Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/iocore/net/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ if(BUILD_TESTING)
unit_tests/benchmark_TLSCertCompression.cc
)
if(SSLLIB_IS_OPENSSL3)
target_sources(test_net PRIVATE unit_tests/test_SSLDHParams.cc)
target_sources(test_net PRIVATE unit_tests/test_SSLDHParams.cc unit_tests/MockHardwareProvider.cc)
endif()
# Use link groups to solve circular dependency
set(LINK_GROUP_LIBS
Expand Down
38 changes: 38 additions & 0 deletions src/iocore/net/P_SSLUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#ifdef OPENSSL_IS_OPENSSL3
#include <openssl/decoder.h>
#include <openssl/evp.h>
#include <openssl/store.h>
#include <openssl/ui.h>
#endif
#define OPENSSL_THREAD_DEFINES
#if __has_include(<openssl/opensslconf.h>)
Expand Down Expand Up @@ -130,6 +132,38 @@ namespace detail
OSSL_DECODER_CTX_free(dctx);
}
};

struct StoreCTXDeleter {
void
operator()(OSSL_STORE_CTX *sctx)
{
OSSL_STORE_close(sctx);
}
};

struct StoreInfoDeleter {
void
operator()(OSSL_STORE_INFO *info)
{
OSSL_STORE_INFO_free(info);
}
};

struct PKEYDeleter {
void
operator()(EVP_PKEY *pkey)
{
EVP_PKEY_free(pkey);
}
};

struct UIMethodDeleter {
void
operator()(UI_METHOD *ui)
{
UI_destroy_method(ui);
}
};
#endif

} // namespace detail
Expand Down Expand Up @@ -159,4 +193,8 @@ using scoped_BIO = std::unique_ptr<BIO, ssl::detail::BIODeleter>;
#ifdef OPENSSL_IS_OPENSSL3
using scoped_PKEY_CTX = std::unique_ptr<EVP_PKEY_CTX, ssl::detail::PKEYCTXDeleter>;
using scoped_Decoder_CTX = std::unique_ptr<OSSL_DECODER_CTX, ssl::detail::DecoderCTXDeleter>;
using scoped_Store_CTX = std::unique_ptr<OSSL_STORE_CTX, ssl::detail::StoreCTXDeleter>;
using scoped_Store_Info = std::unique_ptr<OSSL_STORE_INFO, ssl::detail::StoreInfoDeleter>;
using scoped_PKEY = std::unique_ptr<EVP_PKEY, ssl::detail::PKEYDeleter>;
using scoped_UI_Method = std::unique_ptr<UI_METHOD, ssl::detail::UIMethodDeleter>;
#endif
128 changes: 121 additions & 7 deletions src/iocore/net/SSLKeyUtils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,26 @@
#include "P_SSLUtils.h"

#include <tscore/Diags.h>
#ifdef OPENSSL_IS_OPENSSL3
#include <tscore/ink_assert.h>
#else
#include <tscore/ink_config.h>

#include <openssl/bio.h>
#include <openssl/err.h>
#include <openssl/evp.h>
#include <openssl/pem.h>
#include <openssl/ssl.h>

#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY
#include <openssl/engine.h>
#endif

#ifdef OPENSSL_IS_OPENSSL3
#include <openssl/evp.h>
#include <openssl/decoder.h>
#include <openssl/params.h>
#include <openssl/ssl.h>
#include <openssl/store.h>
#include <openssl/ui.h>
#else
#include <openssl/bio.h>
#include <openssl/bn.h>
#include <openssl/pem.h>
#include <openssl/ssl.h>
#endif

#ifdef OPENSSL_IS_OPENSSL3
Expand Down Expand Up @@ -188,3 +192,113 @@ set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey)
}

#endif // OPENSSL_IS_OPENSSL3

// A hardware-backed key is named rather than stored: the name resolves through
// the device and there is no file holding the key. Both versions below therefore
// ask the hardware before falling back to reading keyPath as a file, which is
// also the order ATS used when this was ENGINE-only, so that a configured device
// keeps precedence over any same-named file on disk.

#ifdef OPENSSL_IS_OPENSSL3

