tls: plumbing for exception removal (envoyproxy#34394)

Risk Level: low
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

envoy/secret/secret_callbacks.h

@@ -12,7 +12,7 @@ class SecretCallbacks {
 public:
   virtual ~SecretCallbacks() = default;
 
-  virtual void onAddOrUpdateSecret() PURE;
+  virtual absl::Status onAddOrUpdateSecret() PURE;
 };
 
 } // namespace Secret

envoy/server/transport_socket_config.h

@@ -87,13 +87,13 @@ class UpstreamTransportSocketConfigFactory : public virtual TransportSocketConfi
    * @param config const Protobuf::Message& supplies the config message for the transport socket
    *        implementation.
    * @param context TransportSocketFactoryContext& supplies the transport socket's context.
-   * @return Network::UpstreamTransportSocketFactoryPtr the transport socket factory instance. The
-   * returned TransportSocketFactoryPtr should not be nullptr.
+   * @return absl::StatusOr<Network::UpstreamTransportSocketFactoryPtr> the transport socket factory
+   * instance or error status. The returned TransportSocketFactoryPtr should not be nullptr.
    *
    * @throw EnvoyException if the implementation is unable to produce a factory with the provided
    *        parameters.
    */
-  virtual Network::UpstreamTransportSocketFactoryPtr
+  virtual absl::StatusOr<Network::UpstreamTransportSocketFactoryPtr>
   createTransportSocketFactory(const Protobuf::Message& config,
                                TransportSocketFactoryContext& context) PURE;
 
@@ -113,13 +113,13 @@ class DownstreamTransportSocketConfigFactory : public virtual TransportSocketCon
    * @param config const Protobuf::Message& supplies the config message for the transport socket
    *        implementation.
    * @param context TransportSocketFactoryContext& supplies the transport socket's context.
-   * @return Network::DownstreamTransportSocketFactoryPtr the transport socket factory instance. The
-   * returned TransportSocketFactoryPtr should not be nullptr.
+   * @return absl::StatusOr<Network::DownstreamTransportSocketFactoryPtr> the transport socket
+   * factory instance. The returned TransportSocketFactoryPtr should not be nullptr.
    *
    * @throw EnvoyException if the implementation is unable to produce a factory with the provided
    *        parameters.
    */
-  virtual Network::DownstreamTransportSocketFactoryPtr
+  virtual absl::StatusOr<Network::DownstreamTransportSocketFactoryPtr>
   createTransportSocketFactory(const Protobuf::Message& config,
                                TransportSocketFactoryContext& context,
                                const std::vector<std::string>& server_names) PURE;

envoy/ssl/context_config.h

