istio · qiwzhang · Jun 28, 2018
diff --git a/include/envoy/secret/BUILD b/include/envoy/secret/BUILD
@@ -8,10 +8,16 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "secret_callbacks_interface",
+    hdrs = ["secret_callbacks.h"],
+)
+
 envoy_cc_library(
     name = "dynamic_secret_provider_interface",
     hdrs = ["dynamic_secret_provider.h"],
     deps = [
+        "secret_callbacks_interface",
         "//include/envoy/ssl:tls_certificate_config_interface",
     ],
 )

diff --git a/include/envoy/secret/dynamic_secret_provider.h b/include/envoy/secret/dynamic_secret_provider.h
@@ -2,6 +2,7 @@
 
 #include <string>
 
+#include "envoy/secret/secret_callbacks.h"
 #include "envoy/ssl/tls_certificate_config.h"
 
 namespace Envoy {
@@ -18,9 +19,12 @@ class DynamicSecretProvider {
    * @return the TlsCertificate secret. Returns nullptr if the secret is not found.
    */
   virtual const Ssl::TlsCertificateConfig* secret() const PURE;
+
+  virtual void addUpdateCallback(SecretCallbacks& callback) PURE;
+  virtual void removeUpdateCallback(SecretCallbacks& callback) PURE;
 };
 
 typedef std::shared_ptr<DynamicSecretProvider> DynamicSecretProviderSharedPtr;
 
 } // namespace Secret
-} // namespace Envoy
+} // namespace Envoy
diff --git a/include/envoy/secret/secret_callbacks.h b/include/envoy/secret/secret_callbacks.h
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <memory>
+#include <string>
+
+#include "envoy/common/pure.h"
+
+namespace Envoy {
+namespace Secret {
+
+/**
+ * Callbacks invoked by a secret manager.
+ */
+class SecretCallbacks {
+public:
+  virtual ~SecretCallbacks() {}
+
+  virtual void onAddOrUpdateSecret() PURE;
+};
+
+} // namespace Secret
+} // namespace Envoy
diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h
@@ -138,6 +138,8 @@ class ClientContextConfig : public virtual ContextConfig {
   virtual bool allowRenegotiation() const PURE;
 };
 
+typedef std::unique_ptr<ClientContextConfig> ClientContextConfigPtr;
+
 class ServerContextConfig : public virtual ContextConfig {
 public:
   struct SessionTicketKey {
@@ -159,5 +161,7 @@ class ServerContextConfig : public virtual ContextConfig {
   virtual const std::vector<SessionTicketKey>& sessionTicketKeys() const PURE;
 };
 
+typedef std::unique_ptr<ServerContextConfig> ServerContextConfigPtr;
+
 } // namespace Ssl
 } // namespace Envoy
diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD
@@ -17,7 +17,6 @@ envoy_cc_library(
         "//include/envoy/secret:secret_manager_interface",
         "//include/envoy/server:instance_interface",
         "//source/common/common:minimal_logger_lib",
-        "//source/common/ssl:tls_certificate_config_impl_lib",
         "@envoy_api//envoy/api/v2/auth:cert_cc",
     ],
 )
@@ -29,7 +28,6 @@ envoy_cc_library(
     deps = [
         "//include/envoy/config:subscription_interface",
         "//include/envoy/server:instance_interface",
-        "//source/common/common:minimal_logger_lib",
         "//source/common/config:resources_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/ssl:tls_certificate_config_impl_lib",

diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc
@@ -51,9 +51,19 @@ void SdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&)
   const uint64_t new_hash = MessageUtil::hash(secret);
   if (new_hash != secret_hash_ &&
       secret.type_case() == envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate) {
-    tls_certificate_secrets_ =
-        std::make_unique<Ssl::TlsCertificateConfigImpl>(secret.tls_certificate());
     secret_hash_ = new_hash;
+    {
+      std::unique_lock<std::shared_timed_mutex> lhs(secret_mutex_);
+      tls_certificate_secrets_ =
+          std::make_unique<Ssl::TlsCertificateConfigImpl>(secret.tls_certificate());
+    }
+
+    {
+      std::shared_lock<std::shared_timed_mutex> lhs(update_callbacks_mutex_);
+      for (auto cb : update_callbacks_) {
+        cb->onAddOrUpdateSecret();
+      }
+    }
   }
 
   runInitializeCallbackIfAny();
@@ -72,4 +82,4 @@ void SdsApi::runInitializeCallbackIfAny() {
 }
 
 } // namespace Secret
-} // namespace Envoy
+} // namespace Envoy
diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h
@@ -1,10 +1,12 @@
 #pragma once
 
 #include <functional>
+#include <shared_mutex>
 
 #include "envoy/api/v2/auth/cert.pb.h"
 #include "envoy/api/v2/core/config_source.pb.h"
 #include "envoy/config/subscription.h"
