Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: fix OpenSSL quic detection in quiche builds #12162

Closed
wants to merge 2 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 19, 2023

An orphan call to CheckQuicSupportInOpenSSL() remained after a recent
update when checking QUIC for quiche. Move back QUIC detection to
a function and fixup callers to use that. Also make sure that quiche
gets QUIC from BoringSSL, because it doesn't support other forks at this
time.

Regression from dee310d #11555

Reported-by: Casey Bodley cbodley@redhat.com
Fixes #12160
Closes #12162

@vszakats vszakats added cmake HTTP/3 h3 or quic related quiche Cloudflare's QUIC and HTTP/3 library labels Oct 19, 2023
@github-actions github-actions bot added the build label Oct 19, 2023
An orphan call to `CheckQuicSupportInOpenSSL()` remained after
a recent update when checking QUIC for quiche. Restore this function
and fixup the callers to use it.

Regression from dee310d #curl#11555

Reported-by: Casey Bodley <cbodley@redhat.com>
Fixes curl#12160
Closes curl#12162
Copy link
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this does fix the issue with USE_QUICHE and boringssl:

-- Looking for OPENSSL_IS_BORINGSSL
-- Looking for OPENSSL_IS_BORINGSSL - found
-- Looking for OPENSSL_IS_AWSLC
-- Looking for OPENSSL_IS_AWSLC - not found
-- Found ZLIB: /usr/lib64/libz.so (found version "1.2.13")
-- Looking for SSL_set0_wbio
-- Looking for SSL_set0_wbio - found
-- Looking for SSL_CTX_set_srp_username
-- Looking for SSL_CTX_set_srp_username - not found
-- Checking for one of the modules 'quiche'
-- Found QUICHE: /home/cbodley/ceph/build/src/rgw/quiche/target/release/libquiche.a
-- Looking for SSL_CTX_set_quic_method
-- Looking for SSL_CTX_set_quic_method - found
-- Looking for quiche_conn_set_qlog_fd
-- Looking for quiche_conn_set_qlog_fd - found

Comment on lines +658 to +659
elseif(USE_WOLFSSL)
openssl_check_symbol_exists(wolfSSL_set_quic_method "wolfssl/options.h;wolfssl/openssl/ssl.h" HAVE_SSL_CTX_SET_QUIC_METHOD)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know whether quiche actually supports wolfssl? afaik it depends on boringssl: https://github.com/cloudflare/quiche#building

so this cmake configure step may succeed with USE_WOLFSSL and USE_QUICHE, but fail during compilation

the USE_QUICHE block should probably include a check for HAVE_BORINGSSL to make sure that openssl_check_quic() actually checked for the correct SSL_CTX_set_quic_method symbol here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I haven't made tests with quiche yet.

If quiche depends specifically on BoringSSL, it's sensible
indeed to check for that. Pushed a commit to do this.

Do you know if quiche works with AWS-LC as well? It's Amazon's BoringSSL fork.

Copy link
Contributor

@cbodley cbodley Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I haven't made tests with quiche yet.

If quiche depends specifically on BoringSSL, it's sensible indeed to check for that. Pushed a commit to do this.

thanks, updated branch still works for me 👍

Do you know if quiche works with AWS-LC as well? It's Amazon's BoringSSL fork.

i don't see any discussion about AWS-LC on their github. i checked out/built AWS-LC, but when i plugged it into quiche i got undefined references:

/path/to/quiche/quiche/src/tls.rs:672: undefined reference to `SSL_get_curve_id'
/usr/bin/ld: /path/to/quiche/quiche/src/tls.rs:677: undefined reference to `SSL_get_curve_name'

this was quiche v0.18.0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing these.

BoringSSL it is then. We can extend this when quiche adds support for more forks (or vice versa.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who knows, maybe openssl will support quic someday 🙃

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by some definition of "quic" :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know whether quiche actually supports wolfssl

I'm pretty sure it does not. It is written with BoringSSL in mind.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe openssl will support quic someday

They already do, but they implement their own QUIC stack and not the "standard" API for it. curl has no support for OpenSSL's API and AFAIK, neither does nghttp3 so we are quite far off an OpenSSL backend for h3.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, their completely different, newly developed quic API is now in 3.2-alpha.

For extra fun, there was some symbol re-use, but maybe not too bad:
openssl/openssl#22204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build cmake HTTP/3 h3 or quic related quiche Cloudflare's QUIC and HTTP/3 library regression
Development

Successfully merging this pull request may close these issues.

None yet

3 participants