QUIC for 3.3.0

[wbl]

Review carefully: we've tested the code decently well (thanks @nibanks). The branch wladd/quic-comparison-branch has everything squashed down to ease review. I renamed our feature to boring-quic-api in this branch as the first big change, hopefully everywhere it needed to happen in the build system.

[wbl]

Looks like some more work needed for CI to work, will update in a bit.

[wbl]

Looks like all checks pass modulo the fuzzer, but that seems to have run fine other times?

[wbl]

I do not understand what the difference is between the two fuzzer builds. @tmshort any ideas? Going to fiddle with the docs some more, and then a review would be very appreciated.

[tmshort]

i wouldn't worry about it. The one tagged (pull_request) is likely only run on an actual pull request, and is more comprehensive than the one tagged (push) which would happen on all pushes to the repo (even if they don't become a PR).

[tmshort]

The CIFuzz (pull_request) has almost always failed.

[tmshort]

Any updates here @wbl ?

[wbl]

Sorry I think balls ended up on floors as Ive forgotten to poke. I've made the comparison branch and gotten this one to what I think is reviewable state but another set of eyes would be helpful. So take a look or reremind me what I need to do first.

[wbl]

Thanks @richsalz for looking it over. I'll fix up the comments if no one else chimes in with more soon in a separate commit.

[nibanks]

Sorry, I haven't followed up here in a while. I just updated https://github.com/microsoft/msquic/pull/4274 to use the latest commit here with the latest version of MsQuic and I am seeing build failures on Ubuntu 24.04. Note, the last time I tried this, we didn't build for 24.04 yet, so this isn't necessarily a recent break, but it is a regression from previous v3 openssl.

https://github.com/microsoft/msquic/actions/runs/9974479617/job/27562154754#step:8:2184

