Skip to content

Commit

Permalink
fix loading of dhparams for DHE with Openssl >= 3
Browse files Browse the repository at this point in the history
The #elif to #if OPENSSL_MAJOR_VERSION < 3 new code block introduced
with 742236c does not work. Instead of trying to load parameters for
DHE, it tries to parse the input file as EC Key if any curve is given to
the tls-dh parameter, e.g.
tls-dh=prime256v1:<path-to-dhparams>/dhparams4096.pem
This causes TLS handshakes to fail if the client uses a configuration
that does not accept ECDHE.

This commit corrects this by setting the type to "DH" and to further also
correctly load dhparams into the SSL_CTX from an EVP_PKEY (in contrast
to the DH used with openssl < 3), the correct function is used.
  • Loading branch information
Andreas Weigel committed Nov 19, 2024
1 parent 22b2a7a commit 2041b85
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/security/ServerOptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,12 @@ Security::ServerOptions::loadDhParams()
parsedDhParams.resetWithoutLocking(dhp);

#else // OpenSSL 3.0+
const auto type = eecdhCurve.isEmpty() ? "DH" : "EC";
const auto type = "DH";

Ssl::ForgetErrors();
EVP_PKEY *rawPkey = nullptr;
using DecoderContext = std::unique_ptr<OSSL_DECODER_CTX, HardFun<void, OSSL_DECODER_CTX*, &OSSL_DECODER_CTX_free> >;
if (const DecoderContext dctx{OSSL_DECODER_CTX_new_for_pkey(&rawPkey, "PEM", nullptr, type, 0, nullptr, nullptr)}) {
if (const DecoderContext dctx{OSSL_DECODER_CTX_new_for_pkey(&rawPkey, "PEM", nullptr, type, OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS, nullptr, nullptr)}) {

// OpenSSL documentation is vague on this, but OpenSSL code and our
// tests suggest that rawPkey remains nil here while rawCtx keeps
Expand All @@ -414,7 +414,6 @@ Security::ServerOptions::loadDhParams()
assert(rawPkey);
const Security::DhePointer pkey(rawPkey);
// TODO: verify that the loaded parameters match the curve named in eecdhCurve

if (const Ssl::EVP_PKEY_CTX_Pointer pkeyCtx{EVP_PKEY_CTX_new_from_pkey(nullptr, pkey.get(), nullptr)}) {
switch (EVP_PKEY_param_check(pkeyCtx.get())) {
case 1: // success
Expand Down Expand Up @@ -566,9 +565,19 @@ Security::ServerOptions::updateContextEecdh(Security::ContextPointer &ctx)
// set DH parameters into the server context
#if USE_OPENSSL
if (parsedDhParams) {
SSL_CTX_set_tmp_dh(ctx.get(), parsedDhParams.get());
}
#if OPENSSL_VERSION_MAJOR < 3
if (1 != SSL_CTX_set_tmp_dh(ctx.get(), parsedDhParams.get())) {
debugs(83, DBG_IMPORTANT, "ERROR: failed to set DH parameters on SSL CTX " << Ssl::ReportAndForgetErrors);
}
#else
EVP_PKEY *tmp = EVP_PKEY_dup(parsedDhParams.get());
if (1 != SSL_CTX_set0_tmp_dh_pkey(ctx.get(), tmp)) {
EVP_PKEY_free(tmp);
debugs(83, DBG_IMPORTANT, "ERROR: failed to set DH parameters on SSL CTX " << Ssl::ReportAndForgetErrors);
}
#endif
}
#endif // USE_OPENSSL
}

void
Expand Down

0 comments on commit 2041b85

Please sign in to comment.