diff --git a/include/envoy/secret/secret_manager.h b/include/envoy/secret/secret_manager.h index 0fd237bc909b8..5e17f533aa64b 100644 --- a/include/envoy/secret/secret_manager.h +++ b/include/envoy/secret/secret_manager.h @@ -8,6 +8,17 @@ namespace Envoy { namespace Secret { + class DynamicSecretProvider { + public: + virtual ~SecretProvider() {} + + virtual const Ssl::TlsCertificateConfigSharedPtr secret() const PURE; + + virtual void addUpdateCallback(SecretCallbacks& callback) PURE; + virtual void removeUpdateCallback(SecretCallbacks& callback) PURE; + } + typedef std::shared_ptr DynamicSecretProviderSharedPtr; + /** * A manager for static and dynamic secrets. */ @@ -21,8 +32,7 @@ class SecretManager { * @param secret a protobuf message of envoy::api::v2::auth::Secret. * @throw an EnvoyException if the secret is invalid or not supported. */ - virtual void addOrUpdateSecret(const std::string& config_source_hash, - const envoy::api::v2::auth::Secret& secret) PURE; + virtual void addStaticSecret(const envoy::api::v2::auth::Secret& secret) PURE; /** * @param sds_config_source_hash hash string of normalized config source. @@ -30,7 +40,7 @@ class SecretManager { * @return the TlsCertificate secret. Returns nullptr if the secret is not found. */ virtual const Ssl::TlsCertificateConfigSharedPtr - findTlsCertificate(const std::string& config_source_hash, const std::string& name) const PURE; + findStaticTlsCertificate(const std::string& name) const PURE; /** * Add or update SDS config source. SecretManager starts downloading secrets from registered @@ -40,19 +50,9 @@ class SecretManager { * @param config_name a name that uniquely refers to the SDS config source. * @return a hash string of normalized config source. */ - virtual std::string addOrUpdateSdsService(const envoy::api::v2::core::ConfigSource& config_source, - std::string config_name) PURE; + virtual DynamicSecretProviderSharedPtr createDynamicSecretProvider(const envoy::api::v2::core::ConfigSource& config_source, + std::string config_name) PURE; - /** - * Register callback function which is to be invoked on secret update. - * - * @param config_source_hash Hash code of ConfigSource. - * @param secret_name name of the secret. - * @param callback SecretCallbacks class. - */ - virtual void registerTlsCertificateConfigCallbacks(const std::string& config_source_hash, - const std::string& secret_name, - SecretCallbacks& callback) PURE; }; } // namespace Secret diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index e30630e438891..338f73ca70b88 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -34,15 +34,30 @@ void SdsApi::initialize(std::function callback) { } void SdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&) { - for (const auto& resource : resources) { - switch (resource.type_case()) { - case envoy::api::v2::auth::Secret::kTlsCertificate: - server_.secretManager().addOrUpdateSecret(sds_config_source_hash_, resource); - break; - case envoy::api::v2::auth::Secret::kSessionTicketKeys: - NOT_IMPLEMENTED; - default: - throw EnvoyException("sds: invalid configuration"); + if (resources.empty()) { + ENVOY_LOG(debug, "Missing RouteConfiguration for {} in onConfigUpdate()", route_config_name_); + runInitializeCallbackIfAny(); + return; + } + if (resources.size() != 1) { + throw EnvoyException(fmt::format("Unexpected RDS resource length: {}", resources.size())); + } + const auto& secret = resources[0]; + MessageUtil::validate(secret); + // TODO(PiotrSikora): Remove this hack once fixed internally. + if (!(secret.name() == sds_config_name_)) { + throw EnvoyException(fmt::format("Unexpected RDS configuration (expecting {}): {}", + sds_config_name_, secret.name())); + } + const uint64_t new_hash = MessageUtil::hash(secret); + if (new_hash != secret_hash_) + if (secret.type_case() == envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate) { + tls_certificate_secret_ = + std::make_shared(secret.tls_certificate()); + + for (auto cb : update_callbakcs_) { + cb->onAddOrUpdateSecret(); + } } } @@ -62,4 +77,4 @@ void SdsApi::runInitializeCallbackIfAny() { } } // namespace Secret -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index faf95bbfb2e21..bbdf31857fb76 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -13,7 +13,9 @@ namespace Secret { /** * SDS API implementation that fetches secrets from SDS server via Subscription. */ -class SdsApi : public Init::Target, Config::SubscriptionCallbacks { +class SdsApi : public Init::Target, + public DynamicSecretProvider, + public Config::SubscriptionCallbacks { public: SdsApi(Server::Instance& server, const envoy::api::v2::core::ConfigSource& sds_config, std::string sds_config_hash, std::string sds_config_name); @@ -28,6 +30,17 @@ class SdsApi : public Init::Target, Config::SubscriptionCallbacks(resource).name(); } + // DynamicSecretProvider + const Ssl::TlsCertificateConfigSharedPtr secret() const override { + return tls_certificate_secrets_; + } + void addUpdateCallback(SecretCallbacks& callback) override { + update_callbacks_.push_back(*callback); + } + void removeUpdateCallback(SecretCallbacks& callback) override { + update_callbacks_.remove(*callback); + } + private: void runInitializeCallbackIfAny(); @@ -37,9 +50,13 @@ class SdsApi : public Init::Target, Config::SubscriptionCallbacks> subscription_; std::function initialize_callback_; std::string sds_config_name_; + + std::string secret_hash_; + Ssl::TlsCertificateConfigSharedPtr tls_certificate_secrets_; + std::list update_callbacks_; }; typedef std::unique_ptr SdsApiPtr; } // namespace Secret -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 36dda28098776..6726510b5f38b 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -8,36 +8,17 @@ namespace Envoy { namespace Secret { -void SecretManagerImpl::addOrUpdateSecret(const std::string& config_source_hash, - const envoy::api::v2::auth::Secret& secret) { +void SecretManagerImpl::addStaticSecret( const envoy::api::v2::auth::Secret& secret) { switch (secret.type_case()) { case envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate: { std::unique_lock lhs(tls_certificate_secrets_mutex_); auto tls_certificate_secret = std::make_shared(secret.tls_certificate()); - tls_certificate_secrets_[config_source_hash][secret.name()] = tls_certificate_secret; + tls_certificate_secrets_[secret.name()] = tls_certificate_secret; if (config_source_hash.empty()) { return; } - - std::string secret_name = secret.name(); - server_.dispatcher().post([this, config_source_hash, secret_name, secret]() { - std::unique_lock lhs(tls_certificate_secret_update_callbacks_mutex_); - auto config_source_it = tls_certificate_secret_update_callbacks_.find(config_source_hash); - if (config_source_it != tls_certificate_secret_update_callbacks_.end()) { - auto callback_it = config_source_it->second.find(secret_name); - if (callback_it != config_source_it->second.end()) { - if (callback_it->second.first == nullptr || - !callback_it->second.first->equalTo(*tls_certificate_secret)) { - for (auto& callback : callback_it->second.second) { - callback->onAddOrUpdateSecret(); - } - callback_it->second.first = secret; - } - } - } - }); } break; default: throw EnvoyException("Secret type not implemented"); @@ -45,17 +26,12 @@ void SecretManagerImpl::addOrUpdateSecret(const std::string& config_source_hash, } const Ssl::TlsCertificateConfigSharedPtr -SecretManagerImpl::findTlsCertificate(const std::string& config_source_hash, +SecretManagerImpl::findStaticTlsCertificate(const std::string& config_source_hash, const std::string& name) const { std::shared_lock lhs(tls_certificate_secrets_mutex_); - auto config_source_it = tls_certificate_secrets_.find(config_source_hash); - if (config_source_it == tls_certificate_secrets_.end()) { - return nullptr; - } - - auto secret = config_source_it->second.find(name); - return (secret != config_source_it->second.end()) ? secret->second : nullptr; + auto secret = tls_certificate_secrets_.find(name); + return (secret != tls_certificate_secrets_.end()) ? secret->second : nullptr; } std::string SecretManagerImpl::addOrUpdateSdsService( @@ -64,36 +40,13 @@ std::string SecretManagerImpl::addOrUpdateSdsService( auto hash = SecretManagerUtil::configSourceHash(sds_config_source); std::string sds_apis_key = hash + config_name; - if (sds_apis_.find(sds_apis_key) != sds_apis_.end()) { - return hash; - } - - sds_apis_[sds_apis_key] = std::make_unique(server_, sds_config_source, hash, config_name); - - return hash; -} - -void SecretManagerImpl::registerTlsCertificateConfigCallbacks(const std::string& config_source_hash, - const std::string& secret_name, - SecretCallbacks& callback) { - auto secret = findTlsCertificate(config_source_hash, secret_name); - - std::unique_lock lhs(tls_certificate_secret_update_callbacks_mutex_); - - auto config_source_it = tls_certificate_secret_update_callbacks_.find(config_source_hash); - if (config_source_it == tls_certificate_secret_update_callbacks_.end()) { - tls_certificate_secret_update_callbacks_[config_source_hash][secret_name] = {secret, - {&callback}}; - return; - } - - auto name_it = config_source_it->second.find(secret_name); - if (name_it == config_source_it->second.end()) { - config_source_it->second[secret_name] = {secret, {&callback}}; - return; + auto sds_api = sds_apis_[sds_apis_key].lock(); + if (!sds_api) { + sds_api = std::make_shared(server_, sds_config_source, hash, config_name); + sds_apis_[sds_apis_key] = sds_api; } - name_it->second.second.push_back(&callback); + return sds_api; } } // namespace Secret diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index 2b5fd74cb3827..2e7f0bf79022a 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -17,30 +17,27 @@ class SecretManagerImpl : public SecretManager, Logger::Loggable sds_apis_; + std::unordered_map> sds_apis_; mutable std::shared_timed_mutex sds_api_mutex_; // Manages pairs of name and Ssl::TlsCertificateConfig grouped by SDS config source hash. // If SDS config source hash is empty, it is a static secret. - std::unordered_map> - tls_certificate_secrets_; + std::unordered_map + tls_certificate_secrets_; mutable std::shared_timed_mutex tls_certificate_secrets_mutex_; // callback functions for secret update diff --git a/source/common/ssl/context_config_impl.cc b/source/common/ssl/context_config_impl.cc index f3fd055903306..8c7bef08f7959 100644 --- a/source/common/ssl/context_config_impl.cc +++ b/source/common/ssl/context_config_impl.cc @@ -16,51 +16,6 @@ namespace Envoy { namespace Ssl { -namespace { - -std::string readSdsSecretName(const envoy::api::v2::auth::CommonTlsContext& config) { - return (!config.tls_certificate_sds_secret_configs().empty()) - ? config.tls_certificate_sds_secret_configs()[0].name() - : EMPTY_STRING; -} - -std::string readConfigSourceHash(const envoy::api::v2::auth::CommonTlsContext& config, - Secret::SecretManager& secret_manager) { - return (!config.tls_certificate_sds_secret_configs().empty() && - config.tls_certificate_sds_secret_configs()[0].has_sds_config()) - ? secret_manager.addOrUpdateSdsService( - config.tls_certificate_sds_secret_configs()[0].sds_config(), - config.tls_certificate_sds_secret_configs()[0].name()) - : EMPTY_STRING; -} - -std::string readConfig( - const envoy::api::v2::auth::CommonTlsContext& config, Secret::SecretManager& secret_manager, - const std::string& config_source_hash, const std::string& secret_name, - const std::function& - read_inline_config, - const std::function& - read_managed_secret) { - if (!config.tls_certificates().empty()) { - return read_inline_config(config.tls_certificates()[0]); - } else if (!config.tls_certificate_sds_secret_configs().empty()) { - const auto secret = secret_manager.findTlsCertificate(config_source_hash, secret_name); - if (!secret) { - if (config_source_hash.empty()) { - throw EnvoyException( - fmt::format("Unknown static secret: {} : {}", config_source_hash, secret_name)); - } else { - return EMPTY_STRING; - } - } - return read_managed_secret(*secret); - } else { - return EMPTY_STRING; - } -} - -} // namespace - const std::string ContextConfigImpl::DEFAULT_CIPHER_SUITES = "[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]:" "[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]:" @@ -79,8 +34,7 @@ const std::string ContextConfigImpl::DEFAULT_ECDH_CURVES = "X25519:P-256"; ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContext& config, Secret::SecretManager& secret_manager) - : secret_manager_(secret_manager), sds_secret_name_(readSdsSecretName(config)), - sds_config_source_hash_(readConfigSourceHash(config, secret_manager)), + : secret_manager_(secret_manager), alpn_protocols_(RepeatedPtrUtil::join(config.alpn_protocols(), ",")), alt_alpn_protocols_(config.deprecated_v1().alt_alpn_protocols()), cipher_suites_(StringUtil::nonEmptyStringOrDefault( @@ -93,26 +47,10 @@ ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContex Config::DataSource::read(config.validation_context().crl(), true)), certificate_revocation_list_path_( Config::DataSource::getPath(config.validation_context().crl())), - cert_chain_(readConfig( - config, secret_manager, sds_config_source_hash_, sds_secret_name_, - [](const envoy::api::v2::auth::TlsCertificate& tls_certificate) -> std::string { - return Config::DataSource::read(tls_certificate.certificate_chain(), true); - }, - [](const Ssl::TlsCertificateConfig& secret) -> std::string { - return secret.certificateChain(); - })), cert_chain_path_( config.tls_certificates().empty() ? "" : Config::DataSource::getPath(config.tls_certificates()[0].certificate_chain())), - private_key_(readConfig( - config, secret_manager, sds_config_source_hash_, sds_secret_name_, - [](const envoy::api::v2::auth::TlsCertificate& tls_certificate) -> std::string { - return Config::DataSource::read(tls_certificate.private_key(), true); - }, - [](const Ssl::TlsCertificateConfig& secret) -> std::string { - return secret.privateKey(); - })), private_key_path_( config.tls_certificates().empty() ? "" @@ -128,6 +66,9 @@ ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContex tlsVersionFromProto(config.tls_params().tls_minimum_protocol_version(), TLS1_VERSION)), max_protocol_version_( tlsVersionFromProto(config.tls_params().tls_maximum_protocol_version(), TLS1_2_VERSION)) { + + readConfig(config); + if (ca_cert_.empty()) { if (!certificate_revocation_list_.empty()) { throw EnvoyException(fmt::format("Failed to load CRL from {} without trusted CA", @@ -144,6 +85,35 @@ ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContex } } +std::string ContextConfigImpl::readConfig( + const envoy::api::v2::auth::CommonTlsContext& config) { + if (!config.tls_certificates().empty()) { + cert_chain_ = Config::DataSource::read(config.tls_certificates()[0].certificate_chain(), true); + private_key_ = Config::DataSource::read(config.tls_certificates()[0].private_key(), true); + return; + } + if (!config.tls_certificate_sds_secret_configs().empty()) { + auto secret_name = config.tls_certificate_sds_secret_configs()[0].name(); + if (!config.tls_certificate_sds_secret_configs()[0].has_sds_config()) { + // static secret + const auto secret = secret_manager.findStaticTlsCertificate(secret_name); + if (secret) { + cert_chain_ = secret->certificateChain(); + private_key_ = secret->privateKey(); + return; + } else { + throw EnvoyException(fmt::format("Unknown static secret: {}", secret_name)); + } + } else { + secret_provider_ = secret_manager.createDynamicSecretProvider(config.tls_certificate_sds_secret_configs()[0].sds_config() + secret_name); + return; + } + } + throw EnvoyException(fmt::format("Unknown invalid config")); +} + + unsigned ContextConfigImpl::tlsVersionFromProto( const envoy::api::v2::auth::TlsParameters_TlsProtocol& version, unsigned default_version) { switch (version) { @@ -165,29 +135,17 @@ unsigned ContextConfigImpl::tlsVersionFromProto( } const std::string& ContextConfigImpl::certChain() const { - if (!cert_chain_.empty()) { - return cert_chain_; + if (secret_provider_ && secret_provider_->secret()) { + return secret_provider_->secret()->certificateChain(); } - - auto secret = secret_manager_.findTlsCertificate(sds_config_source_hash_, sds_secret_name_); - if (!secret) { - return cert_chain_; - } - - return secret->certificateChain(); + return cert_chain_; } const std::string& ContextConfigImpl::privateKey() const { - if (!private_key_.empty()) { - return private_key_; + if (secret_provider_ && secret_provider_->secret()) { + return secret_provider_->secret()->privateKey(); } - - auto secret = secret_manager_.findTlsCertificate(sds_config_source_hash_, sds_secret_name_); - if (!secret) { - return private_key_; - } - - return secret->privateKey(); + return private_key_; } ClientContextConfigImpl::ClientContextConfigImpl( diff --git a/source/common/ssl/context_config_impl.h b/source/common/ssl/context_config_impl.h index ddaeef9aa519d..c2f272aa1cef1 100644 --- a/source/common/ssl/context_config_impl.h +++ b/source/common/ssl/context_config_impl.h @@ -54,8 +54,10 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { unsigned minProtocolVersion() const override { return min_protocol_version_; }; unsigned maxProtocolVersion() const override { return max_protocol_version_; }; - const std::string& sdsConfigShourceHash() const override { return sds_config_source_hash_; } - const std::string& sdsSecretName() const override { return sds_secret_name_; } + bool isValid() const { + // either secret_provider_ is nullptr or secret_provider_->secret() is NOT nullptr. + return !secret_provider_ || secret_provider_->secret(); + } protected: ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContext& config, @@ -70,8 +72,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { static const std::string DEFAULT_ECDH_CURVES; Secret::SecretManager& secret_manager_; - const std::string sds_secret_name_; - const std::string sds_config_source_hash_; + Secret::DynamicSecretProviderSharedPtr secret_provider_; const std::string alpn_protocols_; const std::string alt_alpn_protocols_; const std::string cipher_suites_; diff --git a/source/common/ssl/context_manager_impl.cc b/source/common/ssl/context_manager_impl.cc index eb07512ce5385..ff2119254dbb8 100644 --- a/source/common/ssl/context_manager_impl.cc +++ b/source/common/ssl/context_manager_impl.cc @@ -22,8 +22,7 @@ void ContextManagerImpl::releaseContext(Context* context) { ClientContextPtr ContextManagerImpl::createSslClientContext(Stats::Scope& scope, const ClientContextConfig& config) { - if (!config.sdsConfigShourceHash().empty() && !config.sdsSecretName().empty() && - config.certChain().empty() && config.privateKey().empty()) { + if (!config.isValid()) { return nullptr; } @@ -51,8 +50,7 @@ ContextManagerImpl::updateSslClientContext(const Ssl::ClientContextPtr& client_c ServerContextPtr ContextManagerImpl::createSslServerContext(Stats::Scope& scope, const ServerContextConfig& config, const std::vector& server_names) { - if (!config.sdsConfigShourceHash().empty() && !config.sdsSecretName().empty() && - config.certChain().empty() && config.privateKey().empty()) { + if (!config.isValid()) { return nullptr; } diff --git a/source/common/ssl/ssl_socket.cc b/source/common/ssl/ssl_socket.cc index b433358f16532..585c2c7511be0 100644 --- a/source/common/ssl/ssl_socket.cc +++ b/source/common/ssl/ssl_socket.cc @@ -376,7 +376,18 @@ std::string SslSocket::subjectLocalCertificate() const { ClientSslSocketFactory::ClientSslSocketFactory(const ClientContextConfig& config, Ssl::ContextManager& manager, Stats::Scope& stats_scope) - : ssl_ctx_(manager.createSslClientContext(stats_scope, config)) {} + : ssl_ctx_(manager.createSslClientContext(stats_scope, *config.get())), + config_(std::move(config)), manager_(manager), stats_scope_(stats_scope) { + if (config_) { + config_->addUpdateCallback(*this); + } +} + + ClientSslSocketFactory::~ClientSslSocketFactory() { + if (config_) { + config_->removeUpdateCallback(*this); + } + } Network::TransportSocketPtr ClientSslSocketFactory::createTransportSocket() const { return std::make_unique(*ssl_ctx_, Ssl::InitialState::Client); @@ -388,7 +399,19 @@ ServerSslSocketFactory::ServerSslSocketFactory(const ServerContextConfig& config Ssl::ContextManager& manager, Stats::Scope& stats_scope, const std::vector& server_names) - : ssl_ctx_(manager.createSslServerContext(stats_scope, config, server_names)) {} + : ssl_ctx_(manager.createSslServerContext(stats_scope, *config.get(), server_names)), + config_(std::move(config)), manager_(manager), stats_scope_(stats_scope), + server_names_(server_names) { + if (config_) { + config_->addUpdateCallback(*this); + } +} + + ServerSslSocketFactory::~ServerSslSocketFactory() { + if (config_) { + config_->removeUpdateCallback(*this); + } + } Network::TransportSocketPtr ServerSslSocketFactory::createTransportSocket() const { return std::make_unique(*ssl_ctx_, Ssl::InitialState::Server); diff --git a/source/common/ssl/ssl_socket.h b/source/common/ssl/ssl_socket.h index 30e7931d323b5..5fc6e47052789 100644 --- a/source/common/ssl/ssl_socket.h +++ b/source/common/ssl/ssl_socket.h @@ -71,6 +71,8 @@ class ClientSslSocketFactory : public Network::TransportSocketFactory, public: ClientSslSocketFactory(const ClientContextConfigPtr config, Ssl::ContextManager& manager, Stats::Scope& stats_scope); + virtual ~ClientSslSocketFactory(); + Network::TransportSocketPtr createTransportSocket() const override; bool implementsSecureTransport() const override; @@ -92,6 +94,8 @@ class ServerSslSocketFactory : public Network::TransportSocketFactory, public: ServerSslSocketFactory(const ServerContextConfigPtr config, Ssl::ContextManager& manager, Stats::Scope& stats_scope, const std::vector& server_names); + virtual ~ServerSslSocketFactory(); + Network::TransportSocketPtr createTransportSocket() const override; bool implementsSecureTransport() const override; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index dd9b8b3165197..7f80a2c33f0bb 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -51,7 +51,7 @@ void MainImpl::initialize(const envoy::config::bootstrap::v2::Bootstrap& bootstr ENVOY_LOG(info, "loading {} static secret(s)", secrets.size()); for (ssize_t i = 0; i < secrets.size(); i++) { ENVOY_LOG(debug, "static secret #{}: {}", i, secrets[i].name()); - server.secretManager().addOrUpdateSecret(EMPTY_STRING, secrets[i]); + server.secretManager().addStaticSecret(secrets[i]); } cluster_manager_ = cluster_manager_factory.clusterManagerFromProto(