bool
use_pkey_from_file(SSL_CTX *ctx, const char *keyPath)
{
ink_assert(keyPath && keyPath[0] != '\0');

// A store prompts for a PIN or passphrase through a UI_METHOD rather than the
// pem_password_cb an SSL_CTX carries, so wrap the configured callback. The
// wrapper invokes the callback unconditionally once prompted, so leave the
// UI_METHOD null when there is nothing to wrap rather than hand it a null
// callback to call.
pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx);
scoped_UI_Method ui{password_cb ? UI_UTIL_wrap_read_pem_callback(password_cb, 0) : nullptr};

// OpenSSL 3 reaches a hardware key store through a provider, which exposes
// its keys as an OSSL_STORE under its own URI scheme -- "pkcs11:" for a
// PKCS#11 provider fronting an HSM, for instance.
scoped_Store_CTX store{OSSL_STORE_open(keyPath, ui.get(), SSL_CTX_get_default_passwd_cb_userdata(ctx), nullptr, nullptr)};

if (store) {
// Announcing the one type wanted lets a loader retrieve it directly rather
// than enumerate the whole store. It does not make a single load sufficient:
// a loader that ignores the hint still yields other objects first, which the
// generic layer drops by returning nullptr without reaching EOF.
OSSL_STORE_expect(store.get(), OSSL_STORE_INFO_PKEY);
}

// A store URI may name a whole collection -- a token's worth of objects, of
// which only some are keys -- so scan until a private key turns up.
scoped_PKEY pkey;
while (store && !pkey && !OSSL_STORE_eof(store.get())) {
scoped_Store_Info info{OSSL_STORE_load(store.get())};
if (info && OSSL_STORE_INFO_get_type(info.get()) == OSSL_STORE_INFO_PKEY) {
pkey.reset(OSSL_STORE_INFO_get1_PKEY(info.get()));
}
// A single object failing to load is not fatal; OSSL_STORE_eof reports true
// once the store itself gives up, so the loop terminates either way.
}

if (pkey && SSL_CTX_use_PrivateKey(ctx, pkey.get())) {
return true;
}

// OSSL_STORE also handles plain paths, but only for keys it can decode
// itself, so getting here says nothing about whether keyPath names a loadable
// file. 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();

return 1 == SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM);
}

#else

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
// Before providers, a hardware key store was reached through an ENGINE.
// Absent a configured engine there is nothing to ask.
if (ENGINE *e = ENGINE_get_default_RSA(); e != nullptr) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Release the functional ENGINE reference

ENGINE_get_default_RSA() returns an incremented functional reference that must be released with ENGINE_finish() before it is discarded. Neither the successful early return nor the failure/fallback path releases e, so repeated certificate loads and configuration reloads leak ENGINE references. Please manage this with an RAII deleter or explicitly finish the ENGINE on every path.

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

return 1 == SSL_CTX_use_PrivateKey_file(ctx, keyPath, SSL_FILETYPE_PEM);
}

#endif // OPENSSL_IS_OPENSSL3

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;
}
3 changes: 3 additions & 0 deletions src/iocore/net/SSLKeyUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_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);
53 changes: 17 additions & 36 deletions src/iocore/net/SSLUtils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -856,43 +856,24 @@ SSLMultiCertConfigLoader::default_server_ssl_ctx()
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') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep SSL-secret plugin data authoritative

Trying keyPath before the secret_data returned by SSLSecret::getOrLoadSecret() reverses the documented TS_LIFECYCLE_SSL_SECRET_HOOK contract, where plugin data replaces disk loading. If the configured path contains a stale but parseable key, this helper succeeds and skips the plugin key; the later certificate check can then fail without ever trying the matching secret. This also defeats consistent updates made through TSSslSecretSet(). Please retain secret-data precedence for ordinary file names and use the path-first store lookup only for actual provider URIs.

result = use_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 = 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;
}

if (!SSL_CTX_check_private_key(ctx)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not require this comparison for opaque hardware keys

The previous e == nullptr guard was deliberate: hardware-backed ENGINE keys, including HSM/TPM implementations, may support signing without exposing enough key material for SSL_CTX_check_private_key() to compare them. Making the check unconditional rejects those keys in the retained pre-OpenSSL-3 ENGINE path and can likewise reject opaque provider keys. The mock provider does not cover this because it returns an ordinary exportable PEM key. Please preserve the hardware-key distinction or use a validation path that provider/ENGINE-backed opaque keys can support.

Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key");
return false;
}

return true;
Expand Down
Loading