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>

46

@@ -0,0 +1 @@
+22 -

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/network/utility.cc

@@ -53,16 +53,21 @@ std::string Utility::urlFromDatagramAddress(const Address::Instance& addr) {
   }
 }
 
-Address::InstanceConstSharedPtr Utility::resolveUrl(const std::string& url) {
+absl::StatusOr<Address::InstanceConstSharedPtr> Utility::resolveUrl(const std::string& url) {
+  Address::InstanceConstSharedPtr address{};
   if (urlIsTcpScheme(url)) {
-    return parseInternetAddressAndPort(url.substr(TCP_SCHEME.size()));
+    address = parseInternetAddressAndPortNoThrow(url.substr(TCP_SCHEME.size()));
   } else if (urlIsUdpScheme(url)) {
-    return parseInternetAddressAndPort(url.substr(UDP_SCHEME.size()));
+    address = parseInternetAddressAndPortNoThrow(url.substr(UDP_SCHEME.size()));
   } else if (urlIsUnixScheme(url)) {
     return std::make_shared<Address::PipeInstance>(url.substr(UNIX_SCHEME.size()));
   } else {
-    throwEnvoyExceptionOrPanic(absl::StrCat("unknown protocol scheme: ", url));
+    return absl::InvalidArgumentError(absl::StrCat("unknown protocol scheme: ", url));
+  }
+  if (!address) {
+    return absl::InvalidArgumentError(absl::StrCat("malformed IP address: ", url));
   }
+  return address;
 }
 
 StatusOr<Socket::Type> Utility::socketTypeFromUrl(const std::string& url) {
@@ -160,7 +165,7 @@ Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string&
   const Address::InstanceConstSharedPtr address =
       parseInternetAddressNoThrow(ip_address, port, v6only);
   if (address == nullptr) {
-    throwWithMalformedIp(ip_address);
+    throwEnvoyExceptionOrPanic(absl::StrCat("malformed IP address: ", ip_address));
   }
   return address;
 }
@@ -208,28 +213,13 @@ Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool
   return nullptr;
 }
 
-Address::InstanceConstSharedPtr Utility::parseInternetAddressAndPort(const std::string& ip_address,
-                                                                     bool v6only) {
-
-  const Address::InstanceConstSharedPtr address =
-      parseInternetAddressAndPortNoThrow(ip_address, v6only);
-  if (address == nullptr) {
-    throwWithMalformedIp(ip_address);
-  }
-  return address;
-}
-
 Address::InstanceConstSharedPtr Utility::copyInternetAddressAndPort(const Address::Ip& ip) {
   if (ip.version() == Address::IpVersion::v4) {
     return std::make_shared<Address::Ipv4Instance>(ip.addressAsString(), ip.port());
   }
   return std::make_shared<Address::Ipv6Instance>(ip.addressAsString(), ip.port());
 }
 
-void Utility::throwWithMalformedIp(absl::string_view ip_address) {
-  throwEnvoyExceptionOrPanic(absl::StrCat("malformed IP address: ", ip_address));
-}
-
 // TODO(hennna): Currently getLocalAddress does not support choosing between
 // multiple interfaces and addresses not returned by getifaddrs. In addition,
 // the default is to return a loopback address of type version. This function may
@@ -428,45 +418,6 @@ Address::InstanceConstSharedPtr Utility::getOriginalDst(Socket& sock) {
 #endif
 }
 
