From 8ef1ecc4fae1699b8fcea839a510a8a760ddecd8 Mon Sep 17 00:00:00 2001 From: Greg Greenway Date: Fri, 8 Nov 2024 18:32:59 -0800 Subject: [PATCH] tls: add options to validate SANs and send SNI for upstream hostname (#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 --- api/envoy/config/core/v3/protocol.proto | 6 + .../transport_sockets/tls/v3/tls.proto | 22 ++- changelogs/current.yaml | 10 ++ docs/root/faq/configuration/sni.rst | 9 +- .../_include/envoy-demo-tls-sni.yaml | 8 + docs/root/start/quick-start/securing.rst | 30 +++- envoy/common/optref.h | 2 + .../certificate_validation_context_config.h | 12 ++ envoy/ssl/context_config.h | 13 ++ source/common/common/matchers.h | 14 ++ ...tificate_validation_context_config_impl.cc | 9 +- ...rtificate_validation_context_config_impl.h | 7 +- .../tls/cert_validator/default_validator.cc | 52 ++++-- .../tls/cert_validator/default_validator.h | 5 +- .../common/tls/cert_validator/san_matcher.h | 8 +- source/common/tls/client_context_impl.cc | 26 ++- source/common/tls/client_context_impl.h | 4 +- source/common/tls/client_ssl_socket.cc | 4 +- source/common/tls/context_config_impl.cc | 22 ++- source/common/tls/context_config_impl.h | 6 +- source/common/tls/context_impl.cc | 3 +- source/common/tls/context_impl.h | 3 +- .../common/tls/server_context_config_impl.cc | 5 +- source/common/tls/ssl_socket.cc | 10 +- source/common/tls/ssl_socket.h | 6 +- .../quic/envoy_quic_proof_verifier_test.cc | 1 + test/common/secret/sds_api_test.cc | 5 +- .../common/secret/secret_manager_impl_test.cc | 10 +- .../cert_validator/default_validator_test.cc | 1 + test/common/tls/cert_validator/test_common.h | 2 + test/common/tls/context_impl_test.cc | 32 ++++ test/common/tls/integration/BUILD | 19 ++ .../tls/integration/ssl_integration_test.cc | 3 +- .../ssl_upstream_integration_test.cc | 167 ++++++++++++++++++ test/config/utility.cc | 8 +- test/config/utility.h | 2 +- test/mocks/ssl/mocks.h | 3 + 37 files changed, 474 insertions(+), 75 deletions(-) create mode 100644 test/common/tls/integration/ssl_upstream_integration_test.cc diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index d8ce3cd817c0..7160cfb641a3 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -123,6 +123,9 @@ message UpstreamHttpProtocolOptions { // header when :ref:`override_auto_sni_header ` // is set, as seen by the :ref:`router filter `. // Does nothing if a filter before the http router filter sets the corresponding metadata. + // + // See :ref:`SNI configuration ` 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 @@ -130,6 +133,9 @@ message UpstreamHttpProtocolOptions { // is set, as seen by the :ref:`router filter `. // 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 ` 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. diff --git a/api/envoy/extensions/transport_sockets/tls/v3/tls.proto b/api/envoy/extensions/transport_sockets/tls/v3/tls.proto index c305ff74f42a..44b8d269324c 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/tls.proto +++ b/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 ` is set on + // the host. + // + // See :ref:`SNI configuration ` 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 ` for how this interacts with + // other validation options. + bool auto_sni_san_validation = 7; + // If true, server-initiated TLS renegotiation will be allowed. // // .. attention:: diff --git a/changelogs/current.yaml b/changelogs/current.yaml index e790f677f804..ab34ed54420f 100644 --- a/changelogs/current.yaml +++ b/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 + ` to change the upstream + SNI to the configured hostname for the upstream. +- area: tls + change: | + Added an :ref:`option + ` 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. diff --git a/docs/root/faq/configuration/sni.rst b/docs/root/faq/configuration/sni.rst index d01586e429e1..fd40ca31f56e 100644 --- a/docs/root/faq/configuration/sni.rst +++ b/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 `. -To derive SNI from a downstream HTTP header like, ``host`` or ``:authority``, turn on -:ref:`auto_sni ` to override the fixed SNI in -:ref:`UpstreamTlsContext `. A custom header other than the ``host`` or ``:authority`` can also be supplied using the optional -:ref:`override_auto_sni_header ` field. -If upstream will present certificates with the hostname in SAN, turn on -:ref:`auto_san_validation ` too. -It still needs a trust CA in validation context in :ref:`UpstreamTlsContext ` for trust anchor. +See :ref:`SNI configuration ` and :ref:`validation configuration `. diff --git a/docs/root/start/quick-start/_include/envoy-demo-tls-sni.yaml b/docs/root/start/quick-start/_include/envoy-demo-tls-sni.yaml index 2a699b3eb36d..1e598c8fe585 100644 --- a/docs/root/start/quick-start/_include/envoy-demo-tls-sni.yaml +++ b/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 diff --git a/docs/root/start/quick-start/securing.rst b/docs/root/start/quick-start/securing.rst index 268ea1eaf171..0b2bb2f94444 100644 --- a/docs/root/start/quick-start/securing.rst +++ b/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 ` +which was sent, you can enable :ref:`auto_sni_san_validation ` +and omit :ref:`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 `. + +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 ` set in the endpoint's :ref:`filter_chain_match `, as :ref:`described above `. + +To derive SNI from a downstream HTTP header ``host`` or ``:authority``, turn on +:ref:`auto_sni ` to override the fixed SNI in +:ref:`UpstreamTlsContext `. A custom header other than +the ``host`` or ``:authority`` can also be supplied using the optional +:ref:`override_auto_sni_header ` field. + +To derive SNI from the host Envoy is connecting to, turn on :ref:`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``. diff --git a/envoy/common/optref.h b/envoy/common/optref.h index 63ff2bff154c..0e460cd1bea5 100644 --- a/envoy/common/optref.h +++ b/envoy/common/optref.h @@ -71,6 +71,8 @@ template 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. */ diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index c9488798eb29..d95d49bb4f4c 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/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 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; diff --git a/envoy/ssl/context_config.h b/envoy/ssl/context_config.h index 53619a58805b..b256302dff75 100644 --- a/envoy/ssl/context_config.h +++ b/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. */ diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index 043eafa626c5..33942e6e20e9 100644 --- a/source/common/common/matchers.h +++ b/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_; diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index e6c9264024c8..409f8132eda6 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -19,7 +19,7 @@ static const std::string INLINE_STRING = ""; 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(config.max_verify_depth().value()) - : absl::nullopt) {} + : absl::nullopt), + auto_sni_san_match_(auto_sni_san_match) {} absl::StatusOr> 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( - new CertificateValidationContextConfigImpl(context, api)); + new CertificateValidationContextConfigImpl(context, auto_sni_san_match, api)); absl::Status status = config->initialize(); if (status.ok()) { return config; diff --git a/source/common/ssl/certificate_validation_context_config_impl.h b/source/common/ssl/certificate_validation_context_config_impl.h index b8386f13d366..8bce207f6a56 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.h +++ b/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> 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 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 @@ -82,6 +84,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte Api::Api& api_; const bool only_verify_leaf_cert_crl_; absl::optional max_verify_depth_; + const bool auto_sni_san_match_; }; } // namespace Ssl diff --git a/source/common/tls/cert_validator/default_validator.cc b/source/common/tls/cert_validator/default_validator.cc index a1e0b203e957..cea31717b526 100644 --- a/source/common/tls/cert_validator/default_validator.cc +++ b/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 DefaultCertValidator::initializeSslContexts(std::vectorautoSniSanMatch() && !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_new_mem_buf(const_cast(config_->certificateRevocationList().data()), @@ -193,15 +200,26 @@ absl::StatusOr DefaultCertValidator::initializeSslContexts(std::vectorverifySubjectAltNameListOverride() - : std::vector{}, - subject_alt_name_matchers_, error_details, out_alert); + + std::vector match_sni_san; + OptRef> verify_san_override; + OptRef> 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(sni)); + match_san_override = match_sni_san; + } + Envoy::Ssl::ClientValidationStatus validated = verifyCertificate( + leaf_cert, verify_san_override.value_or(std::vector()), + 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("")); } } diff --git a/source/common/tls/cert_validator/default_validator.h b/source/common/tls/cert_validator/default_validator.h index ed34f7dba735..21d7afcbaced 100644 --- a/source/common/tls/cert_validator/default_validator.h +++ b/source/common/tls/cert_validator/default_validator.h @@ -104,7 +104,7 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable& subject_alt_name_matchers); private: - bool verifyCertAndUpdateStatus(X509* leaf_cert, + bool verifyCertAndUpdateStatus(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); @@ -113,13 +113,14 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable ca_cert_; std::string ca_file_path_; std::vector subject_alt_name_matchers_; std::vector> verify_certificate_hash_list_; std::vector> verify_certificate_spki_list_; + bool allow_untrusted_certificate_{false}; bool verify_trusted_ca_{false}; + const bool auto_sni_san_match_{false}; }; DECLARE_FACTORY(DefaultCertValidatorFactory); diff --git a/source/common/tls/cert_validator/san_matcher.h b/source/common/tls/cert_validator/san_matcher.h index 135cf17f701c..898c7d7a1454 100644 --- a/source/common/tls/cert_validator/san_matcher.h +++ b/source/common/tls/cert_validator/san_matcher.h @@ -32,17 +32,23 @@ using SanMatcherPtr = std::unique_ptr; class StringSanMatcher : public SanMatcher { public: + using StringMatcherImpl = Matchers::StringMatcherImpl; bool match(const GENERAL_NAME* general_name) const override; ~StringSanMatcher() override = default; + StringSanMatcher(int general_name_type, envoy::type::matcher::v3::StringMatcher matcher, Server::Configuration::CommonFactoryContext& context, bssl::UniquePtr&& general_name_oid = nullptr) : general_name_type_(general_name_type), matcher_(matcher, context), general_name_oid_(std::move(general_name_oid)) {} + StringSanMatcher(absl::string_view dns_exact_match) + : general_name_type_(GEN_DNS), + matcher_(StringMatcherImpl::createExactMatcher(dns_exact_match)) {} + private: const int general_name_type_; - const Matchers::StringMatcherImpl matcher_; + const StringMatcherImpl matcher_; bssl::UniquePtr general_name_oid_; }; diff --git a/source/common/tls/client_context_impl.cc b/source/common/tls/client_context_impl.cc index 973075b5cd1c..02bc63ad293b 100644 --- a/source/common/tls/client_context_impl.cc +++ b/source/common/tls/client_context_impl.cc @@ -60,12 +60,21 @@ ClientContextImpl::ClientContextImpl(Stats::Scope& scope, absl::Status& creation_status) : ContextImpl(scope, config, factory_context, nullptr /* additional_init */, creation_status), server_name_indication_(config.serverNameIndication()), + auto_host_sni_(config.autoHostServerNameIndication()), allow_renegotiation_(config.allowRenegotiation()), enforce_rsa_key_usage_(config.enforceRsaKeyUsage()), max_session_keys_(config.maxSessionKeys()) { if (!creation_status.ok()) { return; } + + // Disallow insecure configuration. + if (config.autoSniSanMatch() && config.certificateValidationContext() == nullptr) { + creation_status = absl::InvalidArgumentError( + "'auto_sni_san_validation' was configured without a validation context"); + return; + } + // This should be guaranteed during configuration ingestion for client contexts. ASSERT(tls_contexts_.size() == 1); if (!parsed_alpn_protocols_.empty()) { @@ -90,17 +99,24 @@ ClientContextImpl::ClientContextImpl(Stats::Scope& scope, } absl::StatusOr> -ClientContextImpl::newSsl(const Network::TransportSocketOptionsConstSharedPtr& options) { - absl::StatusOr> ssl_con_or_status(ContextImpl::newSsl(options)); +ClientContextImpl::newSsl(const Network::TransportSocketOptionsConstSharedPtr& options, + Upstream::HostDescriptionConstSharedPtr host) { + absl::StatusOr> ssl_con_or_status(ContextImpl::newSsl(options, host)); if (!ssl_con_or_status.ok()) { return ssl_con_or_status; } bssl::UniquePtr ssl_con = std::move(ssl_con_or_status.value()); - const std::string server_name_indication = options && options->serverNameOverride().has_value() - ? options->serverNameOverride().value() - : server_name_indication_; + std::string server_name_indication; + if (options && options->serverNameOverride().has_value()) { + server_name_indication = options->serverNameOverride().value(); + } else if (auto_host_sni_ && host != nullptr && !host->hostname().empty()) { + server_name_indication = host->hostname(); + } else { + server_name_indication = server_name_indication_; + } + if (!server_name_indication.empty()) { const int rc = SSL_set_tlsext_host_name(ssl_con.get(), server_name_indication.c_str()); if (rc != 1) { diff --git a/source/common/tls/client_context_impl.h b/source/common/tls/client_context_impl.h index 2c01ebaa83c9..ee879160b96b 100644 --- a/source/common/tls/client_context_impl.h +++ b/source/common/tls/client_context_impl.h @@ -44,7 +44,8 @@ class ClientContextImpl : public ContextImpl, public Envoy::Ssl::ClientContext { Server::Configuration::CommonFactoryContext& factory_context); absl::StatusOr> - newSsl(const Network::TransportSocketOptionsConstSharedPtr& options) override; + newSsl(const Network::TransportSocketOptionsConstSharedPtr& options, + Upstream::HostDescriptionConstSharedPtr host) override; private: ClientContextImpl(Stats::Scope& scope, const Envoy::Ssl::ClientContextConfig& config, @@ -54,6 +55,7 @@ class ClientContextImpl : public ContextImpl, public Envoy::Ssl::ClientContext { int newSessionKey(SSL_SESSION* session); const std::string server_name_indication_; + const bool auto_host_sni_; const bool allow_renegotiation_; const bool enforce_rsa_key_usage_; const size_t max_session_keys_; diff --git a/source/common/tls/client_ssl_socket.cc b/source/common/tls/client_ssl_socket.cc index ce31ecfb7332..70343e92462f 100644 --- a/source/common/tls/client_ssl_socket.cc +++ b/source/common/tls/client_ssl_socket.cc @@ -57,7 +57,7 @@ ClientSslSocketFactory::~ClientSslSocketFactory() { manager_.removeContext(ssl_c Network::TransportSocketPtr ClientSslSocketFactory::createTransportSocket( Network::TransportSocketOptionsConstSharedPtr transport_socket_options, - Upstream::HostDescriptionConstSharedPtr) const { + Upstream::HostDescriptionConstSharedPtr host) const { // onAddOrUpdateSecret() could be invoked in the middle of checking the existence of ssl_ctx and // creating SslSocket using ssl_ctx. Capture ssl_ctx_ into a local variable so that we check and // use the same ssl_ctx to create SslSocket. @@ -69,7 +69,7 @@ Network::TransportSocketPtr ClientSslSocketFactory::createTransportSocket( if (ssl_ctx) { auto status_or_socket = SslSocket::create(std::move(ssl_ctx), InitialState::Client, transport_socket_options, - config_->createHandshaker()); + config_->createHandshaker(), host); if (status_or_socket.ok()) { return std::move(*status_or_socket); } diff --git a/source/common/tls/context_config_impl.cc b/source/common/tls/context_config_impl.cc index 3b5247eb848b..7616607c14e3 100644 --- a/source/common/tls/context_config_impl.cc +++ b/source/common/tls/context_config_impl.cc @@ -121,14 +121,16 @@ getCertificateValidationContextConfigProvider( ContextConfigImpl::ContextConfigImpl( const envoy::extensions::transport_sockets::tls::v3::CommonTlsContext& config, - const unsigned default_min_protocol_version, const unsigned default_max_protocol_version, - const std::string& default_cipher_suites, const std::string& default_curves, + bool auto_sni_san_match, const unsigned default_min_protocol_version, + const unsigned default_max_protocol_version, const std::string& default_cipher_suites, + const std::string& default_curves, Server::Configuration::TransportSocketFactoryContext& factory_context, absl::Status& creation_status) : api_(factory_context.serverFactoryContext().api()), options_(factory_context.serverFactoryContext().options()), singleton_manager_(factory_context.serverFactoryContext().singletonManager()), lifecycle_notifier_(factory_context.serverFactoryContext().lifecycleNotifier()), + auto_sni_san_match_(auto_sni_san_match), alpn_protocols_(RepeatedPtrUtil::join(config.alpn_protocols(), ",")), cipher_suites_(StringUtil::nonEmptyStringOrDefault( RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"), default_cipher_suites)), @@ -178,7 +180,7 @@ ContextConfigImpl::ContextConfigImpl( validation_context_config_ = std::move(*context_or_error); } else { auto config_or_status = Envoy::Ssl::CertificateValidationContextConfigImpl::create( - *certificate_validation_context_provider_->secret(), api_); + *certificate_validation_context_provider_->secret(), auto_sni_san_match, api_); SET_AND_RETURN_IF_NOT_OK(config_or_status.status(), creation_status); validation_context_config_ = std::move(config_or_status.value()); } @@ -225,8 +227,8 @@ ContextConfigImpl::getCombinedValidationContextConfig( envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext combined_cvc = *default_cvc_; combined_cvc.MergeFrom(dynamic_cvc); - auto config_or_status = - Envoy::Ssl::CertificateValidationContextConfigImpl::create(combined_cvc, api_); + auto config_or_status = Envoy::Ssl::CertificateValidationContextConfigImpl::create( + combined_cvc, auto_sni_san_match_, api_); RETURN_IF_NOT_OK(config_or_status.status()); return std::move(config_or_status.value()); } @@ -270,7 +272,7 @@ void ContextConfigImpl::setSecretUpdateCallback(std::function ca cvc_update_callback_handle_ = certificate_validation_context_provider_->addUpdateCallback([this, callback]() { auto config_or_status = Envoy::Ssl::CertificateValidationContextConfigImpl::create( - *certificate_validation_context_provider_->secret(), api_); + *certificate_validation_context_provider_->secret(), auto_sni_san_match_, api_); RETURN_IF_NOT_OK(config_or_status.status()); validation_context_config_ = std::move(config_or_status.value()); return callback(); @@ -337,9 +339,11 @@ ClientContextConfigImpl::ClientContextConfigImpl( const envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& config, Server::Configuration::TransportSocketFactoryContext& factory_context, absl::Status& creation_status) - : ContextConfigImpl(config.common_tls_context(), DEFAULT_MIN_VERSION, DEFAULT_MAX_VERSION, - DEFAULT_CIPHER_SUITES, DEFAULT_CURVES, factory_context, creation_status), - server_name_indication_(config.sni()), allow_renegotiation_(config.allow_renegotiation()), + : ContextConfigImpl(config.common_tls_context(), config.auto_sni_san_validation(), + DEFAULT_MIN_VERSION, DEFAULT_MAX_VERSION, DEFAULT_CIPHER_SUITES, + DEFAULT_CURVES, factory_context, creation_status), + server_name_indication_(config.sni()), auto_host_sni_(config.auto_host_sni()), + allow_renegotiation_(config.allow_renegotiation()), enforce_rsa_key_usage_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enforce_rsa_key_usage, false)), max_session_keys_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_session_keys, 1)) { // BoringSSL treats this as a C string, so embedded NULL characters will not diff --git a/source/common/tls/context_config_impl.h b/source/common/tls/context_config_impl.h index 28ecdbbb37e9..3b815cb15b53 100644 --- a/source/common/tls/context_config_impl.h +++ b/source/common/tls/context_config_impl.h @@ -70,7 +70,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { protected: ContextConfigImpl(const envoy::extensions::transport_sockets::tls::v3::CommonTlsContext& config, - const unsigned default_min_protocol_version, + bool auto_sni_san_match, const unsigned default_min_protocol_version, const unsigned default_max_protocol_version, const std::string& default_cipher_suites, const std::string& default_curves, Server::Configuration::TransportSocketFactoryContext& factory_context, @@ -79,6 +79,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { const Server::Options& options_; Singleton::Manager& singleton_manager_; Server::ServerLifecycleNotifier& lifecycle_notifier_; + const bool auto_sni_san_match_; private: static unsigned tlsVersionFromProto( @@ -128,6 +129,8 @@ class ClientContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Cli // Ssl::ClientContextConfig const std::string& serverNameIndication() const override { return server_name_indication_; } + bool autoHostServerNameIndication() const override { return auto_host_sni_; } + bool autoSniSanMatch() const override { return auto_sni_san_match_; } bool allowRenegotiation() const override { return allow_renegotiation_; } size_t maxSessionKeys() const override { return max_session_keys_; } bool enforceRsaKeyUsage() const override { return enforce_rsa_key_usage_; } @@ -142,6 +145,7 @@ class ClientContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Cli static const unsigned DEFAULT_MAX_VERSION; const std::string server_name_indication_; + const bool auto_host_sni_; const bool allow_renegotiation_; const bool enforce_rsa_key_usage_; const size_t max_session_keys_; diff --git a/source/common/tls/context_impl.cc b/source/common/tls/context_impl.cc index 865cbe8a2167..1f9a2b5a3d9f 100644 --- a/source/common/tls/context_impl.cc +++ b/source/common/tls/context_impl.cc @@ -458,7 +458,8 @@ std::vector ContextImpl::parseAlpnProtocols(const std::string& alpn_pro } absl::StatusOr> -ContextImpl::newSsl(const Network::TransportSocketOptionsConstSharedPtr& options) { +ContextImpl::newSsl(const Network::TransportSocketOptionsConstSharedPtr& options, + Upstream::HostDescriptionConstSharedPtr) { // We use the first certificate for a new SSL object, later in the // SSL_CTX_set_select_certificate_cb() callback following ClientHello, we replace with the // selected certificate via SSL_set_SSL_CTX(). diff --git a/source/common/tls/context_impl.h b/source/common/tls/context_impl.h index 846cf1922de4..c692f72bc969 100644 --- a/source/common/tls/context_impl.h +++ b/source/common/tls/context_impl.h @@ -82,7 +82,8 @@ class ContextImpl : public virtual Envoy::Ssl::Context, protected Logger::Loggable { public: virtual absl::StatusOr> - newSsl(const Network::TransportSocketOptionsConstSharedPtr& options); + newSsl(const Network::TransportSocketOptionsConstSharedPtr& options, + Upstream::HostDescriptionConstSharedPtr host); /** * Logs successful TLS handshake and updates stats. diff --git a/source/common/tls/server_context_config_impl.cc b/source/common/tls/server_context_config_impl.cc index b63c1ec4a076..49eef1470584 100644 --- a/source/common/tls/server_context_config_impl.cc +++ b/source/common/tls/server_context_config_impl.cc @@ -115,8 +115,9 @@ ServerContextConfigImpl::ServerContextConfigImpl( const envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext& config, Server::Configuration::TransportSocketFactoryContext& factory_context, absl::Status& creation_status, bool for_quic) - : ContextConfigImpl(config.common_tls_context(), DEFAULT_MIN_VERSION, DEFAULT_MAX_VERSION, - DEFAULT_CIPHER_SUITES, DEFAULT_CURVES, factory_context, creation_status), + : ContextConfigImpl(config.common_tls_context(), false /* auto_sni_san_match */, + DEFAULT_MIN_VERSION, DEFAULT_MAX_VERSION, DEFAULT_CIPHER_SUITES, + DEFAULT_CURVES, factory_context, creation_status), require_client_certificate_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, require_client_certificate, false)), ocsp_staple_policy_(ocspStaplePolicyFromProto(config.ocsp_staple_policy())), diff --git a/source/common/tls/ssl_socket.cc b/source/common/tls/ssl_socket.cc index 0e279330fd1f..445fa6fdc2ef 100644 --- a/source/common/tls/ssl_socket.cc +++ b/source/common/tls/ssl_socket.cc @@ -32,9 +32,10 @@ absl::string_view NotReadySslSocket::failureReason() const { return NotReadyReas absl::StatusOr> SslSocket::create(Envoy::Ssl::ContextSharedPtr ctx, InitialState state, const Network::TransportSocketOptionsConstSharedPtr& transport_socket_options, - Ssl::HandshakerFactoryCb handshaker_factory_cb) { + Ssl::HandshakerFactoryCb handshaker_factory_cb, + Upstream::HostDescriptionConstSharedPtr host) { std::unique_ptr socket(new SslSocket(ctx, transport_socket_options)); - auto status = socket->initialize(state, handshaker_factory_cb); + auto status = socket->initialize(state, handshaker_factory_cb, host); if (status.ok()) { return socket; } else { @@ -48,8 +49,9 @@ SslSocket::SslSocket(Envoy::Ssl::ContextSharedPtr ctx, ctx_(std::dynamic_pointer_cast(ctx)) {} absl::Status SslSocket::initialize(InitialState state, - Ssl::HandshakerFactoryCb handshaker_factory_cb) { - auto status_or_ssl = ctx_->newSsl(transport_socket_options_); + Ssl::HandshakerFactoryCb handshaker_factory_cb, + Upstream::HostDescriptionConstSharedPtr host) { + auto status_or_ssl = ctx_->newSsl(transport_socket_options_, host); if (!status_or_ssl.ok()) { return status_or_ssl.status(); } diff --git a/source/common/tls/ssl_socket.h b/source/common/tls/ssl_socket.h index 4cdb34393f3f..fa8dd582e452 100644 --- a/source/common/tls/ssl_socket.h +++ b/source/common/tls/ssl_socket.h @@ -51,7 +51,8 @@ class SslSocket : public Network::TransportSocket, static absl::StatusOr> create(Envoy::Ssl::ContextSharedPtr ctx, InitialState state, const Network::TransportSocketOptionsConstSharedPtr& transport_socket_options, - Ssl::HandshakerFactoryCb handshaker_factory_cb); + Ssl::HandshakerFactoryCb handshaker_factory_cb, + Upstream::HostDescriptionConstSharedPtr host = {}); // Network::TransportSocket void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override; @@ -83,7 +84,8 @@ class SslSocket : public Network::TransportSocket, private: SslSocket(Envoy::Ssl::ContextSharedPtr ctx, const Network::TransportSocketOptionsConstSharedPtr& transport_socket_options); - absl::Status initialize(InitialState state, Ssl::HandshakerFactoryCb handshaker_factory_cb); + absl::Status initialize(InitialState state, Ssl::HandshakerFactoryCb handshaker_factory_cb, + Upstream::HostDescriptionConstSharedPtr host); struct ReadResult { uint64_t bytes_read_{0}; diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 32eb738389d1..6234f4da6264 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -81,6 +81,7 @@ class EnvoyQuicProofVerifierTest : public testing::Test { .WillRepeatedly(ReturnRef(empty_string_list_)); EXPECT_CALL(cert_validation_ctx_config_, customValidatorConfig()) .WillRepeatedly(ReturnRef(custom_validator_config_)); + EXPECT_CALL(cert_validation_ctx_config_, autoSniSanMatch()).WillRepeatedly(Return(false)); auto context_or_error = Extensions::TransportSockets::Tls::ClientContextImpl::create( *store_.rootScope(), client_context_config_, factory_context_); THROW_IF_NOT_OK(context_or_error.status()); diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 1f73cacb7e39..6ec27b135749 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -640,7 +640,7 @@ TEST_F(SdsApiTest, DynamicCertificateValidationContextUpdateSuccess) { EXPECT_TRUE(subscription_factory_.callbacks_->onConfigUpdate(decoded_resources.refvec_, "").ok()); auto cvc_config = - Ssl::CertificateValidationContextConfigImpl::create(*sds_api.secret(), *api_).value(); + Ssl::CertificateValidationContextConfigImpl::create(*sds_api.secret(), false, *api_).value(); const std::string ca_cert = "{{ test_rundir }}/test/common/tls/test_data/ca_cert.pem"; EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(ca_cert)), cvc_config->caCert()); @@ -716,7 +716,8 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext merged_cvc = default_cvc; merged_cvc.MergeFrom(*sds_api.secret()); - auto cvc_config = Ssl::CertificateValidationContextConfigImpl::create(merged_cvc, *api_).value(); + auto cvc_config = + Ssl::CertificateValidationContextConfigImpl::create(merged_cvc, false, *api_).value(); // Verify that merging CertificateValidationContext applies logical OR to bool // field. EXPECT_TRUE(cvc_config->allowExpiredCertificate()); diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index d5f5b9406581..7a338e621542 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -137,7 +137,7 @@ TEST_F(SecretManagerImplTest, CertificateValidationContextSecretLoadSuccess) { auto cvc_config = Ssl::CertificateValidationContextConfigImpl::create( *secret_manager->findStaticCertificateValidationContextProvider("abc.com")->secret(), - *api_) + false, *api_) .value(); const std::string cert_pem = "{{ test_rundir }}/test/common/tls/test_data/ca_cert.pem"; EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(cert_pem)), @@ -530,9 +530,9 @@ name: "abc.com.validation" EXPECT_TRUE(secret_context.cluster_manager_.subscription_factory_.callbacks_ ->onConfigUpdate(decoded_resources_2.refvec_, "validation-context-v1") .ok()); - auto cert_validation_context = - Ssl::CertificateValidationContextConfigImpl::create(*context_secret_provider->secret(), *api_) - .value(); + auto cert_validation_context = Ssl::CertificateValidationContextConfigImpl::create( + *context_secret_provider->secret(), false, *api_) + .value(); EXPECT_EQ("DUMMY_INLINE_STRING_TRUSTED_CA", cert_validation_context->caCert()); const std::string updated_config_dump = R"EOF( dynamic_active_secrets: @@ -1152,7 +1152,7 @@ TEST_F(SecretManagerImplTest, DeprecatedSanMatcher) { auto cvc_config = Ssl::CertificateValidationContextConfigImpl::create( *secret_manager->findStaticCertificateValidationContextProvider("abc.com")->secret(), - *api_) + false, *api_) .value(); EXPECT_EQ(cvc_config->subjectAltNameMatchers().size(), 4); EXPECT_EQ("example.foo", cvc_config->subjectAltNameMatchers()[0].matcher().exact()); diff --git a/test/common/tls/cert_validator/default_validator_test.cc b/test/common/tls/cert_validator/default_validator_test.cc index b876ca65df6a..12906544100b 100644 --- a/test/common/tls/cert_validator/default_validator_test.cc +++ b/test/common/tls/cert_validator/default_validator_test.cc @@ -698,6 +698,7 @@ class MockCertificateValidationContextConfig : public Ssl::CertificateValidation MOCK_METHOD(Api::Api&, api, (), (const override)); bool onlyVerifyLeafCertificateCrl() const override { return false; } absl::optional maxVerifyDepth() const override { return absl::nullopt; } + bool autoSniSanMatch() const override { return false; } private: std::string s_; diff --git a/test/common/tls/cert_validator/test_common.h b/test/common/tls/cert_validator/test_common.h index b948977a3e39..18fbe7022dbc 100644 --- a/test/common/tls/cert_validator/test_common.h +++ b/test/common/tls/cert_validator/test_common.h @@ -105,6 +105,7 @@ class TestCertificateValidationContextConfig bool onlyVerifyLeafCertificateCrl() const override { return false; } absl::optional maxVerifyDepth() const override { return max_verify_depth_; } + bool autoSniSanMatch() const override { return auto_sni_san_match_; } private: bool allow_expired_certificate_{false}; @@ -115,6 +116,7 @@ class TestCertificateValidationContextConfig const std::string ca_cert_; const std::string ca_cert_path_{"TEST_CA_CERT_PATH"}; const absl::optional max_verify_depth_{absl::nullopt}; + const bool auto_sni_san_match_{false}; }; } // namespace Tls diff --git a/test/common/tls/context_impl_test.cc b/test/common/tls/context_impl_test.cc index 1ad54a85b213..7385b3caa586 100644 --- a/test/common/tls/context_impl_test.cc +++ b/test/common/tls/context_impl_test.cc @@ -1252,6 +1252,38 @@ TEST_F(ClientContextConfigImplTest, EmptyServerNameIndication) { "SNI names containing NULL-byte are not allowed"); } +// Validate that it is an error configure `auto_sni_san_validation` without configuring +// a validation context. +TEST_F(ClientContextConfigImplTest, AutoSniSanValidationWithoutValidationContext) { + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + tls_context.set_auto_sni_san_validation(true); + NiceMock factory_context; + auto client_context_config = *ClientContextConfigImpl::create(tls_context, factory_context); + Stats::IsolatedStoreImpl store; + EXPECT_EQ(manager_.createSslClientContext(*store.rootScope(), *client_context_config) + .status() + .message(), + "'auto_sni_san_validation' was configured without a validation context"); +} + +// Validate that it is an error configure `auto_sni_san_validation` without configuring +// a trusted CA. +TEST_F(ClientContextConfigImplTest, AutoSniSanValidationWithoutTrustedCa) { + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + tls_context.set_auto_sni_san_validation(true); + tls_context.mutable_common_tls_context() + ->mutable_validation_context() + ->set_trust_chain_verification(envoy::extensions::transport_sockets::tls::v3:: + CertificateValidationContext::ACCEPT_UNTRUSTED); + NiceMock factory_context; + auto client_context_config = *ClientContextConfigImpl::create(tls_context, factory_context); + Stats::IsolatedStoreImpl store; + EXPECT_EQ(manager_.createSslClientContext(*store.rootScope(), *client_context_config) + .status() + .message(), + "'auto_sni_san_validation' was configured without configuring a trusted CA"); +} + // Validate that values other than a hex-encoded SHA-256 fail config validation. TEST_F(ClientContextConfigImplTest, InvalidCertificateHash) { envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; diff --git a/test/common/tls/integration/BUILD b/test/common/tls/integration/BUILD index 1179ea1acead..1fa8da34d262 100644 --- a/test/common/tls/integration/BUILD +++ b/test/common/tls/integration/BUILD @@ -72,3 +72,22 @@ envoy_cc_test_library( "//test/integration/filters:common_lib", ], ) + +envoy_cc_test( + name = "ssl_upstream_integration_test", + size = "large", + srcs = [ + "ssl_upstream_integration_test.cc", + ], + data = [ + "//test/config/integration/certs", + ], + rbe_pool = "6gig", + deps = [ + "//source/common/event:dispatcher_includes", + "//source/common/event:dispatcher_lib", + "//source/extensions/transport_sockets/tls:config", + "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/common/tls/integration/ssl_integration_test.cc b/test/common/tls/integration/ssl_integration_test.cc index 0b823ef1cfa6..819b505ca001 100644 --- a/test/common/tls/integration/ssl_integration_test.cc +++ b/test/common/tls/integration/ssl_integration_test.cc @@ -104,7 +104,8 @@ TEST_P(SslIntegrationTest, StatsTagExtraction) { setUpstreamProtocol(Http::CodecType::HTTP2); config_helper_.configureUpstreamTls( false, false, absl::nullopt, - [](envoy::extensions::transport_sockets::tls::v3::CommonTlsContext& ctx) { + [](envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& upstream_ctx) { + auto& ctx = *upstream_ctx.mutable_common_tls_context(); auto& params = *ctx.mutable_tls_params(); params.set_tls_minimum_protocol_version( envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_2); diff --git a/test/common/tls/integration/ssl_upstream_integration_test.cc b/test/common/tls/integration/ssl_upstream_integration_test.cc new file mode 100644 index 000000000000..800b6a074c58 --- /dev/null +++ b/test/common/tls/integration/ssl_upstream_integration_test.cc @@ -0,0 +1,167 @@ +#include "test/integration/http_integration.h" + +namespace Envoy { +namespace Ssl { + +class SslUpstreamIntegrationTest : public testing::TestWithParam, + public HttpIntegrationTest { +public: + SslUpstreamIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP2, GetParam()) {} + + void initialize() override { + upstream_tls_ = true; + config_helper_.configureUpstreamTls( + true /* use alpn */, false /* http3 */, absl::nullopt, + [this](envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& ctx) { + ctx.set_auto_host_sni(auto_host_sni_); + ctx.set_auto_sni_san_validation(auto_sni_san_validation_); + + ctx.set_sni(default_sni_); + }); + + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* cluster = bootstrap.mutable_static_resources()->mutable_clusters(0); + cluster->mutable_load_assignment() + ->mutable_endpoints(0) + ->mutable_lb_endpoints(0) + ->mutable_endpoint() + ->set_hostname(configured_upstream_hostname_); + + auto protocol_options = + MessageUtil::anyConvert( + (*cluster->mutable_typed_extension_protocol_options()) + ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"]); + auto* upstream_http_options = protocol_options.mutable_upstream_http_protocol_options(); + + upstream_http_options->set_auto_sni(auto_sni_); + upstream_http_options->set_auto_san_validation(auto_san_validation_); + + (*cluster->mutable_typed_extension_protocol_options()) + ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"] + .PackFrom(protocol_options); + }); + + HttpIntegrationTest::initialize(); + } + + std::string testSni() { + initialize(); + + codec_client_ = makeHttpConnection((lookupPort("http"))); + default_request_headers_.setHost(sni_header_); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + + EXPECT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + EXPECT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + std::string sni = fake_upstream_connection_->connection().ssl()->sni(); + upstream_request_->encodeHeaders(default_response_headers_, true); + EXPECT_TRUE(fake_upstream_connection_->close(Network::ConnectionCloseType::FlushWrite)); + EXPECT_TRUE(response->waitForEndStream()); + codec_client_->close(); + + return sni; + } + + void expectCertValidationFailure() { + autonomous_upstream_ = true; + initialize(); + + codec_client_ = makeHttpConnection((lookupPort("http"))); + default_request_headers_.setHost(sni_header_); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + EXPECT_TRUE(response->waitForEndStream()); + EXPECT_EQ("503", response->headers().getStatusValue()); + codec_client_->close(); + + test_server_->waitForCounterEq("cluster.cluster_0.ssl.fail_verify_san", 1); + } + + void expectCertValidationSuccess() { testSni(); } + + static constexpr absl::string_view upstream_hostname_ = "upstream_name.example.com"; + static constexpr absl::string_view default_sni_ = "default_sni.example.com"; + static constexpr absl::string_view default_sni_header_ = "sni_header.example.com"; + + bool auto_sni_{false}; // Get SNI from downstream :authority header. + bool auto_san_validation_{false}; // Validate SAN against value of :authority header. + bool auto_host_sni_{false}; // Get SNI from cluster host config. + bool auto_sni_san_validation_{false}; // Validate SAN against transmitted SNI value. + + std::string configured_upstream_hostname_{upstream_hostname_}; + std::string sni_header_{default_sni_header_}; +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, SslUpstreamIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +TEST_P(SslUpstreamIntegrationTest, DefaultSni) { EXPECT_EQ(default_sni_, testSni()); } + +TEST_P(SslUpstreamIntegrationTest, AutoSni) { + auto_sni_ = true; + EXPECT_EQ(sni_header_, testSni()); +} + +TEST_P(SslUpstreamIntegrationTest, AutoHostSni) { + auto_host_sni_ = true; + EXPECT_EQ(upstream_hostname_, testSni()); +} + +// When both `auto_sni` and `auto_host_sni` are configured, `auto_sni` takes precedence. +TEST_P(SslUpstreamIntegrationTest, SniHeaderOverridesHost) { + auto_sni_ = true; + auto_host_sni_ = true; + EXPECT_EQ(sni_header_, testSni()); +} + +// When `auto_host_sni` is configured but the upstream host does not have a hostname, fallback +// to the default SNI. +TEST_P(SslUpstreamIntegrationTest, AutoHostSniMissingConfig) { + auto_host_sni_ = true; + configured_upstream_hostname_.clear(); + EXPECT_EQ(default_sni_, testSni()); +} + +TEST_P(SslUpstreamIntegrationTest, AutoSniSanMatchFail) { + auto_host_sni_ = true; + auto_sni_san_validation_ = true; + expectCertValidationFailure(); +} + +TEST_P(SslUpstreamIntegrationTest, AutoSniSanMatchSuccess) { + // Upstream cert has SAN `*.lyft.com`. + configured_upstream_hostname_ = "test.lyft.com"; + + auto_host_sni_ = true; + auto_sni_san_validation_ = true; + expectCertValidationSuccess(); +} + +// When both `auto_sni_san_validation` and and `auto_san_validation` are set, the latter takes +// precedence. +TEST_P(SslUpstreamIntegrationTest, BothAutoValidationPreferenceFailure) { + // Upstream cert has SAN `*.lyft.com`. + configured_upstream_hostname_ = "test.lyft.com"; + sni_header_ = "test.example.com"; + + auto_host_sni_ = true; + auto_sni_san_validation_ = true; + auto_san_validation_ = true; + expectCertValidationFailure(); +} + +// When both `auto_sni_san_validation` and and `auto_san_validation` are set, the latter takes +// precedence. +TEST_P(SslUpstreamIntegrationTest, BothAutoValidationPreferenceSuccess) { + // Upstream cert has SAN `*.lyft.com`. + configured_upstream_hostname_ = "test.example.com"; + sni_header_ = "test.lyft.com"; + + auto_host_sni_ = true; + auto_sni_san_validation_ = true; + auto_san_validation_ = true; + expectCertValidationSuccess(); +} + +} // namespace Ssl +} // namespace Envoy diff --git a/test/config/utility.cc b/test/config/utility.cc index a41d93793e67..c603215b36e8 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -882,7 +882,7 @@ void ConfigHelper::configureUpstreamTls( bool use_alpn, bool http3, absl::optional alternate_protocol_cache_config, - std::function + std::function configure_tls_context) { addConfigModifier([use_alpn, http3, alternate_protocol_cache_config, configure_tls_context](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { @@ -928,15 +928,15 @@ void ConfigHelper::configureUpstreamTls( .PackFrom(new_protocol_options); } envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; - if (configure_tls_context != nullptr) { - configure_tls_context(*tls_context.mutable_common_tls_context()); - } auto* validation_context = tls_context.mutable_common_tls_context()->mutable_validation_context(); validation_context->mutable_trusted_ca()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); // The test certs are for *.lyft.com, so make sure SNI matches. tls_context.set_sni("foo.lyft.com"); + if (configure_tls_context != nullptr) { + configure_tls_context(tls_context); + } if (http3) { envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_context; quic_context.mutable_upstream_tls_context()->CopyFrom(tls_context); diff --git a/test/config/utility.h b/test/config/utility.h index 180cdcba87b9..0ec30fd4554f 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -424,7 +424,7 @@ class ConfigHelper { bool use_alpn = false, bool http3 = false, absl::optional alternate_protocol_cache_config = {}, - std::function + std::function configure_tls_context = nullptr); // Skip validation that ensures that all upstream ports are referenced by the diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 4d76f6c533e1..9f783331b85e 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -111,6 +111,8 @@ class MockClientContextConfig : public ClientContextConfig { MOCK_METHOD(Ssl::SslCtxCb, sslctxCb, (), (const, override)); MOCK_METHOD(const std::string&, serverNameIndication, (), (const)); + MOCK_METHOD(bool, autoHostServerNameIndication, (), (const)); + MOCK_METHOD(bool, autoSniSanMatch, (), (const)); MOCK_METHOD(bool, allowRenegotiation, (), (const)); MOCK_METHOD(bool, enforceRsaKeyUsage, (), (const)); MOCK_METHOD(size_t, maxSessionKeys, (), (const)); @@ -210,6 +212,7 @@ class MockCertificateValidationContextConfig : public CertificateValidationConte trustChainVerification, (), (const)); MOCK_METHOD(bool, onlyVerifyLeafCertificateCrl, (), (const)); MOCK_METHOD(absl::optional, maxVerifyDepth, (), (const)); + MOCK_METHOD(bool, autoSniSanMatch, (), (const)); }; class MockPrivateKeyMethodManager : public PrivateKeyMethodManager {