forward proxy: per host SNI and SAN verification

docs/root/configuration/http_filters/dynamic_forward_proxy_filter.rst

@@ -25,14 +25,12 @@ HTTP dynamic forward proxy.
   <envoy_api_field_core.Http1ProtocolOptions.allow_absolute_url>` parameter has been set to true
   to allow Envoy to proxy absolute HTTP URLs.
 
-.. attention::
+.. note::
 
-  While configuring a :ref:`tls_context <envoy_api_field_Cluster.tls_Context>` on the cluster with
+  Configuring a :ref:`tls_context <envoy_api_field_Cluster.tls_Context>` on the cluster with
   *trusted_ca* certificates instructs Envoy to use TLS when connecting to upstream hosts and verify
-  the certificate chain, currently it is not possible to configure per-host TLS configuration
-  parameters including SNI, subject alt name verification, etc. This will be added in a future
-  change. **This means that the following configuration will not fully validate TLS certificates**.
-  Use with care until full support for per-host validation is implemented.
+  the certificate chain. Additionally, Envoy will automatically perform SAN verification for the
+  resolved host name as well as specify the host name via SNI.
 
 .. code-block:: yaml
 

docs/root/intro/arch_overview/http/http_proxy.rst

@@ -27,6 +27,8 @@ follows:
 * A special load balancer will select the right host to use based on the HTTP host/authority header
   during forwarding.
 * Hosts that have not been used for a period of time are subject to a TTL that will purge them.
+* When the upstream cluster has been configured with a TLS context, Envoy will automatically perform
+  SAN verification for the resolved host name as well as specify the host name via SNI.
 
 The above implementation details mean that at steady state Envoy can forward a large volume of
 HTTP proxy traffic while all DNS resolution happens asynchronously in the background. Additionally,

include/envoy/network/transport_socket.h

@@ -165,6 +165,12 @@ class TransportSocketOptions {
    */
   virtual const absl::optional<std::string>& serverNameOverride() const PURE;
 
+  /**
+   * @return the optional overridden SAN names to verify, if the transport socket supports SAN
+   *         verification.
+   */
+  virtual const std::vector<std::string>& verifySubjectAltNameListOverride() const PURE;
+
   /**
    * @param vector of bytes to which the option should append hash key data that will be used
    *        to separate connections based on the option. Any data already in the key vector must
@@ -173,7 +179,8 @@ class TransportSocketOptions {
   virtual void hashKey(std::vector<uint8_t>& key) const PURE;
 };
 
-using TransportSocketOptionsSharedPtr = std::shared_ptr<TransportSocketOptions>;
+// TODO(mattklein123): Rename to TransportSocketOptionsConstSharedPtr in a dedicated follow up.
+using TransportSocketOptionsSharedPtr = std::shared_ptr<const TransportSocketOptions>;
 
 /**
  * A factory for creating transport socket. It will be associated to filter chains and clusters.

source/common/config/tls_context_json.cc

@@ -9,20 +9,6 @@
 namespace Envoy {
 namespace Config {
 
-void TlsContextJson::translateDownstreamTlsContext(
-    const Json::Object& json_tls_context,
-    envoy::api::v2::auth::DownstreamTlsContext& downstream_tls_context) {
-  translateCommonTlsContext(json_tls_context, *downstream_tls_context.mutable_common_tls_context());
-  JSON_UTIL_SET_BOOL(json_tls_context, downstream_tls_context, require_client_certificate);
-
-  const std::vector<std::string> paths =
-      json_tls_context.getStringArray("session_ticket_key_paths", true);
-  for (const std::string& path : paths) {
-    downstream_tls_context.mutable_session_ticket_keys()->mutable_keys()->Add()->set_filename(path);
-  }
-  MessageUtil::validate(downstream_tls_context);
-}
-
 void TlsContextJson::translateUpstreamTlsContext(
     const Json::Object& json_tls_context,
     envoy::api::v2::auth::UpstreamTlsContext& upstream_tls_context) {

source/common/config/tls_context_json.h

@@ -8,15 +8,6 @@ namespace Config {
 
 class TlsContextJson {
 public:
-  /**
-   * Translate a v1 JSON TLS context to v2 envoy::api::v2::auth::DownstreamTlsContext.
-   * @param json_tls_context source v1 JSON TLS context object.
-   * @param downstream_tls_context destination v2 envoy::api::v2::Cluster.
-   */
-  static void
-  translateDownstreamTlsContext(const Json::Object& json_tls_context,
-                                envoy::api::v2::auth::DownstreamTlsContext& downstream_tls_context);
-
   /**
    * Translate a v1 JSON TLS context to v2 envoy::api::v2::auth::UpstreamTlsContext.
    * @param json_tls_context source v1 JSON TLS context object.

source/common/network/transport_socket_options_impl.h

@@ -7,19 +7,25 @@ namespace Network {
 
 class TransportSocketOptionsImpl : public TransportSocketOptions {
 public:
-  TransportSocketOptionsImpl(absl::string_view override_server_name = "")
+  TransportSocketOptionsImpl(absl::string_view override_server_name = "",
+                             std::vector<std::string>&& override_verify_san_list = {})
       : override_server_name_(override_server_name.empty()
                                   ? absl::nullopt
-                                  : absl::optional<std::string>(override_server_name)) {}
+                                  : absl::optional<std::string>(override_server_name)),
+        override_verify_san_list_{std::move(override_verify_san_list)} {}
 
   // Network::TransportSocketOptions
   const absl::optional<std::string>& serverNameOverride() const override {
     return override_server_name_;
   }
+  const std::vector<std::string>& verifySubjectAltNameListOverride() const override {
+    return override_verify_san_list_;
+  }
   void hashKey(std::vector<uint8_t>& key) const override;
 
 private:
   const absl::optional<std::string> override_server_name_;
+  const std::vector<std::string> override_verify_san_list_;
 };
 
 } // namespace Network

source/common/upstream/logical_dns_cluster.cc

@@ -89,8 +89,8 @@ void LogicalDnsCluster::startResolve() {
           }
 
           if (!logical_host_) {
-            logical_host_.reset(
-                new LogicalHost(info_, hostname_, new_address, localityLbEndpoint(), lbEndpoint()));
+            logical_host_.reset(new LogicalHost(info_, hostname_, new_address, localityLbEndpoint(),
+                                                lbEndpoint(), nullptr));
 
             const auto& locality_lb_endpoint = localityLbEndpoint();
             PriorityStateManager priority_state_manager(*this, local_info_, nullptr);

source/common/upstream/logical_host.cc

@@ -8,7 +8,9 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection(
     Network::TransportSocketOptionsSharedPtr transport_socket_options) const {
   const auto current_address = address();
   return {HostImpl::createConnection(dispatcher, cluster(), current_address, options,
-                                     transport_socket_options),
+                                     override_transport_socket_options_ != nullptr
+                                         ? override_transport_socket_options_
+                                         : transport_socket_options),
           std::make_shared<RealHostDescription>(current_address, shared_from_this())};
 }
 

source/common/upstream/logical_host.h

@@ -14,11 +14,13 @@ class LogicalHost : public HostImpl {
   LogicalHost(const ClusterInfoConstSharedPtr& cluster, const std::string& hostname,
               const Network::Address::InstanceConstSharedPtr& address,
               const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint,
-              const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint)
+              const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint,
+              const Network::TransportSocketOptionsSharedPtr& override_transport_socket_options)
       : HostImpl(cluster, hostname, address, lb_endpoint.metadata(),
                  lb_endpoint.load_balancing_weight().value(), locality_lb_endpoint.locality(),
                  lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority(),
-                 lb_endpoint.health_status()) {}
+                 lb_endpoint.health_status()),
+        override_transport_socket_options_(override_transport_socket_options) {}
 
   // Set the new address. Updates are typically rare so a R/W lock is used for address updates.
   // Note that the health check address update requires no lock to be held since it is only
@@ -51,6 +53,7 @@ class LogicalHost : public HostImpl {
   }
 
 private:
+  const Network::TransportSocketOptionsSharedPtr override_transport_socket_options_;
   mutable absl::Mutex address_lock_;
 };
 

source/extensions/clusters/dynamic_forward_proxy/BUILD

@@ -13,6 +13,7 @@ envoy_cc_library(
     srcs = ["cluster.cc"],
     hdrs = ["cluster.h"],
     deps = [
+        "//source/common/network:transport_socket_options_lib",
         "//source/common/upstream:cluster_factory_lib",
         "//source/common/upstream:logical_host_lib",
         "//source/extensions/clusters:well_known_names",

source/extensions/clusters/dynamic_forward_proxy/cluster.cc

@@ -1,16 +1,14 @@
 #include "extensions/clusters/dynamic_forward_proxy/cluster.h"
 
+#include "common/network/transport_socket_options_impl.h"
+
 #include "extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h"
 
 namespace Envoy {
 namespace Extensions {
 namespace Clusters {
 namespace DynamicForwardProxy {
 
-// TODO(mattklein123): Make sure that the cluster's hosts display their host name in admin output.
-// TODO(mattklein123): Allow customizing TLS on a per-host basis. For example, setting SNI and
-//                     doing certificate validation.
-
 Cluster::Cluster(
     const envoy::api::v2::Cluster& cluster,
     const envoy::config::cluster::dynamic_forward_proxy::v2alpha::ClusterConfig& config,
@@ -58,14 +56,15 @@ void Cluster::onDnsHostAddOrUpdate(
   HostInfoMapSharedPtr current_map = getCurrentHostMap();
   const auto host_map_it = current_map->find(host);
   if (host_map_it != current_map->end()) {
-    // If we only have an address change, we can do that swap inline without any other updates. The
-    // appropriate R/W locking is in place to allow this. The details of this locking are:
+    // If we only have an address change, we can do that swap inline without any other updates.
+    // The appropriate R/W locking is in place to allow this. The details of this locking are:
     //  - Hosts are not thread local, they are global.
     //  - We take a read lock when reading the address and a write lock when changing it.
     //  - Address updates are very rare.
-    //  - Address reads are only done when a connection is being made and a "real" host description
-    //    is created or the host is queries via the admin endpoint. Both of these operations are
-    //    relatively rare and the read lock is held for a short period of time.
+    //  - Address reads are only done when a connection is being made and a "real" host
+    //    description is created or the host is queried via the admin endpoint. Both of
+    //    these operations are relatively rare and the read lock is held for a short period
+    //    of time.
     //
     // TODO(mattklein123): Right now the dynamic forward proxy / DNS cache works similar to how
     //                     logical DNS works, meaning that we only store a single address per
@@ -82,11 +81,20 @@ void Cluster::onDnsHostAddOrUpdate(
   }
 
   ENVOY_LOG(debug, "adding new dfproxy cluster host '{}'", host);
+
+  // Create an override transport socket options that automatically provides both SNI as well as
+  // SAN verification for the resolved host if the cluster has been configured with TLS.
+  // TODO(mattklein123): If the host is an IP address we should not set SNI.
+  Network::TransportSocketOptionsSharedPtr transport_socket_options =
+      std::make_shared<Network::TransportSocketOptionsImpl>(
+          host_info->resolvedHost(), std::vector<std::string>{host_info->resolvedHost()});
+
   const auto new_host_map = std::make_shared<HostInfoMap>(*current_map);
-  const auto emplaced = new_host_map->try_emplace(
-      host, host_info,
-      std::make_shared<Upstream::LogicalHost>(info(), host, host_info->address(),
-                                              dummy_locality_lb_endpoint_, dummy_lb_endpoint_));
+  const auto emplaced =
+      new_host_map->try_emplace(host, host_info,
+                                std::make_shared<Upstream::LogicalHost>(
+                                    info(), host, host_info->address(), dummy_locality_lb_endpoint_,
+                                    dummy_lb_endpoint_, transport_socket_options));
   Upstream::HostVector hosts_added;
   hosts_added.emplace_back(emplaced.first->second.logical_host_);
 

source/extensions/common/dynamic_forward_proxy/dns_cache.h

@@ -23,6 +23,12 @@ class DnsHostInfo {
    */
   virtual Network::Address::InstanceConstSharedPtr address() PURE;
 
+  /**
+   * Returns the host that was actually resolved via DNS. If port was originally specified it will
+   * be stripped from this return value.
+   */
+  virtual const std::string& resolvedHost() PURE;
+
   /**
    * Indicates that the host has been used and should not be purged depending on any configured
    * TTL policy

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc

@@ -132,12 +132,12 @@ void DnsCacheImpl::onReResolve(const std::string& host) {
 
 void DnsCacheImpl::startResolve(const std::string& host, PrimaryHostInfo& host_info) {
   ENVOY_LOG(debug, "starting main thread resolve for host='{}' dns='{}' port='{}'", host,
-            host_info.host_to_resolve_, host_info.port_);
+            host_info.host_info_->resolved_host_, host_info.port_);
   ASSERT(host_info.active_query_ == nullptr);
 
   stats_.dns_query_attempt_.inc();
   host_info.active_query_ =
-      resolver_->resolve(host_info.host_to_resolve_, dns_lookup_family_,
+      resolver_->resolve(host_info.host_info_->resolved_host_, dns_lookup_family_,
                          [this, host](std::list<Network::DnsResponse>&& response) {
                            finishResolve(host, std::move(response));
                          });
@@ -151,11 +151,8 @@ void DnsCacheImpl::finishResolve(const std::string& host,
 
   auto& primary_host_info = *primary_host_it->second;
   primary_host_info.active_query_ = nullptr;
-  const bool first_resolve = primary_host_info.host_info_ == nullptr;
-  if (primary_host_info.host_info_ == nullptr) {
-    primary_host_info.host_info_ =
-        std::make_shared<DnsHostInfoImpl>(main_thread_dispatcher_.timeSource());
-  }
+  const bool first_resolve = !primary_host_info.host_info_->first_resolve_complete_;
+  primary_host_info.host_info_->first_resolve_complete_ = true;
 
   const auto new_address = !response.empty()
                                ? Network::Utility::getAddressWithPort(*(response.front().address_),
@@ -211,9 +208,8 @@ void DnsCacheImpl::runRemoveCallbacks(const std::string& host) {
 void DnsCacheImpl::updateTlsHostsMap() {
   TlsHostMapSharedPtr new_host_map = std::make_shared<TlsHostMap>();
   for (const auto& primary_host : primary_hosts_) {
-    // Do not include hosts without host info. This only happens before we get the first
-    // resolution.
-    if (primary_host.second->host_info_ != nullptr) {
+    // Do not include hosts that have not resolved at least once.
+    if (primary_host.second->host_info_->first_resolve_complete_) {
       new_host_map->emplace(primary_host.first, primary_host.second->host_info_);
     }
   }
@@ -249,8 +245,10 @@ void DnsCacheImpl::ThreadLocalHostInfo::updateHostMap(const TlsHostMapSharedPtr&
 DnsCacheImpl::PrimaryHostInfo::PrimaryHostInfo(DnsCacheImpl& parent,
                                                absl::string_view host_to_resolve, uint16_t port,
                                                const Event::TimerCb& timer_cb)
-    : parent_(parent), host_to_resolve_(host_to_resolve), port_(port),
-      refresh_timer_(parent.main_thread_dispatcher_.createTimer(timer_cb)) {
+    : parent_(parent), port_(port),
+      refresh_timer_(parent.main_thread_dispatcher_.createTimer(timer_cb)),
+      host_info_(std::make_shared<DnsHostInfoImpl>(parent.main_thread_dispatcher_.timeSource(),
+                                                   host_to_resolve)) {
   parent_.stats_.host_added_.inc();
   parent_.stats_.num_hosts_.inc();
 }

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.h

@@ -71,13 +71,19 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
   };
 
   struct DnsHostInfoImpl : public DnsHostInfo {
-    DnsHostInfoImpl(TimeSource& time_source) : time_source_(time_source) { touch(); }
+    DnsHostInfoImpl(TimeSource& time_source, absl::string_view resolved_host)
+        : time_source_(time_source), resolved_host_(resolved_host) {
+      touch();
+    }
 
     // DnsHostInfo
     Network::Address::InstanceConstSharedPtr address() override { return address_; }
+    const std::string& resolvedHost() override { return resolved_host_; }
     void touch() override { last_used_time_ = time_source_.monotonicTime().time_since_epoch(); }
 
     TimeSource& time_source_;
+    const std::string resolved_host_;
+    bool first_resolve_complete_{};
     Network::Address::InstanceConstSharedPtr address_;
     // Using std::chrono::steady_clock::duration is required for compilation within an atomic vs.
     // using MonotonicTime.
@@ -93,10 +99,9 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
     ~PrimaryHostInfo();
 
     DnsCacheImpl& parent_;
-    const std::string host_to_resolve_;
     const uint16_t port_;
     const Event::TimerPtr refresh_timer_;
-    DnsHostInfoImplSharedPtr host_info_;
+    const DnsHostInfoImplSharedPtr host_info_;
     Network::ActiveDnsQuery* active_query_{};
   };
 

source/extensions/transport_sockets/tls/context_config_impl.cc

@@ -369,17 +369,6 @@ ServerContextConfigImpl::ServerContextConfigImpl(
   }
 }
 
-ServerContextConfigImpl::ServerContextConfigImpl(
-    const Json::Object& config,
-    Server::Configuration::TransportSocketFactoryContext& factory_context)
-    : ServerContextConfigImpl(
-          [&config] {
-            envoy::api::v2::auth::DownstreamTlsContext downstream_tls_context;
-            Config::TlsContextJson::translateDownstreamTlsContext(config, downstream_tls_context);
-            return downstream_tls_context;
-          }(),
-          factory_context) {}
-
 // Append a SessionTicketKey to keys, initializing it with key_data.
 // Throws if key_data is invalid.
 void ServerContextConfigImpl::validateAndAppendKey(

source/extensions/transport_sockets/tls/context_config_impl.h

@@ -130,9 +130,6 @@ class ServerContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Ser
   ServerContextConfigImpl(
       const envoy::api::v2::auth::DownstreamTlsContext& config,
       Server::Configuration::TransportSocketFactoryContext& secret_provider_context);
-  ServerContextConfigImpl(
-      const Json::Object& config,
-      Server::Configuration::TransportSocketFactoryContext& secret_provider_context);
 
   // Ssl::ServerContextConfig
   bool requireClientCertificate() const override { return require_client_certificate_; }