From 32d563c5708b2b195854ae0016533f8ea7e12b81 Mon Sep 17 00:00:00 2001 From: htuch Date: Tue, 27 Nov 2018 12:18:53 -0500 Subject: [PATCH] tls: plumbing for multiple TLS certificate ingestion. (#5095) This PR starts to plumb multiple TLS certs from the proto level into the SSL context. We stop short of enabling multiple TLS certificates, but instead have sufficient mechanism and interface changes to propagate them to the SSL context. Future PRs will extend this with the SSL context implementation. Risk Level: Low Testing: bazel test //test/... Part of #1319. Signed-off-by: Harvey Tuch Signed-off-by: Fred Douglas --- api/envoy/api/v2/auth/cert.proto | 9 ++--- include/envoy/ssl/context_config.h | 6 ++- source/common/ssl/context_config_impl.cc | 50 ++++++++++++++---------- source/common/ssl/context_config_impl.h | 16 +++++--- source/common/ssl/context_impl.cc | 7 ++-- test/common/ssl/context_impl_test.cc | 46 ++++++++++++++++++---- test/common/ssl/ssl_socket_test.cc | 4 +- 7 files changed, 92 insertions(+), 46 deletions(-) diff --git a/api/envoy/api/v2/auth/cert.proto b/api/envoy/api/v2/auth/cert.proto index cd1df6b3c347..c28152607b65 100644 --- a/api/envoy/api/v2/auth/cert.proto +++ b/api/envoy/api/v2/auth/cert.proto @@ -229,11 +229,10 @@ message CommonTlsContext { // Multiple TLS certificates can be associated with the same context. // E.g. to allow both RSA and ECDSA certificates, two TLS certificates can be configured. // - // .. attention:: - // - // Although this is a list, currently only a single certificate is supported. This will be - // relaxed in the future. - repeated TlsCertificate tls_certificates = 2 [(validate.rules).repeated .max_items = 1]; + // Only a single TLS certificate is supported in client contexts. In server contexts, the first + // RSA certificate is used for clients that only support RSA and the first ECDSA certificate is + // used for clients that support ECDSA. + repeated TlsCertificate tls_certificates = 2; // Configs for fetching TLS certificates via SDS API. repeated SdsSecretConfig tls_certificate_sds_secret_configs = 6; diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h index a7afb1aedf87..6647e7f4dd18 100644 --- a/include/envoy/ssl/context_config.h +++ b/include/envoy/ssl/context_config.h @@ -36,9 +36,11 @@ class ContextConfig { virtual const std::string& ecdhCurves() const PURE; /** - * @return TlsCertificateConfig the certificate config used to identify the local side. + * @return std::vector> TLS + * certificate configs. */ - virtual const TlsCertificateConfig* tlsCertificate() const PURE; + virtual std::vector> + tlsCertificates() const PURE; /** * @return CertificateValidationContextConfig the certificate validation context config. diff --git a/source/common/ssl/context_config_impl.cc b/source/common/ssl/context_config_impl.cc index eeb3acab8de9..baea5ab00409 100644 --- a/source/common/ssl/context_config_impl.cc +++ b/source/common/ssl/context_config_impl.cc @@ -10,7 +10,6 @@ #include "common/protobuf/utility.h" #include "common/secret/sds_api.h" #include "common/ssl/certificate_validation_context_config_impl.h" -#include "common/ssl/tls_certificate_config_impl.h" #include "openssl/ssl.h" @@ -19,23 +18,26 @@ namespace Ssl { namespace { -Secret::TlsCertificateConfigProviderSharedPtr getTlsCertificateConfigProvider( +std::vector getTlsCertificateConfigProviders( const envoy::api::v2::auth::CommonTlsContext& config, Server::Configuration::TransportSocketFactoryContext& factory_context) { if (!config.tls_certificates().empty()) { - const auto& tls_certificate = config.tls_certificates(0); - if (!tls_certificate.has_certificate_chain() && !tls_certificate.has_private_key()) { - return nullptr; + std::vector providers; + for (const auto& tls_certificate : config.tls_certificates()) { + if (!tls_certificate.has_certificate_chain() && !tls_certificate.has_private_key()) { + continue; + } + providers.push_back( + factory_context.secretManager().createInlineTlsCertificateProvider(tls_certificate)); } - return factory_context.secretManager().createInlineTlsCertificateProvider( - config.tls_certificates(0)); + return providers; } if (!config.tls_certificate_sds_secret_configs().empty()) { const auto& sds_secret_config = config.tls_certificate_sds_secret_configs(0); if (sds_secret_config.has_sds_config()) { // Fetch dynamic secret. - return factory_context.secretManager().findOrCreateTlsCertificateProvider( - sds_secret_config.sds_config(), sds_secret_config.name(), factory_context); + return {factory_context.secretManager().findOrCreateTlsCertificateProvider( + sds_secret_config.sds_config(), sds_secret_config.name(), factory_context)}; } else { // Load static secret. auto secret_provider = factory_context.secretManager().findStaticTlsCertificateProvider( @@ -43,10 +45,10 @@ Secret::TlsCertificateConfigProviderSharedPtr getTlsCertificateConfigProvider( if (!secret_provider) { throw EnvoyException(fmt::format("Unknown static secret: {}", sds_secret_config.name())); } - return secret_provider; + return {secret_provider}; } } - return nullptr; + return {}; } Secret::CertificateValidationContextConfigProviderSharedPtr @@ -126,7 +128,7 @@ ContextConfigImpl::ContextConfigImpl( RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"), DEFAULT_CIPHER_SUITES)), ecdh_curves_(StringUtil::nonEmptyStringOrDefault( RepeatedPtrUtil::join(config.tls_params().ecdh_curves(), ":"), DEFAULT_ECDH_CURVES)), - tls_certficate_provider_(getTlsCertificateConfigProvider(config, factory_context)), + tls_certficate_providers_(getTlsCertificateConfigProviders(config, factory_context)), certficate_validation_context_provider_( getCertificateValidationContextConfigProvider(config, factory_context, &default_cvc_)), min_protocol_version_( @@ -150,9 +152,12 @@ ContextConfigImpl::ContextConfigImpl( }); } // Load inline or static secret into tls_certificate_config_. - if (tls_certficate_provider_ != nullptr && tls_certficate_provider_->secret() != nullptr) { - tls_certificate_config_ = - std::make_unique(*tls_certficate_provider_->secret()); + if (!tls_certficate_providers_.empty()) { + for (auto& provider : tls_certficate_providers_) { + if (provider->secret() != nullptr) { + tls_certificate_configs_.emplace_back(*provider->secret()); + } + } } // Load inline or static secret into validation_context_config_. if (certficate_validation_context_provider_ != nullptr && @@ -170,17 +175,20 @@ Ssl::CertificateValidationContextConfigPtr ContextConfigImpl::getCombinedValidat } void ContextConfigImpl::setSecretUpdateCallback(std::function callback) { - if (tls_certficate_provider_) { + if (!tls_certficate_providers_.empty()) { if (tc_update_callback_handle_) { tc_update_callback_handle_->remove(); } // Once tls_certificate_config_ receives new secret, this callback updates // ContextConfigImpl::tls_certificate_config_ with new secret. - tc_update_callback_handle_ = tls_certficate_provider_->addUpdateCallback([this, callback]() { - tls_certificate_config_ = - std::make_unique(*tls_certficate_provider_->secret()); - callback(); - }); + tc_update_callback_handle_ = + tls_certficate_providers_[0]->addUpdateCallback([this, callback]() { + // This breaks multiple certificate support, but today SDS is only single cert. + // TODO(htuch): Fix this when SDS goes multi-cert. + tls_certificate_configs_.clear(); + tls_certificate_configs_.emplace_back(*tls_certficate_providers_[0]->secret()); + callback(); + }); } if (certficate_validation_context_provider_) { if (cvc_update_callback_handle_) { diff --git a/source/common/ssl/context_config_impl.h b/source/common/ssl/context_config_impl.h index 2914e91410f7..07fd2f8182ba 100644 --- a/source/common/ssl/context_config_impl.h +++ b/source/common/ssl/context_config_impl.h @@ -11,6 +11,7 @@ #include "common/common/empty_string.h" #include "common/json/json_loader.h" +#include "common/ssl/tls_certificate_config_impl.h" namespace Envoy { namespace Ssl { @@ -25,8 +26,13 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { const std::string& alpnProtocols() const override { return alpn_protocols_; } const std::string& cipherSuites() const override { return cipher_suites_; } const std::string& ecdhCurves() const override { return ecdh_curves_; } - const TlsCertificateConfig* tlsCertificate() const override { - return tls_certificate_config_.get(); + // TODO(htuch): This needs to be made const again and/or zero copy and/or callers fixed. + std::vector> tlsCertificates() const override { + std::vector> configs; + for (const auto& config : tls_certificate_configs_) { + configs.push_back(config); + } + return configs; } const CertificateValidationContextConfig* certificateValidationContext() const override { return validation_context_config_.get(); @@ -36,7 +42,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { bool isReady() const override { const bool tls_is_ready = - (tls_certficate_provider_ == nullptr || tls_certificate_config_ != nullptr); + (tls_certficate_providers_.empty() || !tls_certificate_configs_.empty()); const bool combined_cvc_is_ready = (default_cvc_ == nullptr || validation_context_config_ != nullptr); const bool cvc_is_ready = (certficate_validation_context_provider_ == nullptr || @@ -65,13 +71,13 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { const std::string cipher_suites_; const std::string ecdh_curves_; - Ssl::TlsCertificateConfigPtr tls_certificate_config_; + std::vector tls_certificate_configs_; Ssl::CertificateValidationContextConfigPtr validation_context_config_; // If certificate validation context type is combined_validation_context. default_cvc_ // holds a copy of CombinedCertificateValidationContext::default_validation_context. // Otherwise, default_cvc_ is nullptr. std::unique_ptr default_cvc_; - Secret::TlsCertificateConfigProviderSharedPtr tls_certficate_provider_; + std::vector tls_certficate_providers_; // Handle for TLS certificate dyanmic secret callback. Common::CallbackHandle* tc_update_callback_handle_{}; Secret::CertificateValidationContextConfigProviderSharedPtr diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index 6fd8e3230320..751f942796b7 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -182,9 +182,10 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const ContextConfig& config, TimeS SSL_CTX_set_cert_verify_callback(ctx_.get(), ContextImpl::verifyCallback, this); } - if (config.tlsCertificate() != nullptr) { + const auto tls_certificates = config.tlsCertificates(); + if (!tls_certificates.empty()) { // Load certificate chain. - const auto& tls_certificate = *config.tlsCertificate(); + const auto& tls_certificate = tls_certificates[0].get(); cert_chain_file_path_ = tls_certificate.certificateChainPath(); bssl::UniquePtr bio( BIO_new_mem_buf(const_cast(tls_certificate.certificateChain().data()), @@ -574,7 +575,7 @@ ServerContextImpl::ServerContextImpl(Stats::Scope& scope, const ServerContextCon const std::vector& server_names, TimeSource& time_source) : ContextImpl(scope, config, time_source), session_ticket_keys_(config.sessionTicketKeys()) { - if (config.tlsCertificate() == nullptr) { + if (config.tlsCertificates().empty()) { throw EnvoyException("Server TlsCertificates must have a certificate specified"); } if (config.certificateValidationContext() != nullptr && diff --git a/test/common/ssl/context_impl_test.cc b/test/common/ssl/context_impl_test.cc index 076d9317812c..3390f3747825 100644 --- a/test/common/ssl/context_impl_test.cc +++ b/test/common/ssl/context_impl_test.cc @@ -522,8 +522,16 @@ TEST(ClientContextConfigImplTest, InvalidCertificateSpki) { TEST(ClientContextConfigImplTest, MultipleTlsCertificates) { envoy::api::v2::auth::UpstreamTlsContext tls_context; NiceMock factory_context; - tls_context.mutable_common_tls_context()->add_tls_certificates(); - tls_context.mutable_common_tls_context()->add_tls_certificates(); + const std::string tls_certificate_yaml = R"EOF( + certificate_chain: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_key.pem" + )EOF"; + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); EXPECT_THROW_WITH_MESSAGE( ClientContextConfigImpl client_context_config(tls_context, factory_context), EnvoyException, "Multiple TLS certificates are not supported for client contexts"); @@ -534,7 +542,14 @@ TEST(ClientContextConfigImplTest, MultipleTlsCertificates) { TEST(ClientContextConfigImplTest, TlsCertificatesAndSdsConfig) { envoy::api::v2::auth::UpstreamTlsContext tls_context; NiceMock factory_context; - tls_context.mutable_common_tls_context()->add_tls_certificates(); + const std::string tls_certificate_yaml = R"EOF( + certificate_chain: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_key.pem" + )EOF"; + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs(); EXPECT_THROW_WITH_MESSAGE( ClientContextConfigImpl client_context_config(tls_context, factory_context), EnvoyException, @@ -636,10 +651,10 @@ name: "abc.com" const std::string cert_pem = "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_cert.pem"; EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(cert_pem)), - client_context_config.tlsCertificate()->certificateChain()); + client_context_config.tlsCertificates()[0].get().certificateChain()); const std::string key_pem = "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_key.pem"; EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(key_pem)), - client_context_config.tlsCertificate()->privateKey()); + client_context_config.tlsCertificates()[0].get().privateKey()); } // Validate that client context config with static certificate validation context is created @@ -763,8 +778,16 @@ TEST(ServerContextConfigImplTest, MultipleTlsCertificates) { EXPECT_THROW_WITH_MESSAGE( ServerContextConfigImpl client_context_config(tls_context, factory_context), EnvoyException, "No TLS certificates found for server context"); - tls_context.mutable_common_tls_context()->add_tls_certificates(); - tls_context.mutable_common_tls_context()->add_tls_certificates(); + const std::string tls_certificate_yaml = R"EOF( + certificate_chain: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_key.pem" + )EOF"; + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); EXPECT_THROW_WITH_MESSAGE( ServerContextConfigImpl client_context_config(tls_context, factory_context), EnvoyException, "A single TLS certificate is required for server contexts"); @@ -776,7 +799,14 @@ TEST(ServerContextConfigImplTest, TlsCertificatesAndSdsConfig) { EXPECT_THROW_WITH_MESSAGE( ServerContextConfigImpl server_context_config(tls_context, factory_context), EnvoyException, "No TLS certificates found for server context"); - tls_context.mutable_common_tls_context()->add_tls_certificates(); + const std::string tls_certificate_yaml = R"EOF( + certificate_chain: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/common/ssl/test_data/selfsigned_key.pem" + )EOF"; + MessageUtil::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), + *tls_context.mutable_common_tls_context()->add_tls_certificates()); tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs(); EXPECT_THROW_WITH_MESSAGE( ServerContextConfigImpl server_context_config(tls_context, factory_context), EnvoyException, diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index e667d6861ca9..5a2ec15e8eb5 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -3193,7 +3193,7 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { sds_secret_configs->set_name("abc.com"); sds_secret_configs->mutable_sds_config(); auto server_cfg = std::make_unique(tls_context, factory_context); - EXPECT_EQ(nullptr, server_cfg->tlsCertificate()); + EXPECT_TRUE(server_cfg->tlsCertificates().empty()); EXPECT_FALSE(server_cfg->isReady()); Event::SimulatedTimeSystem time_system; @@ -3234,7 +3234,7 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { sds_secret_configs->set_name("abc.com"); sds_secret_configs->mutable_sds_config(); auto client_cfg = std::make_unique(tls_context, factory_context); - EXPECT_EQ(nullptr, client_cfg->tlsCertificate()); + EXPECT_TRUE(client_cfg->tlsCertificates().empty()); EXPECT_FALSE(client_cfg->isReady()); ContextManagerImpl manager(time_system);