+#include "envoy/secret/dynamic_secret_provider.h"
 #include "envoy/server/instance.h"
 
 namespace Envoy {
@@ -33,9 +35,19 @@ class SdsApi : public Init::Target,
 
   // DynamicSecretProvider
   const Ssl::TlsCertificateConfig* secret() const override {
+    std::shared_lock<std::shared_timed_mutex> lhs(secret_mutex_);
     return tls_certificate_secrets_.get();
   }
 
+  void addUpdateCallback(SecretCallbacks& callback) override {
+    std::unique_lock<std::shared_timed_mutex> lhs(update_callbacks_mutex_);
+    update_callbacks_.push_back(&callback);
+  }
+  void removeUpdateCallback(SecretCallbacks& callback) override {
+    std::unique_lock<std::shared_timed_mutex> lhs(update_callbacks_mutex_);
+    update_callbacks_.remove(&callback);
+  }
+
 private:
   void runInitializeCallbackIfAny();
 
@@ -45,11 +57,15 @@ class SdsApi : public Init::Target,
   std::function<void()> initialize_callback_;
   std::string sds_config_name_;
 
-  uint64_t secret_hash_;
+  std::size_t secret_hash_{};
   Ssl::TlsCertificateConfigPtr tls_certificate_secrets_;
+  mutable std::shared_timed_mutex secret_mutex_;
+
+  std::list<SecretCallbacks*> update_callbacks_;
+  mutable std::shared_timed_mutex update_callbacks_mutex_;
 };
 
 typedef std::unique_ptr<SdsApi> SdsApiPtr;
 
 } // namespace Secret
-} // namespace Envoy
+} // namespace Envoy
diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc
@@ -58,4 +58,4 @@ DynamicSecretProviderSharedPtr SecretManagerImpl::findOrCreateDynamicSecretProvi
 }
 
 } // namespace Secret
-} // namespace Envoy
+} // namespace Envoy
diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h
@@ -18,6 +18,7 @@ class SecretManagerImpl : public SecretManager, Logger::Loggable<Logger::Id::ups
   SecretManagerImpl(Server::Instance& server) : server_(server) {}
 
   void addStaticSecret(const envoy::api::v2::auth::Secret& secret) override;
+
   const Ssl::TlsCertificateConfig* findStaticTlsCertificate(const std::string& name) const override;
 
   DynamicSecretProviderSharedPtr

diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD
@@ -18,6 +18,7 @@ envoy_cc_library(
         ":context_lib",
         "//include/envoy/network:connection_interface",
         "//include/envoy/network:transport_socket_interface",
+        "//include/envoy/secret:secret_manager_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:empty_string",
         "//source/common/common:minimal_logger_lib",

diff --git a/source/common/ssl/context_config_impl.cc b/source/common/ssl/context_config_impl.cc
@@ -16,8 +16,6 @@
 namespace Envoy {
 namespace Ssl {
 
-namespace {} // 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]:"
@@ -68,6 +66,7 @@ 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)) {
+
   readCertChainConfig(config);
 
   if (ca_cert_.empty()) {
@@ -93,10 +92,11 @@ void ContextConfigImpl::readCertChainConfig(const envoy::api::v2::auth::CommonTl
     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()) {
+    const auto& secret_config = config.tls_certificate_sds_secret_configs()[0];
+    const auto& secret_name = secret_config.name();
+    if (!secret_config.has_sds_config()) {
       // static secret
-      const auto secret = secret_manager_.findStaticTlsCertificate(secret_name);
+      const auto& secret = secret_manager_.findStaticTlsCertificate(secret_name);
       if (secret) {
         cert_chain_ = secret->certificateChain();
         private_key_ = secret->privateKey();
@@ -106,7 +106,7 @@ void ContextConfigImpl::readCertChainConfig(const envoy::api::v2::auth::CommonTl
       }
     } else {
       secret_provider_ = secret_manager_.findOrCreateDynamicSecretProvider(
-          config.tls_certificate_sds_secret_configs()[0].sds_config(), secret_name);
+        secret_config.sds_config(), secret_name);
       return;
     }
   }
@@ -136,15 +136,13 @@ const std::string& ContextConfigImpl::certChain() const {
   if (secret_provider_ && secret_provider_->secret()) {
     return secret_provider_->secret()->certificateChain();
   }
-
   return cert_chain_;
 }
 
 const std::string& ContextConfigImpl::privateKey() const {
   if (secret_provider_ && secret_provider_->secret()) {
     return secret_provider_->secret()->privateKey();
   }
-
   return private_key_;
 }
 

diff --git a/source/common/ssl/context_manager_impl.cc b/source/common/ssl/context_manager_impl.cc
@@ -22,6 +22,10 @@ void ContextManagerImpl::removeContext(Context* context) {
 
 ClientContextSharedPtr
 ContextManagerImpl::createSslClientContext(Stats::Scope& scope, const ClientContextConfig& config) {
+  if (!config.isValid()) {
+    return nullptr;
+  }
+
   ClientContextSharedPtr context(new ClientContextImpl(*this, scope, config));
   std::unique_lock<std::shared_timed_mutex> lock(contexts_lock_);
   contexts_.emplace_back(context.get());
@@ -31,6 +35,10 @@ ContextManagerImpl::createSslClientContext(Stats::Scope& scope, const ClientCont
 ServerContextSharedPtr
 ContextManagerImpl::createSslServerContext(Stats::Scope& scope, const ServerContextConfig& config,
                                            const std::vector<std::string>& server_names) {
+  if (!config.isValid()) {
+    return nullptr;
+  }
+
   ServerContextSharedPtr context(
       new ServerContextImpl(*this, scope, config, server_names, runtime_));
   std::unique_lock<std::shared_timed_mutex> lock(contexts_lock_);

diff --git a/source/common/ssl/ssl_socket.cc b/source/common/ssl/ssl_socket.cc
@@ -373,33 +373,73 @@ std::string SslSocket::subjectLocalCertificate() const {
   return getSubjectFromCertificate(cert);
 }
 
-ClientSslSocketFactory::ClientSslSocketFactory(const ClientContextConfig& config,
+ClientSslSocketFactory::ClientSslSocketFactory(ClientContextConfigPtr config,
                                                Ssl::ContextManager& manager,
                                                Stats::Scope& stats_scope)
-    : manager_(manager), ssl_ctx_(manager.createSslClientContext(stats_scope, config)) {}
+    : manager_(manager), stats_scope_(stats_scope), config_(std::move(config)),
+      ssl_ctx_(manager.createSslClientContext(stats_scope, *config_)) {
+  if (config_->getDynamicSecretProvider()) {
+    config_->getDynamicSecretProvider()->addUpdateCallback(*this);
+  }
+}
 
-ClientSslSocketFactory::~ClientSslSocketFactory() { manager_.removeContext(ssl_ctx_.get()); }
+ClientSslSocketFactory::~ClientSslSocketFactory() {
+  if (ssl_ctx_) {
+    manager_.removeContext(ssl_ctx_.get());
+  }
+  if (config_->getDynamicSecretProvider()) {
+    config_->getDynamicSecretProvider()->removeUpdateCallback(*this);
+  }
+}
 
 Network::TransportSocketPtr ClientSslSocketFactory::createTransportSocket() const {
   return std::make_unique<Ssl::SslSocket>(ssl_ctx_, Ssl::InitialState::Client);
 }
 
 bool ClientSslSocketFactory::implementsSecureTransport() const { return true; }
 
-ServerSslSocketFactory::ServerSslSocketFactory(const ServerContextConfig& config,
+void ClientSslSocketFactory::onAddOrUpdateSecret() {
+  if (ssl_ctx_) {
+    ENVOY_LOG(debug, "listener socket updated");
+    manager_.removeContext(ssl_ctx_.get());
+  }
+  ssl_ctx_ = manager_.createSslClientContext(stats_scope_, *config_);
+}
+
+ServerSslSocketFactory::ServerSslSocketFactory(ServerContextConfigPtr config,
                                                Ssl::ContextManager& manager,
                                                Stats::Scope& stats_scope,
                                                const std::vector<std::string>& server_names)
-    : manager_(manager),
-      ssl_ctx_(manager.createSslServerContext(stats_scope, config, server_names)) {}
+    : manager_(manager), stats_scope_(stats_scope), config_(std::move(config)),
+      ssl_ctx_(manager.createSslServerContext(stats_scope, *config_, server_names)),
+      server_names_(server_names) {
+  if (config_->getDynamicSecretProvider()) {
+    config_->getDynamicSecretProvider()->addUpdateCallback(*this);
+  }
+}
 
-ServerSslSocketFactory::~ServerSslSocketFactory() { manager_.removeContext(ssl_ctx_.get()); }
+ServerSslSocketFactory::~ServerSslSocketFactory() {
+  if (ssl_ctx_) {
+    manager_.removeContext(ssl_ctx_.get());
+  }
+  if (config_->getDynamicSecretProvider()) {
+    config_->getDynamicSecretProvider()->removeUpdateCallback(*this);
+  }
+}
 
 Network::TransportSocketPtr ServerSslSocketFactory::createTransportSocket() const {
   return std::make_unique<Ssl::SslSocket>(ssl_ctx_, Ssl::InitialState::Server);
 }
 
 bool ServerSslSocketFactory::implementsSecureTransport() const { return true; }
 
+void ServerSslSocketFactory::onAddOrUpdateSecret() {
+  if (ssl_ctx_) {
+    ENVOY_LOG(debug, "listener socket updated");
+    manager_.removeContext(ssl_ctx_.get());
+  }
+  ssl_ctx_ = manager_.createSslServerContext(stats_scope_, *config_, server_names_);
+}
+
 } // namespace Ssl
 } // namespace Envoy