Closed marekr closed 1 year ago
Ref: https://github.com/microsoft/vcpkg/pull/24348 Ref: https://github.com/curl/curl/pull/8864 Ref: https://github.com/curl/curl/pull/9314
Basically, to summarize, it looks like @elms had put wolfssl first and @vszakats had put openssl first. I think if wolfSSL can use the same paths as OpenSSL includes (@elms mentioned this in his PR) then we would want to check wolfSSL first, otherwise our OpenSSL section could include wolfSSL includes? If I understand this right. @vszakats do you remember the reason for the change to put OpenSSL first?
Also fyi, if I fix the issue in curl_ntlm_core by reordering the includes, another issue appears in \lib\vtls\wolfssl.c with a header conflict.
This second conflict is caused by https://github.com/curl/curl/commit/dafdb20a26d0c890e83dea61a104b75408481ebd
It introduced vtls_int.h, this header includes all the backend headers outright. openssl.h
is first, it includes the openssl/ssl.h header which includes all the openssl types. wolfssl.c
then includes vtls_int.h
before the wolfssl headers and the result is the same compiler barf
Can't remember what was the exact error without this patch. But, is it really possible to use both wolfSSL and OpenSSL at the same time? In my tests this didn't seem to be the case, and they were conflicting. One (main) reason is that wolfSSL is supported by curl via its OpenSSL compatibility interface, sharing most (but not all) interface code.
It would be nice to sort out inconsistencies when picking one over the other, but building against both at the same time I don't think is possible. It means a viable fix is to deselect one of them at build time.
UPDATE: The likely reason I had changed the order, is the comment at the top of curl_ntlm_core.c
which specifies the expected priority order, USE_OPENSSL
being the first on the list. I think the order should be synced with this also in curl_ntlm_core.h
. [ Back then, after finding out these two are conflicting, I did sort this out at the build level, and may have skipped bumping into the header issue in further tests. ]
UPDATE 2: When I added this priority list in 2017 (6f86022df26243cc8a035fe8b4c89033b6a04bc0), wolfSSL support wasn't present yet. When wolfSSL support was added later on, the list wasn't updated, so it's still missing from it. I'd be also nice to add it there, right below USE_OPENSSL
.
Patch proposal moved here: https://github.com/curl/curl/pull/10322
Can't remember what was the exact error without this patch. But, is it really possible to use both wolfSSL and OpenSSL at the same time?
I just checked and curl 7.84.0 builds with both backends just fine
Do you mean that curl -V
would display both OpenSSL and wolfSSL at the same time? Or do you mean the build finishes successfully (with one of the TLS backends appearing in curl -V
)?
Do you mean that
curl -V
would display both OpenSSL and wolfSSL at the same time? Or do you mean the build finishes successfully (with one of the TLS backends appearing incurl -V
)?
PS E:\vcpkgup\installed\x64-windows\tools\curl> ./curl -V
curl 7.84.0-DEV (Windows) libcurl/7.84.0-DEV wolfSSL/5.5.0 (OpenSSL/3.0.7) (Schannel) zlib/1.2.11
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS IPv6 Kerberos Largefile libz MultiSSL NTLM SPNEGO SSL SSPI threadsafe UnixSockets
Yes, also schannel for bonus points
Thanks! I've made my tests with HTTP/3 enabled, which may change the situation (e.g. due to ngtcp2 supporting one or the other TLS backend). I'll make some tests without HTTP/3.
Thanks! I've made my tests with HTTP/3 enabled, which may change the situation (e.g. due to ngtcp2 supporting one or the other TLS backend). I'll make some tests without HTTP/3.
In the meantime, did you have a chance to test the patch above?
Your patch does fix the build in curl_ntlm_core however if you see my other comment https://github.com/curl/curl/issues/10321#issuecomment-1396462823
there is a second breakage by a more recent commit in november. I poked it but it seems more problematic to fix since vtls_int.h also carries necessary types/functions for the backend headers now after that referenced commit.
Testing with 7.87.0, with the above patch, I bumped into the dafdb20a26d0c890e83dea61a104b75408481ebd issue. If that sorted, the next issue will be ngtcp2.
Yep, got it! The dafdb20a26d0c890e83dea61a104b75408481ebd issue wasn't present (nor was HTTP/3 supported by wolfSSL) at the time, but there were further issues down the line with HTTP/3 enabled, according to my notes both at compile and link time. So, mixing these two may or may not be possible depending on other features enabled. With wolfSSL now also supporting HTTP/3, there are some more combinations since.
I can unlock this combo in lib/Makefile.mk
(patch above updated, and converted to #10322) and in curl-for-win, but checking all valid and invalid combinations seems a little bit too complex at this point. Not to mention CMake and autotools builds (CMake doesn't support wolfSSL with HTTP/3 yet).
I would be okay with documenting that wolfSSL and OpenSSL cannot be used together in a curl build.
This commit made a change to curl_ntlm_core https://github.com/curl/curl/commit/5fd7cd7302d3ea265661822b5eea200f97005133
It rearranged the includes in the curl_ntlm_core.c so that openssl headers are included before wolfssl if both backends are enabled. However, the curl_ntlm_core.h header was not updated and it still includes wolfssl before openssl.
The problem is curl_ntlm_core.h since updated, if you attempt to build, the source file will include openssl and the header will include wolfssl. Wolfssl's compat types immediately clash into openssl types
The result is
== Build Environment