diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index af2b1c3fb002..db382082b84c 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -128,6 +128,9 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider return nullptr; } Common::CallbackHandle* addUpdateCallback(std::function callback) override { + if (secret()) { + callback(); + } return update_callback_manager_.add(callback); } @@ -180,6 +183,9 @@ class CertificateValidationContextSdsApi : public SdsApi, return certificate_validation_context_secrets_.get(); } Common::CallbackHandle* addUpdateCallback(std::function callback) override { + if (secret()) { + callback(); + } return update_callback_manager_.add(callback); } @@ -247,6 +253,9 @@ class TlsSessionTicketKeysSdsApi : public SdsApi, public TlsSessionTicketKeysCon } Common::CallbackHandle* addUpdateCallback(std::function callback) override { + if (secret()) { + callback(); + } return update_callback_manager_.add(callback); } diff --git a/source/extensions/transport_sockets/tls/context_config_impl.cc b/source/extensions/transport_sockets/tls/context_config_impl.cc index 49f5a4811a4a..36bbb867c33e 100644 --- a/source/extensions/transport_sockets/tls/context_config_impl.cc +++ b/source/extensions/transport_sockets/tls/context_config_impl.cc @@ -166,21 +166,33 @@ ContextConfigImpl::ContextConfigImpl( default_min_protocol_version)), max_protocol_version_(tlsVersionFromProto(config.tls_params().tls_maximum_protocol_version(), default_max_protocol_version)) { - if (default_cvc_ && certificate_validation_context_provider_ != nullptr) { - // We need to validate combined certificate validation context. - // The default certificate validation context and dynamic certificate validation - // context could only contain partial fields, which is okay to fail the validation. - // But the combined certificate validation context should pass validation. If - // validation of combined certificate validation context fails, - // getCombinedValidationContextConfig() throws exception, validation_context_config_ will not - // get updated. - cvc_validation_callback_handle_ = - certificate_validation_context_provider_->addValidationCallback( - [this]( - const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& - dynamic_cvc) { getCombinedValidationContextConfig(dynamic_cvc); }); + if (certificate_validation_context_provider_ != nullptr) { + if (default_cvc_) { + // We need to validate combined certificate validation context. + // The default certificate validation context and dynamic certificate validation + // context could only contain partial fields, which is okay to fail the validation. + // But the combined certificate validation context should pass validation. If + // validation of combined certificate validation context fails, + // getCombinedValidationContextConfig() throws exception, validation_context_config_ will not + // get updated. + cvc_validation_callback_handle_ = + certificate_validation_context_provider_->addValidationCallback( + [this]( + const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& + dynamic_cvc) { getCombinedValidationContextConfig(dynamic_cvc); }); + } + // Load inlined, static or dynamic secret that's already available. + if (certificate_validation_context_provider_->secret() != nullptr) { + if (default_cvc_) { + validation_context_config_ = + getCombinedValidationContextConfig(*certificate_validation_context_provider_->secret()); + } else { + validation_context_config_ = std::make_unique( + *certificate_validation_context_provider_->secret(), api_); + } + } } - // Load inline or static secret into tls_certificate_config_. + // Load inlined, static or dynamic secrets that are already available. if (!tls_certificate_providers_.empty()) { for (auto& provider : tls_certificate_providers_) { if (provider->secret() != nullptr) { @@ -188,12 +200,6 @@ ContextConfigImpl::ContextConfigImpl( } } } - // Load inline or static secret into validation_context_config_. - if (certificate_validation_context_provider_ != nullptr && - certificate_validation_context_provider_->secret() != nullptr) { - validation_context_config_ = std::make_unique( - *certificate_validation_context_provider_->secret(), api_); - } } Ssl::CertificateValidationContextConfigPtr ContextConfigImpl::getCombinedValidationContextConfig( @@ -381,12 +387,10 @@ ServerContextConfigImpl::ServerContextConfigImpl( [this](const envoy::extensions::transport_sockets::tls::v3::TlsSessionTicketKeys& keys) { getSessionTicketKeys(keys); }); - } - - // Load inline or static secrets. - if (session_ticket_keys_provider_ != nullptr && - session_ticket_keys_provider_->secret() != nullptr) { - session_ticket_keys_ = getSessionTicketKeys(*session_ticket_keys_provider_->secret()); + // Load inlined, static or dynamic secret that's already available. + if (session_ticket_keys_provider_->secret() != nullptr) { + session_ticket_keys_ = getSessionTicketKeys(*session_ticket_keys_provider_->secret()); + } } if ((config.common_tls_context().tls_certificates().size() + diff --git a/test/integration/sds_dynamic_integration_test.cc b/test/integration/sds_dynamic_integration_test.cc index 68d7e3c83101..13242efd4a9c 100644 --- a/test/integration/sds_dynamic_integration_test.cc +++ b/test/integration/sds_dynamic_integration_test.cc @@ -15,6 +15,7 @@ #include "extensions/transport_sockets/tls/context_config_impl.h" #include "extensions/transport_sockets/tls/context_manager_impl.h" +#include "extensions/transport_sockets/tls/ssl_socket.h" #include "test/common/grpc/grpc_client_integration.h" #include "test/config/integration/certs/clientcert_hash.h" @@ -263,21 +264,7 @@ class SdsDynamicDownstreamCertValidationContextTest : public SdsDynamicDownstrea TestEnvironment::runfilesPath("test/config/integration/certs/servercert.pem")); tls_certificate->mutable_private_key()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/serverkey.pem")); - - if (use_combined_validation_context_) { - // Modify the listener context validation type to use combined certificate validation - // context. - auto* combined_config = common_tls_context->mutable_combined_validation_context(); - auto* default_validation_context = combined_config->mutable_default_validation_context(); - default_validation_context->add_verify_certificate_hash(TEST_CLIENT_CERT_HASH); - auto* secret_config = combined_config->mutable_validation_context_sds_secret_config(); - setUpSdsConfig(secret_config, validation_secret_); - } else { - // Modify the listener context validation type to use dynamic certificate validation - // context. - auto* secret_config = common_tls_context->mutable_validation_context_sds_secret_config(); - setUpSdsConfig(secret_config, validation_secret_); - } + setUpSdsValidationContext(common_tls_context); transport_socket->set_name("envoy.transport_sockets.tls"); transport_socket->mutable_typed_config()->PackFrom(tls_context); @@ -286,16 +273,90 @@ class SdsDynamicDownstreamCertValidationContextTest : public SdsDynamicDownstrea sds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); sds_cluster->set_name("sds_cluster"); sds_cluster->mutable_http2_protocol_options(); + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext upstream_tls_context; + if (share_validation_secret_) { + // Configure static cluster with SDS config referencing "validation_secret", + // which is going to be processed before LDS resources. + ASSERT(use_lds_); + setUpSdsValidationContext(upstream_tls_context.mutable_common_tls_context()); + } + // Enable SSL/TLS with a client certificate in the first cluster. + auto* upstream_tls_certificate = + upstream_tls_context.mutable_common_tls_context()->add_tls_certificates(); + upstream_tls_certificate->mutable_certificate_chain()->set_filename( + TestEnvironment::runfilesPath("test/config/integration/certs/clientcert.pem")); + upstream_tls_certificate->mutable_private_key()->set_filename( + TestEnvironment::runfilesPath("test/config/integration/certs/clientkey.pem")); + auto* upstream_transport_socket = + bootstrap.mutable_static_resources()->mutable_clusters(0)->mutable_transport_socket(); + upstream_transport_socket->set_name("envoy.transport_sockets.tls"); + upstream_transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); }); HttpIntegrationTest::initialize(); + registerTestServerPorts({"http"}); client_ssl_ctx_ = createClientSslTransportSocketFactory({}, context_manager_, *api_); } + void setUpSdsValidationContext( + envoy::extensions::transport_sockets::tls::v3::CommonTlsContext* common_tls_context) { + if (use_combined_validation_context_) { + // Modify the listener context validation type to use combined certificate validation + // context. + auto* combined_config = common_tls_context->mutable_combined_validation_context(); + auto* default_validation_context = combined_config->mutable_default_validation_context(); + default_validation_context->add_verify_certificate_hash(TEST_CLIENT_CERT_HASH); + auto* secret_config = combined_config->mutable_validation_context_sds_secret_config(); + setUpSdsConfig(secret_config, validation_secret_); + } else { + // Modify the listener context validation type to use dynamic certificate validation + // context. + auto* secret_config = common_tls_context->mutable_validation_context_sds_secret_config(); + setUpSdsConfig(secret_config, validation_secret_); + } + } + + void createUpstreams() override { + // Fake upstream with SSL/TLS for the first cluster. + fake_upstreams_.emplace_back(new FakeUpstream( + createUpstreamSslContext(), 0, FakeHttpConnection::Type::HTTP1, version_, timeSystem())); + create_xds_upstream_ = true; + } + + Network::TransportSocketFactoryPtr createUpstreamSslContext() { + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; + auto* common_tls_context = tls_context.mutable_common_tls_context(); + auto* tls_certificate = common_tls_context->add_tls_certificates(); + tls_certificate->mutable_certificate_chain()->set_filename( + TestEnvironment::runfilesPath("test/config/integration/certs/clientcert.pem")); + tls_certificate->mutable_private_key()->set_filename( + TestEnvironment::runfilesPath("test/config/integration/certs/clientkey.pem")); + + auto cfg = std::make_unique( + tls_context, factory_context_); + static Stats::Scope* upstream_stats_store = new Stats::TestIsolatedStoreImpl(); + return std::make_unique( + std::move(cfg), context_manager_, *upstream_stats_store, std::vector{}); + } + + void TearDown() override { + cleanUpXdsConnection(); + + client_ssl_ctx_.reset(); + cleanupUpstreamAndDownstream(); + codec_client_.reset(); + + test_server_.reset(); + fake_upstreams_.clear(); + } + void enableCombinedValidationContext(bool enable) { use_combined_validation_context_ = enable; } + void shareValidationSecret(bool share) { share_validation_secret_ = share; } private: bool use_combined_validation_context_{false}; + bool share_validation_secret_{false}; }; INSTANTIATE_TEST_SUITE_P(IpVersionsClientType, SdsDynamicDownstreamCertValidationContextTest, @@ -333,6 +394,53 @@ TEST_P(SdsDynamicDownstreamCertValidationContextTest, CombinedCertValidationCont testRouterHeaderOnlyRequestAndResponse(&creator); } +// A test that verifies that both: static cluster and LDS listener are updated when using +// the same verification secret (standalone validation context) from the SDS server. +TEST_P(SdsDynamicDownstreamCertValidationContextTest, BasicWithSharedSecret) { + shareValidationSecret(true); + on_server_init_function_ = [this]() { + createSdsStream(*(fake_upstreams_[1])); + sendSdsResponse(getCvcSecret()); + }; + initialize(); + + // Wait for "ssl_context_updated_by_sds" counters to indicate that both resources + // depending on the verification_secret were updated. + test_server_->waitForCounterGe( + "cluster.cluster_0.client_ssl_socket_factory.ssl_context_update_by_sds", 1); + test_server_->waitForCounterGe( + listenerStatPrefix("server_ssl_socket_factory.ssl_context_update_by_sds"), 1); + + ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { + return makeSslClientConnection(); + }; + testRouterHeaderOnlyRequestAndResponse(&creator); +} + +// A test that verifies that both: static cluster and LDS listener are updated when using +// the same verification secret (combined validation context) from the SDS server. +TEST_P(SdsDynamicDownstreamCertValidationContextTest, CombinedValidationContextWithSharedSecret) { + enableCombinedValidationContext(true); + shareValidationSecret(true); + on_server_init_function_ = [this]() { + createSdsStream(*(fake_upstreams_[1])); + sendSdsResponse(getCvcSecretWithOnlyTrustedCa()); + }; + initialize(); + + // Wait for "ssl_context_updated_by_sds" counters to indicate that both resources + // depending on the verification_secret were updated. + test_server_->waitForCounterGe( + "cluster.cluster_0.client_ssl_socket_factory.ssl_context_update_by_sds", 1); + test_server_->waitForCounterGe( + listenerStatPrefix("server_ssl_socket_factory.ssl_context_update_by_sds"), 1); + + ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { + return makeSslClientConnection(); + }; + testRouterHeaderOnlyRequestAndResponse(&creator); +} + // Upstream SDS integration test: a static cluster has ssl cert from SDS. class SdsDynamicUpstreamIntegrationTest : public SdsDynamicIntegrationBaseTest { public: