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
configure: error out on multissl + http3 #12807
Conversation
@@ -4691,6 +4691,9 @@ fi | |||
|
|||
if test "x$USE_NGTCP2_H3" = "x1" -o "x$USE_QUICHE" = "x1" \ | |||
-o "x$USE_OPENSSL_H3" = "x1" -o "x$USE_MSH3" = "x1"; then | |||
if test "x$CURL_WITH_MULTI_SSL" = "x1"; then | |||
AC_MSG_ERROR([MultiSSL cannot be enabled with HTTP/3 and vice versa]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP3 + Schannel/SecureTransport are working fine and the above disallows them too. It'd be useful to let these combos through. (curl-for-win uses them in default builds.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are working fine
In what combinations have you tested this? We don't test anything of this in CI and the h3 code has no multissl awareness. It feels brittle and risky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not doing extra testing with these builds, but they were built like this (OpenSSL + Schannel) since MultiSSL became available.
Since the introduction of HTTP/3 (around 2022-05-23), I haven't re-evaluated that, but quictls/LibreSSL + Schannel continued to build and appeared to work fine in basic tests and without any obvious issues or bug reports. They also have fairly distinct codepaths, except some crypto backend stuff unrelated to HTTP/3.
It'd be a big shock if this combo was broken all along.
...testing with fresh debug builds under WINE (with script #12806, but with curl.se
, --disable
and just -v
; not -v4
), I see no leaks, but schannel doesn't use H2, only H1 (also true with pure schannel build) and it does actually touch the QUIC codepath (with QUIC connect to
), but works. LibreSSL also works with no leaks, in H3 mode. (-v4
resets H3 to H2. Is it a valid curl option?)
[ If this is truly broken, or unsupported, curl-for-win will need a reshuffle. Back then without Schannel, it was not possible to download cacert.pem
(to bootstrap OpenSSL) via HTTPS because of the lack of native CA support. Maybe --ca-native
/CURLSSLOPT_NATIVE_CA
options fix that now? Schannel is also useful to do cross-checks for bug reports, and also to keep the Schannel codepath build-tested with curl-for-win options. I'd also expect at least some users relying on this in some form or another. ]
edit: -4
is for IPv4 of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't there also be something like this:
#if defined(CURL_WITH_MULTI_SSL) && defined(USE_NGHTTP3)
#error "Multi-SSL with Nghttp3 is unsupported"
#endif
Ref. the snippet in curl_setup.h
:
#if defined(USE_LIBIDN2) && defined(USE_WIN32_IDN)
#error "Both libidn2 and WinIDN are enabled, choose one."
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The QUIC code has no awareness of multissl. Nobody has worked on that yet. The QUIC backends can't change TLS at run-time. It is not about nghttp3 specifically. It goes for all QUIC related code in libcurl.
If we in the multssl code initialize the selected TLS, it might be a separate TLS library used there than is used in QUIC which then is not initialized properly before use.
I have not actually tried this myself, but I can see how things like this can happen. I rather be cautious and say that QUIC/h3 is not tested or confirmed to work with multissl.
I like to believe that multissl is less needed these days since the introduction of --ca-native
, but I don't know exactly for which other purposes people previously have selected to use the Schannel backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess in that case, the best is to shut down MultiSSL. (* in curl-for-win)
Just confirmed on a real Windows machine with network, that --ca-native
works, so Schannel is no longer necessary for using HTTPS (without having a hand-rolled CA bundle that is.) curl-for-win also ships with a CA bundle.
As for macOS, --ca-native
doesn't work, but there is a CA bundle at /etc/ssl/cert.pem
(and also one provided for example by Homebrew.) I'm guessing these are still not native (and thus missing custom entries added via Keychain Access), but good enough to boot.
This will not actively detect and prevent enabling multiple TLS backends (aka MultiSSL) enabled with HTTP/3, e.g. OpenSSL[-fork] + mbedTLS/wolfSSL, but those cases might be rejected by the curl build itself. In effect MultiSSL and HTTP/3 are mutually exclusive options in curl. Ref: curl/curl#12807
This will not actively detect and prevent enabling multiple TLS backends (aka MultiSSL) enabled with HTTP/3, e.g. OpenSSL[-fork] + mbedTLS/wolfSSL, but those cases might be rejected by the curl build itself. In effect MultiSSL and HTTP/3 are mutually exclusive options in curl. after: ``` curl 8.5.0 (x86_64-w64-mingw32) libcurl/8.5.0 LibreSSL/3.8.2 zlib/1.3.1 brotli/1.1.0 zstd/1.5.5 WinIDN libpsl/0.21.5 libssh2/1.11.0 nghttp2/1.59.0 ngtcp2/1.2.0 nghttp3/1.1.0 Release-Date: 2023-12-06 Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp ws wss Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTP3 HTTPS-proxy IDN IPv6 Largefile libz NTLM PSL SSL threadsafe UnixSockets zstd ``` before: ``` curl 8.5.0 (x86_64-w64-mingw32) libcurl/8.5.0 LibreSSL/3.8.2 (Schannel) zlib/1.3.1 brotli/1.1.0 zstd/1.5.5 WinIDN libpsl/0.21.5 libssh2/1.11.0 nghttp2/1.59.0 ngtcp2/1.2.0 nghttp3/1.1.0 Release-Date: 2023-12-06 Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp ws wss Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTP3 HTTPS-proxy IDN IPv6 Kerberos Largefile libz MultiSSL NTLM PSL SPNEGO SSL SSPI threadsafe UnixSockets zstd ``` Ref: curl/curl#12807
Same for CMake: --- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -720,6 +720,10 @@ if(USE_MSH3)
list(APPEND CURL_LIBS ${MSH3_LIBRARIES})
endif()
+if(CURL_WITH_MULTI_SSL AND (USE_NGTCP2 OR USE_QUICHE OR USE_MSH3))
+ message(FATAL_ERROR "MultiSSL cannot be enabled with HTTP/3 and vice versa.")
+endif()
+
if(NOT CURL_DISABLE_SRP AND (HAVE_GNUTLS_SRP OR HAVE_OPENSSL_SRP))
set(USE_TLS_SRP 1)
endif() |
Since the QUIC/h3 code has no knowledge or handling of multissl it might bring unintended consequences if we allow it. Ref: #12806
6a28411
to
c7ccfda
Compare
--with-secure-transport is commented (I don't think it's actually used) (ref: curl/curl#12807)
--with-secure-transport is removed (I don't think it's actually used) (ref: curl/curl#12807)
Since the QUIC/h3 code has not knowledge or handling of multissl it might bring unintended consequences if we allow it.
Ref: #12806