@@ -81,7 +81,7 @@ class ContextConfig {
    * are downloaded from SDS server, this callback is invoked to update SSL context.
    * @param callback callback that is executed by context config.
    */
-  virtual void setSecretUpdateCallback(std::function<void()> callback) PURE;
+  virtual void setSecretUpdateCallback(std::function<absl::Status()> callback) PURE;
 
   /**
    * @return a callback which can be used to create Handshaker instances.

mobile/test/common/integration/test_server.cc

@@ -535,7 +535,8 @@ Network::DownstreamTransportSocketFactoryPtr TestServer::createQuicUpstreamTlsCo
       Server::Configuration::DownstreamTransportSocketConfigFactory>(
       "envoy.transport_sockets.quic");
 
-  return config_factory.createTransportSocketFactory(quic_config, factory_context, server_names);
+  return config_factory.createTransportSocketFactory(quic_config, factory_context, server_names)
+      .value();
 }
 
 Network::DownstreamTransportSocketFactoryPtr TestServer::createUpstreamTlsContext(

source/common/listener_manager/listener_manager_impl.cc

@@ -1097,9 +1097,11 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF
   std::vector<std::string> server_names(filter_chain.filter_chain_match().server_names().begin(),
                                         filter_chain.filter_chain_match().server_names().end());
 
+  auto factory_or_error = config_factory.createTransportSocketFactory(*message, factory_context_,
+                                                                      std::move(server_names));
+  THROW_IF_NOT_OK(factory_or_error.status());
   auto filter_chain_res = std::make_shared<FilterChainImpl>(
-      config_factory.createTransportSocketFactory(*message, factory_context_,
-                                                  std::move(server_names)),
+      std::move(factory_or_error.value()),
       listener_component_factory_.createNetworkFilterFactoryList(filter_chain.filters(),
                                                                  *filter_chain_factory_context),
       std::chrono::milliseconds(

source/common/quic/quic_client_transport_socket_factory.cc

@@ -13,7 +13,7 @@
 namespace Envoy {
 namespace Quic {
 
-Network::UpstreamTransportSocketFactoryPtr
+absl::StatusOr<Network::UpstreamTransportSocketFactoryPtr>
 QuicClientTransportSocketConfigFactory::createTransportSocketFactory(
     const Protobuf::Message& config,
     Server::Configuration::TransportSocketFactoryContext& context) {

source/common/quic/quic_client_transport_socket_factory.h

@@ -69,7 +69,7 @@ class QuicClientTransportSocketConfigFactory
       public Server::Configuration::UpstreamTransportSocketConfigFactory {
 public:
   // Server::Configuration::UpstreamTransportSocketConfigFactory
-  Network::UpstreamTransportSocketFactoryPtr createTransportSocketFactory(
+  absl::StatusOr<Network::UpstreamTransportSocketFactoryPtr> createTransportSocketFactory(
       const Protobuf::Message& config,
       Server::Configuration::TransportSocketFactoryContext& context) override;
 

source/common/quic/quic_server_transport_socket_factory.cc

@@ -12,7 +12,7 @@
 namespace Envoy {
 namespace Quic {
 
-Network::DownstreamTransportSocketFactoryPtr
+absl::StatusOr<Network::DownstreamTransportSocketFactoryPtr>
 QuicServerTransportSocketConfigFactory::createTransportSocketFactory(
     const Protobuf::Message& config, Server::Configuration::TransportSocketFactoryContext& context,
     const std::vector<std::string>& server_names) {
@@ -121,6 +121,7 @@ void QuicServerTransportSocketFactory::initialize() {
   config_->setSecretUpdateCallback([this]() {
     // The callback also updates config_ with the new secret.
     onSecretUpdated();
+    return absl::OkStatus();
   });
   if (!config_->alpnProtocols().empty()) {
     supported_alpns_ = absl::StrSplit(config_->alpnProtocols(), ',');

source/common/quic/quic_server_transport_socket_factory.h

@@ -72,7 +72,7 @@ class QuicServerTransportSocketConfigFactory
       public Server::Configuration::DownstreamTransportSocketConfigFactory {
 public:
   // Server::Configuration::DownstreamTransportSocketConfigFactory
-  Network::DownstreamTransportSocketFactoryPtr
+  absl::StatusOr<Network::DownstreamTransportSocketFactoryPtr>
   createTransportSocketFactory(const Protobuf::Message& config,
                                Server::Configuration::TransportSocketFactoryContext& context,
                                const std::vector<std::string>& server_names) override;

source/common/tls/client_ssl_socket.cc

@@ -34,7 +34,7 @@ ClientSslSocketFactory::ClientSslSocketFactory(Envoy::Ssl::ClientContextConfigPt
     : manager_(manager), stats_scope_(stats_scope), stats_(generateStats(stats_scope)),
       config_(std::move(config)),
       ssl_ctx_(manager_.createSslClientContext(stats_scope_, *config_)) {
-  config_->setSecretUpdateCallback([this]() { onAddOrUpdateSecret(); });
+  config_->setSecretUpdateCallback([this]() { return onAddOrUpdateSecret(); });
 }
 
 ClientSslSocketFactory::~ClientSslSocketFactory() { manager_.removeContext(ssl_ctx_); }
@@ -67,7 +67,7 @@ Network::TransportSocketPtr ClientSslSocketFactory::createTransportSocket(
 
 bool ClientSslSocketFactory::implementsSecureTransport() const { return true; }
 
-void ClientSslSocketFactory::onAddOrUpdateSecret() {
+absl::Status ClientSslSocketFactory::onAddOrUpdateSecret() {
   ENVOY_LOG(debug, "Secret is updated.");
   auto ctx = manager_.createSslClientContext(stats_scope_, *config_);
   {
@@ -76,6 +76,7 @@ void ClientSslSocketFactory::onAddOrUpdateSecret() {
   }
   manager_.removeContext(ctx);
   stats_.ssl_context_update_by_sds_.inc();
+  return absl::OkStatus();
 }
 
 Envoy::Ssl::ClientContextSharedPtr ClientSslSocketFactory::sslCtx() {

source/common/tls/client_ssl_socket.h

@@ -49,7 +49,7 @@ class ClientSslSocketFactory : public Network::CommonUpstreamTransportSocketFact
   bool supportsAlpn() const override { return true; }
 
   // Secret::SecretCallbacks
-  void onAddOrUpdateSecret() override;
+  absl::Status onAddOrUpdateSecret() override;
 
   OptRef<const Ssl::ClientContextConfig> clientContextConfig() const override { return {*config_}; }
 

source/common/tls/context_config_impl.cc

@@ -272,7 +272,7 @@ Ssl::CertificateValidationContextConfigPtr ContextConfigImpl::getCombinedValidat
   return std::move(config_or_status.value());
 }
 
-void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback) {
+void ContextConfigImpl::setSecretUpdateCallback(std::function<absl::Status()> callback) {
   // When any of tls_certificate_providers_ receives a new secret, this callback updates
   // ContextConfigImpl::tls_certificate_configs_ with new secret.
   for (const auto& tls_certificate_provider : tls_certificate_providers_) {
@@ -287,8 +287,7 @@ void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback)
                   std::unique_ptr<Ssl::TlsCertificateConfigImpl>));
             }
           }
-          callback();
-          return absl::OkStatus();
+          return callback();
         }));
   }
   if (certificate_validation_context_provider_) {
@@ -301,8 +300,7 @@ void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback)
           certificate_validation_context_provider_->addUpdateCallback([this, callback]() {
             validation_context_config_ = getCombinedValidationContextConfig(
                 *certificate_validation_context_provider_->secret());
-            callback();
-            return absl::OkStatus();
+            return callback();
           });
     } else {
       // Once certificate_validation_context_provider_ receives new secret, this callback updates
@@ -315,8 +313,7 @@ void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback)
               throwEnvoyExceptionOrPanic(std::string(config_or_status.status().message()));
             }
             validation_context_config_ = std::move(config_or_status.value());
-            callback();
-            return absl::OkStatus();
+            return callback();
           });
     }
   }
@@ -452,16 +449,15 @@ ServerContextConfigImpl::ServerContextConfigImpl(
   }
 }
 
-void ServerContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback) {
+void ServerContextConfigImpl::setSecretUpdateCallback(std::function<absl::Status()> callback) {
   ContextConfigImpl::setSecretUpdateCallback(callback);
   if (session_ticket_keys_provider_) {
     // Once session_ticket_keys_ receives new secret, this callback updates
     // ContextConfigImpl::session_ticket_keys_ with new session ticket keys.
     stk_update_callback_handle_ =
         session_ticket_keys_provider_->addUpdateCallback([this, callback]() {
           session_ticket_keys_ = getSessionTicketKeys(*session_ticket_keys_provider_->secret());
-          callback();
-          return absl::OkStatus();
+          return callback();
         });
   }
 }

source/common/tls/context_config_impl.h

@@ -59,7 +59,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
     return tls_is_ready && combined_cvc_is_ready && cvc_is_ready;
   }
 
-  void setSecretUpdateCallback(std::function<void()> callback) override;
+  void setSecretUpdateCallback(std::function<absl::Status()> callback) override;
   Ssl::HandshakerFactoryCb createHandshaker() const override;
   Ssl::HandshakerCapabilities capabilities() const override { return capabilities_; }
   Ssl::SslCtxCb sslctxCb() const override { return sslctx_cb_; }
@@ -162,7 +162,7 @@ class ServerContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Ser
     return parent_is_ready && session_ticket_keys_are_ready;
   }
 
-  void setSecretUpdateCallback(std::function<void()> callback) override;
+  void setSecretUpdateCallback(std::function<absl::Status()> callback) override;
   bool disableStatelessSessionResumption() const override {
     return disable_stateless_session_resumption_;
   }

source/common/tls/server_ssl_socket.cc

@@ -35,7 +35,7 @@ ServerSslSocketFactory::ServerSslSocketFactory(Envoy::Ssl::ServerContextConfigPt
     : manager_(manager), stats_scope_(stats_scope), stats_(generateStats(stats_scope)),
       config_(std::move(config)), server_names_(server_names),
       ssl_ctx_(manager_.createSslServerContext(stats_scope_, *config_, server_names_, nullptr)) {
-  config_->setSecretUpdateCallback([this]() { onAddOrUpdateSecret(); });
+  config_->setSecretUpdateCallback([this]() { return onAddOrUpdateSecret(); });
 }
 
 ServerSslSocketFactory::~ServerSslSocketFactory() { manager_.removeContext(ssl_ctx_); }
@@ -65,7 +65,7 @@ Network::TransportSocketPtr ServerSslSocketFactory::createDownstreamTransportSoc
 
 bool ServerSslSocketFactory::implementsSecureTransport() const { return true; }
 
-void ServerSslSocketFactory::onAddOrUpdateSecret() {
+absl::Status ServerSslSocketFactory::onAddOrUpdateSecret() {
   ENVOY_LOG(debug, "Secret is updated.");
   auto ctx = manager_.createSslServerContext(stats_scope_, *config_, server_names_, nullptr);
   {
@@ -75,6 +75,7 @@ void ServerSslSocketFactory::onAddOrUpdateSecret() {
   manager_.removeContext(ctx);
 
   stats_.ssl_context_update_by_sds_.inc();
+  return absl::OkStatus();
 }
 
 } // namespace Tls

source/common/tls/server_ssl_socket.h

@@ -44,7 +44,7 @@ class ServerSslSocketFactory : public Network::DownstreamTransportSocketFactory,
   bool implementsSecureTransport() const override;
 
   // Secret::SecretCallbacks
-  void onAddOrUpdateSecret() override;
+  absl::Status onAddOrUpdateSecret() override;
 
 private:
   Ssl::ContextManager& manager_;

source/common/upstream/health_discovery_service.cc

@@ -541,10 +541,13 @@ ProdClusterInfoFactory::createClusterInfo(const CreateClusterInfoParams& params)
       params.server_context_.clusterManager(), params.server_context_.messageValidationVisitor());
 
   // TODO(JimmyCYJ): Support SDS for HDS cluster.
-  Network::UpstreamTransportSocketFactoryPtr socket_factory =
-      Upstream::createTransportSocketFactory(params.cluster_, factory_context);
-  auto socket_matcher = std::make_unique<TransportSocketMatcherImpl>(
-      params.cluster_.transport_socket_matches(), factory_context, socket_factory, *scope);
+  Network::UpstreamTransportSocketFactoryPtr socket_factory = THROW_OR_RETURN_VALUE(
+      Upstream::createTransportSocketFactory(params.cluster_, factory_context),
+      Network::UpstreamTransportSocketFactoryPtr);
+  auto socket_matcher = THROW_OR_RETURN_VALUE(
+      TransportSocketMatcherImpl::create(params.cluster_.transport_socket_matches(),
+                                         factory_context, socket_factory, *scope),
+      std::unique_ptr<TransportSocketMatcherImpl>);
 
   return std::make_unique<ClusterInfoImpl>(
       params.server_context_.initManager(), params.server_context_, params.cluster_,

source/common/upstream/transport_socket_match_impl.cc

@@ -9,11 +9,24 @@
 namespace Envoy {
 namespace Upstream {
 
+absl::StatusOr<std::unique_ptr<TransportSocketMatcherImpl>> TransportSocketMatcherImpl::create(
+    const Protobuf::RepeatedPtrField<envoy::config::cluster::v3::Cluster::TransportSocketMatch>&
+        socket_matches,
+    Server::Configuration::TransportSocketFactoryContext& factory_context,
+    Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope) {
+  absl::Status creation_status = absl::OkStatus();
+  auto ret = std::unique_ptr<TransportSocketMatcherImpl>(new TransportSocketMatcherImpl(
+      socket_matches, factory_context, default_factory, stats_scope, creation_status));
+  RETURN_IF_NOT_OK(creation_status);
+  return ret;
+}
+
 TransportSocketMatcherImpl::TransportSocketMatcherImpl(
     const Protobuf::RepeatedPtrField<envoy::config::cluster::v3::Cluster::TransportSocketMatch>&
         socket_matches,
     Server::Configuration::TransportSocketFactoryContext& factory_context,
-    Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope)
+    Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope,
+    absl::Status& creation_status)
     : stats_scope_(stats_scope),
       default_match_("default", std::move(default_factory), generateStats("default")) {
   for (const auto& socket_match : socket_matches) {
@@ -22,9 +35,10 @@ TransportSocketMatcherImpl::TransportSocketMatcherImpl(
         Server::Configuration::UpstreamTransportSocketConfigFactory>(socket_config);
     ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig(
         socket_config, factory_context.messageValidationVisitor(), config_factory);
-    FactoryMatch factory_match(
-        socket_match.name(), config_factory.createTransportSocketFactory(*message, factory_context),
-        generateStats(absl::StrCat(socket_match.name(), ".")));
+    auto factory_or_error = config_factory.createTransportSocketFactory(*message, factory_context);
+    SET_AND_RETURN_IF_NOT_OK(factory_or_error.status(), creation_status);
+    FactoryMatch factory_match(socket_match.name(), std::move(factory_or_error.value()),
+                               generateStats(absl::StrCat(socket_match.name(), ".")));
     for (const auto& kv : socket_match.match().fields()) {
       factory_match.label_set.emplace_back(kv.first, kv.second);
     }

source/common/upstream/transport_socket_match_impl.h

@@ -22,6 +22,12 @@ namespace Upstream {
 class TransportSocketMatcherImpl : public Logger::Loggable<Logger::Id::upstream>,
                                    public TransportSocketMatcher {
 public:
+  static absl::StatusOr<std::unique_ptr<TransportSocketMatcherImpl>> create(
+      const Protobuf::RepeatedPtrField<envoy::config::cluster::v3::Cluster::TransportSocketMatch>&
+          socket_matches,
+      Server::Configuration::TransportSocketFactoryContext& factory_context,
+      Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope);
+
   struct FactoryMatch {
     FactoryMatch(std::string match_name, Network::UpstreamTransportSocketFactoryPtr socket_factory,
                  TransportSocketMatchStats match_stats)
@@ -32,12 +38,6 @@ class TransportSocketMatcherImpl : public Logger::Loggable<Logger::Id::upstream>
     mutable TransportSocketMatchStats stats;
   };
 
-  TransportSocketMatcherImpl(
-      const Protobuf::RepeatedPtrField<envoy::config::cluster::v3::Cluster::TransportSocketMatch>&
-          socket_matches,
-      Server::Configuration::TransportSocketFactoryContext& factory_context,
-      Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope);
-
   MatchData resolve(const envoy::config::core::v3::Metadata* metadata) const override;
 
   bool allMatchesSupportAlpn() const override {
@@ -53,6 +53,13 @@ class TransportSocketMatcherImpl : public Logger::Loggable<Logger::Id::upstream>
   }
 
 protected:
+  TransportSocketMatcherImpl(
+      const Protobuf::RepeatedPtrField<envoy::config::cluster::v3::Cluster::TransportSocketMatch>&
+          socket_matches,
+      Server::Configuration::TransportSocketFactoryContext& factory_context,
+      Network::UpstreamTransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope,
+      absl::Status& creation_status);
+
   TransportSocketMatchStats generateStats(const std::string& prefix);
   Stats::Scope& stats_scope_;
   FactoryMatch default_match_;