openssl: fix building with v3 no-deprecated + add CI test#12384
Closed
vszakats wants to merge 7 commits into
Closed
openssl: fix building with v3 no-deprecated + add CI test#12384vszakats wants to merge 7 commits into
no-deprecated + add CI test#12384vszakats wants to merge 7 commits into
Conversation
Member
Author
|
/cc @ajbozarth |
Contributor
|
Thanks for the catch, I missed that those were deprecated when we added the version checks |
Member
Author
|
@ajbozarth: These calls seem redundant, but can you confirm their removal didn't break this feature? |
ajbozarth
reviewed
Nov 22, 2023
ajbozarth
left a comment
ajbozarth
left a comment
Contributor
There was a problem hiding this comment.
I've checkout and run the code and it's still fully functional
LGTM (can't make this an approve cause of perms)
Member
Author
|
Thank you @ajbozarth! |
Don't call `OpenSSL_add_all_algorithms(), `OpenSSL_add_all_digests()`. The caller code is meant for OpenSSL 3, while these two functions were only necessary before OpenSSL 1.1.0. They are missing from OpenSSL 3 if built with option `no-deprecated`, causing a build error. Regression from b6e6d4f curl#12030 Bug: curl#12380 (comment) Closes #xxxxx
With OpenSSL 3 `no-deprecated`.
```
curl_ntlm_core.c: In function ‘Curl_ntlm_core_lm_resp’:
curl_ntlm_core.c:309:50: error: unused parameter ‘keys’ [-Werror=unused-parameter]
309 | void Curl_ntlm_core_lm_resp(const unsigned char *keys,
| ~~~~~~~~~~~~~~~~~~~~~^~~~
curl_ntlm_core.c:310:50: error: unused parameter ‘plaintext’ [-Werror=unused-parameter]
310 | const unsigned char *plaintext,
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
curl_ntlm_core.c:311:44: error: unused parameter ‘results’ [-Werror=unused-parameter]
311 | unsigned char *results)
| ~~~~~~~~~~~~~~~^~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/6960319321/job/18939543529?pr=12384#step:24:134
With OpenSSL 3 `no-deprecated`.
These happen after our intentional `#error` directive when building
without necessary TLS-backend with DES support but let's fix them
anyway for good measure.
```
curl_ntlm_core.c: In function ‘Curl_ntlm_core_mk_lm_hash’:
curl_ntlm_core.c:350:30: error: unused variable ‘magic’ [-Werror=unused-variable]
350 | static const unsigned char magic[] = {
| ^~~~~
At top level:
curl_ntlm_core.c:350:30: error: ‘magic’ defined but not used [-Werror=unused-const-variable=]
curl_ntlm_core.c:136:13: error: ‘extend_key_56_to_64’ defined but not used [-Werror=unused-function]
136 | static void extend_key_56_to_64(const unsigned char *key_56, char *key)
| ^~~~~~~~~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/6960319321/job/18939543529?pr=12384#step:24:144
We build quictls with `no-deprecated` which implies `no-des` and NTLM support needs DES.
This function is not present in OpenSSL 3 `no-deprecated` builds.
Fix it by moving an existing solution for this from `vtls/openssl.c`
to `vtls/openssl.h` and adjusting the caller code.
```
vquic/curl_ngtcp2.c: In function ‘qng_verify_peer’:
vquic/curl_ngtcp2.c:1950:19: error: implicit declaration of function ‘SSL_get_peer_certificate’; did you mean ‘SSL_get1_peer_certificate’? [-Wimplicit-function-declaration]
1950 | server_cert = SSL_get_peer_certificate(ctx->ssl);
| ^~~~~~~~~~~~~~~~~~~~~~~~
| SSL_get1_peer_certificate
```
Ref: https://github.com/curl/curl/actions/runs/6960723097/job/18940818625#step:24:1178
vszakats
force-pushed
the
openssl-fix-no-deprecated
branch
from
75626e8 to
ded5315
Compare
vszakats
changed the title
no-deprecatedno-deprecated OpenSSL 3
vszakats
changed the title
no-deprecated OpenSSL 3no-deprecated + add CI test
3 tasks
vszakats
added a commit
that referenced
this pull request
Oct 24, 2025
Make autotools and cmake detect DES support in OpenSSL and mbedTLS. Forward feature macros to C and omit NTLM from the feature preview list. Use the feature macros in source. This ensure that `-V` output matches the preview. OpenSSL doesn't support DES when built with `no-des` or `no-deprecated`. mbedTLS 4.x no longer supports it, and it's possible to disable it in <4 with `scripts/config.py unset MBEDTLS_DES_C`. Before this patch this worked for mbedTLS 4 only, and with a regression for pending PR #16973. Also: - drop NTLM feature check from `curl_setup.h` in favour of autotools/ cmake feature macros. This makes `curl_setup.h` no longer need to include an mbedTLS header, which in turn makes tests/server build without depending on mbedTLS. Fixing, in #16973: ``` In file included from tests/server/first.h:40, from bld/tests/server/servers.c:3: lib/curl_setup.h:741:10: fatal error: mbedtls/version.h: No such file or directory 741 | #include <mbedtls/version.h> | ^~~~~~~~~~~~~~~~~~~ ``` Ref: https://github.com/curl/curl/actions/runs/18689537893/job/53291322012?pr=16973 Ref: #19181 (initial fix idea) Follow-up to 3a30583 #19077 - move back mbedTLS header include and version check from `curl_setup.h` to each source which consumes mbedTLS. - GHA/http3-linux: drop workaround that disabled NTLM for `no-deprecated` OpenSSL builds. Follow-up to 0069778 #12384 - curl_ntlm_core: drop pointless macro `CURL_NTLM_NOT_SUPPORTED`. Follow-up to 0069778 #12384 Closes #19206
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
build quictls with
no-deprecatedin CI to have test coverage forthis OpenSSL 3 configuration.
don't call
OpenSSL_add_all_algorithms(),OpenSSL_add_all_digests().The caller code is meant for OpenSSL 3, while these two functions were
only necessary before OpenSSL 1.1.0. They are missing from OpenSSL 3
if built with option
no-deprecated, causing build errors:Ref: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48587418?fullLog=true#L7667
Regression from b6e6d4f OpenSSL: Include SIG and KEM algorithms in verbose #12030
Bug: curl 8.3 build failing with OPENSSL 3 due to absence of DES functions #12380 (comment)
vquic/curl_ngtcp2: fix using
SSL_get_peer_certificatewithno-deprecatedquictls 3 builds.Do it by moving an existing solution for this from
vtls/openssl.cto
vtls/openssl.hand adjusting caller code.Ref: https://github.com/curl/curl/actions/runs/6960723097/job/18940818625#step:24:1178
curl_ntlm_core: fix
-Wunused-parameter,-Wunused-variableand-Wunused-functionwhen trying to build curl with NTLM enabled butwithout the necessary TLS backend (with DES) support.
Closes #12384
no-deprecatedOpenSSL 3 builds.Though OpenSSL 3 deprecated the DES functions needed for the NTLM
feature, its
no-deprecatedbuild option doesn't actually disablethese DES functions. It means we can use them for NTLM even if
OPENSSL_NO_DEPRECATED_3_0is set. Adjust our logic to allow it.This failed tests due to some types missing. Dropping this.
https://github.com/curl/curl/actions/runs/6961417865/job/18942957524?pr=12384#step:24:132