/usr/bin/ld: ../../../../_deps/opensslquic-build/openssl3/lib/libssl.a(libssl-lib-ssl_lib.o): in function `SSL_CTX_free':
ssl_lib.c:(.text+0x93f1): undefined reference to `OSSL_STACK_OF_X509_free'
/usr/bin/ld: ../../../../_deps/opensslquic-build/openssl3/lib/libssl.a(libssl-lib-ssl_lib.o): in function `SSL_dup':
ssl_lib.c:(.text+0x9e44): undefined reference to `OSSL_STACK_OF_X509_free'
/usr/bin/ld: ../../../../_deps/opensslquic-build/openssl3/lib/libssl.a(libssl-lib-ssl_lib.o): in function `ossl_ssl_connection_free':
ssl_lib.c:(.text+0xa262): undefined reference to `OSSL_STACK_OF_X509_free'
/usr/bin/ld: ssl_lib.c:(.text+0xa540): undefined reference to `OSSL_STACK_OF_X509_free'
/usr/bin/ld: ../../../../_deps/opensslquic-build/openssl3/lib/libssl.a(libssl-lib-ssl_lib.o): in function `SSL_CTX_new_ex':
ssl_lib.c:(.text+0xa6fc): undefined reference to `OPENSSL_LH_set_thunks'
/u
...

[wbl]

@nibanks I'll look into that failure: if those things got redefined to be macros or functions across versions, maybe its a build picking up wrong things sort of issue, but we should know that for sure and I'll go attack it. What's the easiest way for me to locally reproduce where I can poke around the image/reproduce in CI: open a junk pr to MSQuic?

[nibanks]

@nibanks I'll look into that failure: if those things got redefined to be macros or functions across versions, maybe its a build picking up wrong things sort of issue, but we should know that for sure and I'll go attack it. What's the easiest way for me to locally reproduce where I can poke around the image/reproduce in CI: open a junk pr to MSQuic?

Do you want to try to repro locally or just let MsQuic CI/CD do the heavy lifting? If you want to use MsQuic, yeah, just make a draft PR and I will approve the automation to run.

[wbl]

I think I've got to play with it locally just to understand. But wait,

cd /__w/msquic/msquic/build/linux/x86_openssl3/src/tools/interopserver && /usr/bin/cmake -E cmake_link_script CMakeFiles/quicinteropserver.dir/link.txt --verbose=1 /usr/bin/c++ --sysroot=/ -Og -fno-omit-frame-pointer -ggdb3 CMakeFiles/quicinteropserver.dir/InteropServer.cpp.o -o /w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3/quicinteropserver -Wl,-rpath,"\$ORIGIN:w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3" /__w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3/libmsquic.so.2.4.0 ../../../obj/Debug/libplatform.a ../../../obj/Debug/liblogging.a ../../../_deps/opensslquic-build/openssl3/lib/libssl.a /usr/lib/x86_64-linux-gnu/libcrypto.so -ldl /usr/lib/x86_64-linux-gnu/libatomic.so.1 /usr/lib/x86_64-linux-gnu/libnuma.so.1

Is it just me or is it linking the new libssl.a to the system libcrypto.so? Then I'm not surprised it doesn't work, as those got introduced in patch https://github.com/openssl/openssl/commit/5c42ced0ff974a59af98b75e54136f4282718266

[nibanks]

I think I've got to play with it locally just to understand. But wait,

cd /__w/msquic/msquic/build/linux/x86_openssl3/src/tools/interopserver && /usr/bin/cmake -E cmake_link_script CMakeFiles/quicinteropserver.dir/link.txt --verbose=1 /usr/bin/c++ --sysroot=/ -Og -fno-omit-frame-pointer -ggdb3 CMakeFiles/quicinteropserver.dir/InteropServer.cpp.o -o /w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3/quicinteropserver -Wl,-rpath,"$ORIGIN:w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3" /__w/msquic/msquic/artifacts/bin/linux/x86_Debug_openssl3/libmsquic.so.2.4.0 ../../../obj/Debug/libplatform.a ../../../obj/Debug/liblogging.a ../../../_deps/opensslquic-build/openssl3/lib/libssl.a /usr/lib/x86_64-linux-gnu/libcrypto.so -ldl /usr/lib/x86_64-linux-gnu/libatomic.so.1 /usr/lib/x86_64-linux-gnu/libnuma.so.1

Is it just me or is it linking the new libssl.a to the system libcrypto.so? Then I'm not surprised it doesn't work, as those got introduced in patch openssl@5c42ced

Ah, you're right! It's all our builds with the -UseSystemOpenSSLCrypto flag, which statically links libssl, but dynamically links libcrytpo (for FIPS). This passed on previous versions/commits, but is failing now. How are we supposed to handle this?

[wbl]

My first reaction is why on earth would you expect this to work out nicely, and why do you need it: is there some other way to get what you want here? Note that the fips module is entirely unaffected by this issue as that dynamically loads with a much more restricted interface.

[wbl]

Alternatively try 3.3 as the system version, but that might have other issues

[nibanks]

My first reaction is why on earth would you expect this to work out nicely, and why do you need it: is there some other way to get what you want here? Note that the fips module is entirely unaffected by this issue as that dynamically loads with a much more restricted interface.

We need to dynamically link to the system libcrypto for FIPS compliance (because we can't bring our own binary). But we cannot link to the system libssl because it's not the quictls fork. This has worked out so far for previous v1 and v3 versions of openssl.

Alternatively try 3.3 as the system version, but that might have other issues

I don't think we had to specify anything version specific in the past. Any suggestions on how we'd test this out?

[wbl]

My first reaction is why on earth would you expect this to work out nicely, and why do you need it: is there some other way to get what you want here? Note that the fips module is entirely unaffected by this issue as that dynamically loads with a much more restricted interface.

We need to dynamically link to the system libcrypto for FIPS compliance (because we can't bring our own binary). But we cannot link to the system libssl because it's not the quictls fork. This has worked out so far for previous v1 and v3 versions of openssl.

That's odd: they were supposed to have limited fips issues to fips.so, but maybe you're doing something different.

Alternatively try 3.3 as the system version, but that might have other issues

I don't think we had to specify anything version specific in the past. Any suggestions on how we'd test this out?

Ideally just an apt-get update and install the new version ahead of running the existing script. Not really sure about where it is in ubuntu.

[tmshort]

What's the status of this PR?

[wbl]

I'd like more than just Rich's approval on it, given the size and scope and the number of decisions I had to make that people could possibly have issues with. Please take a look and either approve or say what you want to change.

[tatsuhiro-t]

Built fine with the latest ngtcp2 and all tests passed. Thank you for great work. Once thing, that is I do not know whether it is the issue of original openssl, but the generated pkgconfig file is broken. libdir lacks lib directory.

[nibanks]

I just pushed the latest to https://github.com/microsoft/msquic/pull/4274 to run it all again. If only our 'use system libcrypto' tests fail, I'm fine for merging this. I still need to figure out a solution for that though...

[richsalz]

@nibanks I agree, let's merge this once your results are known.

[wbl]

@nibanks it looks like it's just the usesystem openssl tests failing, but I'm not that familiar with your tests.

@tatsuhiro-t pkgconfig is a bit of a black box to me I'm afraid. If you would be so kind, open another issue after we merge this where we can solve that problem.

[nibanks]

I'm sorry. I just haven't had the time to follow up on this. I will see if I can make progress tomorrow morning, otherwise, don't wait on me.

[tatsuhiro-t]

@wbl All right. Lets move on. Will check after the merge to check that it is still an issue.