diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 369ee4d657eb..c638578392a9 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -9,6 +9,7 @@ import "envoy/type/matcher/v3/string.proto"; import "google/protobuf/any.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "udpa/annotations/sensitive.proto"; import "udpa/annotations/status.proto"; @@ -268,7 +269,26 @@ message CertificateProviderPluginInstance { string certificate_name = 2; } -// [#next-free-field: 15] +// Matcher for subject alternative names, to match both type and value of the SAN. +message SubjectAltNameMatcher { + // Indicates the choice of GeneralName as defined in section 4.2.1.5 of RFC 5280 to match + // against. + enum SanType { + SAN_TYPE_UNSPECIFIED = 0; + EMAIL = 1; + DNS = 2; + URI = 3; + IP_ADDRESS = 4; + } + + // Specification of type of SAN. Note that the default enum value is an invalid choice. + SanType san_type = 1 [(validate.rules).enum = {defined_only: true not_in: 0}]; + + // Matcher for SAN value. + type.matcher.v3.StringMatcher matcher = 2 [(validate.rules).message = {required: true}]; +} + +// [#next-free-field: 16] message CertificateValidationContext { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.auth.CertificateValidationContext"; @@ -298,8 +318,8 @@ message CertificateValidationContext { // `, // :ref:`verify_certificate_hash // `, or - // :ref:`match_subject_alt_names - // `) is also + // :ref:`match_typed_subject_alt_names + // `) is also // specified. // // It can optionally contain certificate revocation lists, in which case Envoy will verify @@ -406,6 +426,8 @@ message CertificateValidationContext { // An optional list of Subject Alternative name matchers. If specified, Envoy will verify that the // Subject Alternative Name of the presented certificate matches one of the specified matchers. + // The matching uses "any" semantics, that is to say, the SAN is verified if at least one matcher is + // matched. // // When a certificate has wildcard DNS SAN entries, to match a specific client, it should be // configured with exact match type in the :ref:`string matcher `. @@ -414,15 +436,22 @@ message CertificateValidationContext { // // .. code-block:: yaml // - // match_subject_alt_names: - // exact: "api.example.com" + // match_typed_subject_alt_names: + // - san_type: DNS + // matcher: + // exact: "api.example.com" // // .. attention:: // // Subject Alternative Names are easily spoofable and verifying only them is insecure, // therefore this option must be used together with :ref:`trusted_ca // `. - repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9; + repeated SubjectAltNameMatcher match_typed_subject_alt_names = 15; + + // This field is deprecated in favor of ref:`match_typed_subject_alt_names + // ` + repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // [#not-implemented-hide:] Must present signed certificate time-stamp. google.protobuf.BoolValue require_signed_certificate_timestamp = 6; diff --git a/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto b/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto index cfb5e5c07e90..382fe985daff 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto @@ -42,7 +42,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Note that SPIFFE validator inherits and uses the following options from :ref:`CertificateValidationContext `. // // - :ref:`allow_expired_certificate ` to allow expired certificates. -// - :ref:`match_subject_alt_names ` to match **URI** SAN of certificates. Unlike the default validator, SPIFFE validator only matches **URI** SAN (which equals to SVID in SPIFFE terminology) and ignore other SAN types. +// - :ref:`match_typed_subject_alt_names ` to match **URI** SAN of certificates. Unlike the default validator, SPIFFE validator only matches **URI** SAN (which equals to SVID in SPIFFE terminology) and ignore other SAN types. // message SPIFFECertValidatorConfig { message TrustDomain { diff --git a/configs/envoy_double_proxy.template.yaml b/configs/envoy_double_proxy.template.yaml index b574d6a518c5..56f4d17ad201 100644 --- a/configs/envoy_double_proxy.template.yaml +++ b/configs/envoy_double_proxy.template.yaml @@ -149,8 +149,10 @@ static_resources: validation_context: trusted_ca: filename: certs/cacert.pem - match_subject_alt_names: - exact: "front-proxy.yourcompany.net" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "front-proxy.yourcompany.net" typed_extension_protocol_options: envoy.extensions.upstreams.http.v3.HttpProtocolOptions: "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions @@ -188,8 +190,10 @@ static_resources: validation_context: trusted_ca: filename: certs/cacert.pem - match_subject_alt_names: - exact: "collector-grpc.lightstep.com" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "collector-grpc.lightstep.com" flags_path: "/etc/envoy/flags" stats_sinks: - name: envoy.stat_sinks.statsd diff --git a/configs/envoy_service_to_service.template.yaml b/configs/envoy_service_to_service.template.yaml index 6ec2f0bde905..7bdd6a4d0fad 100644 --- a/configs/envoy_service_to_service.template.yaml +++ b/configs/envoy_service_to_service.template.yaml @@ -350,8 +350,10 @@ static_resources: trusted_ca: filename: certs/cacert.pem {% if host.get('verify_subject_alt_name', False) %} - match_subject_alt_names: - exact: "{{host['verify_subject_alt_name'] }}" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "{{host['verify_subject_alt_name'] }}" {% endif %} {% if host.get('sni', False) %} sni: "{{ host['sni'] }}" @@ -520,8 +522,10 @@ static_resources: validation_context: trusted_ca: filename: certs/cacert.pem - match_subject_alt_names: - exact: "collector-grpc.lightstep.com" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "collector-grpc.lightstep.com" - name: cds_cluster connect_timeout: 0.25s type: STRICT_DNS diff --git a/docs/root/intro/arch_overview/security/_include/ssl.yaml b/docs/root/intro/arch_overview/security/_include/ssl.yaml index 6f666e4d92a5..00bd6083239b 100644 --- a/docs/root/intro/arch_overview/security/_include/ssl.yaml +++ b/docs/root/intro/arch_overview/security/_include/ssl.yaml @@ -50,7 +50,9 @@ static_resources: private_key: {"filename": "certs/serverkey.pem"} ocsp_staple: {"filename": "certs/server_ocsp_resp.der"} validation_context: - match_subject_alt_names: - - exact: "foo" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "foo" trusted_ca: filename: /etc/ssl/certs/ca-certificates.crt diff --git a/docs/root/intro/arch_overview/security/ssl.rst b/docs/root/intro/arch_overview/security/ssl.rst index e1192833232c..8878c1faf419 100644 --- a/docs/root/intro/arch_overview/security/ssl.rst +++ b/docs/root/intro/arch_overview/security/ssl.rst @@ -76,7 +76,7 @@ Example configuration */etc/ssl/certs/ca-certificates.crt* is the default path for the system CA bundle on Debian systems. :ref:`trusted_ca ` along with -:ref:`match_subject_alt_names ` +:ref:`match_typed_subject_alt_names ` makes Envoy verify the server identity of *127.0.0.1:1234* as "foo" in the same way as e.g. cURL does on standard Debian installations. Common paths for system CA bundles on Linux and BSD are: diff --git a/docs/root/start/quick-start/_include/envoy-demo-tls-client-auth.yaml b/docs/root/start/quick-start/_include/envoy-demo-tls-client-auth.yaml index 84367c637f68..dd48870fbda6 100644 --- a/docs/root/start/quick-start/_include/envoy-demo-tls-client-auth.yaml +++ b/docs/root/start/quick-start/_include/envoy-demo-tls-client-auth.yaml @@ -34,8 +34,10 @@ static_resources: validation_context: trusted_ca: filename: certs/cacert.pem - match_subject_alt_names: - - exact: proxy-postgres-frontend.example.com + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: proxy-postgres-frontend.example.com tls_certificates: - certificate_chain: filename: certs/servercert.pem diff --git a/docs/root/start/quick-start/_include/envoy-demo-tls-validation.yaml b/docs/root/start/quick-start/_include/envoy-demo-tls-validation.yaml index b9ad6cc0635e..054f79e55f36 100644 --- a/docs/root/start/quick-start/_include/envoy-demo-tls-validation.yaml +++ b/docs/root/start/quick-start/_include/envoy-demo-tls-validation.yaml @@ -48,5 +48,7 @@ static_resources: validation_context: trusted_ca: filename: certs/cacert.pem - match_subject_alt_names: - - exact: proxy-postgres-backend.example.com + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: proxy-postgres-backend.example.com diff --git a/docs/root/start/quick-start/securing.rst b/docs/root/start/quick-start/securing.rst index ccfd6bd0ce06..35ff830450a6 100644 --- a/docs/root/start/quick-start/securing.rst +++ b/docs/root/start/quick-start/securing.rst @@ -100,7 +100,7 @@ certificate is valid for. .. 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_subject_alt_names``. + this is what you should use when matching with ``match_typed_subject_alt_names``. .. note:: @@ -122,20 +122,20 @@ and specify a mutually trusted certificate authority: :language: yaml :linenos: :lineno-start: 27 - :lines: 27-39 + :lines: 27-41 :emphasize-lines: 6, 8-10 :caption: :download:`envoy-demo-tls-client-auth.yaml <_include/envoy-demo-tls-client-auth.yaml>` You can further restrict the authentication of connecting clients by specifying the allowed "Subject Alternative Names" in -:ref:`match_subject_alt_names `, +:ref:`match_typed_subject_alt_names `, similar to validating upstream certificates :ref:`described above `. .. literalinclude:: _include/envoy-demo-tls-client-auth.yaml :language: yaml :linenos: :lineno-start: 27 - :lines: 27-39 + :lines: 27-41 :emphasize-lines: 7, 11-12 :caption: :download:`envoy-demo-tls-client-auth.yaml <_include/envoy-demo-tls-client-auth.yaml>` @@ -154,8 +154,8 @@ When connecting to an upstream with client certificates you can set them as foll .. literalinclude:: _include/envoy-demo-tls-client-auth.yaml :language: yaml :linenos: - :lineno-start: 44 - :lines: 44-68 + :lineno-start: 46 + :lines: 46-70 :emphasize-lines: 20-25 :caption: :download:`envoy-demo-tls-client-auth.yaml <_include/envoy-demo-tls-client-auth.yaml>` diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 5297fe4e80a2..1937c5298b41 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -80,6 +80,7 @@ New Features * thrift_proxy: add host level success/error metrics where success is a reply of type success and error is any other response to a call. * thrift_proxy: support header flags. * thrift_proxy: support subset lb when using request or route metadata. +* tls: added support for :ref:`match_typed_subject_alt_names ` for subject alternative names to enforce specifying the subject alternative name type for the matcher to prevent matching against an unintended type in the certificate. * tls: added support for only verifying the leaf CRL in the certificate chain with :ref:`only_verify_leaf_cert_crl `. * tls: support loading certificate chain and private key via :ref:`pkcs12 `. * tls_inspector filter: added :ref:`enable_ja3_fingerprinting ` to create JA3 fingerprint hash from Client Hello message. @@ -95,4 +96,5 @@ Deprecated * bootstrap: :ref:`dns_resolution_config ` is deprecated in favor of :ref:`typed_dns_resolver_config `. * cluster: :ref:`dns_resolution_config ` is deprecated in favor of :ref:`typed_dns_resolver_config `. * dns_cache: :ref:`dns_resolution_config ` is deprecated in favor of :ref:`typed_dns_resolver_config `. +* tls: :ref:`match_subject_alt_names ` has been deprecated in favor of the :ref:`match_typed_subject_alt_names `. * dns_filter: :ref:`dns_resolution_config ` is deprecated in favor of :ref:`typed_dns_resolver_config `. diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index d8a1bf6b0af2..d331c45ce254 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -7,6 +7,7 @@ #include "envoy/api/api.h" #include "envoy/common/pure.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/type/matcher/v3/string.pb.h" #include "absl/types/optional.h" @@ -43,7 +44,7 @@ class CertificateValidationContextConfig { /** * @return The subject alt name matchers to be verified, if enabled. */ - virtual const std::vector& + virtual const std::vector& subjectAltNameMatchers() const PURE; /** diff --git a/examples/double-proxy/envoy-backend.yaml b/examples/double-proxy/envoy-backend.yaml index 07cc1a7905f1..0636354c3771 100644 --- a/examples/double-proxy/envoy-backend.yaml +++ b/examples/double-proxy/envoy-backend.yaml @@ -26,8 +26,10 @@ static_resources: private_key: filename: certs/serverkey.pem validation_context: - match_subject_alt_names: - - exact: proxy-postgres-frontend.example.com + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: proxy-postgres-frontend.example.com trusted_ca: filename: certs/cacert.pem diff --git a/examples/double-proxy/envoy-frontend.yaml b/examples/double-proxy/envoy-frontend.yaml index 37acbf334124..21fa643e62ed 100644 --- a/examples/double-proxy/envoy-frontend.yaml +++ b/examples/double-proxy/envoy-frontend.yaml @@ -36,7 +36,9 @@ static_resources: private_key: filename: certs/clientkey.pem validation_context: - match_subject_alt_names: - - exact: proxy-postgres-backend.example.com + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: proxy-postgres-backend.example.com trusted_ca: filename: certs/cacert.pem diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD index 714c426b930f..4526fe428f73 100644 --- a/source/common/ssl/BUILD +++ b/source/common/ssl/BUILD @@ -32,6 +32,7 @@ envoy_cc_library( "//envoy/ssl:certificate_validation_context_config_interface", "//source/common/common:empty_string", "//source/common/config:datasource_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", ], diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 564325c3fb66..376f2b65a085 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -1,10 +1,13 @@ #include "source/common/ssl/certificate_validation_context_config_impl.h" #include "envoy/common/exception.h" +#include "envoy/config/core/v3/extension.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "source/common/common/empty_string.h" #include "source/common/common/fmt.h" +#include "source/common/common/logger.h" #include "source/common/config/datasource.h" namespace Envoy { @@ -22,8 +25,7 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( certificate_revocation_list_path_( Config::DataSource::getPath(config.crl()) .value_or(certificate_revocation_list_.empty() ? EMPTY_STRING : INLINE_STRING)), - subject_alt_name_matchers_(config.match_subject_alt_names().begin(), - config.match_subject_alt_names().end()), + subject_alt_name_matchers_(getSubjectAltNameMatchers(config)), verify_certificate_hash_list_(config.verify_certificate_hash().begin(), config.verify_certificate_hash().end()), verify_certificate_spki_list_(config.verify_certificate_spki().begin(), @@ -51,5 +53,34 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( } } +std::vector +CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( + const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config) { + if (!config.match_typed_subject_alt_names().empty() && + !config.match_subject_alt_names().empty()) { + throw EnvoyException("SAN-based verification using both match_typed_subject_alt_names and " + "the deprecated match_subject_alt_names is not allowed"); + } + std::vector + subject_alt_name_matchers(config.match_typed_subject_alt_names().begin(), + config.match_typed_subject_alt_names().end()); + // Handle deprecated string type san matchers without san type specified, by + // creating a matcher for each supported type. + for (const envoy::type::matcher::v3::StringMatcher& matcher : config.match_subject_alt_names()) { + static constexpr std::array< + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType, 4> + san_types{envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS}; + for (const auto san_type : san_types) { + subject_alt_name_matchers.emplace_back(); + subject_alt_name_matchers.back().set_san_type(san_type); + *subject_alt_name_matchers.back().mutable_matcher() = matcher; + } + } + return subject_alt_name_matchers; +} + } // namespace Ssl } // namespace Envoy diff --git a/source/common/ssl/certificate_validation_context_config_impl.h b/source/common/ssl/certificate_validation_context_config_impl.h index 038abe08b1e9..6e00605ff5cd 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.h +++ b/source/common/ssl/certificate_validation_context_config_impl.h @@ -4,6 +4,7 @@ #include "envoy/api/api.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/ssl/certificate_validation_context_config.h" #include "envoy/type/matcher/v3/string.pb.h" @@ -24,7 +25,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte const std::string& certificateRevocationListPath() const final { return certificate_revocation_list_path_; } - const std::vector& + const std::vector& subjectAltNameMatchers() const override { return subject_alt_name_matchers_; } @@ -51,11 +52,15 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte bool onlyVerifyLeafCertificateCrl() const override { return only_verify_leaf_cert_crl_; } private: + static std::vector + getSubjectAltNameMatchers( + const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config); const std::string ca_cert_; const std::string ca_cert_path_; const std::string certificate_revocation_list_; const std::string certificate_revocation_list_path_; - const std::vector subject_alt_name_matchers_; + const std::vector + subject_alt_name_matchers_; const std::vector verify_certificate_hash_list_; const std::vector verify_certificate_spki_list_; const bool allow_expired_certificate_; diff --git a/source/extensions/transport_sockets/tls/cert_validator/BUILD b/source/extensions/transport_sockets/tls/cert_validator/BUILD index ce92df41e80c..93f745f8faa3 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/source/extensions/transport_sockets/tls/cert_validator/BUILD @@ -13,12 +13,14 @@ envoy_cc_library( srcs = [ "default_validator.cc", "factory.cc", + "san_matcher.cc", "utility.cc", ], hdrs = [ "cert_validator.h", "default_validator.h", "factory.h", + "san_matcher.h", "utility.h", ], external_deps = [ @@ -35,9 +37,13 @@ envoy_cc_library( "//source/common/common:hex_lib", "//source/common/common:minimal_logger_lib", "//source/common/common:utility_lib", + "//source/common/config:utility_lib", "//source/common/stats:symbol_table_lib", "//source/common/stats:utility_lib", "//source/extensions/transport_sockets/tls:stats_lib", "//source/extensions/transport_sockets/tls:utility_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", + "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc index e638b98b4284..93bbe8b17a04 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -18,6 +18,7 @@ #include "source/common/common/hex.h" #include "source/common/common/matchers.h" #include "source/common/common/utility.h" +#include "source/common/config/utility.h" #include "source/common/network/address_impl.h" #include "source/common/protobuf/utility.h" #include "source/common/runtime/runtime_features.h" @@ -147,9 +148,9 @@ int DefaultCertValidator::initializeSslContexts(std::vector contexts, const Envoy::Ssl::CertificateValidationContextConfig* cert_validation_config = config_; if (cert_validation_config != nullptr) { if (!cert_validation_config->subjectAltNameMatchers().empty()) { - for (const envoy::type::matcher::v3::StringMatcher& matcher : + for (const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher : cert_validation_config->subjectAltNameMatchers()) { - subject_alt_name_matchers_.push_back(Matchers::StringMatcherImpl(matcher)); + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher)); } verify_mode = verify_mode_validation_context; } @@ -221,8 +222,8 @@ int DefaultCertValidator::doVerifyCertChain( // If `trusted_ca` exists, it is already verified in the code above. Thus, we just need to make // sure the verification for other validation context configurations doesn't fail (i.e. either - // `NotValidated` or `Validated`). If `trusted_ca` doesn't exist, we will need to make sure other - // configurations are verified and the verification succeed. + // `NotValidated` or `Validated`). If `trusted_ca` doesn't exist, we will need to make sure + // other configurations are verified and the verification succeed. int validation_status = verify_trusted_ca_ ? validated != Envoy::Ssl::ClientValidationStatus::Failed : validated == Envoy::Ssl::ClientValidationStatus::Validated; @@ -232,8 +233,7 @@ int DefaultCertValidator::doVerifyCertChain( Envoy::Ssl::ClientValidationStatus DefaultCertValidator::verifyCertificate( X509* cert, const std::vector& verify_san_list, - const std::vector>& - subject_alt_name_matchers) { + const std::vector& subject_alt_name_matchers) { Envoy::Ssl::ClientValidationStatus validated = Envoy::Ssl::ClientValidationStatus::NotValidated; if (!verify_san_list.empty()) { @@ -291,23 +291,15 @@ bool DefaultCertValidator::verifySubjectAltName(X509* cert, } bool DefaultCertValidator::matchSubjectAltName( - X509* cert, - const std::vector>& - subject_alt_name_matchers) { + X509* cert, const std::vector& subject_alt_name_matchers) { bssl::UniquePtr san_names( static_cast(X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr))); if (san_names == nullptr) { return false; } - for (const GENERAL_NAME* general_name : san_names.get()) { - const std::string san = Utility::generalNameAsString(general_name); - for (auto& config_san_matcher : subject_alt_name_matchers) { - // For DNS SAN, if the StringMatcher type is exact, we have to follow DNS matching semantics. - if (general_name->type == GEN_DNS && - config_san_matcher.matcher().match_pattern_case() == - envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kExact - ? Utility::dnsNameMatch(config_san_matcher.matcher().exact(), absl::string_view(san)) - : config_san_matcher.match(san)) { + for (const auto& config_san_matcher : subject_alt_name_matchers) { + for (const GENERAL_NAME* general_name : san_names.get()) { + if (config_san_matcher->match(general_name)) { return true; } } diff --git a/source/extensions/transport_sockets/tls/cert_validator/default_validator.h b/source/extensions/transport_sockets/tls/cert_validator/default_validator.h index 0cfe2766d137..64f52bc38728 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.h +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "envoy/common/pure.h" @@ -18,6 +19,7 @@ #include "source/common/common/matchers.h" #include "source/common/stats/symbol_table_impl.h" #include "source/extensions/transport_sockets/tls/cert_validator/cert_validator.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "absl/synchronization/mutex.h" @@ -53,10 +55,9 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable& verify_san_list, - const std::vector>& - subject_alt_name_matchers); + Envoy::Ssl::ClientValidationStatus + verifyCertificate(X509* cert, const std::vector& verify_san_list, + const std::vector& subject_alt_name_matchers); /** * Verifies certificate hash for pinning. The hash is a hex-encoded SHA-256 of the DER-encoded @@ -94,10 +95,8 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable>& - subject_alt_name_matchers); + static bool matchSubjectAltName(X509* cert, + const std::vector& subject_alt_name_matchers); private: const Envoy::Ssl::CertificateValidationContextConfig* config_; @@ -107,8 +106,7 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable ca_cert_; std::string ca_file_path_; - std::vector> - subject_alt_name_matchers_; + std::vector subject_alt_name_matchers_; std::vector> verify_certificate_hash_list_; std::vector> verify_certificate_spki_list_; bool verify_trusted_ca_{false}; diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc new file mode 100644 index 000000000000..4be1c64ec096 --- /dev/null +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc @@ -0,0 +1,53 @@ +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" + +#include + +#include "envoy/config/core/v3/extension.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/ssl/certificate_validation_context_config.h" + +#include "source/extensions/transport_sockets/tls/utility.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +bool StringSanMatcher::match(const GENERAL_NAME* general_name) const { + if (general_name->type != general_name_type_) { + return false; + } + // For DNS SAN, if the StringMatcher type is exact, we have to follow DNS matching semantics. + const std::string san = Utility::generalNameAsString(general_name); + return general_name->type == GEN_DNS && + matcher_.matcher().match_pattern_case() == + envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kExact + ? Utility::dnsNameMatch(matcher_.matcher().exact(), absl::string_view(san)) + : matcher_.match(san); +} + +SanMatcherPtr createStringSanMatcher( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher) { + // Verify that a new san type has not been added. + static_assert(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType_MAX == + 4); + + switch (matcher.san_type()) { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: + return SanMatcherPtr{std::make_unique(GEN_DNS, matcher.matcher())}; + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL: + return SanMatcherPtr{std::make_unique(GEN_EMAIL, matcher.matcher())}; + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI: + return SanMatcherPtr{std::make_unique(GEN_URI, matcher.matcher())}; + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: + return SanMatcherPtr{std::make_unique(GEN_IPADD, matcher.matcher())}; + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.h new file mode 100644 index 000000000000..5c2141f4b570 --- /dev/null +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.h @@ -0,0 +1,51 @@ +#pragma once + +#include + +#include "envoy/config/core/v3/extension.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" +#include "envoy/ssl/certificate_validation_context_config.h" +#include "envoy/type/matcher/v3/string.pb.h" + +#include "source/common/common/hash.h" +#include "source/common/common/matchers.h" +#include "source/common/protobuf/protobuf.h" +#include "source/extensions/transport_sockets/tls/utility.h" + +#include "openssl/x509v3.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +/** Interface to verify if there is a match in a list of subject alternative + * names. + */ +class SanMatcher { +public: + virtual bool match(GENERAL_NAME const*) const PURE; + virtual ~SanMatcher() = default; +}; + +using SanMatcherPtr = std::unique_ptr; + +class StringSanMatcher : public SanMatcher { +public: + bool match(const GENERAL_NAME* general_name) const override; + ~StringSanMatcher() override = default; + StringSanMatcher(int general_name_type, envoy::type::matcher::v3::StringMatcher matcher) + : general_name_type_(general_name_type), matcher_(matcher) {} + +private: + const int general_name_type_; + const Matchers::StringMatcherImpl matcher_; +}; + +SanMatcherPtr createStringSanMatcher( + const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher); + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc index 63df8297c76d..c3b8d338fcae 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc @@ -1,5 +1,6 @@ #include "source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.pb.h" #include "envoy/network/transport_socket.h" #include "envoy/registry/registry.h" @@ -37,7 +38,14 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC if (!config->subjectAltNameMatchers().empty()) { for (const auto& matcher : config->subjectAltNameMatchers()) { - subject_alt_name_matchers_.push_back(Matchers::StringMatcherImpl(matcher)); + if (matcher.san_type() == + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI) { + // Only match against URI SAN since SPIFFE specification does not restrict values in other + // SAN types. See the discussion: https://github.com/envoyproxy/envoy/issues/15392 + // TODO(pradeepcrao): Throw an exception when a non-URI matcher is encountered after the + // deprecated field match_subject_alt_names is removed + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher)); + } } } @@ -224,15 +232,10 @@ bool SPIFFEValidator::matchSubjectAltName(X509& leaf_cert) { ASSERT(san_names != nullptr, "san_names should have at least one name after SPIFFE cert validation"); - // Only match against URI SAN since SPIFFE specification does not restrict values in other SAN - // types. See the discussion: https://github.com/envoyproxy/envoy/issues/15392 for (const GENERAL_NAME* general_name : san_names.get()) { - if (general_name->type == GEN_URI) { - const std::string san = Utility::generalNameAsString(general_name); - for (const auto& config_san_matcher : subject_alt_name_matchers_) { - if (config_san_matcher.match(san)) { - return true; - } + for (const auto& config_san_matcher : subject_alt_name_matchers_) { + if (config_san_matcher->match(general_name)) { + return true; } } } diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h index b4cc068e908e..669d77aec7d0 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h +++ b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h @@ -17,6 +17,7 @@ #include "source/common/common/matchers.h" #include "source/common/stats/symbol_table_impl.h" #include "source/extensions/transport_sockets/tls/cert_validator/cert_validator.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "openssl/ssl.h" @@ -67,8 +68,7 @@ class SPIFFEValidator : public CertValidator { bool allow_expired_certificate_{false}; std::vector> ca_certs_; std::string ca_file_name_; - std::vector> - subject_alt_name_matchers_{}; + std::vector subject_alt_name_matchers_{}; absl::flat_hash_map trust_bundle_stores_; SslStats& stats_; diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index b481768eb91f..3c49ad23f35e 100644 --- a/test/common/quic/envoy_quic_proof_source_test.cc +++ b/test/common/quic/envoy_quic_proof_source_test.cc @@ -58,7 +58,8 @@ class SignatureVerifier { ON_CALL(cert_validation_ctx_config_, certificateRevocationListPath()) .WillByDefault(ReturnRef(path_string)); const std::vector empty_string_list; - const std::vector san_matchers; + const std::vector + san_matchers; ON_CALL(cert_validation_ctx_config_, subjectAltNameMatchers()) .WillByDefault(ReturnRef(san_matchers)); ON_CALL(cert_validation_ctx_config_, verifyCertificateHashList()) diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index dea9daa038f5..06b3405b7d2a 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -75,7 +75,8 @@ class EnvoyQuicProofVerifierTest : public testing::Test { const std::string path_string_{"some_path"}; const std::string alpn_{"h2,http/1.1"}; const std::string sig_algs_{"rsa_pss_rsae_sha256"}; - const std::vector san_matchers_; + const std::vector + san_matchers_; const std::string empty_string_; const std::vector empty_string_list_; const std::string cert_chain_{quic::test::kTestCertificateChainPem}; diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index a0cea30a2f8a..f91734a588b9 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -665,7 +665,10 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { dynamic_cvc->set_allow_expired_certificate(false); dynamic_cvc->mutable_trusted_ca()->set_filename(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem")); - dynamic_cvc->add_match_subject_alt_names()->set_exact("second san"); + auto* san_matcher = dynamic_cvc->add_match_typed_subject_alt_names(); + san_matcher->mutable_matcher()->set_exact("second san"); + san_matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); const std::string dynamic_verify_certificate_spki = "QGJRPdmx/r5EGOFLb2MTiZp2isyC0Whht7iazhzXaCM="; dynamic_cvc->add_verify_certificate_spki(dynamic_verify_certificate_spki); @@ -681,7 +684,10 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext default_cvc; default_cvc.set_allow_expired_certificate(true); default_cvc.mutable_trusted_ca()->set_inline_bytes("fake trusted ca"); - default_cvc.add_match_subject_alt_names()->set_exact("first san"); + san_matcher = default_cvc.add_match_typed_subject_alt_names(); + san_matcher->mutable_matcher()->set_exact("first san"); + san_matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); default_cvc.add_verify_certificate_hash(default_verify_certificate_hash); envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext merged_cvc = default_cvc; @@ -697,8 +703,12 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { cvc_config.caCert()); // Verify that repeated fields are concatenated. EXPECT_EQ(2, cvc_config.subjectAltNameMatchers().size()); - EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[0].exact()); - EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[1].exact()); + EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[0].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, + cvc_config.subjectAltNameMatchers()[0].san_type()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[1].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, + cvc_config.subjectAltNameMatchers()[1].san_type()); // Verify that if dynamic CertificateValidationContext does not set certificate hash list, the new // secret contains hash list from default CertificateValidationContext. EXPECT_EQ(1, cvc_config.verifyCertificateHashList().size()); diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index a73f143ba861..1109b7621256 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -1128,6 +1128,43 @@ name: "abc.com" EnvoyException, "Failed to load private key provider: test"); } +// Verify that using the match_subject_alt_names will result in a typed matcher, one for each of +// DNS, URI, EMAIL and IP_ADDRESS. +// TODO(pradeepcrao): Delete this test once the deprecated field is removed. +TEST_F(SecretManagerImplTest, DeprecatedSanMatcher) { + envoy::extensions::transport_sockets::tls::v3::Secret secret_config; + const std::string yaml = + R"EOF( + name: "abc.com" + validation_context: + trusted_ca: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" } + allow_expired_certificate: true + match_subject_alt_names: + exact: "example.foo" + )EOF"; + TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); + secret_manager->addStaticSecret(secret_config); + + ASSERT_EQ(secret_manager->findStaticCertificateValidationContextProvider("undefined"), nullptr); + ASSERT_NE(secret_manager->findStaticCertificateValidationContextProvider("abc.com"), nullptr); + Ssl::CertificateValidationContextConfigImpl cvc_config( + *secret_manager->findStaticCertificateValidationContextProvider("abc.com")->secret(), *api_); + EXPECT_EQ(cvc_config.subjectAltNameMatchers().size(), 4); + EXPECT_EQ("example.foo", cvc_config.subjectAltNameMatchers()[0].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, + cvc_config.subjectAltNameMatchers()[0].san_type()); + EXPECT_EQ("example.foo", cvc_config.subjectAltNameMatchers()[1].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, + cvc_config.subjectAltNameMatchers()[1].san_type()); + EXPECT_EQ("example.foo", cvc_config.subjectAltNameMatchers()[2].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, + cvc_config.subjectAltNameMatchers()[2].san_type()); + EXPECT_EQ("example.foo", cvc_config.subjectAltNameMatchers()[3].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS, + cvc_config.subjectAltNameMatchers()[3].san_type()); +} + } // namespace } // namespace Secret } // namespace Envoy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index b177712ca4cc..f413d4cfd8a3 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3554,9 +3554,13 @@ TEST_F(ClusterInfoImplTest, Http3) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS )EOF", Network::Address::IpVersion::v4); auto cluster1 = makeCluster(yaml); @@ -3627,9 +3631,13 @@ TEST_F(ClusterInfoImplTest, Http3BadConfig) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS typed_extension_protocol_options: envoy.extensions.upstreams.http.v3.HttpProtocolOptions: "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions @@ -3672,9 +3680,13 @@ TEST_F(ClusterInfoImplTest, Http3Auto) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS )EOF", Network::Address::IpVersion::v4); @@ -3731,9 +3743,13 @@ TEST_F(ClusterInfoImplTest, UseDownstreamHttpProtocolWithoutDowngrade) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS typed_extension_protocol_options: envoy.extensions.upstreams.http.v3.HttpProtocolOptions: "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions diff --git a/test/config/utility.cc b/test/config/utility.cc index 320fa3957c75..cfd6fa10e612 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1215,8 +1215,8 @@ void ConfigHelper::initializeTls( } } if (!options.san_matchers_.empty()) { - *validation_context->mutable_match_subject_alt_names() = {options.san_matchers_.begin(), - options.san_matchers_.end()}; + *validation_context->mutable_match_typed_subject_alt_names() = {options.san_matchers_.begin(), + options.san_matchers_.end()}; } } diff --git a/test/config/utility.h b/test/config/utility.h index 5ebdc1484019..33fa29e13b69 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -14,6 +14,7 @@ #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/extensions/upstreams/http/v3/http_protocol_options.pb.h" #include "envoy/http/codes.h" @@ -80,7 +81,8 @@ class ConfigHelper { } ServerSslOptions& - setSanMatchers(std::vector san_matchers) { + setSanMatchers(std::vector + san_matchers) { san_matchers_ = san_matchers; return *this; } @@ -94,7 +96,8 @@ class ConfigHelper { bool ocsp_staple_required_{false}; bool tlsv1_3_{false}; bool expect_client_ecdsa_cert_{false}; - std::vector san_matchers_{}; + std::vector + san_matchers_{}; }; // Set up basic config, using the specified IpVersion for all connections: listeners, upstream, diff --git a/test/extensions/transport_sockets/tls/cert_validator/BUILD b/test/extensions/transport_sockets/tls/cert_validator/BUILD index 7fd2c97084a1..9d880d51fa05 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/test/extensions/transport_sockets/tls/cert_validator/BUILD @@ -57,3 +57,16 @@ envoy_cc_test_library( "//test/test_common:utility_lib", ], ) + +envoy_cc_test( + name = "san_matcher_test", + srcs = [ + "san_matcher_test.cc", + ], + deps = [ + "//source/common/protobuf:utility_lib", + "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc index 036c3679fa1f..d25eb20c0a03 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc @@ -2,6 +2,7 @@ #include #include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include "test/extensions/transport_sockets/tls/ssl_test_utility.h" #include "test/test_common/environment.h" @@ -28,22 +29,33 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; - matcher.MergeFrom(TestUtility::createRegexMatcher(".*.example.com")); - std::vector> - subject_alt_name_matchers; - subject_alt_name_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } +TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameIncorrectTypeMatched) { + bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); + envoy::type::matcher::v3::StringMatcher matcher; + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); + EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameWildcardDNSMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir " "}}/test/extensions/transport_sockets/tls/test_data/san_multiple_dns_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; matcher.set_exact("api.example.com"); - std::vector> - subject_alt_name_matchers; - subject_alt_name_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -54,9 +66,9 @@ TEST(DefaultCertValidatorTest, TestMultiLevelMatch) { "}}/test/extensions/transport_sockets/tls/test_data/san_multiple_dns_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; matcher.set_exact("foo.api.example.com"); - std::vector> - subject_alt_name_matchers; - subject_alt_name_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -81,10 +93,10 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURIMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; - matcher.MergeFrom(TestUtility::createRegexMatcher("spiffe://lyft.com/.*-team")); - std::vector> - subject_alt_name_matchers; - subject_alt_name_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw(spiffe://lyft.com/[^/]*-team)raw")); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -100,10 +112,16 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; - matcher.MergeFrom(TestUtility::createRegexMatcher(".*.foo.com")); - std::vector> - subject_alt_name_matchers; - subject_alt_name_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example\.net)raw")); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_IPADD, matcher)}); + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_EMAIL, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -119,18 +137,18 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithSANMatcher) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; - matcher.MergeFrom(TestUtility::createRegexMatcher(".*.example.com")); - std::vector> san_matchers; - san_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); + std::vector san_matchers; + san_matchers.push_back(SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); // Verify the certificate with correct SAN regex matcher. EXPECT_EQ(default_validator->verifyCertificate(cert.get(), /*verify_san_list=*/{}, san_matchers), Envoy::Ssl::ClientValidationStatus::Validated); EXPECT_EQ(stats.fail_verify_san_.value(), 0); matcher.MergeFrom(TestUtility::createExactMatcher("hello.example.com")); - std::vector> - invalid_san_matchers; - invalid_san_matchers.push_back(Matchers::StringMatcherImpl(matcher)); + std::vector invalid_san_matchers; + invalid_san_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); // Verify the certificate with incorrect SAN exact matcher. EXPECT_EQ(default_validator->verifyCertificate(cert.get(), /*verify_san_list=*/{}, invalid_san_matchers), @@ -158,6 +176,17 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithNoValidationContex 0); } +TEST(DefaultCertValidatorTest, NoSanInCert) { + bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/fake_ca_cert.pem")); + envoy::type::matcher::v3::StringMatcher matcher; + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example\.net)raw")); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); + EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc new file mode 100644 index 000000000000..b7729de8d41e --- /dev/null +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc @@ -0,0 +1,57 @@ +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.validate.h" + +#include "source/common/protobuf/message_validator_impl.h" +#include "source/common/protobuf/utility.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" + +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +// Verify that we get a valid string san matcher for all valid san types. +TEST(SanMatcherConfigTest, TestValidSanType) { + // Iterate over all san type enums. + for (envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType san_type = + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType_MIN; + san_type <= + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType_MAX; + san_type = static_cast< + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType>( + static_cast(san_type + 1))) { + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; + san_matcher.mutable_matcher()->set_exact("foo.example"); + san_matcher.set_san_type(san_type); + if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: + SAN_TYPE_UNSPECIFIED) { + continue; + } else { + const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); + EXPECT_NE(matcher.get(), nullptr); + // Verify that the message is valid. + TestUtility::validate(san_matcher); + } + } +} + +TEST(SanMatcherConfigTest, UnspecifiedSanType) { + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; + san_matcher.mutable_matcher()->set_exact("foo.example"); + // Do not set san_type + EXPECT_THROW_WITH_REGEX(TestUtility::validate(san_matcher), EnvoyException, + "Proto constraint validation failed"); + san_matcher.set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SAN_TYPE_UNSPECIFIED); + EXPECT_THROW_WITH_REGEX(TestUtility::validate(san_matcher), EnvoyException, + "Proto constraint validation failed"); +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc index 47acf98c4f53..fe47ef14f6e3 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc @@ -53,6 +53,26 @@ void SslSPIFFECertValidatorIntegrationTest::checkVerifyErrorCouter(uint64_t valu counter->reset(); } +void SslSPIFFECertValidatorIntegrationTest::addStringMatcher( + const envoy::type::matcher::v3::StringMatcher& matcher) { + san_matchers_.emplace_back(); + *san_matchers_.back().mutable_matcher() = matcher; + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + san_matchers_.emplace_back(); + *san_matchers_.back().mutable_matcher() = matcher; + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI); + san_matchers_.emplace_back(); + *san_matchers_.back().mutable_matcher() = matcher; + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL); + san_matchers_.emplace_back(); + *san_matchers_.back().mutable_matcher() = matcher; + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS); +} + INSTANTIATE_TEST_SUITE_P( IpVersionsClientVersions, SslSPIFFECertValidatorIntegrationTest, testing::Combine( @@ -124,7 +144,7 @@ name: envoy.tls.cert_validator.spiffe envoy::type::matcher::v3::StringMatcher matcher; matcher.set_prefix("spiffe://lyft.com/"); - san_matchers_ = {matcher}; + addStringMatcher(matcher); ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { return makeSslClientConnection({}); @@ -152,7 +172,7 @@ name: envoy.tls.cert_validator.spiffe matcher.set_prefix("spiffe://example.com/"); // The cert has "DNS.1 = lyft.com" but SPIFFE validator must ignore SAN types other than URI. matcher.set_prefix("www.lyft.com"); - san_matchers_ = {matcher}; + addStringMatcher(matcher); initialize(); auto conn = makeSslClientConnection({}); if (tls_version_ == envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_2) { @@ -223,8 +243,8 @@ name: envoy.tls.cert_validator.spiffe checkVerifyErrorCouter(1); } -// clientcert.pem's san is "spiffe://lyft.com/frontend-team" but the corresponding trust bundle does -// not match with the client cert. So this should also be rejected. +// clientcert.pem's san is "spiffe://lyft.com/frontend-team" but the corresponding trust bundle +// does not match with the client cert. So this should also be rejected. TEST_P(SslSPIFFECertValidatorIntegrationTest, ServerRsaSPIFFEValidatorRejected2) { auto typed_conf = new envoy::config::core::v3::TypedExtensionConfig(); TestUtility::loadFromYaml(TestEnvironment::substitute(R"EOF( diff --git a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h index b1e57169ea18..01d08f5a811d 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h +++ b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h @@ -39,10 +39,11 @@ class SslSPIFFECertValidatorIntegrationTest } protected: + void addStringMatcher(envoy::type::matcher::v3::StringMatcher const& matcher); bool allow_expired_cert_{}; envoy::config::core::v3::TypedExtensionConfig* custom_validator_config_{nullptr}; std::unique_ptr context_manager_; - std::vector san_matchers_; + std::vector san_matchers_; const envoy::extensions::transport_sockets::tls::v3::TlsParameters::TlsProtocol tls_version_{ std::get<1>(GetParam())}; }; diff --git a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc index 07206e156a72..3be9706e76be 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc @@ -64,13 +64,34 @@ class TestSPIFFEValidator : public testing::Test { // Setter. void setAllowExpiredCertificate(bool val) { allow_expired_certificate_ = val; } void setSanMatchers(std::vector san_matchers) { - san_matchers_ = san_matchers; + san_matchers_.clear(); + for (auto& matcher : san_matchers) { + san_matchers_.emplace_back(); + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + *san_matchers_.back().mutable_matcher() = matcher; + + san_matchers_.emplace_back(); + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI); + *san_matchers_.back().mutable_matcher() = matcher; + + san_matchers_.emplace_back(); + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL); + *san_matchers_.back().mutable_matcher() = matcher; + + san_matchers_.emplace_back(); + san_matchers_.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS); + *san_matchers_.back().mutable_matcher() = matcher; + } }; private: bool allow_expired_certificate_{false}; TestCertificateValidationContextConfigPtr config_; - std::vector san_matchers_{}; + std::vector san_matchers_{}; Stats::TestUtil::TestStore store_; SslStats stats_; Event::TestRealTimeSystem time_system_; @@ -193,7 +214,8 @@ TEST_F(TestSPIFFEValidator, TestGetTrustBundleStore) { // Non-SPIFFE SAN cert = readCertFromFile(TestEnvironment::substitute( - "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/non_spiffe_san_cert.pem")); + "{{ test_rundir " + "}}/test/extensions/transport_sockets/tls/test_data/non_spiffe_san_cert.pem")); EXPECT_FALSE(validator().getTrustBundleStore(cert.get())); // SPIFFE SAN diff --git a/test/extensions/transport_sockets/tls/cert_validator/test_common.h b/test/extensions/transport_sockets/tls/cert_validator/test_common.h index 11d4022cf043..0eb5066e1c76 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/test_common.h +++ b/test/extensions/transport_sockets/tls/cert_validator/test_common.h @@ -33,7 +33,8 @@ class TestCertificateValidationContextConfig public: TestCertificateValidationContextConfig( envoy::config::core::v3::TypedExtensionConfig config, bool allow_expired_certificate = false, - std::vector san_matchers = {}) + std::vector + san_matchers = {}) : allow_expired_certificate_(allow_expired_certificate), api_(Api::createApiForTest()), custom_validator_config_(config), san_matchers_(san_matchers){}; TestCertificateValidationContextConfig() @@ -47,7 +48,7 @@ class TestCertificateValidationContextConfig const std::string& certificateRevocationListPath() const final { CONSTRUCT_ON_FIRST_USE(std::string, ""); } - const std::vector& + const std::vector& subjectAltNameMatchers() const override { return san_matchers_; } @@ -78,7 +79,8 @@ class TestCertificateValidationContextConfig bool allow_expired_certificate_{false}; Api::ApiPtr api_; const absl::optional custom_validator_config_; - const std::vector san_matchers_{}; + const std::vector + san_matchers_{}; }; } // namespace Tls diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 2e6c67a1da17..f4568860b223 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -923,8 +923,10 @@ TEST_F(SslServerContextImplTicketTest, VerifySanWithNoCA) { private_key: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_key.pem" validation_context: - match_subject_alt_names: - exact : "spiffe://lyft.com/testclient" + match_typed_subject_alt_names: + - san_type: URI + matcher: + exact: "spiffe://lyft.com/testclient" )EOF"; EXPECT_THROW_WITH_MESSAGE(loadConfigYaml(yaml), EnvoyException, "SAN-based verification of peer certificates without trusted CA " @@ -1907,6 +1909,35 @@ TEST_F(ServerContextConfigImplTest, PrivateKeyMethodLoadFailureBothKeyAndMethod) "Certificate configuration can't have both private_key and private_key_provider"); } +// Test that we don't allow specification of both typed and untyped matchers for +// sans. +TEST_F(ServerContextConfigImplTest, DeprecatedSanMatcher) { + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; + NiceMock context_manager; + NiceMock private_key_method_manager; + auto private_key_method_provider_ptr = + std::make_shared>(); + const std::string yaml = + R"EOF( + common_tls_context: + validation_context: + trusted_ca: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" } + allow_expired_certificate: true + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "foo.example" + match_subject_alt_names: + exact: "foo.example" + )EOF"; + TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context); + + EXPECT_THROW_WITH_MESSAGE( + ServerContextConfigImpl server_context_config(tls_context, factory_context_), EnvoyException, + "SAN-based verification using both match_typed_subject_alt_names and " + "the deprecated match_subject_alt_names is not allowed"); +} + TEST_F(ServerContextConfigImplTest, Pkcs12LoadFailureBothPkcs12AndMethod) { envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; NiceMock context_manager; diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 751d9e5459b9..af9288bbea31 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -1089,8 +1089,10 @@ TEST_P(SslSocketTest, GetUriWithUriSan) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - exact: "spiffe://lyft.com/test-team" + match_typed_subject_alt_names: + - san_type: URI + matcher: + exact: "spiffe://lyft.com/test-team" )EOF"; TestUtilOptions test_options(client_ctx_yaml, server_ctx_yaml, true, GetParam()); @@ -1105,8 +1107,10 @@ TEST_P(SslSocketTest, Ipv4San) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/config/integration/certs/upstreamcacert.pem" - match_subject_alt_names: - exact: "127.0.0.1" + match_typed_subject_alt_names: + - san_type: IP_ADDRESS + matcher: + exact: "127.0.0.1" )EOF"; const std::string server_ctx_yaml = R"EOF( @@ -1129,8 +1133,10 @@ TEST_P(SslSocketTest, Ipv6San) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/config/integration/certs/upstreamcacert.pem" - match_subject_alt_names: - exact: "::1" + match_typed_subject_alt_names: + - san_type: IP_ADDRESS + matcher: + exact: "::1" )EOF"; const std::string server_ctx_yaml = R"EOF( @@ -1533,8 +1539,10 @@ TEST_P(SslSocketTest, FailedClientAuthSanVerificationNoClientCert) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - exact: "example.com" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "example.com" )EOF"; TestUtilOptions test_options(client_ctx_yaml, server_ctx_yaml, false, GetParam()); @@ -1561,8 +1569,10 @@ TEST_P(SslSocketTest, FailedClientAuthSanVerification) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - exact: "example.com" + match_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "example.com" )EOF"; TestUtilOptions test_options(client_ctx_yaml, server_ctx_yaml, false, GetParam()); diff --git a/test/integration/ads_integration.cc b/test/integration/ads_integration.cc index f97ad753fe52..a85242d72d1b 100644 --- a/test/integration/ads_integration.cc +++ b/test/integration/ads_integration.cc @@ -145,7 +145,10 @@ void AdsIntegrationTest::initializeAds(const bool rate_limiting) { auto* validation_context = context.mutable_common_tls_context()->mutable_validation_context(); validation_context->mutable_trusted_ca()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); - validation_context->add_match_subject_alt_names()->set_suffix("lyft.com"); + auto* san_matcher = validation_context->add_match_typed_subject_alt_names(); + san_matcher->mutable_matcher()->set_suffix("lyft.com"); + san_matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); if (clientType() == Grpc::ClientType::GoogleGrpc) { auto* google_grpc = grpc_service->mutable_google_grpc(); auto* ssl_creds = google_grpc->mutable_channel_credentials()->mutable_ssl_credentials(); diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index 36b93b6f81bc..c629d6f2e3c0 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -66,8 +66,23 @@ void initializeUpstreamTlsContextConfig( common_context->add_alpn_protocols(Http::Utility::AlpnNames::get().Http3); } if (!options.san_.empty()) { - common_context->mutable_validation_context()->add_match_subject_alt_names()->set_exact( - options.san_); + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher* matcher = + common_context->mutable_validation_context()->add_match_typed_subject_alt_names(); + matcher->mutable_matcher()->set_exact(options.san_); + matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + matcher = common_context->mutable_validation_context()->add_match_typed_subject_alt_names(); + matcher->mutable_matcher()->set_exact(options.san_); + matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI); + matcher = common_context->mutable_validation_context()->add_match_typed_subject_alt_names(); + matcher->mutable_matcher()->set_exact(options.san_); + matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL); + matcher = common_context->mutable_validation_context()->add_match_typed_subject_alt_names(); + matcher->mutable_matcher()->set_exact(options.san_); + matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS); } for (const std::string& cipher_suite : options.cipher_suites_) { common_context->mutable_tls_params()->add_cipher_suites(cipher_suite); diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index 19186660db21..0ce4d4a94213 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -6,6 +6,7 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/stats/scope.h" #include "source/common/event/dispatcher_impl.h" @@ -45,10 +46,16 @@ Network::TransportSocketFactoryPtr XfccIntegrationTest::createClientSslContext(b validation_context: trusted_ca: filename: {{ test_rundir }}/test/config/integration/certs/cacert.pem - match_subject_alt_names: - exact: "spiffe://lyft.com/backend-team" - exact: "lyft.com" - exact: "www.lyft.com" + match_typed_subject_alt_names: + - san_type: URI + matcher: + exact: "spiffe://lyft.com/backend-team" + - san_type: DNS + matcher: + exact: "lyft.com" + - san_type: DNS + matcher: + exact: "www.lyft.com" )EOF"; const std::string yaml_mtls = R"EOF( @@ -56,10 +63,16 @@ Network::TransportSocketFactoryPtr XfccIntegrationTest::createClientSslContext(b validation_context: trusted_ca: filename: {{ test_rundir }}/test/config/integration/certs/cacert.pem - match_subject_alt_names: - exact: "spiffe://lyft.com/backend-team" - exact: "lyft.com" - exact: "www.lyft.com" + match_typed_subject_alt_names: + - san_type: URI + matcher: + exact: "spiffe://lyft.com/backend-team" + - san_type: DNS + matcher: + exact: "lyft.com" + - san_type: DNS + matcher: + exact: "www.lyft.com" tls_certificates: certificate_chain: filename: {{ test_rundir }}/test/config/integration/certs/clientcert.pem @@ -135,7 +148,10 @@ void XfccIntegrationTest::initialize() { auto* validation_context = context.mutable_common_tls_context()->mutable_validation_context(); validation_context->mutable_trusted_ca()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); - validation_context->add_match_subject_alt_names()->set_suffix("lyft.com"); + auto* san_matcher = validation_context->add_match_typed_subject_alt_names(); + san_matcher->mutable_matcher()->set_suffix("lyft.com"); + san_matcher->set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); transport_socket->set_name("envoy.transport_sockets.tls"); transport_socket->mutable_typed_config()->PackFrom(context); }); diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 130a78b83c61..104242ce0c92 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -159,8 +159,9 @@ class MockCertificateValidationContextConfig : public CertificateValidationConte MOCK_METHOD(const std::string&, caCertPath, (), (const)); MOCK_METHOD(const std::string&, certificateRevocationList, (), (const)); MOCK_METHOD(const std::string&, certificateRevocationListPath, (), (const)); - MOCK_METHOD(const std::vector&, subjectAltNameMatchers, - (), (const)); + MOCK_METHOD( + const std::vector&, + subjectAltNameMatchers, (), (const)); MOCK_METHOD(const std::vector&, verifyCertificateHashList, (), (const)); MOCK_METHOD(const std::vector&, verifyCertificateSpkiList, (), (const)); MOCK_METHOD(bool, allowExpiredCertificate, (), (const)); diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 184a5485da26..93ba06fd0c3f 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -71,7 +71,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/tracers/zipkin:96.1" "source/extensions/transport_sockets:95.3" "source/extensions/transport_sockets/tls:94.5" -"source/extensions/transport_sockets/tls/cert_validator:95.8" +"source/extensions/transport_sockets/tls/cert_validator:95.7" "source/extensions/transport_sockets/tls/ocsp:96.5" "source/extensions/transport_sockets/tls/private_key:77.8" "source/extensions/wasm_runtime/wamr:0.0" # Not enabled in coverage build diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 84e1a3a200c9..f505b467c99d 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/test/server/listener_manager_impl_quic_only_test.cc @@ -60,9 +60,13 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS udp_listener_config: quic_options: {} )EOF", @@ -161,9 +165,13 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongTransportSoc validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS udp_listener_config: quic_options: {} )EOF", @@ -205,9 +213,13 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongCodec) { validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS udp_listener_config: quic_options: {} )EOF", @@ -259,9 +271,13 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithConnectionBalence validation_context: trusted_ca: filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + match_typed_subject_alt_names: + - matcher: + exact: localhost + san_type: URI + - matcher: + exact: 127.0.0.1 + san_type: IP_ADDRESS udp_listener_config: quic_options: {} connection_balance_config: