-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
openssl + wolfssl combo backend build broken #10321
Comments
Ref: microsoft/vcpkg#24348 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 dafdb20 It introduced vtls_int.h, this header includes all the backend headers outright. |
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 UPDATE 2: When I added this priority list in 2017 (6f86022), 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 Patch proposal moved here: #10322 |
I just checked and curl 7.84.0 builds with both backends just fine |
Do you mean that |
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. |
Your patch does fix the build in curl_ntlm_core however if you see my other comment #10321 (comment) 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 dafdb20 issue. |
Yep, got it! The dafdb20 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 |
Caveat: This is broken in multiple ways depending on curl version, options enabled and build tool. With HTTP/3 enabled, these two backends never worked together and as of 7.87.0, there are multiple compile-time issues even without HTTP/3. Also `lib/Makefile.mk` will need a patch to allow this combination. Ref: curl/curl#10321
1. Add USE_WOLFSSL to the priority list in `lib/curl_ntlm_core.c` 2. Fix `lib/curl_ntlm_core.h` to respect backend priority, bringing this in sync with the above list and `lib/curl_ntlm_core.c` itself. 3. Allow enabling both wolfSSL and OpenSSL at the same time in `lib/Makefile.mk`. Update the logic to select the crypto-specific lib for ngtcp2. Ref: curl#10321 Closes #xxxxx
1. Add USE_WOLFSSL to the priority list in `lib/curl_ntlm_core.c` 2. Fix `lib/curl_ntlm_core.h` to respect backend priority, bringing this in sync with the above list and `lib/curl_ntlm_core.c` itself. 3. Allow enabling both wolfSSL and OpenSSL at the same time in `lib/Makefile.mk`. Update the logic to select the crypto-specific lib for ngtcp2. Ref: curl#10321 Closes curl#10322
I would be okay with documenting that wolfSSL and OpenSSL cannot be used together in a curl build. |
And document how OpenSSL forks and wolfSSL cannot be used at the same time. Reported-by: Mark Roszko Fixes #10321
1. Add `USE_WOLFSSL` to the TLS backend priority list in `lib/curl_ntlm_core.c`. 2. Fix `lib/curl_ntlm_core.h` to respect TLS backend priority, bringing it in sync with the above list and `lib/curl_ntlm_core.c` itself. Reported-by: Mark Roszko Ref: #10321 3. Allow enabling both wolfSSL and OpenSSL at the same time in `lib/Makefile.mk` bringing this in line with cmake/autotools builds. Update logic to select the crypto-specific lib for `ngtcp2`, which supports a single TLS backend at the same time. Closes #10322
And document how OpenSSL forks and wolfSSL cannot be used at the same time. Reported-by: Mark Roszko Fixes curl#10321 Closes curl#10382
1. Add `USE_WOLFSSL` to the TLS backend priority list in `lib/curl_ntlm_core.c`. 2. Fix `lib/curl_ntlm_core.h` to respect TLS backend priority, bringing it in sync with the above list and `lib/curl_ntlm_core.c` itself. Reported-by: Mark Roszko Ref: curl#10321 3. Allow enabling both wolfSSL and OpenSSL at the same time in `lib/Makefile.mk` bringing this in line with cmake/autotools builds. Update logic to select the crypto-specific lib for `ngtcp2`, which supports a single TLS backend at the same time. Closes curl#10322
This commit made a change to curl_ntlm_core 5fd7cd7
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
The text was updated successfully, but these errors were encountered: