Support OpenSSL 3.0 APIs for Diffie-Hellman#13349
Open
JosiahWI wants to merge 4 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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ATS’s server-side Diffie-Hellman (DHE) parameter handling to use OpenSSL 3.x EVP/decoder APIs when available, while preserving OpenSSL 1.1.1 compatibility. It also factors DH key/parameter helpers out of the already-large SSLUtils.cc into a dedicated utility file.
Changes:
- Add OpenSSL 3–only Catch2 unit tests that exercise DH parameter loading via
SSLMultiCertConfigLoader::init_server_ssl_ctx(). - Switch
ssl_context_enable_dhe()to use new helper functions for generating/loading DH parameters and setting them onSSL_CTX. - Introduce
SSLKeyUtils.{h,cc}to isolate OpenSSL-version-specific DH handling and preprocessor logic.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/iocore/net/unit_tests/test_SSLDHParams.cc |
Adds OpenSSL 3–gated unit tests covering success/failure cases for DH parameter loading. |
src/iocore/net/SSLUtils.cc |
Routes DHE configuration through SSLKeyUtils helpers instead of legacy DH APIs. |
src/iocore/net/SSLKeyUtils.h |
Declares OpenSSL-version-abstracted DH parameter/key helper APIs. |
src/iocore/net/SSLKeyUtils.cc |
Implements OpenSSL 3 EVP/decoder path and OpenSSL 1.1.1 legacy path for DH param generation/loading. |
src/iocore/net/P_SSLUtils.h |
Adds RAII deleters for OpenSSL 3 decoder / pkey context types. |
src/iocore/net/CMakeLists.txt |
Builds SSLKeyUtils.cc and conditionally adds the new OpenSSL 3 unit test source. |
Comment on lines
+49
to
+71
| scoped_PKEY_CTX pctx{EVP_PKEY_CTX_new_from_name(NULL, "DH", NULL)}; | ||
| if (!pctx) { | ||
| Error("failed to create OpenSSL pkey context"); | ||
| return nullptr; | ||
| } | ||
|
|
||
| if (EVP_PKEY_keygen_init(pctx.get()) <= 0) { | ||
| Error("failed to initialize OpenSSL keygen"); | ||
| return nullptr; | ||
| } | ||
|
|
||
| char prime_group[]{"dh_2048_256"}; | ||
| OSSL_PARAM const params[]{OSSL_PARAM_utf8_string("group", prime_group, 0), OSSL_PARAM_END}; | ||
|
|
||
| if (!EVP_PKEY_CTX_set_params(pctx.get(), params)) { | ||
| Error("SSL dhparams source returned invalid parameters"); | ||
| return nullptr; | ||
| } | ||
|
|
||
| EVP_PKEY *pkey{}; | ||
| EVP_PKEY_generate(pctx.get(), &pkey); | ||
|
|
||
| return pkey; |
Comment on lines
+74
to
+92
| EVP_PKEY * | ||
| load_dhparams_file(char const *dhparams_file) | ||
| { | ||
| EVP_PKEY *pkey{}; | ||
| scoped_Decoder_CTX dctx{OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, "DH", OSSL_KEYMGMT_SELECT_ALL_PARAMETERS, NULL, NULL)}; | ||
| if (!dctx) { | ||
| Error("failed to create OpenSSL decoder context"); | ||
| return nullptr; | ||
| } | ||
|
|
||
| ink_assert(OSSL_DECODER_CTX_get_num_decoders(dctx.get()) > 0); | ||
| scoped_BIO bio{BIO_new_file(dhparams_file, "r")}; | ||
| if (!OSSL_DECODER_from_bio(dctx.get(), bio.get())) { | ||
| Error("SSL dhparams source returned invalid parameters"); | ||
| return nullptr; | ||
| } | ||
|
|
||
| return pkey; | ||
| } |
| std::string | ||
| make_rsa_pem() | ||
| { | ||
| EVP_PKEY *pkey = EVP_RSA_gen(1024); |
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
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.