tls: add options to validate SANs and send SNI for upstream hostname (e…

…nvoyproxy#36903)

These options are related, but do not have to be used together.

The existing `auto_sni` and `auto_san_validation` make Envoy set
upstream TLS SNI and validate SANs based on the downstream requests's
`:authority` (or override header).

The new `auto_host_sni` option causes Envoy to set the upstream TLS SNI
to the hostname in the cluster for the host being connected to. For
example, in a DNS cluster with server1.example.com and server2.example
as the cluster members, connections would have SNI of
server1.example.com and server2.example.com, respectively.

The new `auto_sni_san_validation` option causes Envoy to validate that
the peer's server certificate has a DNS SAN that matches whatever SNI
value was sent (regardless of how it's configured). This is appropriate
to set in many/most configurations for normally-behaving upstream
servers without special certificate validation requirements.

Signed-off-by: Greg Greenway <ggreenway@apple.com>

api/envoy/config/core/v3/protocol.proto

@@ -123,13 +123,19 @@ message UpstreamHttpProtocolOptions {
   // header when :ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>`
   // is set, as seen by the :ref:`router filter <config_http_filters_router>`.
   // Does nothing if a filter before the http router filter sets the corresponding metadata.
+  //
+  // See :ref:`SNI configuration <start_quick_start_securing_sni_client>` for details on how this
+  // interacts with other validation options.
   bool auto_sni = 1;
 
   // Automatic validate upstream presented certificate for new upstream connections based on the
   // downstream HTTP host/authority header or any other arbitrary header when :ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>`
   // is set, as seen by the :ref:`router filter <config_http_filters_router>`.
   // This field is intended to be set with ``auto_sni`` field.
   // Does nothing if a filter before the http router filter sets the corresponding metadata.
+  //
+  // See :ref:`validation configuration <start_quick_start_securing_validation>` for how this interacts with
+  // other validation options.
   bool auto_san_validation = 2;
 
   // An optional alternative to the host/authority header to be used for setting the SNI value.

api/envoy/extensions/transport_sockets/tls/v3/tls.proto

@@ -25,7 +25,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.transport_sockets.tls]
 // The TLS contexts below provide the transport socket configuration for upstream/downstream TLS.
 
-// [#next-free-field: 6]
+// [#next-free-field: 8]
 message UpstreamTlsContext {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.auth.UpstreamTlsContext";
@@ -42,6 +42,26 @@ message UpstreamTlsContext {
   // SNI string to use when creating TLS backend connections.
   string sni = 2 [(validate.rules).string = {max_bytes: 255}];
 
+  // If true, replaces the SNI for the connection with the hostname of the upstream host, if
+  // the hostname is known due to either a DNS cluster type or the
+  // :ref:`hostname <envoy_v3_api_field_config.endpoint.v3.Endpoint.hostname>` is set on
+  // the host.
+  //
+  // See :ref:`SNI configuration <start_quick_start_securing_sni_client>` for details on how this
+  // interacts with other validation options.
+  bool auto_host_sni = 6;
+
+  // If true, replace any Subject Alternative Name validations with a validation for a DNS SAN matching
+  // the SNI value sent. Note that the validation will be against the actual requested SNI, regardless of how it
+  // is configured.
+  //
+  // For the common case where an SNI value is sent and it is expected that the server certificate contains a SAN
+  // matching that SNI value, this option will do the correct SAN validation.
+  //
+  // See :ref:`validation configuration <start_quick_start_securing_validation>` for how this interacts with
+  // other validation options.
+  bool auto_sni_san_validation = 7;
+
   // If true, server-initiated TLS renegotiation will be allowed.
   //
   // .. attention::

changelogs/current.yaml

@@ -132,6 +132,16 @@ new_features:
 - area: tls
   change: |
     Added support for P-384 and P-521 curves for TLS server certificates.
+- area: tls
+  change: |
+    Added an :ref:`option
+    <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_host_sni>` to change the upstream
+    SNI to the configured hostname for the upstream.
+- area: tls
+  change: |
+    Added an :ref:`option
+    <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_sni_san_validation>` to validate
+    the upstream server certificate SANs against the actual SNI value sent, regardless of the method of configuring SNI.
 - area: xds
   change: |
     Added support for ADS replacement by invoking ``xdsManager().setAdsConfigSource()`` with a new config source.

docs/root/faq/configuration/sni.rst

@@ -71,11 +71,4 @@ The following is a YAML example of the above requirement.
 How do I configure SNI for clusters?
 ====================================
 
-For clusters, a fixed SNI can be set in :ref:`sni <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.sni>`.
-To derive SNI from a downstream HTTP header like, ``host`` or ``:authority``, turn on
-:ref:`auto_sni <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_sni>` to override the fixed SNI in
-:ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>`. A custom header other than the ``host`` or ``:authority`` can also be supplied using the optional
-:ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>` field.
-If upstream will present certificates with the hostname in SAN, turn on
-:ref:`auto_san_validation <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_san_validation>` too.
-It still needs a trust CA in validation context in :ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>` for trust anchor.
+See :ref:`SNI configuration <start_quick_start_securing_sni_client>` and :ref:`validation configuration <start_quick_start_securing_validation>`.

docs/root/start/quick-start/_include/envoy-demo-tls-sni.yaml

@@ -60,3 +60,11 @@ static_resources:
       typed_config:
         "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
         sni: www.envoyproxy.io
+        common_tls_context:
+          validation_context:
+            trusted_ca:
+              filename: certs/cacert.pem
+            match_typed_subject_alt_names:
+            - san_type: DNS
+              matcher:
+                exact: www.envoyproxy.io

docs/root/start/quick-start/securing.rst

@@ -84,11 +84,22 @@ Firstly, you can ensure that the certificates are from a mutually trusted certif
    :emphasize-lines: 8-11
    :caption: :download:`envoy-demo-tls-validation.yaml <_include/envoy-demo-tls-validation.yaml>`
 
-You can also ensure that the "Subject Alternative Names" for the cerficate match.
+You should also ensure that the Subject Alternative Names (SANs) for the certificate match.
 
 This is commonly used by web certificates (X.509) to identify the domain or domains that a
 certificate is valid for.
 
+For the most common case where the certificate should have a SAN matching the :ref:`SNI <start_quick_start_securing_sni_client>`
+which was sent, you can enable :ref:`auto_sni_san_validation <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_sni_san_validation>`
+and omit :ref:`match_typed_subject_alt_names <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.match_typed_subject_alt_names>`
+in the validation context.
+
+To validate that a certificate has a SAN matching the downstream request ``host`` or ``:authority`` header, you can enable
+:ref:`auto_san_validation <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_san_validation>`.
+
+When multiple validation options are configured, ``auto_san_validation`` has the highest priority, followed by ``auto_sni_san_validation``,
+followed by ``match_typed_subject_alt_names``.
+
 .. literalinclude:: _include/envoy-demo-tls-validation.yaml
    :language: yaml
    :linenos:
@@ -97,10 +108,6 @@ certificate is valid for.
    :emphasize-lines: 6-7, 10-11
    :caption: :download:`envoy-demo-tls-validation.yaml <_include/envoy-demo-tls-validation.yaml>`
 
-.. note::
-
-   If the "Subject Alternative Names" for a certificate are for a wildcard domain, eg ``*.example.com``,
-   this is what you should use when matching with ``match_typed_subject_alt_names``.
 
 .. note::
 
@@ -204,3 +211,16 @@ When connecting to an Envoy endpoint that is protected by ``SNI``, this must mat
 :ref:`server_names <envoy_v3_api_field_config.listener.v3.FilterChainMatch.server_names>` set in the endpoint's
 :ref:`filter_chain_match <envoy_v3_api_msg_config.listener.v3.FilterChainMatch>`, as
 :ref:`described above <start_quick_start_securing_sni>`.
+
+To derive SNI from a downstream HTTP header ``host`` or ``:authority``, turn on
+:ref:`auto_sni <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_sni>` to override the fixed SNI in
+:ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>`. A custom header other than
+the ``host`` or ``:authority`` can also be supplied using the optional
+:ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>` field.
+
+To derive SNI from the host Envoy is connecting to, turn on :ref:`auto_host_sni
+<envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_host_sni>`, which will use the hostname
+of the upstream endpoint.
+
+When multiple options are configured, ``auto_sni`` has the highest priority, followed by ``auto_host_sni``, followed by
+the fixed ``sni``.

envoy/common/optref.h

@@ -71,6 +71,8 @@ template <class T> struct OptRef {
    */
   bool has_value() const { return ptr_ != nullptr; }
 
+  T& value_or(T& other) const { return has_value() ? *ptr_ : other; }
+
   /**
    * @return true if the object has a value.
    */

envoy/ssl/certificate_validation_context_config.h

@@ -94,6 +94,18 @@ class CertificateValidationContextConfig {
    * @return the max depth used when verifying the certificate-chain
    */
   virtual absl::optional<uint32_t> maxVerifyDepth() const PURE;
+
+  /**
+   * @return true if the SAN validation rules should be replaced with a rule to validate that the
+   * certificate matches the transmitted SNI.
+   */
+  virtual bool autoSniSanMatch() const PURE;
+
+  // SECURITY NOTE
+  //
+  // When adding or changing this interface, it is likely that a change is needed to
+  // `DefaultCertValidator::updateDigestForSessionId` in
+  // `source/common/tls/cert_validator/default_validator.cc`.
 };
 
 using CertificateValidationContextConfigPtr = std::unique_ptr<CertificateValidationContextConfig>;

envoy/ssl/context_config.h

@@ -127,6 +127,19 @@ class ClientContextConfig : public virtual ContextConfig {
    */
   virtual const std::string& serverNameIndication() const PURE;
 
+  /**
+   * If true, replaces the SNI for the connection with the hostname of the upstream host, if
+   * the hostname is known.
+   */
+  virtual bool autoHostServerNameIndication() const PURE;
+
+  /**
+   * If true, replace any Subject Alternative Name validations with a validation for a DNS SAN
+   * matching the SNI value sent. Note that the validation will be against the actual requested SNI,
+   * regardless of how it is configured.
+   */
+  virtual bool autoSniSanMatch() const PURE;
+
   /**
    * @return true if server-initiated TLS renegotiation will be allowed.
    */

source/common/common/matchers.h

@@ -111,6 +111,13 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
     }
   }
 
+  // Helper to create an exact matcher in contexts where there is no factory context available.
+  // This is a static member instead of constructor so that it can be named for clarity of what it
+  // produces.
+  static StringMatcherImpl createExactMatcher(absl::string_view match) {
+    return StringMatcherImpl(match);
+  }
+
   // StringMatcher
   bool match(const absl::string_view value) const override {
     switch (matcher_.match_pattern_case()) {
@@ -165,6 +172,13 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
   }
 
 private:
+  StringMatcherImpl(absl::string_view exact_match)
+      : matcher_([&]() -> StringMatcherType {
+          StringMatcherType cfg;
+          cfg.set_exact(exact_match);
+          return cfg;
+        }()) {}
+
   const StringMatcherType matcher_;
   Regex::CompiledMatcherPtr regex_;
   std::string lowercase_contains_match_;

source/common/ssl/certificate_validation_context_config_impl.cc

@@ -19,7 +19,7 @@ static const std::string INLINE_STRING = "<inline>";
 
 CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
     const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config,
-    Api::Api& api)
+    bool auto_sni_san_match, Api::Api& api)
     : ca_cert_(THROW_OR_RETURN_VALUE(
           THROW_OR_RETURN_VALUE(Config::DataSource::read(config.trusted_ca(), true, api),
                                 std::string),
@@ -47,14 +47,15 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
       api_(api), only_verify_leaf_cert_crl_(config.only_verify_leaf_cert_crl()),
       max_verify_depth_(config.has_max_verify_depth()
                             ? absl::optional<uint32_t>(config.max_verify_depth().value())
-                            : absl::nullopt) {}
+                            : absl::nullopt),
+      auto_sni_san_match_(auto_sni_san_match) {}
 
 absl::StatusOr<std::unique_ptr<CertificateValidationContextConfigImpl>>
 CertificateValidationContextConfigImpl::create(
     const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& context,
-    Api::Api& api) {
+    bool auto_sni_san_match, Api::Api& api) {
   auto config = std::unique_ptr<CertificateValidationContextConfigImpl>(
-      new CertificateValidationContextConfigImpl(context, api));
+      new CertificateValidationContextConfigImpl(context, auto_sni_san_match, api));
   absl::Status status = config->initialize();
   if (status.ok()) {
     return config;

source/common/ssl/certificate_validation_context_config_impl.h

@@ -18,7 +18,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
   // Create a CertificateValidationContextConfigImpl or return an error status.
   static absl::StatusOr<std::unique_ptr<CertificateValidationContextConfigImpl>>
   create(const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& context,
-         Api::Api& api);
+         bool auto_sni_san_match, Api::Api& api);
 
   absl::Status initialize();
 
@@ -58,10 +58,12 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
 
   absl::optional<uint32_t> maxVerifyDepth() const override { return max_verify_depth_; }
 
+  bool autoSniSanMatch() const override { return auto_sni_san_match_; }
+
 protected:
   CertificateValidationContextConfigImpl(
       const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config,
-      Api::Api& api);
+      bool auto_sni_san_match, Api::Api& api);
 
 private:
   static std::vector<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher>
@@ -82,6 +84,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
   Api::Api& api_;
   const bool only_verify_leaf_cert_crl_;
   absl::optional<uint32_t> max_verify_depth_;
+  const bool auto_sni_san_match_;
 };
 
 } // namespace Ssl

source/common/tls/cert_validator/default_validator.cc

@@ -45,7 +45,8 @@ namespace Tls {
 DefaultCertValidator::DefaultCertValidator(
     const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats,
     Server::Configuration::CommonFactoryContext& context)
-    : config_(config), stats_(stats), context_(context) {
+    : config_(config), stats_(stats), context_(context),
+      auto_sni_san_match_(config_ != nullptr ? config_->autoSniSanMatch() : false) {
   if (config_ != nullptr) {
     allow_untrusted_certificate_ = config_->trustChainVerification() ==
                                    envoy::extensions::transport_sockets::tls::v3::
@@ -118,6 +119,12 @@ absl::StatusOr<int> DefaultCertValidator::initializeSslContexts(std::vector<SSL_
     }
   }
 
+  // Disallow insecure configuration.
+  if (config_ != nullptr && config_->autoSniSanMatch() && !verify_trusted_ca_) {
+    return absl::InvalidArgumentError(
+        "'auto_sni_san_validation' was configured without configuring a trusted CA");
+  }
+
   if (config_ != nullptr && !config_->certificateRevocationList().empty()) {
     bssl::UniquePtr<BIO> bio(
         BIO_new_mem_buf(const_cast<char*>(config_->certificateRevocationList().data()),
@@ -193,15 +200,26 @@ absl::StatusOr<int> DefaultCertValidator::initializeSslContexts(std::vector<SSL_
 }
 
 bool DefaultCertValidator::verifyCertAndUpdateStatus(
-    X509* leaf_cert, const Network::TransportSocketOptions* transport_socket_options,
+    X509* leaf_cert, absl::string_view sni,
+    const Network::TransportSocketOptions* transport_socket_options,
     Envoy::Ssl::ClientValidationStatus& detailed_status, std::string* error_details,
     uint8_t* out_alert) {
-  Envoy::Ssl::ClientValidationStatus validated =
-      verifyCertificate(leaf_cert,
-                        transport_socket_options != nullptr
-                            ? transport_socket_options->verifySubjectAltNameListOverride()
-                            : std::vector<std::string>{},
-                        subject_alt_name_matchers_, error_details, out_alert);
+
+  std::vector<SanMatcherPtr> match_sni_san;
+  OptRef<const std::vector<std::string>> verify_san_override;
+  OptRef<const std::vector<SanMatcherPtr>> match_san_override;
+  if (transport_socket_options != nullptr &&
+      !transport_socket_options->verifySubjectAltNameListOverride().empty()) {
+    // TODO(ggreenway): this validation should be part of `match_sni_san` so that the type is
+    // validated as a DNS SAN, but this change will require a runtime flag for the behavior change.
+    verify_san_override = transport_socket_options->verifySubjectAltNameListOverride();
+  } else if (auto_sni_san_match_ && !sni.empty()) {
+    match_sni_san.push_back(std::make_unique<StringSanMatcher>(sni));
+    match_san_override = match_sni_san;
+  }
+  Envoy::Ssl::ClientValidationStatus validated = verifyCertificate(
+      leaf_cert, verify_san_override.value_or(std::vector<std::string>()),
+      match_san_override.value_or(subject_alt_name_matchers_), error_details, out_alert);
 
   if (detailed_status == Envoy::Ssl::ClientValidationStatus::NotValidated ||
       validated != Envoy::Ssl::ClientValidationStatus::NotValidated) {
@@ -281,7 +299,7 @@ ValidationResults DefaultCertValidator::doVerifyCertChain(
     STACK_OF(X509)& cert_chain, Ssl::ValidateResultCallbackPtr /*callback*/,
     const Network::TransportSocketOptionsConstSharedPtr& transport_socket_options, SSL_CTX& ssl_ctx,
     const CertValidator::ExtraValidationContext& /*validation_context*/, bool is_server,
-    absl::string_view /*host_name*/) {
+    absl::string_view host_name) {
   if (sk_X509_num(&cert_chain) == 0) {
     stats_.fail_verify_error_.inc();
     const char* error = "verify cert failed: empty cert chain";
@@ -332,8 +350,9 @@ ValidationResults DefaultCertValidator::doVerifyCertChain(
   }
   std::string error_details;
   uint8_t tls_alert = SSL_AD_CERTIFICATE_UNKNOWN;
-  const bool succeeded = verifyCertAndUpdateStatus(leaf_cert, transport_socket_options.get(),
-                                                   detailed_status, &error_details, &tls_alert);
+  const bool succeeded =
+      verifyCertAndUpdateStatus(leaf_cert, host_name, transport_socket_options.get(),
+                                detailed_status, &error_details, &tls_alert);
   return succeeded ? ValidationResults{ValidationResults::ValidationStatus::Successful,
                                        detailed_status, absl::nullopt, absl::nullopt}
                    : ValidationResults{ValidationResults::ValidationStatus::Failed, detailed_status,
@@ -475,6 +494,17 @@ void DefaultCertValidator::updateDigestForSessionId(bssl::ScopedEVP_MD_CTX& md,
     auto only_leaf_crl = config_->onlyVerifyLeafCertificateCrl();
     rc = EVP_DigestUpdate(md.get(), &only_leaf_crl, sizeof(only_leaf_crl));
     RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));
+
+    auto max_verify_depth_opt = config_->maxVerifyDepth();
+    if (max_verify_depth_opt.has_value()) {
+      auto max_verify_depth = *max_verify_depth_opt;
+      rc = EVP_DigestUpdate(md.get(), &max_verify_depth, sizeof(max_verify_depth));
+      RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));
+    }
+
+    bool auto_sni_san_match = config_->autoSniSanMatch();
+    rc = EVP_DigestUpdate(md.get(), &auto_sni_san_match, sizeof(auto_sni_san_match));
+    RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));
   }
 }