From caa5f7af34677514270488b8247df6cb7d224faa Mon Sep 17 00:00:00 2001 From: GiantCroc Date: Wed, 6 Sep 2023 08:36:27 +0800 Subject: [PATCH] tls: support fallback for private key provider (#232) * api: revert the private_key_provider_list and add fallback field to ProviderKeyProvider api (#29184) * Revert "api: introduce the private key provider list field (#28215)" This reverts commit b24ea1e75aea899d5106f2a10ddc8f3ef975fe20. Signed-off-by: He Jie Xu * Add fallback to PrivateKeyProvider Signed-off-by: He Jie Xu --------- Signed-off-by: He Jie Xu * tls: support fallback for private key provider Signed-off-by: Giantcroc * fix sgx Signed-off-by: Giantcroc --------- Signed-off-by: He Jie Xu Signed-off-by: Giantcroc Co-authored-by: Alex Xu --- .../transport_sockets/tls/v3/common.proto | 32 +++------------ changelogs/current.yaml | 8 ++++ .../source/cryptomb_private_key_provider.cc | 10 ++++- .../source/cryptomb_private_key_provider.h | 3 ++ .../private_key_providers/test/config_test.cc | 16 +++++--- .../qat/private_key_providers/source/qat.cc | 5 ++- .../qat/private_key_providers/source/qat.h | 3 ++ .../source/qat_private_key_provider.cc | 12 +++++- .../source/qat_private_key_provider.h | 2 + .../private_key_providers/test/config_test.cc | 17 ++++++-- .../private_key_providers/test/ops_test.cc | 6 +-- .../source/sgx_private_key_provider.cc | 1 + .../source/sgx_private_key_provider.h | 1 + envoy/ssl/private_key/private_key.h | 6 +++ .../common/ssl/tls_certificate_config_impl.cc | 22 +++++----- .../tls/context_impl_test.cc | 15 +++++-- .../transport_sockets/tls/ssl_socket_test.cc | 41 +++++++++++++++++++ .../tls/test_private_key_method_provider.cc | 9 ++++ .../tls/test_private_key_method_provider.h | 4 ++ test/mocks/ssl/mocks.h | 1 + 20 files changed, 155 insertions(+), 59 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index e031fa1baa..0d653050f5 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -178,23 +178,14 @@ message PrivateKeyProvider { oneof config_type { google.protobuf.Any typed_config = 3 [(udpa.annotations.sensitive) = true]; } -} -// [#not-implemented-hide:] -// Provides a list of private key providers. Envoy will find out an available private -// key provider from the list on order. If there is none of available private key provider, -// it may fallback to BoringSSL default implementation based on the `fallback` fallback. -message PrivateKeyProviderList { - // A list of private key providers, and at least one private key provider provided. - repeated PrivateKeyProvider private_key_provider = 1 [(validate.rules).repeated = {min_items: 1}]; - - // If there is no available private key provider from the list, Envoy will fallback to - // the BoringSSL default implementation when the `fallback` is true. The default value - // is `false`. - bool fallback = 2; + // If the private key provider isn't available (eg. the required hardware capability doesn't existed), + // Envoy will fallback to the BoringSSL default implementation when the `fallback` is true. + // The default value is `false`. + bool fallback = 4; } -// [#next-free-field: 10] +// [#next-free-field: 9] message TlsCertificate { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.auth.TlsCertificate"; @@ -249,19 +240,6 @@ message TlsCertificate { // error. PrivateKeyProvider private_key_provider = 6; - // [#not-implemented-hide:] - // This provides a list of BoringSSL private key method provider. Envoy will find out - // an available private key method provider. It may fallback to BoringSSL default implementation - // when there is no available one. All the private key provider will share the same private key - // in the :ref:`private_key ` field, - // so the :ref:`private_key ` field - // must be specified when the `proviate_key_provider_list` field is used. The old :ref:`private_key_provider - // ` field will be - // deprecated. If both :ref:`private_key_provider ` - // and `private_key_provider_list` are provided, the old - // :ref:`private_key_provider ` will be ignored. - PrivateKeyProviderList private_key_provider_list = 9; - // The password to decrypt the TLS private key. If this field is not set, it is assumed that the // TLS private key is not password encrypted. config.core.v3.DataSource password = 3 [(udpa.annotations.sensitive) = true]; diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 7a71d56693..ce3505ae31 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -31,5 +31,13 @@ removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` new_features: +- area: tls + change: | + added fallback :ref:`fallback + ` + to support private key provider to fallback to boringssl tls handshake. + If the private key provider isn't available (eg. the required hardware capability doesn't existed), + Envoy will fallback to the BoringSSL default implementation when the fallback is true. + The default value is false. deprecated: diff --git a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc index f94f37db63..82d1bd4a5a 100644 --- a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc +++ b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc @@ -499,6 +499,8 @@ bool CryptoMbPrivateKeyMethodProvider::checkFips() { return false; } +bool CryptoMbPrivateKeyMethodProvider::isAvailable() { return initialized_; } + Ssl::BoringSslPrivateKeyMethodSharedPtr CryptoMbPrivateKeyMethodProvider::getBoringSslPrivateKeyMethod() { return method_; @@ -522,7 +524,8 @@ CryptoMbPrivateKeyMethodProvider::CryptoMbPrivateKeyMethodProvider( stats_(generateCryptoMbStats("cryptomb", factory_context.statsScope())) { if (!ipp->mbxIsCryptoMbApplicable(0)) { - throw EnvoyException("Multi-buffer CPU instructions not available."); + ENVOY_LOG(warn, "Multi-buffer CPU instructions not available."); + return; } std::chrono::milliseconds poll_delay = @@ -565,7 +568,8 @@ CryptoMbPrivateKeyMethodProvider::CryptoMbPrivateKeyMethodProvider( key_size = 4096; break; default: - throw EnvoyException("Only RSA keys of 1024, 2048, 3072, and 4096 bits are supported."); + ENVOY_LOG(warn, "Only RSA keys of 1024, 2048, 3072, and 4096 bits are supported."); + return; } // If longer keys are ever supported, remember to change the signature buffer to be larger. @@ -619,6 +623,8 @@ CryptoMbPrivateKeyMethodProvider::CryptoMbPrivateKeyMethodProvider( ENVOY_LOG(debug, "Created CryptoMb Queue for thread {}", d.name()); return std::make_shared(poll_delay, key_type, key_size, ipp, d, stats_); }); + + initialized_ = true; } namespace { diff --git a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.h b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.h index 90d3efac06..cbd7a42d53 100644 --- a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.h +++ b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.h @@ -166,6 +166,7 @@ class CryptoMbPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodPro Event::Dispatcher& dispatcher) override; void unregisterPrivateKeyMethod(SSL* ssl) override; bool checkFips() override; + bool isAvailable() override; Ssl::BoringSslPrivateKeyMethodSharedPtr getBoringSslPrivateKeyMethod() override; static int connectionIndex(); @@ -191,6 +192,8 @@ class CryptoMbPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodPro ThreadLocal::TypedSlotPtr tls_; CryptoMbStats stats_; + + bool initialized_{}; }; } // namespace CryptoMb diff --git a/contrib/cryptomb/private_key_providers/test/config_test.cc b/contrib/cryptomb/private_key_providers/test/config_test.cc index 158936f490..0ac6ce5ff3 100644 --- a/contrib/cryptomb/private_key_providers/test/config_test.cc +++ b/contrib/cryptomb/private_key_providers/test/config_test.cc @@ -74,6 +74,7 @@ TEST_F(CryptoMbConfigTest, CreateRsa1024) { Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); EXPECT_NE(nullptr, provider); EXPECT_EQ(false, provider->checkFips()); + EXPECT_EQ(provider->isAvailable(), true); Ssl::BoringSslPrivateKeyMethodSharedPtr method = provider->getBoringSslPrivateKeyMethod(); EXPECT_NE(nullptr, method); @@ -96,7 +97,9 @@ TEST_F(CryptoMbConfigTest, CreateRsa2048) { private_key: { "filename": "{{ test_rundir }}/contrib/cryptomb/private_key_providers/test/test_data/rsa-2048.pem" } )EOF"; - EXPECT_NE(nullptr, createWithConfig(yaml)); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); } TEST_F(CryptoMbConfigTest, CreateRsa2048WithExponent3) { @@ -146,8 +149,9 @@ TEST_F(CryptoMbConfigTest, CreateRsa512) { private_key: { "filename": "{{ test_rundir }}/contrib/cryptomb/private_key_providers/test/test_data/rsa-512.pem" } )EOF"; - EXPECT_THROW_WITH_MESSAGE(createWithConfig(yaml), EnvoyException, - "Only RSA keys of 1024, 2048, 3072, and 4096 bits are supported."); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), false); } TEST_F(CryptoMbConfigTest, CreateEcdsaP256) { @@ -266,6 +270,7 @@ TEST_F(CryptoMbConfigTest, CreateOneMillisecondPollDelay) { Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); CryptoMbPrivateKeyMethodProvider* cryptomb_provider = dynamic_cast(provider.get()); EXPECT_EQ(cryptomb_provider->getPollDelayForTest(), std::chrono::microseconds(1000)); @@ -282,6 +287,7 @@ TEST_F(CryptoMbConfigTest, CreateTwoMillisecondPollDelay) { Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); CryptoMbPrivateKeyMethodProvider* cryptomb_provider = dynamic_cast(provider.get()); EXPECT_EQ(cryptomb_provider->getPollDelayForTest(), std::chrono::microseconds(2000)); @@ -309,8 +315,8 @@ TEST_F(CryptoMbConfigTest, CreateNotSupportedInstructionSet) { poll_delay: 0.02s )EOF"; - EXPECT_THROW_WITH_MESSAGE(createWithConfig(yaml, false), EnvoyException, - "Multi-buffer CPU instructions not available."); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml, false); + EXPECT_EQ(provider->isAvailable(), false); } } // namespace CryptoMb diff --git a/contrib/qat/private_key_providers/source/qat.cc b/contrib/qat/private_key_providers/source/qat.cc index 7acb4cbdcd..856dd978f7 100644 --- a/contrib/qat/private_key_providers/source/qat.cc +++ b/contrib/qat/private_key_providers/source/qat.cc @@ -17,7 +17,8 @@ QatManager::QatManager(LibQatCryptoSharedPtr libqat) { libqat_ = libqat; CpaStatus status = libqat_->icpSalUserStart("SSL"); if (status != CPA_STATUS_SUCCESS) { - throw EnvoyException("Failed to start QAT device."); + ENVOY_LOG(warn, "Failed to start QAT device."); + qat_is_supported = false; } } @@ -64,6 +65,8 @@ int QatManager::connectionIndex() { CONSTRUCT_ON_FIRST_USE(int, createIndex()); int QatManager::contextIndex() { CONSTRUCT_ON_FIRST_USE(int, createIndex()); } +bool QatManager::checkQatDevice() { return qat_is_supported; } + QatHandle::~QatHandle() { if (polling_thread_ == nullptr) { return; diff --git a/contrib/qat/private_key_providers/source/qat.h b/contrib/qat/private_key_providers/source/qat.h index 617b39aede..4837ced934 100644 --- a/contrib/qat/private_key_providers/source/qat.h +++ b/contrib/qat/private_key_providers/source/qat.h @@ -93,8 +93,11 @@ class QatManager : public std::enable_shared_from_this, static int connectionIndex(); static int contextIndex(); + bool checkQatDevice(); + private: LibQatCryptoSharedPtr libqat_{}; + bool qat_is_supported{true}; }; /** diff --git a/contrib/qat/private_key_providers/source/qat_private_key_provider.cc b/contrib/qat/private_key_providers/source/qat_private_key_provider.cc index 8636376041..b6a4b0b516 100644 --- a/contrib/qat/private_key_providers/source/qat_private_key_provider.cc +++ b/contrib/qat/private_key_providers/source/qat_private_key_provider.cc @@ -310,6 +310,7 @@ QatPrivateKeyMethodProvider::getBoringSslPrivateKeyMethod() { } bool QatPrivateKeyMethodProvider::checkFips() { return false; } +bool QatPrivateKeyMethodProvider::isAvailable() { return initialized_; } QatPrivateKeyConnection::QatPrivateKeyConnection(Ssl::PrivateKeyConnectionCallbacks& cb, Event::Dispatcher& dispatcher, QatHandle& handle, @@ -355,6 +356,10 @@ QatPrivateKeyMethodProvider::QatPrivateKeyMethodProvider( ASSERT(manager_); + if (!manager_->checkQatDevice()) { + return; + } + std::chrono::milliseconds poll_delay = std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(conf, poll_delay, 5)); @@ -370,13 +375,15 @@ QatPrivateKeyMethodProvider::QatPrivateKeyMethodProvider( if (EVP_PKEY_id(pkey.get()) != EVP_PKEY_RSA) { // TODO(ipuustin): add support also to ECDSA keys. - throw EnvoyException("Only RSA keys are supported."); + ENVOY_LOG(warn, "Only RSA keys are supported."); + return; } pkey_ = std::move(pkey); section_ = std::make_shared(libqat); if (!section_->startSection(api_, poll_delay)) { - throw EnvoyException("Failed to start QAT."); + ENVOY_LOG(warn, "Failed to start QAT."); + return; } method_ = std::make_shared(); @@ -384,6 +391,7 @@ QatPrivateKeyMethodProvider::QatPrivateKeyMethodProvider( method_->decrypt = privateKeyDecrypt; method_->complete = privateKeyComplete; + initialized_ = true; ENVOY_LOG(info, "initialized QAT private key provider"); } diff --git a/contrib/qat/private_key_providers/source/qat_private_key_provider.h b/contrib/qat/private_key_providers/source/qat_private_key_provider.h index bfaf4ac2d7..7636430233 100644 --- a/contrib/qat/private_key_providers/source/qat_private_key_provider.h +++ b/contrib/qat/private_key_providers/source/qat_private_key_provider.h @@ -47,6 +47,7 @@ class QatPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodProvider Event::Dispatcher& dispatcher) override; void unregisterPrivateKeyMethod(SSL* ssl) override; bool checkFips() override; + bool isAvailable() override; Ssl::BoringSslPrivateKeyMethodSharedPtr getBoringSslPrivateKeyMethod() override; private: @@ -56,6 +57,7 @@ class QatPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodProvider Api::Api& api_; bssl::UniquePtr pkey_; LibQatCryptoSharedPtr libqat_{}; + bool initialized_{}; }; } // namespace Qat diff --git a/contrib/qat/private_key_providers/test/config_test.cc b/contrib/qat/private_key_providers/test/config_test.cc index 518c9ba22d..9cbbafada6 100644 --- a/contrib/qat/private_key_providers/test/config_test.cc +++ b/contrib/qat/private_key_providers/test/config_test.cc @@ -87,6 +87,7 @@ TEST_F(QatConfigTest, CreateRsa1024) { Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); EXPECT_NE(nullptr, provider); EXPECT_EQ(false, provider->checkFips()); + EXPECT_EQ(provider->isAvailable(), true); Ssl::BoringSslPrivateKeyMethodSharedPtr method = provider->getBoringSslPrivateKeyMethod(); EXPECT_NE(nullptr, method); } @@ -100,7 +101,9 @@ TEST_F(QatConfigTest, CreateRsa2048) { private_key: { "filename": "{{ test_rundir }}/contrib/qat/private_key_providers/test/test_data/rsa-2048.pem" } )EOF"; - EXPECT_NE(nullptr, createWithConfig(yaml)); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); } TEST_F(QatConfigTest, CreateRsa3072) { @@ -112,7 +115,9 @@ TEST_F(QatConfigTest, CreateRsa3072) { private_key: { "filename": "{{ test_rundir }}/contrib/qat/private_key_providers/test/test_data/rsa-3072.pem" } )EOF"; - EXPECT_NE(nullptr, createWithConfig(yaml)); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); } TEST_F(QatConfigTest, CreateRsa4096) { @@ -124,7 +129,9 @@ TEST_F(QatConfigTest, CreateRsa4096) { private_key: { "filename": "{{ test_rundir }}/contrib/qat/private_key_providers/test/test_data/rsa-4096.pem" } )EOF"; - EXPECT_NE(nullptr, createWithConfig(yaml)); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), true); } TEST_F(QatConfigTest, CreateEcdsaP256) { @@ -136,7 +143,9 @@ TEST_F(QatConfigTest, CreateEcdsaP256) { private_key: { "filename": "{{ test_rundir }}/contrib/qat/private_key_providers/test/test_data/ecdsa-p256.pem" } )EOF"; - EXPECT_THROW_WITH_MESSAGE(createWithConfig(yaml), EnvoyException, "Only RSA keys are supported."); + Ssl::PrivateKeyMethodProviderSharedPtr provider = createWithConfig(yaml); + EXPECT_NE(nullptr, provider); + EXPECT_EQ(provider->isAvailable(), false); } TEST_F(QatConfigTest, CreateMissingPrivateKeyFile) { diff --git a/contrib/qat/private_key_providers/test/ops_test.cc b/contrib/qat/private_key_providers/test/ops_test.cc index 68922f08bf..3f61f68c63 100644 --- a/contrib/qat/private_key_providers/test/ops_test.cc +++ b/contrib/qat/private_key_providers/test/ops_test.cc @@ -241,9 +241,9 @@ TEST_F(QatProviderRsaTest, TestQatDeviceInit) { // no device found libqat_->icpSalUserStart_return_value_ = CPA_STATUS_FAIL; - EXPECT_THROW_WITH_REGEX( - std::make_shared(conf, factory_context_, libqat_), - EnvoyException, "Failed to start QAT device."); + Ssl::PrivateKeyMethodProviderSharedPtr provider = + std::make_shared(conf, factory_context_, libqat_); + EXPECT_EQ(provider->isAvailable(), false); delete private_key; } diff --git a/contrib/sgx/private_key_providers/source/sgx_private_key_provider.cc b/contrib/sgx/private_key_providers/source/sgx_private_key_provider.cc index e050d470d0..ba20e74d3f 100644 --- a/contrib/sgx/private_key_providers/source/sgx_private_key_provider.cc +++ b/contrib/sgx/private_key_providers/source/sgx_private_key_provider.cc @@ -263,6 +263,7 @@ void SgxPrivateKeyMethodProvider::registerPrivateKeyMethod(SSL* ssl, } bool SgxPrivateKeyMethodProvider::checkFips() { return true; } +bool SgxPrivateKeyMethodProvider::isAvailable() { return true; } Ssl::BoringSslPrivateKeyMethodSharedPtr SgxPrivateKeyMethodProvider::getBoringSslPrivateKeyMethod() { diff --git a/contrib/sgx/private_key_providers/source/sgx_private_key_provider.h b/contrib/sgx/private_key_providers/source/sgx_private_key_provider.h index c4222e6c4b..6a387fb71b 100644 --- a/contrib/sgx/private_key_providers/source/sgx_private_key_provider.h +++ b/contrib/sgx/private_key_providers/source/sgx_private_key_provider.h @@ -61,6 +61,7 @@ class SgxPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodProvider void unregisterPrivateKeyMethod(SSL* ssl) override; bool checkFips() override; + bool isAvailable() override; Ssl::BoringSslPrivateKeyMethodSharedPtr getBoringSslPrivateKeyMethod() override; diff --git a/envoy/ssl/private_key/private_key.h b/envoy/ssl/private_key/private_key.h index b1048a09c6..861df49516 100644 --- a/envoy/ssl/private_key/private_key.h +++ b/envoy/ssl/private_key/private_key.h @@ -51,6 +51,12 @@ class PrivateKeyMethodProvider { */ virtual bool checkFips() PURE; + /** + * Check whether the private key method is available. + * @return true if the private key method is available, false if not. + */ + virtual bool isAvailable() PURE; + #ifdef OPENSSL_IS_BORINGSSL /** * Get the private key methods from the provider. diff --git a/source/common/ssl/tls_certificate_config_impl.cc b/source/common/ssl/tls_certificate_config_impl.cc index 0d4c1dfb83..c170f72689 100644 --- a/source/common/ssl/tls_certificate_config_impl.cc +++ b/source/common/ssl/tls_certificate_config_impl.cc @@ -46,10 +46,6 @@ TlsCertificateConfigImpl::TlsCertificateConfigImpl( ocsp_staple_path_(Config::DataSource::getPath(config.ocsp_staple()) .value_or(ocsp_staple_.empty() ? EMPTY_STRING : INLINE_STRING)), private_key_method_(nullptr) { - if (config.has_private_key_provider() && config.has_private_key()) { - throw EnvoyException(fmt::format( - "Certificate configuration can't have both private_key and private_key_provider")); - } if (config.has_pkcs12()) { if (config.has_private_key()) { throw EnvoyException( @@ -69,20 +65,24 @@ TlsCertificateConfigImpl::TlsCertificateConfigImpl( factory_context.sslContextManager() .privateKeyMethodManager() .createPrivateKeyMethodProvider(config.private_key_provider(), factory_context); + if (private_key_method_ && !private_key_method_->isAvailable()) { + private_key_method_ = nullptr; + } } if (certificate_chain_.empty()) { throw EnvoyException( fmt::format("Failed to load incomplete certificate from {}: certificate chain not set", certificate_chain_path_)); } + if (config.has_private_key_provider() && !config.private_key_provider().fallback() && + private_key_method_ == nullptr) { + throw EnvoyException(fmt::format("Failed to load private key provider: {}", + config.private_key_provider().provider_name())); + } + if (private_key_.empty() && private_key_method_ == nullptr) { - if (config.has_private_key_provider()) { - throw EnvoyException(fmt::format("Failed to load private key provider: {}", - config.private_key_provider().provider_name())); - } else { - throw EnvoyException( - fmt::format("Failed to load incomplete private key from path: {}", private_key_path_)); - } + throw EnvoyException( + fmt::format("Failed to load incomplete private key from path: {}", private_key_path_)); } } } diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index c7865f2cc2..21e41400bc 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1986,6 +1986,7 @@ TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadFailureNoMethod) { .WillOnce(ReturnRef(private_key_method_manager)); EXPECT_CALL(private_key_method_manager, createPrivateKeyMethodProvider(_, _)) .WillOnce(Return(private_key_method_provider_ptr)); + EXPECT_CALL(*private_key_method_provider_ptr, isAvailable()).WillOnce(Return(true)); const std::string tls_context_yaml = R"EOF( common_tls_context: tls_certificates: @@ -2017,6 +2018,7 @@ TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadSuccess) { .WillOnce(ReturnRef(private_key_method_manager)); EXPECT_CALL(private_key_method_manager, createPrivateKeyMethodProvider(_, _)) .WillOnce(Return(private_key_method_provider_ptr)); + EXPECT_CALL(*private_key_method_provider_ptr, isAvailable()).WillOnce(Return(true)); const std::string tls_context_yaml = R"EOF( common_tls_context: tls_certificates: @@ -2033,12 +2035,18 @@ TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadSuccess) { ServerContextConfigImpl server_context_config(tls_context, factory_context_); } -TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadFailureBothKeyAndMethod) { +TEST_F(ServerContextConfigImplTest, PrivateKeyMethodFallback) { envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; NiceMock context_manager; NiceMock private_key_method_manager; auto private_key_method_provider_ptr = std::make_shared>(); + EXPECT_CALL(factory_context_, sslContextManager()).WillOnce(ReturnRef(context_manager)); + EXPECT_CALL(context_manager, privateKeyMethodManager()) + .WillOnce(ReturnRef(private_key_method_manager)); + EXPECT_CALL(private_key_method_manager, createPrivateKeyMethodProvider(_, _)) + .WillOnce(Return(private_key_method_provider_ptr)); + EXPECT_CALL(*private_key_method_provider_ptr, isAvailable()).WillOnce(Return(false)); const std::string tls_context_yaml = R"EOF( common_tls_context: tls_certificates: @@ -2052,11 +2060,10 @@ TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadFailureBothKeyAndMethod) "@type": type.googleapis.com/google.protobuf.Struct value: test_value: 100 + fallback: true )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(tls_context_yaml), tls_context); - EXPECT_THROW_WITH_MESSAGE( - ServerContextConfigImpl server_context_config(tls_context, factory_context_), EnvoyException, - "Certificate configuration can't have both private_key and private_key_provider"); + ServerContextConfigImpl server_context_config(tls_context, factory_context_); } // Test that if both typed and untyped matchers for sans are specified, we diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 8f59823d3c..f30a49954b 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -6312,6 +6312,47 @@ TEST_P(SslSocketTest, RsaPrivateKeyProviderSyncDecryptSuccess) { testUtil(successful_test_options.setPrivateKeyMethodExpected(true)); } +// Test fallback for key provider. +TEST_P(SslSocketTest, RsaPrivateKeyProviderFallbackSuccess) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + cipher_suites: + - TLS_RSA_WITH_AES_128_GCM_SHA256 + tls_certificates: + certificate_chain: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_key.pem" + private_key_provider: + provider_name: test + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct + value: + private_key_file: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_key.pem" + expected_operation: decrypt + sync_mode: true + mode: rsa + is_available: false + fallback: true + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" + crl: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.crl" +)EOF"; + const std::string successful_client_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + cipher_suites: + - TLS_RSA_WITH_AES_128_GCM_SHA256 +)EOF"; + + TestUtilOptions successful_test_options(successful_client_ctx_yaml, server_ctx_yaml, true, + version_); + testUtil(successful_test_options.setPrivateKeyMethodExpected(true)); +} + // Test asynchronous signing (ECDHE) failure (invalid signature). TEST_P(SslSocketTest, RsaPrivateKeyProviderAsyncSignFailure) { const std::string server_ctx_yaml = R"EOF( diff --git a/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc b/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc index 1f01175716..83e0ce46ed 100644 --- a/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc +++ b/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc @@ -257,6 +257,8 @@ bool TestPrivateKeyMethodProvider::checkFips() { return true; } +bool TestPrivateKeyMethodProvider::isAvailable() { return test_options_.is_available_; } + TestPrivateKeyConnection::TestPrivateKeyConnection( Ssl::PrivateKeyConnectionCallbacks& cb, Event::Dispatcher& dispatcher, bssl::UniquePtr pkey, TestPrivateKeyConnectionTestOptions& test_options) @@ -333,6 +335,9 @@ TestPrivateKeyMethodProvider::TestPrivateKeyMethodProvider( if (value_it.first == "method_error" && value.kind_case() == ProtobufWkt::Value::kBoolValue) { test_options_.method_error_ = value.bool_value(); } + if (value_it.first == "is_available" && value.kind_case() == ProtobufWkt::Value::kBoolValue) { + test_options_.is_available_ = value.bool_value(); + } if (value_it.first == "async_method_error" && value.kind_case() == ProtobufWkt::Value::kBoolValue) { test_options_.async_method_error_ = value.bool_value(); @@ -350,6 +355,10 @@ TestPrivateKeyMethodProvider::TestPrivateKeyMethodProvider( } } + if (!test_options_.is_available_) { + return; + } + std::string private_key = factory_context.serverFactoryContext().api().fileSystem().fileReadToEnd(private_key_path); bssl::UniquePtr bio( diff --git a/test/extensions/transport_sockets/tls/test_private_key_method_provider.h b/test/extensions/transport_sockets/tls/test_private_key_method_provider.h index efe94936d2..133d6a1d0a 100644 --- a/test/extensions/transport_sockets/tls/test_private_key_method_provider.h +++ b/test/extensions/transport_sockets/tls/test_private_key_method_provider.h @@ -31,6 +31,9 @@ struct TestPrivateKeyConnectionTestOptions { // Return an error from the private key method completion function. bool async_method_error_{}; + + // Return true if the private key method is available. + bool is_available_{true}; }; // An example private key method provider for testing the decrypt() and sign() @@ -68,6 +71,7 @@ class TestPrivateKeyMethodProvider : public virtual Ssl::PrivateKeyMethodProvide Event::Dispatcher& dispatcher) override; void unregisterPrivateKeyMethod(SSL* ssl) override; bool checkFips() override; + bool isAvailable() override; Ssl::BoringSslPrivateKeyMethodSharedPtr getBoringSslPrivateKeyMethod() override; static int rsaConnectionIndex(); diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index a6138922eb..fdcac0aa56 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -208,6 +208,7 @@ class MockPrivateKeyMethodProvider : public PrivateKeyMethodProvider { (SSL * ssl, PrivateKeyConnectionCallbacks& cb, Event::Dispatcher& dispatcher)); MOCK_METHOD(void, unregisterPrivateKeyMethod, (SSL * ssl)); MOCK_METHOD(bool, checkFips, ()); + MOCK_METHOD(bool, isAvailable, ()); #ifdef OPENSSL_IS_BORINGSSL MOCK_METHOD(BoringSslPrivateKeyMethodSharedPtr, getBoringSslPrivateKeyMethod, ());