From 43db37cd3a5e68838075a5b42e178243fc06a6f1 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 30 Jun 2026 06:13:46 -0500 Subject: [PATCH 1/9] Add unit tests for RSA certificate loading --- src/iocore/net/unit_tests/test_SSLDHParams.cc | 181 ++++++++++++++++-- src/iocore/net/unit_tests/unit_test_main.cc | 5 + 2 files changed, 172 insertions(+), 14 deletions(-) diff --git a/src/iocore/net/unit_tests/test_SSLDHParams.cc b/src/iocore/net/unit_tests/test_SSLDHParams.cc index 6a32a7458ba..54bf59a2830 100644 --- a/src/iocore/net/unit_tests/test_SSLDHParams.cc +++ b/src/iocore/net/unit_tests/test_SSLDHParams.cc @@ -1,9 +1,14 @@ /** @file - Catch based unit tests for the DH-parameter handling behavior of - SSLMultiCertConfigLoader::init_server_ssl_ctx, which is the inknet - public boundary that transitively invokes ssl_context_enable_dhe - and (when a file is configured) load_dhparams_file. + Catch based unit tests for two pieces of inknet SSL_CTX setup, each + exercised through its public SSLMultiCertConfigLoader boundary: + + * The DH-parameter handling of init_server_ssl_ctx, which transitively + invokes ssl_context_enable_dhe and (when a file is configured) + load_dhparams_file. + + * The private key handling of load_certs, which transitively invokes the + file-static SSLPrivateKeyHandler. @section license License @@ -38,14 +43,25 @@ #include #include #include +#include #include +#include #include +#include #include namespace { +std::string +bio_to_string(BIO *bio) +{ + BUF_MEM *bm = nullptr; + BIO_get_mem_ptr(bio, &bm); + return std::string{bm->data, bm->length}; +} + std::string make_valid_dh_pem() { @@ -63,30 +79,72 @@ make_valid_dh_pem() BIO *bio = BIO_new(BIO_s_mem()); REQUIRE(PEM_write_bio_Parameters(bio, pkey) == 1); - BUF_MEM *bm = nullptr; - BIO_get_mem_ptr(bio, &bm); - std::string out{bm->data, bm->length}; + std::string const out{bio_to_string(bio)}; BIO_free(bio); EVP_PKEY_free(pkey); EVP_PKEY_CTX_free(pctx); return out; } +// PEM-encodes pkey as a private key, optionally encrypting it with the given +// cipher and passphrase (cipher==nullptr leaves it unencrypted). std::string -make_rsa_pem() +key_to_pem(EVP_PKEY *pkey, EVP_CIPHER const *cipher, char *pass) { - EVP_PKEY *pkey = EVP_RSA_gen(2048); - REQUIRE(pkey != nullptr); BIO *bio = BIO_new(BIO_s_mem()); - REQUIRE(PEM_write_bio_PrivateKey(bio, pkey, nullptr, nullptr, 0, nullptr, nullptr) == 1); - BUF_MEM *bm = nullptr; - BIO_get_mem_ptr(bio, &bm); - std::string out{bm->data, bm->length}; + int passlen{pass ? static_cast(std::strlen(pass)) : 0}; + REQUIRE(PEM_write_bio_PrivateKey(bio, pkey, cipher, reinterpret_cast(pass), passlen, nullptr, nullptr) == 1); + std::string out{bio_to_string(bio)}; BIO_free(bio); + return out; +} + +std::string +make_rsa_pem() +{ + EVP_PKEY *pkey = EVP_RSA_gen(2048); + std::string const out{key_to_pem(pkey, nullptr, nullptr)}; EVP_PKEY_free(pkey); return out; } +// A self-signed certificate paired with the matching 2048-bit RSA private key, +// both PEM-encoded. Each call produces a fresh, independent key pair. When a +// cipher is given the key PEM is encrypted under the passphrase. +struct CertAndKey { + std::string cert_pem; + std::string key_pem; +}; + +CertAndKey +make_cert_and_key(EVP_CIPHER const *cipher = nullptr, char *pass = nullptr) +{ + EVP_PKEY *pkey = EVP_RSA_gen(2048); + REQUIRE(pkey != nullptr); + + X509 *x509 = X509_new(); + REQUIRE(x509 != nullptr); + ASN1_INTEGER_set(X509_get_serialNumber(x509), 1); + X509_gmtime_adj(X509_getm_notBefore(x509), 0); + X509_gmtime_adj(X509_getm_notAfter(x509), 60L * 60L * 24L * 365L); + REQUIRE(X509_set_pubkey(x509, pkey) == 1); + + X509_NAME *name = X509_get_subject_name(x509); + X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, reinterpret_cast("ats-test"), -1, -1, 0); + REQUIRE(X509_set_issuer_name(x509, name) == 1); + REQUIRE(X509_sign(x509, pkey, EVP_sha256()) > 0); + + BIO *cert_bio = BIO_new(BIO_s_mem()); + REQUIRE(PEM_write_bio_X509(cert_bio, x509) == 1); + std::string const cert_pem{bio_to_string(cert_bio)}; + BIO_free(cert_bio); + X509_free(x509); + + std::string const key_pem{key_to_pem(pkey, cipher, pass)}; + EVP_PKEY_free(pkey); + return {cert_pem, key_pem}; +} + class TempFile { public: @@ -140,6 +198,55 @@ init_with_dhparams(char const *dhparams_file) return ok; } +// A fixed-passphrase callback, matching how SSLPrivateKeyHandler consults the +// SSL_CTX default password callback to decrypt an encrypted private key. +char test_passphrase[]{"ats-secret-pass"}; + +int +fixed_passphrase_cb(char *buf, int size, int /* rwflag */, void * /* u */) +{ + int len{static_cast(std::strlen(test_passphrase))}; + if (len > size) { + len = size; + } + std::memcpy(buf, test_passphrase, len); + return len; +} + +// Drives SSLPrivateKeyHandler via the public static load_certs boundary, +// holding the certificate fixed and valid so the only variable under test is +// the private key material. The certificate and key are read from real files, +// exactly as a production ssl_multicert entry would be, so that the file-load +// path (load_rsa_pkey_from_file) is genuinely exercised. +// +// An empty key_path selects the "key bundled in the certificate file" branch, +// where the file load is skipped and the key is read from the certificate +// secret. A non-null passwd_cb is installed as the SSL_CTX default password +// callback, exactly as init_server_ssl_ctx's dialog setup would do for an +// encrypted key. +bool +load_key_via_load_certs(char const *cert_path, char const *key_path, pem_password_cb *passwd_cb = nullptr) +{ + SSLConfigParams params; + SSLMultiCertConfigParams settings; + settings.cert = ats_strdup(cert_path); + + SSLMultiCertConfigLoader::CertLoadData data; + data.cert_names_list.emplace_back(cert_path); + data.key_list.emplace_back(key_path); + + SSL_CTX *ctx = SSL_CTX_new(TLS_server_method()); + REQUIRE(ctx != nullptr); + if (passwd_cb != nullptr) { + SSL_CTX_set_default_passwd_cb(ctx, passwd_cb); + } + + bool ok = SSLMultiCertConfigLoader::load_certs(ctx, data.cert_names_list, data.key_list, data, ¶ms, &settings); + + SSL_CTX_free(ctx); + return ok; +} + } // namespace TEST_CASE("ssl_context_enable_dhe: nullptr dhparams file falls back to built-in DH parameters") @@ -184,3 +291,49 @@ TEST_CASE("ssl_context_enable_dhe: truncated DH PEM (missing END marker) is reje TempFile truncated{pem.substr(0, end)}; CHECK_FALSE(init_with_dhparams(truncated.get_path())); } + +TEST_CASE("SSLPrivateKeyHandler: a key file matching the certificate is loaded") +{ + CertAndKey ck = make_cert_and_key(); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an empty key path loads the key bundled in the certificate file") +{ + CertAndKey ck = make_cert_and_key(); + TempFile cert{ck.cert_pem + ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), "")); +} + +TEST_CASE("SSLPrivateKeyHandler: a valid key file not matching the certificate is rejected") +{ + TempFile cert{make_cert_and_key().cert_pem}; + TempFile key{make_cert_and_key().key_pem}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an unparseable key file is rejected") +{ + TempFile cert{make_cert_and_key().cert_pem}; + TempFile key{"-----BEGIN PRIVATE KEY-----\nnot base64\n-----END PRIVATE KEY-----\n"}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an encrypted key file is decrypted via the SSL_CTX password callback") +{ + CertAndKey ck = make_cert_and_key(EVP_aes_256_cbc(), test_passphrase); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), key.get_path(), fixed_passphrase_cb)); +} + +TEST_CASE("SSLPrivateKeyHandler: an encrypted key file with the wrong passphrase is rejected") +{ + char wrong_pass[]{"the-wrong-passphrase"}; + CertAndKey ck = make_cert_and_key(EVP_aes_256_cbc(), wrong_pass); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path(), fixed_passphrase_cb)); +} diff --git a/src/iocore/net/unit_tests/unit_test_main.cc b/src/iocore/net/unit_tests/unit_test_main.cc index 25b355f0b64..41b96438d98 100644 --- a/src/iocore/net/unit_tests/unit_test_main.cc +++ b/src/iocore/net/unit_tests/unit_test_main.cc @@ -23,6 +23,7 @@ #include "iocore/eventsystem/EventSystem.h" #include "../P_SSLConfig.h" +#include "api/LifecycleAPIHooks.h" #include "records/RecordsConfig.h" #include "tscore/BaseLogFile.h" #include "tscore/Diags.h" @@ -55,6 +56,10 @@ class EventProcessorListener final : public Catch::EventListenerBase RecProcessInit(); LibRecordsConfigInit(); + // SSLSecret::loadSecret consults the global lifecycle hooks for the + // SSL_SECRET hook, so they must be allocated before any secret is loaded. + init_global_lifecycle_hooks(); + ink_event_system_init(EVENT_SYSTEM_MODULE_PUBLIC_VERSION); eventProcessor.start(test_threads); From 7c761e3c6a96010c2f0b609d5d850b0ee8d1f7fc Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 30 Jun 2026 08:45:28 -0500 Subject: [PATCH 2/9] Switch OpenSSL APIs to avoid ENGINE use Fixes #13347 This patch changes the implementation of `SSLPrivateKeyHandler` to use `SSL_CTX_use_RSAPrivateKey_file` instead of `ENGINE_` APIs, since those APIs are deprecated in OpenSSL 3.x. --- src/iocore/net/SSLUtils.cc | 86 ++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 36 deletions(-) diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 9c865bbbf80..f54b817b175 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -853,46 +853,60 @@ SSLMultiCertConfigLoader::default_server_ssl_ctx() return SSL_CTX_new(SSLv23_server_method()); } +static bool +load_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath) +{ + ink_assert(keyPath && keyPath[0] != '\0'); + int const result{SSL_CTX_use_RSAPrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; + if (1 != result) { + char err_buf[256]{}; + ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf)); + Error("failed to load RSA key %s: %s", keyPath, err_buf); + } + return 1 == result; +} + +static bool +load_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) +{ + scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); + + pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); + void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); + EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); + if (nullptr == pkey) { + Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, + secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + return false; + } + if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { + Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", + (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + EVP_PKEY_free(pkey); + return false; + } + return true; +} + static bool SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) { - EVP_PKEY *pkey = nullptr; -#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY - ENGINE *e = ENGINE_get_default_RSA(); - if (e != nullptr) { - pkey = ENGINE_load_private_key(e, keyPath, nullptr, nullptr); - if (pkey) { - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key from engine"); - EVP_PKEY_free(pkey); - return false; - } - } + bool result{false}; + if (keyPath && keyPath[0] != '\0') { + result = load_rsa_pkey_from_file(ctx, keyPath); } -#else - void *e = nullptr; -#endif - if (pkey == nullptr) { - scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); - - pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); - void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); - pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); - if (nullptr == pkey) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, - secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - return false; - } - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", - (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - EVP_PKEY_free(pkey); - return false; - } - if (e == nullptr && !SSL_CTX_check_private_key(ctx)) { - Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); - return false; - } + + if (!result) { + result = load_rsa_pkey_from_secret_data(ctx, keyPath, secret_data, secret_data_len); + } + + if (!result) { + return false; + } + + if (!SSL_CTX_check_private_key(ctx)) { + Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); + return false; } return true; From 26602cf7ed390f31be7c9c4938b7ecfab6453ee1 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 30 Jun 2026 09:07:33 -0500 Subject: [PATCH 3/9] Clean up implementation * Rename `load_xxx` to `use_xxx` * Push `Dbg` message back to `SSLPrivateKeyHandler` * Move `use_xxx` functions to SSLKeyUtils.{h,cc} --- src/iocore/net/SSLKeyUtils.cc | 42 ++++++++++++++++++++++++++++++----- src/iocore/net/SSLKeyUtils.h | 3 +++ src/iocore/net/SSLUtils.cc | 41 ++++------------------------------ 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index 4bf14c5f473..f94b5ec25ee 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -29,16 +29,17 @@ #include #endif -#ifdef OPENSSL_IS_OPENSSL3 +#include +#include #include +#include +#include + +#ifdef OPENSSL_IS_OPENSSL3 #include #include -#include #else -#include #include -#include -#include #endif #ifdef OPENSSL_IS_OPENSSL3 @@ -188,3 +189,34 @@ set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey) } #endif // OPENSSL_IS_OPENSSL3 + +bool +use_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath) +{ + ink_assert(keyPath && keyPath[0] != '\0'); + int const result{SSL_CTX_use_RSAPrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; + if (1 != result) { + char err_buf[256]{}; + ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf)); + Error("failed to load RSA key %s: %s", keyPath, err_buf); + } + return 1 == result; +} + +bool +use_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len) +{ + scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); + + pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); + void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); + EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); + if (nullptr == pkey) { + return false; + } + if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { + EVP_PKEY_free(pkey); + return false; + } + return true; +} diff --git a/src/iocore/net/SSLKeyUtils.h b/src/iocore/net/SSLKeyUtils.h index 64bea23e9ea..ffcd3446eb9 100644 --- a/src/iocore/net/SSLKeyUtils.h +++ b/src/iocore/net/SSLKeyUtils.h @@ -40,3 +40,6 @@ dh_key_t *load_dhparams_file(char const *dhparams_file); // Takes ownership of pkey. bool set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey); + +bool use_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath); +bool use_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index f54b817b175..5336542e271 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -853,54 +853,21 @@ SSLMultiCertConfigLoader::default_server_ssl_ctx() return SSL_CTX_new(SSLv23_server_method()); } -static bool -load_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath) -{ - ink_assert(keyPath && keyPath[0] != '\0'); - int const result{SSL_CTX_use_RSAPrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; - if (1 != result) { - char err_buf[256]{}; - ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf)); - Error("failed to load RSA key %s: %s", keyPath, err_buf); - } - return 1 == result; -} - -static bool -load_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) -{ - scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); - - pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); - void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); - EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); - if (nullptr == pkey) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, - secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - return false; - } - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", - (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - EVP_PKEY_free(pkey); - return false; - } - return true; -} - static bool SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) { bool result{false}; if (keyPath && keyPath[0] != '\0') { - result = load_rsa_pkey_from_file(ctx, keyPath); + result = use_rsa_pkey_from_file(ctx, keyPath); } if (!result) { - result = load_rsa_pkey_from_secret_data(ctx, keyPath, secret_data, secret_data_len); + result = use_rsa_pkey_from_secret_data(ctx, secret_data, secret_data_len); } if (!result) { + Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, + secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); return false; } From ce220e5ea59447825f831e7499805c74b06536d4 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 29 Jul 2026 07:12:09 -0500 Subject: [PATCH 4/9] Fix issues found by Copilot --- src/iocore/net/SSLKeyUtils.cc | 12 ++++++------ src/iocore/net/SSLKeyUtils.h | 4 ++-- src/iocore/net/SSLUtils.cc | 4 ++-- src/iocore/net/unit_tests/test_SSLDHParams.cc | 7 +++++-- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index f94b5ec25ee..ddf7154e3c0 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -23,9 +23,8 @@ #include "P_SSLUtils.h" #include -#ifdef OPENSSL_IS_OPENSSL3 #include -#else +#ifndef OPENSSL_IS_OPENSSL3 #include #endif @@ -191,20 +190,20 @@ set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey) #endif // OPENSSL_IS_OPENSSL3 bool -use_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath) +use_pkey_from_file(SSL_CTX *ctx, const char *keyPath) { ink_assert(keyPath && keyPath[0] != '\0'); - int const result{SSL_CTX_use_RSAPrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; + int const result{SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; if (1 != result) { char err_buf[256]{}; ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf)); - Error("failed to load RSA key %s: %s", keyPath, err_buf); + Error("failed to load private key %s: %s", keyPath, err_buf); } return 1 == result; } bool -use_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len) +use_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len) { scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); @@ -218,5 +217,6 @@ use_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_ EVP_PKEY_free(pkey); return false; } + EVP_PKEY_free(pkey); return true; } diff --git a/src/iocore/net/SSLKeyUtils.h b/src/iocore/net/SSLKeyUtils.h index ffcd3446eb9..b207ef10892 100644 --- a/src/iocore/net/SSLKeyUtils.h +++ b/src/iocore/net/SSLKeyUtils.h @@ -41,5 +41,5 @@ dh_key_t *load_dhparams_file(char const *dhparams_file); // Takes ownership of pkey. bool set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey); -bool use_rsa_pkey_from_file(SSL_CTX *ctx, const char *keyPath); -bool use_rsa_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len); +bool use_pkey_from_file(SSL_CTX *ctx, const char *keyPath); +bool use_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 5336542e271..3313a1e029f 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -858,11 +858,11 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, { bool result{false}; if (keyPath && keyPath[0] != '\0') { - result = use_rsa_pkey_from_file(ctx, keyPath); + result = use_pkey_from_file(ctx, keyPath); } if (!result) { - result = use_rsa_pkey_from_secret_data(ctx, secret_data, secret_data_len); + result = use_pkey_from_secret_data(ctx, secret_data, secret_data_len); } if (!result) { diff --git a/src/iocore/net/unit_tests/test_SSLDHParams.cc b/src/iocore/net/unit_tests/test_SSLDHParams.cc index 54bf59a2830..728b8268c13 100644 --- a/src/iocore/net/unit_tests/test_SSLDHParams.cc +++ b/src/iocore/net/unit_tests/test_SSLDHParams.cc @@ -58,7 +58,7 @@ std::string bio_to_string(BIO *bio) { BUF_MEM *bm = nullptr; - BIO_get_mem_ptr(bio, &bm); + REQUIRE(1 == BIO_get_mem_ptr(bio, &bm)); return std::string{bm->data, bm->length}; } @@ -78,6 +78,7 @@ make_valid_dh_pem() REQUIRE(EVP_PKEY_generate(pctx, &pkey) > 0); BIO *bio = BIO_new(BIO_s_mem()); + REQUIRE(bio != nullptr); REQUIRE(PEM_write_bio_Parameters(bio, pkey) == 1); std::string const out{bio_to_string(bio)}; BIO_free(bio); @@ -92,7 +93,8 @@ std::string key_to_pem(EVP_PKEY *pkey, EVP_CIPHER const *cipher, char *pass) { BIO *bio = BIO_new(BIO_s_mem()); - int passlen{pass ? static_cast(std::strlen(pass)) : 0}; + REQUIRE(bio != nullptr); + int passlen{pass ? static_cast(std::strlen(pass)) : 0}; REQUIRE(PEM_write_bio_PrivateKey(bio, pkey, cipher, reinterpret_cast(pass), passlen, nullptr, nullptr) == 1); std::string out{bio_to_string(bio)}; BIO_free(bio); @@ -135,6 +137,7 @@ make_cert_and_key(EVP_CIPHER const *cipher = nullptr, char *pass = nullptr) REQUIRE(X509_sign(x509, pkey, EVP_sha256()) > 0); BIO *cert_bio = BIO_new(BIO_s_mem()); + REQUIRE(cert_bio != nullptr); REQUIRE(PEM_write_bio_X509(cert_bio, x509) == 1); std::string const cert_pem{bio_to_string(cert_bio)}; BIO_free(cert_bio); From 89f0b974b58f0f4d197573a295c1832fdb961ffa Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 29 Jul 2026 11:13:35 -0500 Subject: [PATCH 5/9] Do not print ERROR for keyfile load failure --- src/iocore/net/SSLKeyUtils.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index ddf7154e3c0..32caeffd149 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -194,11 +194,6 @@ use_pkey_from_file(SSL_CTX *ctx, const char *keyPath) { ink_assert(keyPath && keyPath[0] != '\0'); int const result{SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; - if (1 != result) { - char err_buf[256]{}; - ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf)); - Error("failed to load private key %s: %s", keyPath, err_buf); - } return 1 == result; } From 7f9e8742e4bb812379164758152ade54588c5b25 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 30 Jul 2026 09:31:11 -0500 Subject: [PATCH 6/9] Restore ENGINE use --- src/iocore/net/SSLKeyUtils.cc | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index 32caeffd149..7edbaec13d9 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -24,9 +24,7 @@ #include #include -#ifndef OPENSSL_IS_OPENSSL3 #include -#endif #include #include @@ -34,6 +32,10 @@ #include #include +#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY +#include +#endif + #ifdef OPENSSL_IS_OPENSSL3 #include #include @@ -193,7 +195,32 @@ bool use_pkey_from_file(SSL_CTX *ctx, const char *keyPath) { ink_assert(keyPath && keyPath[0] != '\0'); + +#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY + // A key held by an HSM or other hardware store is named rather than stored: + // keyPath is a key identifier the engine resolves, and there is no file to + // read. Ask the engine first so that a configured device takes precedence over + // any same-named file on disk. Absent a configured engine there is nothing to + // ask, and keyPath is left to the file load below. + if (ENGINE *e = ENGINE_get_default_RSA(); e != nullptr) { + EVP_PKEY *pkey = ENGINE_load_private_key(e, keyPath, nullptr, nullptr); + + if (pkey != nullptr) { + bool const result{1 == SSL_CTX_use_PrivateKey(ctx, pkey)}; + + EVP_PKEY_free(pkey); + if (result) { + return true; + } + } + // Not finding the key in hardware is the ordinary case for a file-based + // configuration, so leave no errors behind for the caller to misread. + ERR_clear_error(); + } +#endif + int const result{SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; + return 1 == result; } From c1859605c11807100d929e051d30c61b87af84b5 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 31 Jul 2026 07:47:09 -0500 Subject: [PATCH 7/9] Remove production code changes --- src/iocore/net/SSLUtils.cc | 53 ++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 3313a1e029f..9c865bbbf80 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -856,24 +856,43 @@ SSLMultiCertConfigLoader::default_server_ssl_ctx() static bool SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) { - bool result{false}; - if (keyPath && keyPath[0] != '\0') { - result = use_pkey_from_file(ctx, keyPath); - } - - if (!result) { - result = use_pkey_from_secret_data(ctx, secret_data, secret_data_len); - } - - if (!result) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, - secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - return false; + EVP_PKEY *pkey = nullptr; +#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY + ENGINE *e = ENGINE_get_default_RSA(); + if (e != nullptr) { + pkey = ENGINE_load_private_key(e, keyPath, nullptr, nullptr); + if (pkey) { + if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { + Dbg(dbg_ctl_ssl_load, "failed to load server private key from engine"); + EVP_PKEY_free(pkey); + return false; + } + } } - - if (!SSL_CTX_check_private_key(ctx)) { - Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); - return false; +#else + void *e = nullptr; +#endif + if (pkey == nullptr) { + scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); + + pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); + void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); + pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); + if (nullptr == pkey) { + Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, + secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + return false; + } + if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { + Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", + (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + EVP_PKEY_free(pkey); + return false; + } + if (e == nullptr && !SSL_CTX_check_private_key(ctx)) { + Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); + return false; + } } return true; From 9a2d94a62e4f8f3b0cff6e7ad0355a46e42b7139 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 31 Jul 2026 07:48:13 -0500 Subject: [PATCH 8/9] Remove other production code changes --- src/iocore/net/SSLKeyUtils.cc | 68 ++++------------------------------- src/iocore/net/SSLKeyUtils.h | 3 -- 2 files changed, 7 insertions(+), 64 deletions(-) diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index 7edbaec13d9..4bf14c5f473 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -23,24 +23,22 @@ #include "P_SSLUtils.h" #include +#ifdef OPENSSL_IS_OPENSSL3 #include +#else #include - -#include -#include -#include -#include -#include - -#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY -#include #endif #ifdef OPENSSL_IS_OPENSSL3 +#include #include #include +#include #else +#include #include +#include +#include #endif #ifdef OPENSSL_IS_OPENSSL3 @@ -190,55 +188,3 @@ set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey) } #endif // OPENSSL_IS_OPENSSL3 - -bool -use_pkey_from_file(SSL_CTX *ctx, const char *keyPath) -{ - ink_assert(keyPath && keyPath[0] != '\0'); - -#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY - // A key held by an HSM or other hardware store is named rather than stored: - // keyPath is a key identifier the engine resolves, and there is no file to - // read. Ask the engine first so that a configured device takes precedence over - // any same-named file on disk. Absent a configured engine there is nothing to - // ask, and keyPath is left to the file load below. - if (ENGINE *e = ENGINE_get_default_RSA(); e != nullptr) { - EVP_PKEY *pkey = ENGINE_load_private_key(e, keyPath, nullptr, nullptr); - - if (pkey != nullptr) { - bool const result{1 == SSL_CTX_use_PrivateKey(ctx, pkey)}; - - EVP_PKEY_free(pkey); - if (result) { - return true; - } - } - // Not finding the key in hardware is the ordinary case for a file-based - // configuration, so leave no errors behind for the caller to misread. - ERR_clear_error(); - } -#endif - - int const result{SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM)}; - - return 1 == result; -} - -bool -use_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len) -{ - scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); - - pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); - void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); - EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); - if (nullptr == pkey) { - return false; - } - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - EVP_PKEY_free(pkey); - return false; - } - EVP_PKEY_free(pkey); - return true; -} diff --git a/src/iocore/net/SSLKeyUtils.h b/src/iocore/net/SSLKeyUtils.h index b207ef10892..64bea23e9ea 100644 --- a/src/iocore/net/SSLKeyUtils.h +++ b/src/iocore/net/SSLKeyUtils.h @@ -40,6 +40,3 @@ dh_key_t *load_dhparams_file(char const *dhparams_file); // Takes ownership of pkey. bool set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey); - -bool use_pkey_from_file(SSL_CTX *ctx, const char *keyPath); -bool use_pkey_from_secret_data(SSL_CTX *ctx, const char *secret_data, int secret_data_len); From 32382c35725166d416e3eb69eea1f6fe6c7aa744 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 31 Jul 2026 08:04:23 -0500 Subject: [PATCH 9/9] Check `EVP_RSA_gen` return value for `nullptr` --- src/iocore/net/unit_tests/test_SSLDHParams.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/iocore/net/unit_tests/test_SSLDHParams.cc b/src/iocore/net/unit_tests/test_SSLDHParams.cc index 728b8268c13..3e75f5fe097 100644 --- a/src/iocore/net/unit_tests/test_SSLDHParams.cc +++ b/src/iocore/net/unit_tests/test_SSLDHParams.cc @@ -104,7 +104,8 @@ key_to_pem(EVP_PKEY *pkey, EVP_CIPHER const *cipher, char *pass) std::string make_rsa_pem() { - EVP_PKEY *pkey = EVP_RSA_gen(2048); + EVP_PKEY *pkey = EVP_RSA_gen(2048); + REQUIRE(pkey != nullptr); std::string const out{key_to_pem(pkey, nullptr, nullptr)}; EVP_PKEY_free(pkey); return out;