-void Utility::parsePortRangeList(absl::string_view string, std::list<PortRange>& list) {
-  const auto ranges = StringUtil::splitToken(string, ",");
-  for (const auto& s : ranges) {
-    const std::string s_string{s};
-    std::stringstream ss(s_string);
-    uint32_t min = 0;
-    uint32_t max = 0;
-
-    if (absl::StrContains(s, '-')) {
-      char dash = 0;
-      ss >> min;
-      ss >> dash;
-      ss >> max;
-    } else {
-      ss >> min;
-      max = min;
-    }
-
-    if (s.empty() || (min > 65535) || (max > 65535) || ss.fail() || !ss.eof()) {
-      throwEnvoyExceptionOrPanic(fmt::format("invalid port number or range '{}'", s_string));
-    }
-
-    list.emplace_back(PortRange(min, max));
-  }
-}
-
-bool Utility::portInRangeList(const Address::Instance& address, const std::list<PortRange>& list) {
-  if (address.type() != Address::Type::Ip) {
-    return false;
-  }
-
-  for (const PortRange& p : list) {
-    if (p.contains(address.ip()->port())) {
-      return true;
-    }
-  }
-  return false;
-}
-
 absl::uint128 Utility::Ip6ntohl(const absl::uint128& address) {
 #ifdef ABSL_IS_LITTLE_ENDIAN
   return flipOrder(address);

source/common/network/utility.h

@@ -16,22 +16,6 @@
 namespace Envoy {
 namespace Network {
 
-/**
- * Utility class to represent TCP/UDP port range
- */
-class PortRange {
-public:
-  PortRange(uint32_t min, uint32_t max) : min_(min), max_(max) {}
-
-  bool contains(uint32_t port) const { return (port >= min_ && port <= max_); }
-
-private:
-  const uint32_t min_;
-  const uint32_t max_;
-};
-
-using PortRangeList = std::list<PortRange>;
-
 /**
  * A callback interface used by readFromSocket() to pass packets read from
  * socket.
@@ -119,10 +103,9 @@ class Utility {
   /**
    * Resolve a URL.
    * @param url supplies the url to resolve.
-   * @return Address::InstanceConstSharedPtr the resolved address.
-   * @throw EnvoyException if url is invalid.
+   * @return Address::InstanceConstSharedPtr the resolved address or an error status
    */
-  static Address::InstanceConstSharedPtr resolveUrl(const std::string& url);
+  static absl::StatusOr<Address::InstanceConstSharedPtr> resolveUrl(const std::string& url);
 
   /**
    * Determine the socket type for a URL.
@@ -153,18 +136,6 @@ class Utility {
    */
   static bool urlIsUnixScheme(absl::string_view url);
 
-  /**
-   * Parse an internet host address (IPv4 or IPv6) and create an Instance from it. The address must
-   * not include a port number. Throws EnvoyException if unable to parse the address.
-   * @param ip_address string to be parsed as an internet address.
-   * @param port optional port to include in Instance created from ip_address, 0 by default.
-   * @param v6only disable IPv4-IPv6 mapping for IPv6 addresses?
-   * @return pointer to the Instance.
-   * @throw EnvoyException in case of a malformed IP address.
-   */
-  static Address::InstanceConstSharedPtr
-  parseInternetAddress(const std::string& ip_address, uint16_t port = 0, bool v6only = true);
-
   /**
    * Parse an internet host address (IPv4 or IPv6) and create an Instance from it. The address must
    * not include a port number.
@@ -185,18 +156,6 @@ class Utility {
    */
   static Address::InstanceConstSharedPtr copyInternetAddressAndPort(const Address::Ip& ip);
 
-  /**
-   * Create a new Instance from an internet host address (IPv4 or IPv6) and port.
-   * @param ip_addr string to be parsed as an internet address and port. Examples:
-   *        - "1.2.3.4:80"
-   *        - "[1234:5678::9]:443"
-   * @param v6only disable IPv4-IPv6 mapping for IPv6 addresses?
-   * @return pointer to the Instance.
-   * @throw EnvoyException in case of a malformed IP address.
-   */
-  static Address::InstanceConstSharedPtr parseInternetAddressAndPort(const std::string& ip_address,
-                                                                     bool v6only = true);
-
   /**
    * Create a new Instance from an internet host address (IPv4 or IPv6) and port.
    * @param ip_addr string to be parsed as an internet address and port. Examples:
@@ -281,6 +240,18 @@ class Utility {
   static Address::InstanceConstSharedPtr getAddressWithPort(const Address::Instance& address,
                                                             uint32_t port);
 
+  /**
+   * Parse an internet host address (IPv4 or IPv6) and create an Instance from it. The address must
+   * not include a port number. Throws EnvoyException if unable to parse the address.
+   * @param ip_address string to be parsed as an internet address.
+   * @param port optional port to include in Instance created from ip_address, 0 by default.
+   * @param v6only disable IPv4-IPv6 mapping for IPv6 addresses?
+   * @return pointer to the Instance.
+   * @throw EnvoyException in case of a malformed IP address.
+   */
+  static Address::InstanceConstSharedPtr
+  parseInternetAddress(const std::string& ip_address, uint16_t port = 0, bool v6only = true);
+
   /**
    * Retrieve the original destination address from an accepted socket.
    * The address (IP and port) may be not local and the port may differ from
@@ -290,24 +261,6 @@ class Utility {
    */
   static Address::InstanceConstSharedPtr getOriginalDst(Socket& sock);
 
-  /**
-   * Parses a string containing a comma-separated list of port numbers and/or
-   * port ranges and appends the values to a caller-provided list of PortRange structures.
-   * For example, the string "1-1024,2048-4096,12345" causes 3 PortRange structures
-   * to be appended to the supplied list.
-   * @param str is the string containing the port numbers and ranges
-   * @param list is the list to append the new data structures to
-   */
-  static void parsePortRangeList(absl::string_view string, std::list<PortRange>& list);
-
-  /**
-   * Checks whether a given port number appears in at least one of the port ranges in a list
-   * @param address supplies the IP address to compare.
-   * @param list the list of port ranges in which the port may appear
-   * @return whether the port appears in at least one of the ranges in the list
-   */
-  static bool portInRangeList(const Address::Instance& address, const std::list<PortRange>& list);
-
   /**
    * Converts IPv6 absl::uint128 in network byte order to host byte order.
    * @param address supplies the IPv6 address in network byte order.
@@ -408,8 +361,6 @@ class Utility {
                                                bool allow_mmsg, uint32_t& packets_dropped);
 
 private:
-  static void throwWithMalformedIp(absl::string_view ip_address);
-
   /**
    * Takes a number and flips the order in byte chunks. The last byte of the input will be the
    * first byte in the output. The second to last byte will be the second to first byte in the

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_}; }