Support OpenSSL 3.0 APIs for Diffie-Hellman#13349
Open
JosiahWI wants to merge 3 commits into
Open
Conversation
JosiahWI
commented
Jun 29, 2026
Comment on lines
+60
to
+63
| char prime_group[]{"dh_2048_256"}; | ||
| OSSL_PARAM const params[]{OSSL_PARAM_utf8_string("group", prime_group, sizeof(prime_group) - 1), OSSL_PARAM_END}; | ||
|
|
||
| if (!EVP_PKEY_CTX_set_params(pctx.get(), params)) { |
Contributor
Author
There was a problem hiding this comment.
This is where I want to use the EVP_PKEY_CTX_set_dh_rfc5114 function instead of manually setting parameters.
JosiahWI
commented
Jun 29, 2026
Comment on lines
+95
to
+102
| set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey) | ||
| { | ||
| bool result{SSL_CTX_set_options(ctx, SSL_OP_SINGLE_DH_USE) && SSL_CTX_set0_tmp_dh_pkey(ctx, pkey)}; | ||
| if (!result) { | ||
| EVP_PKEY_free(pkey); | ||
| } | ||
| return result; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Of special note: the OpenSSL 3.0 API takes ownership of the key, whereas the older API does not. Therefore, the control flow with respect to memory management is different between the two set_ctx_dh implementations, which could be surprising.
JosiahWI
commented
Jun 29, 2026
JosiahWI
force-pushed
the
feat/openssl3-pkey-compat
branch
from
948fe92 to
53c6090
Compare
JosiahWI
commented
Jun 30, 2026
| CHECK(init_with_dhparams(nullptr)); | ||
| } | ||
|
|
||
| TEST_CASE("ssl_context_enable_dhe: valid ffdhe2048 DH PEM file is accepted") |
Contributor
Author
There was a problem hiding this comment.
Name needs to be corrected. I changed the format to dh_2048_256.
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
1 participant
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.
This is work for #13347. It does not remove the OpenSSL 1.1.1 compatibility, but it switches the implementation to use OpenSSL 3.x APIs when available.
The change is split into three commits. The first one is a suite of unit tests generated by Claude Opus. They are written to test through a high-level API for now, but target the DH key loading. I reviewed them and fixed some issues in the generated code. The tests use OpenSSL 3.x APIs and are only added to the build if those APIs are available.
The second commit is the OpenSSL 3.x implementation. This is the easiest place to view the meat of the change without distraction.
Finally, I split most of the work here into a new file called SSLKeyUtils.cc in the third commit. SSLUtils.cc needs refactoring according to CodeScene (the file is massive and not cohesive), and I saw an opportunity to make a small chip in it here in that direction, with the special benefit of isolating all the messy precompiler directives in the new file so that the SSLUtils.cc function remains sleek and build-configuration-independent.
When reviewing, please take a look at
EVP_PKEY_CTX_set_dh_rfc5114. In the OpenSSL 3.x implementation ofgen_dh_2048_258I would rather use that helper function than manually set params. When I tried it, OpenSSL happily failed the corresponding unit test. Closer docs inspection reveals that the key type has to beEVP_PK_DHX, even for thedhversion of the function. I don't know the difference between DH and DHX and whether it would be behavior-preserving to switch, so I'm asking for help here.I have been thorough in error-checking and memory management, referring to OpenSSL's API documentation, but I am not familiar with OpenSSL APIs yet. I would appreciate a review from someone with a lot of OpenSSL experience.