make chacha optional on Linux by wfurt · Pull Request #3423 · microsoft/msquic

wfurt · 2023-02-12T02:06:50Z

Description

this is minimal fix for #3422. Instead of linking, we use dlsym to find the symbol and we set the handle accordingly if found. In such case, there is really no functional impact. I also added few extra check to handle cases when chacha is missing.

Lastly, I added few asserts to Tls test to prevent crashes when dereferencing null. This can probably be worked out via filter if we ever get tests running on FIPS openssl.

I noticed that schannel also have chacha optional. There is possibly more chance for some convergence and optional tests.

Testing

I did run Quic tests as well as tests for .NET Quic test suite in Mariner container.

wfurt · 2023-02-12T02:08:28Z

~/github/wfurt-runtime/artifacts/bin/System.Net.Quic.Functional.Tests/Debug/net7.0-unix ~/github/wfurt-runtime/src/libraries/System.Net.Quic/tests/FunctionalTests
    Discovering: System.Net.Quic.Functional.Tests (method display = ClassAndMethod, method display options = None)
    Discovered:  System.Net.Quic.Functional.Tests (found 106 of 112 test cases)
    Starting:    System.Net.Quic.Functional.Tests (parallel test collections = on, max threads = 4)
      System.Net.Quic.Tests.MsQuicPlatformDetectionTests.UnsupportedPlatforms_ThrowsPlatformNotSupportedException [SKIP]
        Condition(s) not met: "IsQuicUnsupported"



    Finished:    System.Net.Quic.Functional.Tests
  === TEST EXECUTION SUMMARY ===
     System.Net.Quic.Functional.Tests  Total: 326, Errors: 0, Failed: 0, Skipped: 1, Time: 44.566s

cat /etc/os-release
NAME="Common Base Linux Mariner"
VERSION="2.0.20230126"
ID=mariner
VERSION_ID="2.0"
PRETTY_NAME="CBL-Mariner/Linux"
ANSI_COLOR="1;34"
HOME_URL="https://aka.ms/cbl-mariner"
BUG_REPORT_URL="https://aka.ms/cbl-mariner"
SUPPORT_URL="https://aka.ms/cbl-mariner"

$ openssl version -a
OpenSSL 1.1.1k  FIPS 25 Mar 2021
built on: Wed Aug 24 22:12:30 2022 UTC
platform: linux-x86_64
options:  bn(64,64) rc4(16x,int) des(int) blowfish(ptr)
compiler: gcc -fPIC -pthread -m64 -Wa,--noexecstack -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/mariner/default-hardened-cc1   -fcommon -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/mariner/default-hardened-cc1 -fcommon -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wa,--noexecstack -Wa,--generate-missing-build-notes=yes -specs=/usr/lib/rpm/mariner/default-hardened-ld -O0 -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DAESNI_ASM -DVPAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPOLY1305_ASM -DNDEBUG -DPURIFY -DDEVRANDOM="\"/dev/urandom\""
OPENSSLDIR: "/etc/pki/tls"
ENGINESDIR: "/usr/lib/engines-1.1"
Seeding source: os-specific

nibanks · 2023-02-12T15:35:48Z

On Windows, please run (in PWSH) .\scripts\update-sidecar.ps1.

nibanks · 2023-02-12T15:36:38Z

From the codecheck build:

/home/vsts/work/1/s/src/platform/tls_openssl.c:1041:11: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
        } else if (CredConfig->AllowedCipherSuites == QUIC_ALLOWED_CIPHER_SUITE_CHACHA20_POLY1305_SHA256 && !CxPlatCryptSupports(CXPLAT_AEAD_CHACHA20_POLY1305)) {
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

nibanks · 2023-02-12T15:38:09Z

Linux test failure:

[ RUN      ] CryptTest.WellKnownChaChaPolyv1
/home/vsts/work/1/s/src/platform/unittest/CryptTest.cpp:307: Failure
Value of: ((int)(QuicPacketKeyDerive(QUIC_PACKET_KEY_1_RTT, &Labels, &Secret, "WellKnownChaChaPoly", 1, &PacketKey)) <= 0)
  Actual: false
Expected: true
[  FAILED  ] CryptTest.WellKnownChaChaPolyv1 (0 ms)
[----------] 1 test from CryptTest (0 ms total)

https://dev.azure.com/ms/msquic/_build/results?buildId=427797&view=logs&j=358fb670-07e5-5f9a-d283-d837feb19209&t=e511c6c5-b9b6-5330-ae54-83dcd7e01e67&l=17

src/platform/platform_internal.h

src/platform/crypt_openssl.c

src/platform/tls_openssl.c

src/platform/unittest/TlsTest.cpp

Co-authored-by: Nick Banks <[email protected]>

nibanks

Minor comments

src/inc/quic_crypt.h

src/platform/crypt_openssl.c

* make chacha optional on Linux (#3423) * another attempt to fix quic loading when chacha is missing (#3447) * Update src/platform/crypt_openssl.c * CxPlatCryptInitialize * CxPlatCryptUninitialize --------- Co-authored-by: Nick Banks <[email protected]>

make chacha optional on Linux

646ad98

wfurt added the TLS: quicTLS label Feb 12, 2023

wfurt requested a review from a team as a code owner February 12, 2023 02:06

wfurt added 2 commits February 11, 2023 18:55

fix build

fcc062c

fix tests

384c26b

wfurt added 2 commits February 12, 2023 16:41

fix failures

83ea7f0

update clog

1b7d1ec