Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Feb 1, 2023

builds up on #2103
Contributes to #2039

Unlike the original PR this does not tries to switch OpenSSL but it adds OpenSSL as alternative TLS.
By default it adds additional OpenSSL drop but it can can be controlled during checkout (instead of default recursive)

It adds openssl3 submodule so MsQuic can be either build with OpenSSSL 1.1 or 3.x
The UseSystemOpenSSLCrypto may still need some work. For now, it is required to select Tls matching OS version.
It would be interesting to run pwsh scripts/build.ps1 -Tls openssl3 -UseSystemOpenSSLCrypto on Arch Linux @ManickaP

PGO files should be probably re-generated. For now, this depends on original PR.

I had issues liking system OpenSSL 3. It seems like the new OpenSSL has some trickery linking different sources for static and dynamic libraries. Extra rule in submodules/CMakeLists.tx tries to compensate for it. There may be better long term way to solve.

@wfurt wfurt requested a review from a team as a code owner February 1, 2023 08:12
@wfurt wfurt self-assigned this Feb 1, 2023
@nibanks
Copy link
Collaborator

nibanks commented Feb 1, 2023

/azp run ci

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@nibanks nibanks mentioned this pull request Feb 1, 2023
@nibanks nibanks merged commit 0bf9e09 into microsoft:main Feb 1, 2023
@ManickaP
Copy link
Member

ManickaP commented Feb 1, 2023

My experience with this change is not very good, so I've picked some interesting print lines from the build:


-- Configuring for OpenSSL 1.1
-- Setting openssldir to /etc/ssl
-- Configuring OpenSSL: /home/manicka/repositories/msquic/submodules/openssl/config;CC=/usr/bin/cc;CXX=/usr/bin/c++ enable-tls1_3;no-makedepend;no-dgram;no-ssl3;no-psk;no-srp;no-zlib;no-egd;no-idea;no-rc5;no-rc4;no-afalgeng;no-comp;no-cms;no-ct;no-srp;no-srtp;no-ts;no-gost;no-dso;no-ec2m;no-tls1;no-tls1_1;no-tls1_2;no-dtls;no-dtls1;no-dtls1_2;no-ssl;no-ssl3-method;no-tls1-method;no-tls1_1-method;no-tls1_2-method;no-dtls1-method;no-dtls1_2-method;no-siphash;no-whirlpool;no-aria;no-bf;no-blake2;no-sm2;no-sm3;no-sm4;no-camellia;no-cast;no-md4;no-mdc2;no-ocb;no-rc2;no-rmd160;no-scrypt;no-weak-ssl-ciphers;no-shared;no-tests;--openssldir="/etc/ssl";--prefix=/home/manicka/repositories/msquic/build/_deps/opensslquic-build/openssl
-- Found OpenSSL: /usr/lib/libcrypto.so (found version "3.0.7")

Note the Configuring for Configuring for OpenSSL 1.1 + found version "3.0.7". This is a thing that would fail the build before this change, now it doesn't.


Configuring OpenSSL version 1.1.1s+quic (0x1010113fL) for linux-x86_64

Picks wrongly OpenSSL 1.1 submodule (or is wrongly using system OpenSSL 3.0.7)


[100%] Built target msquic

Regardless of the previous mismatch, successfully builds libmsquic.


After installation of this frankenlib in the system, System.Net.Quic tests crash with:

/home/manicka/repositories/runtime/artifacts/bin/testhost/net8.0-linux-Debug-x64/dotnet: symbol lookup error: /usr/lib/libmsquic.so.2: undefined symbol: EVP_MD_size

  • Behavior is identical regardless whether I have or don't OpenSSL 1.1 on the system.
  • I'm using -DQUIC_USE_SYSTEM_LIBCRYPTO=true

@wfurt
Copy link
Member Author

wfurt commented Feb 1, 2023

The selection is not automatic @ManickaP - at least not with this change. You have to pass -Tls openssl3 to build script or set the cmake values accordingly.

There should be guard for the mismatch. I can check why it is not working.

@ManickaP
Copy link
Member

ManickaP commented Feb 1, 2023

There should be guard for the mismatch. I can check why it is not working.

That doesn't work.

The selection is not automatic

I hope it's planned though.

You have to pass ...

Works with -DQUIC_TLS=openssl3, but not otherwise, regardless whether I have OpenSSL 1.1 or not. I.e. the moment your system upgrades to OpenSSL 3, it's not possible to build for anything else and you have to explicitly state it in the build script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants