From 3c47ebb7b8cf2940a4764ac4b1380828f51079cd Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 14 Oct 2021 14:40:10 +0000 Subject: [PATCH 01/32] Specify type for matching Subject Alternative Name. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 36 ++++- .../certificate_validation_context_config.h | 25 ++- ...tificate_validation_context_config_impl.cc | 25 ++- ...rtificate_validation_context_config_impl.h | 9 +- .../tls/cert_validator/BUILD | 5 + .../tls/cert_validator/default_validator.cc | 55 ++++--- .../tls/cert_validator/default_validator.h | 22 +-- .../tls/cert_validator/san_matcher_config.cc | 64 ++++++++ .../tls/cert_validator/san_matcher_config.h | 92 +++++++++++ .../quic/envoy_quic_proof_source_test.cc | 3 +- .../quic/envoy_quic_proof_verifier_test.cc | 3 +- test/common/secret/sds_api_test.cc | 7 +- .../cert_validator/default_validator_test.cc | 145 +++++++++++++++--- .../tls/cert_validator/test_common.h | 8 +- test/mocks/ssl/mocks.h | 5 +- 15 files changed, 437 insertions(+), 67 deletions(-) create mode 100644 source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc create mode 100644 source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 82dcb37cd7ca..55782e10fcd9 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -253,6 +253,10 @@ message CertificateProviderPluginInstance { string certificate_name = 2; } +message SubjectAltNameMatcher { + google.protobuf.Any typed_config = 1; +} + // [#next-free-field: 14] message CertificateValidationContext { option (udpa.annotations.versioning).previous_message_type = @@ -396,15 +400,21 @@ message CertificateValidationContext { // // .. code-block:: yaml // - // match_subject_alt_names: - // exact: "api.example.com" + // match_subject_alt_names_with_type: + // typed_config: + // "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.common.DnsSanMatcher + // 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_subject_alt_names_with_type = 14; + + // This field is deprecated in favor of ref:`match_subject_alt_names_with_type + // ` + repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 [deprecated = true]; // [#not-implemented-hide:] Must present signed certificate time-stamp. google.protobuf.BoolValue require_signed_certificate_timestamp = 6; @@ -434,3 +444,23 @@ message CertificateValidationContext { // [#extension-category: envoy.tls.cert_validator] config.core.v3.TypedExtensionConfig custom_validator_config = 12; } + +// Matcher for DNS type subject alternative name. +message DnsSanMatcher { + type.matcher.v3.StringMatcher matcher = 1; +} + +// Matcher for email type subject alternative name. +message EmailSanMatcher { + type.matcher.v3.StringMatcher matcher = 1; +} + +// Matcher for URI type subject alternative name. +message UriSanMatcher { + type.matcher.v3.StringMatcher matcher = 1; +} + +// Matcher for IP address type subject alternative name. +message IpAddSanMatcher { + type.matcher.v3.StringMatcher matcher = 1; +} diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 544215a73dae..00b8f6e7ee46 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -6,14 +6,37 @@ #include "envoy/api/api.h" #include "envoy/common/pure.h" +#include "envoy/config/typed_config.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" +#include "openssl/x509v3.h" namespace Envoy { namespace Ssl { +/** Interface to verify if there is a match in a list of subject alternative + * names. + */ +class SanMatcher { +public: + virtual bool match(GENERAL_NAMES const*) const PURE; + virtual ~SanMatcher() = default; +}; + +using SanMatcherPtr = std::unique_ptr; + +class SanMatcherFactory : public Config::TypedFactory { +public: + ~SanMatcherFactory() override = default; + + virtual SanMatcherPtr createSanMatcher(const Protobuf::Message* config) PURE; + + std::string category() const override { return "envoy.san_matchers"; } +}; + class CertificateValidationContextConfig { public: virtual ~CertificateValidationContextConfig() = default; @@ -43,7 +66,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/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 40dc20f6ef3a..d8aa261f7c97 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -1,10 +1,12 @@ #include "source/common/ssl/certificate_validation_context_config_impl.h" #include "envoy/common/exception.h" +#include "envoy/extensions//transport_sockets/tls/v3/common.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.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 +24,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(), @@ -36,6 +37,7 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( config.custom_validator_config()) : absl::nullopt), api_(api) { + if (ca_cert_.empty() && custom_validator_config_ == absl::nullopt) { if (!certificate_revocation_list_.empty()) { throw EnvoyException(fmt::format("Failed to load CRL from {} without trusted CA", @@ -51,5 +53,24 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( } } +std::vector +CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( + const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config) { + if (!config.match_subject_alt_names_with_type().empty() && + !config.match_subject_alt_names().empty()) { + throw EnvoyException("SAN-based verification using both match_subject_alt_names_with_type and " + "the deprecated match_subject_alt_names is not allowed"); + } + std::vector + subject_alt_name_matchers(config.match_subject_alt_names_with_type().begin(), + config.match_subject_alt_names_with_type().end()); + for (auto& matcher : config.match_subject_alt_names()) { + subject_alt_name_matchers.emplace_back(); + subject_alt_name_matchers.back().set_allocated_typed_config(new ProtobufWkt::Any()); + subject_alt_name_matchers.back().mutable_typed_config()->PackFrom(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 7cf045a35184..f58ac1ae0a54 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_; } @@ -49,11 +50,15 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte Api::Api& api() const override { return api_; } 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..e1096a03cd7e 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_config.cc", "utility.cc", ], hdrs = [ "cert_validator.h", "default_validator.h", "factory.h", + "san_matcher_config.h", "utility.h", ], external_deps = [ @@ -35,9 +37,12 @@ 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/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 369c2aa7375b..542ae389e238 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" @@ -25,6 +26,7 @@ #include "source/common/stats/utility.h" #include "source/extensions/transport_sockets/tls/cert_validator/cert_validator.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" #include "source/extensions/transport_sockets/tls/cert_validator/utility.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "source/extensions/transport_sockets/tls/utility.h" @@ -144,9 +146,21 @@ 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)); + if (matcher.typed_config().Is()) { + // Special handling for deprecated san matcher that does not check + // type. + subject_alt_name_matchers_.emplace_back(createBackwardsCompatibleSanMatcher(matcher)); + } else { + auto const factory = + Envoy::Config::Utility::getFactoryByType( + matcher.typed_config()); + if (factory != nullptr) { + subject_alt_name_matchers_.emplace_back( + factory->createSanMatcher(&matcher.typed_config())); + } + } } verify_mode = verify_mode_validation_context; } @@ -218,8 +232,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; @@ -229,8 +243,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()) { @@ -308,26 +321,28 @@ bool DefaultCertValidator::dnsNameMatch(const absl::string_view dns_name, return false; } +bool DefaultCertValidator::verifySubjectAltName( + const GENERAL_NAME* general_name, + Matchers::StringMatcherImpl const& matcher) { + // 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 + ? dnsNameMatch(matcher.matcher().exact(), absl::string_view(san)) + : matcher.match(san); +} + 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 - ? dnsNameMatch(config_san_matcher.matcher().exact(), absl::string_view(san)) - : config_san_matcher.match(san)) { - return true; - } + for (auto& config_san_matcher : subject_alt_name_matchers) { + if (config_san_matcher->match(san_names.get())) { + return true; } } return false; 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 163eb0118d42..c2aad839c511 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" @@ -53,10 +54,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 @@ -88,6 +88,10 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable& subject_alt_names); + static bool verifySubjectAltName( + const GENERAL_NAME* general_name, + Matchers::StringMatcherImpl const& matcher); + /** * Determines whether the given name matches 'pattern' which may optionally begin with a wildcard. * NOTE: public for testing @@ -103,10 +107,9 @@ 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_; @@ -116,8 +119,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_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc new file mode 100644 index 000000000000..af14e918d984 --- /dev/null +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -0,0 +1,64 @@ +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" + +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" +#include "envoy/ssl/certificate_validation_context_config.h" + +#include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +Envoy::Ssl::SanMatcherPtr DnsSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { + auto san_matcher = + dynamic_cast(config); + ASSERT(san_matcher != nullptr); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; +} + +Envoy::Ssl::SanMatcherPtr +EmailSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { + auto san_matcher = + dynamic_cast(config); + ASSERT(san_matcher != nullptr); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; +} + +Envoy::Ssl::SanMatcherPtr UriSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { + auto san_matcher = + dynamic_cast(config); + ASSERT(san_matcher != nullptr); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; +} + +Envoy::Ssl::SanMatcherPtr +IpAddSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { + auto san_matcher = + dynamic_cast(config); + ASSERT(san_matcher != nullptr); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; +} + +bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAMES* general_names) const { + for (const GENERAL_NAME* general_name : general_names) { + if (DefaultCertValidator::verifySubjectAltName(general_name, matcher_)) { + return true; + } + } + return false; +} + +Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher) { + ASSERT(matcher.typed_config().Is()); + + envoy::type::matcher::v3::StringMatcher string_matcher; + matcher.typed_config().MessageUtil::unpackTo(&string_matcher); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h new file mode 100644 index 000000000000..ca271b9fdd8c --- /dev/null +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -0,0 +1,92 @@ +#pragma once + +#include + +#include "envoy/ssl/certificate_validation_context_config.h" +#include "envoy/type/matcher/v3/string.pb.h" + +#include "source/common/common/matchers.h" +#include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" +#include "source/extensions/transport_sockets/tls/utility.h" + +#include "openssl/x509v3.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +template class DefaultSanMatcher : public Envoy::Ssl::SanMatcher { +public: + bool match(const GENERAL_NAMES* general_names) const override { + for (const GENERAL_NAME* general_name : general_names) { + if (general_name->type == SanTypeId && + DefaultCertValidator::verifySubjectAltName(general_name, matcher_)) { + return true; + } + } + return false; + } + + ~DefaultSanMatcher() override = default; + + DefaultSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) : matcher_(matcher) {} + +private: + Matchers::StringMatcherImpl matcher_; +}; + +using DnsSanMatcher = DefaultSanMatcher; +using EmailSanMatcher = DefaultSanMatcher; +using UriSanMatcher = DefaultSanMatcher; +using IpAddSanMatcher = DefaultSanMatcher; + +class DnsSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { +public: + ~DnsSanMatcherFactory() override = default; + Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; + std::string name() const override { return "envoy.san_matchers.dns_san_matcher"; } +}; + +class EmailSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { +public: + ~EmailSanMatcherFactory() override = default; + Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; + std::string name() const override { return "envoy.san_matchers.email_san_matcher"; } +}; + +class UriSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { +public: + ~UriSanMatcherFactory() override = default; + Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; + std::string name() const override { return "envoy.san_matchers.uri_san_matcher"; } +}; + +class IpAddSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { +public: + ~IpAddSanMatcherFactory() override = default; + Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; + std::string name() const override { return "envoy.san_matchers.ip_add_san_matcher"; } +}; + +class BackwardsCompatibleSanMatcher : public Envoy::Ssl::SanMatcher { + +public: + bool match(const GENERAL_NAMES* general_names) const override; + + ~BackwardsCompatibleSanMatcher() override = default; + + BackwardsCompatibleSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) + : matcher_(matcher) {} + +private: + Matchers::StringMatcherImpl matcher_; +}; + +Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher); + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index 9ef2e3fe5470..4e27141773ee 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 304488c49a66..c6b075fe03ea 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..6c5cdd09c90c 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -697,8 +697,11 @@ 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()); + envoy::type::matcher::v3::StringMatcher string_matcher; + cvc_config.subjectAltNameMatchers()[0].typed_config().UnpackTo(&string_matcher); + EXPECT_EQ("first san", string_matcher.exact()); + cvc_config.subjectAltNameMatchers()[1].typed_config().UnpackTo(&string_matcher); + EXPECT_EQ("second san", string_matcher.exact()); // 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/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc index 68925f05930d..a15c41ba08c6 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_config.h" #include "test/extensions/transport_sockets/tls/ssl_test_utility.h" #include "test/test_common/environment.h" @@ -43,21 +44,48 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSMatched) { "{{ 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)); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } +TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSTypeMatched) { + 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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); + subject_alt_name_matchers.clear(); + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + +TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameWildcardDNSTypeMatched) { + 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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -68,9 +96,22 @@ 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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + +TEST(DefaultCertValidatorTest, TestMultiLevelTypeMatch) { + // san_multiple_dns_cert matches *.example.com + 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("foo.api.example.com"); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -96,12 +137,27 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURIMatched) { "{{ 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)); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } +TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURITypeMatched) { + 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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); + subject_alt_name_matchers.clear(); + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + TEST(DefaultCertValidatorTest, TestVerifySubjectAltNameNotMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); @@ -115,9 +171,26 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotMatched) { "{{ 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)); + std::vector subject_alt_name_matchers; + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); +} + +TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotTypeMatched) { + 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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + subject_alt_name_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -134,17 +207,49 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithSANMatcher) { "{{ 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)); + std::vector san_matchers; + san_matchers.push_back( + Envoy::Ssl::SanMatcherPtr{std::make_unique(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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + // Verify the certificate with incorrect SAN exact matcher. + EXPECT_EQ(default_validator->verifyCertificate(cert.get(), /*verify_san_list=*/{}, + invalid_san_matchers), + Envoy::Ssl::ClientValidationStatus::Failed); + EXPECT_EQ(stats.fail_verify_san_.value(), 1); +} + +TEST(DefaultCertValidatorTest, TestCertificateVerificationWithTypedSANMatcher) { + Stats::TestUtil::TestStore test_store; + SslStats stats = generateSslStats(test_store); + // Create the default validator object. + auto default_validator = + std::make_unique( + /*CertificateValidationContextConfig=*/nullptr, stats, + Event::GlobalTimeSystem().timeSystem()); + + 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(Envoy::Ssl::SanMatcherPtr{std::make_unique(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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); // Verify the certificate with incorrect SAN exact matcher. EXPECT_EQ(default_validator->verifyCertificate(cert.get(), /*verify_san_list=*/{}, invalid_san_matchers), 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 b958f1727207..cad834c9f259 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_; } @@ -77,7 +78,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/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 43ad275fce27..e36b06fbd99d 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -156,8 +156,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)); From a554236ca84b9e266d96a86c0dbb31181b04960a Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 14 Oct 2021 17:18:43 +0000 Subject: [PATCH 02/32] Fix build and presubmit issues Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 7 ++++--- source/common/ssl/BUILD | 1 + .../tls/cert_validator/san_matcher_config.cc | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 55782e10fcd9..1ef2ab2589dd 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"; @@ -257,7 +258,7 @@ message SubjectAltNameMatcher { google.protobuf.Any typed_config = 1; } -// [#next-free-field: 14] +// [#next-free-field: 15] message CertificateValidationContext { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.auth.CertificateValidationContext"; @@ -414,8 +415,8 @@ message CertificateValidationContext { // This field is deprecated in favor of ref:`match_subject_alt_names_with_type // ` - repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 [deprecated = true]; - + 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/source/common/ssl/BUILD b/source/common/ssl/BUILD index 714c426b930f..adce51a71c82 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/extensions//transport_sockets/tls/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/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index af14e918d984..a902d17246ba 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -54,7 +54,7 @@ Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( ASSERT(matcher.typed_config().Is()); envoy::type::matcher::v3::StringMatcher string_matcher; - matcher.typed_config().MessageUtil::unpackTo(&string_matcher); + MessageUtil::unpackTo(matcher.typed_config(), string_matcher); return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; } From 3d6272e96c3db0467da96f54ba93bdec50d0dbf4 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 14 Oct 2021 17:54:50 +0000 Subject: [PATCH 03/32] fix build Signed-off-by: Pradeep Rao --- source/common/ssl/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD index adce51a71c82..714c426b930f 100644 --- a/source/common/ssl/BUILD +++ b/source/common/ssl/BUILD @@ -32,7 +32,6 @@ envoy_cc_library( "//envoy/ssl:certificate_validation_context_config_interface", "//source/common/common:empty_string", "//source/common/config:datasource_lib", - "@envoy_api//envoy/extensions//transport_sockets/tls/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", ], From f3c87a610ea568e48b93e1e4626b9e2ecf06b539 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 14 Oct 2021 19:02:19 +0000 Subject: [PATCH 04/32] fix whitespace Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 1ef2ab2589dd..0ba7548d2aab 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -415,7 +415,7 @@ message CertificateValidationContext { // This field is deprecated in favor of ref:`match_subject_alt_names_with_type // ` - repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 + 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; From ff9cd5feb576a8cf6eb61c31411787224d73f3a9 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Sun, 17 Oct 2021 23:51:18 +0000 Subject: [PATCH 05/32] Added StringSanMatcher message, and the ability to provide either StringSanMatchers or TypedExtensionConfigs for san matchers. Made test regexes stricter. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 47 +++++++------- .../certificate_validation_context_config.h | 4 +- ...tificate_validation_context_config_impl.cc | 13 +++- .../tls/cert_validator/default_validator.cc | 10 ++- .../tls/cert_validator/san_matcher_config.cc | 65 +++++++++---------- .../tls/cert_validator/san_matcher_config.h | 61 +++++++---------- .../cert_validator/default_validator_test.cc | 16 ++--- .../tls/cert_validator/factory_test.cc | 18 +++++ 8 files changed, 122 insertions(+), 112 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 0ba7548d2aab..66f75d26ad1b 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -254,8 +254,23 @@ message CertificateProviderPluginInstance { string certificate_name = 2; } +// String matcher for subject alternative names, to match both type and value of the SAN. +message StringSanMatcher { + enum SanType { + EMAIL_ID = 0; + DNS_ID = 1; + URI_ID = 2; + IP_ADD = 3; + } + SanType san_type = 1 [(validate.rules).enum = {in: [0, 3]}]; + type.matcher.v3.StringMatcher matcher = 2; +} + message SubjectAltNameMatcher { - google.protobuf.Any typed_config = 1; + oneof matcher { + StringSanMatcher string_matcher = 1; + config.core.v3.TypedExtensionConfig typed_config = 2; + } } // [#next-free-field: 15] @@ -393,6 +408,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 `. @@ -402,9 +419,10 @@ message CertificateValidationContext { // .. code-block:: yaml // // match_subject_alt_names_with_type: - // typed_config: - // "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.common.DnsSanMatcher - // exact: "api.example.com" + // string_matcher: + // san_type: DNS_ID + // matcher: + // exact: "api.example.com" // // .. attention:: // @@ -417,6 +435,7 @@ message CertificateValidationContext { // ` 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; @@ -445,23 +464,3 @@ message CertificateValidationContext { // [#extension-category: envoy.tls.cert_validator] config.core.v3.TypedExtensionConfig custom_validator_config = 12; } - -// Matcher for DNS type subject alternative name. -message DnsSanMatcher { - type.matcher.v3.StringMatcher matcher = 1; -} - -// Matcher for email type subject alternative name. -message EmailSanMatcher { - type.matcher.v3.StringMatcher matcher = 1; -} - -// Matcher for URI type subject alternative name. -message UriSanMatcher { - type.matcher.v3.StringMatcher matcher = 1; -} - -// Matcher for IP address type subject alternative name. -message IpAddSanMatcher { - type.matcher.v3.StringMatcher matcher = 1; -} diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 00b8f6e7ee46..c451512ab1ab 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -9,6 +9,7 @@ #include "envoy/config/typed_config.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" +#include "envoy/config/core/v3/extension.pb.h" #include "envoy/type/matcher/v3/string.pb.h" #include "absl/types/optional.h" @@ -32,7 +33,8 @@ class SanMatcherFactory : public Config::TypedFactory { public: ~SanMatcherFactory() override = default; - virtual SanMatcherPtr createSanMatcher(const Protobuf::Message* config) PURE; + virtual SanMatcherPtr + createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig* config) PURE; std::string category() const override { return "envoy.san_matchers"; } }; diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index d8aa261f7c97..7f6553cf5ba7 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -1,6 +1,7 @@ #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/common.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" @@ -64,10 +65,18 @@ CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( std::vector subject_alt_name_matchers(config.match_subject_alt_names_with_type().begin(), config.match_subject_alt_names_with_type().end()); + // Handle deprecated string type san matchers without san type specified, by + // creating backwards compatible san matcher configs. for (auto& matcher : config.match_subject_alt_names()) { subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_allocated_typed_config(new ProtobufWkt::Any()); - subject_alt_name_matchers.back().mutable_typed_config()->PackFrom(matcher); + subject_alt_name_matchers.back().set_allocated_typed_config( + new ::envoy::config::core::v3::TypedExtensionConfig()); + subject_alt_name_matchers.back().mutable_typed_config()->set_allocated_typed_config( + new ProtobufWkt::Any()); + subject_alt_name_matchers.back().mutable_typed_config()->mutable_typed_config()->PackFrom( + matcher); + subject_alt_name_matchers.back().mutable_typed_config()->mutable_name()->assign( + "envoy.san_matchers.backward_compatible_san_matcher"); } return subject_alt_name_matchers; } 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 542ae389e238..e2d9bd6e27a1 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -148,14 +148,12 @@ int DefaultCertValidator::initializeSslContexts(std::vector contexts, if (!cert_validation_config->subjectAltNameMatchers().empty()) { for (const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher : cert_validation_config->subjectAltNameMatchers()) { - if (matcher.typed_config().Is()) { - // Special handling for deprecated san matcher that does not check - // type. - subject_alt_name_matchers_.emplace_back(createBackwardsCompatibleSanMatcher(matcher)); + if (matcher.has_string_matcher()) { + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher.string_matcher())); } else { auto const factory = - Envoy::Config::Utility::getFactoryByType( - matcher.typed_config()); + Envoy::Config::Utility::getAndCheckFactory( + matcher.typed_config(), true); if (factory != nullptr) { subject_alt_name_matchers_.emplace_back( factory->createSanMatcher(&matcher.typed_config())); diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index a902d17246ba..48112f879bb7 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -1,43 +1,24 @@ #include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#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/registry/registry.h" #include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" +#include namespace Envoy { namespace Extensions { namespace TransportSockets { namespace Tls { -Envoy::Ssl::SanMatcherPtr DnsSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { - auto san_matcher = - dynamic_cast(config); - ASSERT(san_matcher != nullptr); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; -} - -Envoy::Ssl::SanMatcherPtr -EmailSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { - auto san_matcher = - dynamic_cast(config); - ASSERT(san_matcher != nullptr); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; -} - -Envoy::Ssl::SanMatcherPtr UriSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { - auto san_matcher = - dynamic_cast(config); - ASSERT(san_matcher != nullptr); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; -} - -Envoy::Ssl::SanMatcherPtr -IpAddSanMatcherFactory::createSanMatcher(const Protobuf::Message* config) { - auto san_matcher = - dynamic_cast(config); - ASSERT(san_matcher != nullptr); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(san_matcher->matcher())}; +Envoy::Ssl::SanMatcherPtr BackwardsCompatibleSanMatcherFactory::createSanMatcher( + const envoy::config::core::v3::TypedExtensionConfig* config) { + ASSERT(config->typed_config().Is()); + envoy::type::matcher::v3::StringMatcher string_matcher; + MessageUtil::unpackTo(config->typed_config(), string_matcher); + return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; } bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAMES* general_names) const { @@ -49,15 +30,29 @@ bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAMES* general_names) co return false; } -Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher) { - ASSERT(matcher.typed_config().Is()); - - envoy::type::matcher::v3::StringMatcher string_matcher; - MessageUtil::unpackTo(matcher.typed_config(), string_matcher); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; +Envoy::Ssl::SanMatcherPtr createStringSanMatcher( + envoy::extensions::transport_sockets::tls::v3::StringSanMatcher const& matcher) { + switch (matcher.san_type()) { + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::DNS_ID: { + return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + } + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::EMAIL_ID: { + return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + } + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI_ID: { + return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + } + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::IP_ADD: { + return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + } + default: + ASSERT(true, + "Invalid san type for envoy::extensions::transport_sockets::tls::v3::StringSanMatcher"); + return Envoy::Ssl::SanMatcherPtr(); + } } +REGISTER_FACTORY(BackwardsCompatibleSanMatcherFactory, Envoy::Ssl::SanMatcherFactory); } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index ca271b9fdd8c..41984197f3c1 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -2,10 +2,14 @@ #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/cert_validator/default_validator.h" #include "source/extensions/transport_sockets/tls/utility.h" @@ -16,11 +20,11 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -template class DefaultSanMatcher : public Envoy::Ssl::SanMatcher { +template class StringSanMatcher : public Envoy::Ssl::SanMatcher { public: bool match(const GENERAL_NAMES* general_names) const override { for (const GENERAL_NAME* general_name : general_names) { - if (general_name->type == SanTypeId && + if (general_name->type == general_name_type && DefaultCertValidator::verifySubjectAltName(general_name, matcher_)) { return true; } @@ -28,46 +32,18 @@ template class DefaultSanMatcher : public Envoy::Ssl::SanMatcher return false; } - ~DefaultSanMatcher() override = default; + ~StringSanMatcher() override = default; - DefaultSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) : matcher_(matcher) {} + StringSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) : matcher_(matcher) {} private: Matchers::StringMatcherImpl matcher_; }; -using DnsSanMatcher = DefaultSanMatcher; -using EmailSanMatcher = DefaultSanMatcher; -using UriSanMatcher = DefaultSanMatcher; -using IpAddSanMatcher = DefaultSanMatcher; - -class DnsSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { -public: - ~DnsSanMatcherFactory() override = default; - Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; - std::string name() const override { return "envoy.san_matchers.dns_san_matcher"; } -}; - -class EmailSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { -public: - ~EmailSanMatcherFactory() override = default; - Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; - std::string name() const override { return "envoy.san_matchers.email_san_matcher"; } -}; - -class UriSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { -public: - ~UriSanMatcherFactory() override = default; - Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; - std::string name() const override { return "envoy.san_matchers.uri_san_matcher"; } -}; - -class IpAddSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { -public: - ~IpAddSanMatcherFactory() override = default; - Envoy::Ssl::SanMatcherPtr createSanMatcher(const Protobuf::Message* config) override; - std::string name() const override { return "envoy.san_matchers.ip_add_san_matcher"; } -}; +using DnsSanMatcher = StringSanMatcher; +using EmailSanMatcher = StringSanMatcher; +using UriSanMatcher = StringSanMatcher; +using IpAddSanMatcher = StringSanMatcher; class BackwardsCompatibleSanMatcher : public Envoy::Ssl::SanMatcher { @@ -83,9 +59,22 @@ class BackwardsCompatibleSanMatcher : public Envoy::Ssl::SanMatcher { Matchers::StringMatcherImpl matcher_; }; +class BackwardsCompatibleSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { +public: + ~BackwardsCompatibleSanMatcherFactory() override = default; + Envoy::Ssl::SanMatcherPtr + createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig* config) override; + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } + std::string name() const override { return "envoy.san_matchers.backward_compatible_san_matcher"; } +}; + Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher); +Envoy::Ssl::SanMatcherPtr createStringSanMatcher( + envoy::extensions::transport_sockets::tls::v3::StringSanMatcher const& matcher); } // namespace Tls } // namespace TransportSockets } // namespace Extensions 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 a15c41ba08c6..b3b7fd3093d6 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 @@ -43,7 +43,7 @@ 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -54,7 +54,7 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSTypeMatched) { 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -136,7 +136,7 @@ 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw(spiffe://lyft.com/[^/]*-team)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -147,7 +147,7 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURITypeMatched) { 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw(spiffe://lyft.com/[^/]*-team)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -170,7 +170,7 @@ 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example\.net)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -181,7 +181,7 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotTypeMatched) { 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example\.net)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -206,7 +206,7 @@ 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example\.com)raw")); std::vector san_matchers; san_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); @@ -238,7 +238,7 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithTypedSANMatcher) { 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")); + matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); std::vector san_matchers; san_matchers.push_back(Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); // Verify the certificate with correct SAN regex matcher. diff --git a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc index 94cf5e7063fa..a402ca8ef216 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc @@ -1,6 +1,8 @@ #include #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#include "source/common/config/utility.h" #include "test/extensions/transport_sockets/tls/cert_validator/test_common.h" @@ -22,6 +24,22 @@ TEST(FactoryTest, TestGetCertValidatorName) { EXPECT_EQ(custom_config.name(), getCertValidatorName(config.get())); } +TEST(FactoryTest, BackwardsCompatibleSanMatcherFactoryTest) { + // Create proto for BackwardsCompatibleSanMatcher. + envoy::config::core::v3::TypedExtensionConfig typed_config = {}; + typed_config.set_name("envoy.san_matchers.backward_compatible_san_matcher"); + envoy::type::matcher::v3::StringMatcher matcher; + typed_config.set_allocated_typed_config(new ProtobufWkt::Any()); + typed_config.mutable_typed_config()->PackFrom(matcher); + + // Create san matcher. + Envoy::Ssl::SanMatcherFactory* factory = + Envoy::Config::Utility::getAndCheckFactory(typed_config, true); + ASSERT_NE(factory, nullptr); + Envoy::Ssl::SanMatcherPtr san_matcher = factory->createSanMatcher(&typed_config); + EXPECT_NE(dynamic_cast(san_matcher.get()), nullptr); +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions From 3aae6db85244e797c1d493c9f2abc7a34c2ee3b2 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 18 Oct 2021 13:41:17 +0000 Subject: [PATCH 06/32] Fix formatting Signed-off-by: Pradeep Rao --- envoy/ssl/BUILD | 1 + envoy/ssl/certificate_validation_context_config.h | 2 +- source/common/ssl/BUILD | 1 + source/extensions/transport_sockets/tls/cert_validator/BUILD | 1 + .../tls/cert_validator/san_matcher_config.cc | 5 +++-- 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/envoy/ssl/BUILD b/envoy/ssl/BUILD index 3862d837cf75..5132e0067c14 100644 --- a/envoy/ssl/BUILD +++ b/envoy/ssl/BUILD @@ -60,6 +60,7 @@ envoy_cc_library( external_deps = ["abseil_optional"], deps = [ "//source/common/common:matchers_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/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index c451512ab1ab..12bc4223ee3e 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -6,10 +6,10 @@ #include "envoy/api/api.h" #include "envoy/common/pure.h" +#include "envoy/config/core/v3/extension.pb.h" #include "envoy/config/typed_config.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" -#include "envoy/config/core/v3/extension.pb.h" #include "envoy/type/matcher/v3/string.pb.h" #include "absl/types/optional.h" 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/extensions/transport_sockets/tls/cert_validator/BUILD b/source/extensions/transport_sockets/tls/cert_validator/BUILD index e1096a03cd7e..a8bea3139578 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/source/extensions/transport_sockets/tls/cert_validator/BUILD @@ -42,6 +42,7 @@ envoy_cc_library( "//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/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index 48112f879bb7..ca7476b4f520 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -1,12 +1,13 @@ #include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#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/registry/registry.h" +#include "envoy/ssl/certificate_validation_context_config.h" #include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" -#include namespace Envoy { namespace Extensions { From ab5cd115da60f93f628cfb9cecba7aad2953c344 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 18 Oct 2021 14:33:53 +0000 Subject: [PATCH 07/32] Fix formatting. Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 66f75d26ad1b..6dca56ca6986 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -262,13 +262,16 @@ message StringSanMatcher { URI_ID = 2; IP_ADD = 3; } - SanType san_type = 1 [(validate.rules).enum = {in: [0, 3]}]; + + SanType san_type = 1 [(validate.rules).enum.defined_only = true]; + type.matcher.v3.StringMatcher matcher = 2; } message SubjectAltNameMatcher { oneof matcher { StringSanMatcher string_matcher = 1; + config.core.v3.TypedExtensionConfig typed_config = 2; } } From 135391efac04a37c80fbca841d31712b2528d052 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 18 Oct 2021 15:42:52 +0000 Subject: [PATCH 08/32] Fix formatting Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 2 +- .../transport_sockets/tls/cert_validator/factory_test.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 6dca56ca6986..660c23f9078b 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -263,7 +263,7 @@ message StringSanMatcher { IP_ADD = 3; } - SanType san_type = 1 [(validate.rules).enum.defined_only = true]; + SanType san_type = 1 [(validate.rules).enum = {defined_only: true}]; type.matcher.v3.StringMatcher matcher = 2; } diff --git a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc index a402ca8ef216..f5162a369107 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc @@ -1,8 +1,8 @@ #include +#include "source/common/config/utility.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" #include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" -#include "source/common/config/utility.h" #include "test/extensions/transport_sockets/tls/cert_validator/test_common.h" From 2a65d300db504f32fe21c8fc1a21f25e28f78a68 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 18 Oct 2021 20:49:04 +0000 Subject: [PATCH 09/32] Fix include Signed-off-by: Pradeep Rao --- source/common/ssl/certificate_validation_context_config_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 7f6553cf5ba7..b75bb1df2845 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -2,7 +2,7 @@ #include "envoy/common/exception.h" #include "envoy/config/core/v3/extension.pb.h" -#include "envoy/extensions//transport_sockets/tls/v3/common.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" #include "source/common/common/empty_string.h" From e02838805268452c7d42917484448f2d5a0f1f75 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 19 Oct 2021 19:50:01 +0000 Subject: [PATCH 10/32] Fix merge issue Signed-off-by: Pradeep Rao --- .../tls/cert_validator/default_validator.cc | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) 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 c02458802483..42d37e6d2822 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -298,27 +298,6 @@ bool DefaultCertValidator::verifySubjectAltName(X509* cert, return false; } -bool DefaultCertValidator::dnsNameMatch(const absl::string_view dns_name, - const absl::string_view pattern) { - const std::string lower_case_dns_name = absl::AsciiStrToLower(dns_name); - const std::string lower_case_pattern = absl::AsciiStrToLower(pattern); - if (lower_case_dns_name == lower_case_pattern) { - return true; - } - - size_t pattern_len = lower_case_pattern.length(); - if (pattern_len > 1 && lower_case_pattern[0] == '*' && lower_case_pattern[1] == '.') { - if (lower_case_dns_name.length() > pattern_len - 1) { - const size_t off = lower_case_dns_name.length() - pattern_len + 1; - return lower_case_dns_name.substr(0, off).find('.') == std::string::npos && - lower_case_dns_name.substr(off, pattern_len - 1) == - lower_case_pattern.substr(1, pattern_len - 1); - } - } - - return false; -} - bool DefaultCertValidator::verifySubjectAltName( const GENERAL_NAME* general_name, Matchers::StringMatcherImpl const& matcher) { @@ -327,7 +306,7 @@ bool DefaultCertValidator::verifySubjectAltName( return general_name->type == GEN_DNS && matcher.matcher().match_pattern_case() == envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kExact - ? dnsNameMatch(matcher.matcher().exact(), absl::string_view(san)) + ? Utility::dnsNameMatch(matcher.matcher().exact(), absl::string_view(san)) : matcher.match(san); } From cebac65227973100d6c1d945d827bf08a75369a1 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 19 Oct 2021 20:34:57 +0000 Subject: [PATCH 11/32] Fix formatting. Signed-off-by: Pradeep Rao --- source/common/ssl/certificate_validation_context_config_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index b75bb1df2845..2f499b40277d 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -2,8 +2,8 @@ #include "envoy/common/exception.h" #include "envoy/config/core/v3/extension.pb.h" -#include "envoy/extensions/transport_sockets/tls/v3/common.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" From e8226dd571650c8b1699aedf6e5411955fb9a53d Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 21 Oct 2021 01:11:24 +0000 Subject: [PATCH 12/32] Fix spiffe validator. Signed-off-by: Pradeep Rao --- .../certificate_validation_context_config.h | 2 +- .../tls/cert_validator/default_validator.cc | 6 ++++-- .../tls/cert_validator/san_matcher_config.cc | 9 ++------ .../tls/cert_validator/san_matcher_config.h | 13 ++++-------- .../cert_validator/spiffe/spiffe_validator.cc | 21 ++++++++++++++++--- .../cert_validator/spiffe/spiffe_validator.h | 3 +-- .../spiffe/spiffe_validator_test.cc | 17 ++++++++++++--- 7 files changed, 44 insertions(+), 27 deletions(-) diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 12bc4223ee3e..50f2fa5b7c98 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -23,7 +23,7 @@ namespace Ssl { */ class SanMatcher { public: - virtual bool match(GENERAL_NAMES const*) const PURE; + virtual bool match(GENERAL_NAME const*) const PURE; virtual ~SanMatcher() = default; }; 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 42d37e6d2822..c19d85c9d732 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -318,8 +318,10 @@ bool DefaultCertValidator::matchSubjectAltName( return false; } for (auto& config_san_matcher : subject_alt_name_matchers) { - if (config_san_matcher->match(san_names.get())) { - return true; + for (const GENERAL_NAME* general_name : san_names.get()) { + if (config_san_matcher->match(general_name)) { + return true; + } } } return false; diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index ca7476b4f520..1f332d3dcd11 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -22,13 +22,8 @@ Envoy::Ssl::SanMatcherPtr BackwardsCompatibleSanMatcherFactory::createSanMatcher return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; } -bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAMES* general_names) const { - for (const GENERAL_NAME* general_name : general_names) { - if (DefaultCertValidator::verifySubjectAltName(general_name, matcher_)) { - return true; - } - } - return false; +bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAME* general_name) const { + return DefaultCertValidator::verifySubjectAltName(general_name, matcher_); } Envoy::Ssl::SanMatcherPtr createStringSanMatcher( diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index 41984197f3c1..8bbab9b0d79a 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -22,14 +22,9 @@ namespace Tls { template class StringSanMatcher : public Envoy::Ssl::SanMatcher { public: - bool match(const GENERAL_NAMES* general_names) const override { - for (const GENERAL_NAME* general_name : general_names) { - if (general_name->type == general_name_type && - DefaultCertValidator::verifySubjectAltName(general_name, matcher_)) { - return true; - } - } - return false; + bool match(const GENERAL_NAME* general_name) const override { + return general_name->type == general_name_type && + DefaultCertValidator::verifySubjectAltName(general_name, matcher_); } ~StringSanMatcher() override = default; @@ -48,7 +43,7 @@ using IpAddSanMatcher = StringSanMatcher; class BackwardsCompatibleSanMatcher : public Envoy::Ssl::SanMatcher { public: - bool match(const GENERAL_NAMES* general_names) const override; + bool match(const GENERAL_NAME* general_name) const override; ~BackwardsCompatibleSanMatcher() override = default; 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..df1db96f53b5 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" @@ -11,6 +12,7 @@ #include "source/common/protobuf/message_validator_impl.h" #include "source/common/stats/symbol_table_impl.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" #include "source/extensions/transport_sockets/tls/cert_validator/utility.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "source/extensions/transport_sockets/tls/utility.h" @@ -37,7 +39,21 @@ 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.has_string_matcher() && + matcher.string_matcher().san_type() == + envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI_ID) { + // 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 + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher.string_matcher())); + } else { + auto const factory = + Envoy::Config::Utility::getAndCheckFactory( + matcher.typed_config(), true); + if (factory != nullptr) { + subject_alt_name_matchers_.emplace_back( + factory->createSanMatcher(&matcher.typed_config())); + } + } } } @@ -228,9 +244,8 @@ bool SPIFFEValidator::matchSubjectAltName(X509& leaf_cert) { // 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)) { + 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..dd519f04efeb 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 @@ -67,8 +67,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/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 cf77bdf3737a..70d1a26f0514 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,23 @@ 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_allocated_typed_config( + new ::envoy::config::core::v3::TypedExtensionConfig()); + san_matchers_.back().mutable_typed_config()->set_allocated_typed_config( + new ProtobufWkt::Any()); + san_matchers_.back().mutable_typed_config()->mutable_typed_config()->PackFrom(matcher); + san_matchers_.back().mutable_typed_config()->mutable_name()->assign( + "envoy.san_matchers.backward_compatible_san_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 +203,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 From af0c18b8fc44823b2e25212ecf77d109ee26533a Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 21 Oct 2021 14:47:02 +0000 Subject: [PATCH 13/32] Address review comments. Signed-off-by: Pradeep Rao --- .../extensions/transport_sockets/tls/v3/common.proto | 6 +++--- envoy/ssl/certificate_validation_context_config.h | 2 +- .../certificate_validation_context_config_impl.cc | 4 ---- .../tls/cert_validator/default_validator.cc | 2 +- .../tls/cert_validator/san_matcher_config.cc | 12 ++++++------ .../tls/cert_validator/san_matcher_config.h | 2 +- .../tls/cert_validator/spiffe/spiffe_validator.cc | 6 ++++-- test/common/secret/sds_api_test.cc | 4 ++-- .../tls/cert_validator/factory_test.cc | 2 +- .../cert_validator/spiffe/spiffe_validator_test.cc | 4 ---- 10 files changed, 19 insertions(+), 25 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 660c23f9078b..fee21d008488 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -257,9 +257,9 @@ message CertificateProviderPluginInstance { // String matcher for subject alternative names, to match both type and value of the SAN. message StringSanMatcher { enum SanType { - EMAIL_ID = 0; - DNS_ID = 1; - URI_ID = 2; + EMAIL = 0; + DNS = 1; + URI = 2; IP_ADD = 3; } diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 50f2fa5b7c98..22ac62c46401 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -34,7 +34,7 @@ class SanMatcherFactory : public Config::TypedFactory { ~SanMatcherFactory() override = default; virtual SanMatcherPtr - createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig* config) PURE; + createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig& config) PURE; std::string category() const override { return "envoy.san_matchers"; } }; diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 2f499b40277d..6ccebb1cf583 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -69,10 +69,6 @@ CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( // creating backwards compatible san matcher configs. for (auto& matcher : config.match_subject_alt_names()) { subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_allocated_typed_config( - new ::envoy::config::core::v3::TypedExtensionConfig()); - subject_alt_name_matchers.back().mutable_typed_config()->set_allocated_typed_config( - new ProtobufWkt::Any()); subject_alt_name_matchers.back().mutable_typed_config()->mutable_typed_config()->PackFrom( matcher); subject_alt_name_matchers.back().mutable_typed_config()->mutable_name()->assign( 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 c19d85c9d732..17d0ba856d48 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -156,7 +156,7 @@ int DefaultCertValidator::initializeSslContexts(std::vector contexts, matcher.typed_config(), true); if (factory != nullptr) { subject_alt_name_matchers_.emplace_back( - factory->createSanMatcher(&matcher.typed_config())); + factory->createSanMatcher(matcher.typed_config())); } } } diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index 1f332d3dcd11..ee4547d2b13b 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -15,10 +15,10 @@ namespace TransportSockets { namespace Tls { Envoy::Ssl::SanMatcherPtr BackwardsCompatibleSanMatcherFactory::createSanMatcher( - const envoy::config::core::v3::TypedExtensionConfig* config) { - ASSERT(config->typed_config().Is()); + const envoy::config::core::v3::TypedExtensionConfig& config) { + ASSERT(config.typed_config().Is()); envoy::type::matcher::v3::StringMatcher string_matcher; - MessageUtil::unpackTo(config->typed_config(), string_matcher); + MessageUtil::unpackTo(config.typed_config(), string_matcher); return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; } @@ -29,13 +29,13 @@ bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAME* general_name) cons Envoy::Ssl::SanMatcherPtr createStringSanMatcher( envoy::extensions::transport_sockets::tls::v3::StringSanMatcher const& matcher) { switch (matcher.san_type()) { - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::DNS_ID: { + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::DNS: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::EMAIL_ID: { + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::EMAIL: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI_ID: { + case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::IP_ADD: { diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index 8bbab9b0d79a..ee82d1ac0b97 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -58,7 +58,7 @@ class BackwardsCompatibleSanMatcherFactory : public Envoy::Ssl::SanMatcherFactor public: ~BackwardsCompatibleSanMatcherFactory() override = default; Envoy::Ssl::SanMatcherPtr - createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig* config) override; + createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig& config) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } 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 df1db96f53b5..cd957c1d9815 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 @@ -41,7 +41,7 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC for (const auto& matcher : config->subjectAltNameMatchers()) { if (matcher.has_string_matcher() && matcher.string_matcher().san_type() == - envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI_ID) { + envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::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 subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher.string_matcher())); @@ -51,7 +51,7 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC matcher.typed_config(), true); if (factory != nullptr) { subject_alt_name_matchers_.emplace_back( - factory->createSanMatcher(&matcher.typed_config())); + factory->createSanMatcher(matcher.typed_config())); } } } @@ -243,6 +243,8 @@ bool SPIFFEValidator::matchSubjectAltName(X509& leaf_cert) { // 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()) { + // This check can be removed when match_subject_alt_names is removed from + // envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext. if (general_name->type == GEN_URI) { for (const auto& config_san_matcher : subject_alt_name_matchers_) { if (config_san_matcher->match(general_name)) { diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 6c5cdd09c90c..2713d7bdf803 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -698,9 +698,9 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { // Verify that repeated fields are concatenated. EXPECT_EQ(2, cvc_config.subjectAltNameMatchers().size()); envoy::type::matcher::v3::StringMatcher string_matcher; - cvc_config.subjectAltNameMatchers()[0].typed_config().UnpackTo(&string_matcher); + cvc_config.subjectAltNameMatchers()[0].typed_config().typed_config().UnpackTo(&string_matcher); EXPECT_EQ("first san", string_matcher.exact()); - cvc_config.subjectAltNameMatchers()[1].typed_config().UnpackTo(&string_matcher); + cvc_config.subjectAltNameMatchers()[1].typed_config().typed_config().UnpackTo(&string_matcher); EXPECT_EQ("second san", string_matcher.exact()); // Verify that if dynamic CertificateValidationContext does not set certificate hash list, the new // secret contains hash list from default CertificateValidationContext. diff --git a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc index f5162a369107..d8df3ae681ba 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc @@ -36,7 +36,7 @@ TEST(FactoryTest, BackwardsCompatibleSanMatcherFactoryTest) { Envoy::Ssl::SanMatcherFactory* factory = Envoy::Config::Utility::getAndCheckFactory(typed_config, true); ASSERT_NE(factory, nullptr); - Envoy::Ssl::SanMatcherPtr san_matcher = factory->createSanMatcher(&typed_config); + Envoy::Ssl::SanMatcherPtr san_matcher = factory->createSanMatcher(typed_config); EXPECT_NE(dynamic_cast(san_matcher.get()), nullptr); } 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 70d1a26f0514..40241d7153d1 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 @@ -67,10 +67,6 @@ class TestSPIFFEValidator : public testing::Test { san_matchers_.clear(); for (auto& matcher : san_matchers) { san_matchers_.emplace_back(); - san_matchers_.back().set_allocated_typed_config( - new ::envoy::config::core::v3::TypedExtensionConfig()); - san_matchers_.back().mutable_typed_config()->set_allocated_typed_config( - new ProtobufWkt::Any()); san_matchers_.back().mutable_typed_config()->mutable_typed_config()->PackFrom(matcher); san_matchers_.back().mutable_typed_config()->mutable_name()->assign( "envoy.san_matchers.backward_compatible_san_matcher"); From 67d05d19066d974f972201606301fa0eeff4ae31 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 27 Oct 2021 11:07:10 +0000 Subject: [PATCH 14/32] Remove TypedExtensionConfig for san matcher. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 31 +++---- .../tls/v3/tls_spiffe_validator_config.proto | 3 +- configs/envoy_double_proxy.template.yaml | 12 ++- .../envoy_service_to_service.template.yaml | 12 ++- .../arch_overview/security/_include/ssl.yaml | 6 +- .../root/intro/arch_overview/security/ssl.rst | 2 +- .../_include/envoy-demo-tls-client-auth.yaml | 6 +- .../_include/envoy-demo-tls-validation.yaml | 6 +- docs/root/start/quick-start/securing.rst | 12 +-- docs/root/version_history/current.rst | 5 + examples/double-proxy/envoy-backend.yaml | 6 +- examples/double-proxy/envoy-frontend.yaml | 6 +- ...tificate_validation_context_config_impl.cc | 32 +++++-- .../tls/cert_validator/default_validator.cc | 12 +-- .../tls/cert_validator/san_matcher_config.cc | 27 ++---- .../tls/cert_validator/san_matcher_config.h | 31 +------ .../cert_validator/spiffe/spiffe_validator.cc | 27 ++---- test/common/secret/sds_api_test.cc | 31 +++++-- .../cert_validator/default_validator_test.cc | 91 +------------------ .../tls/cert_validator/factory_test.cc | 18 ---- .../spiffe/spiffe_validator_test.cc | 21 ++++- 21 files changed, 145 insertions(+), 252 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index fee21d008488..461e9e9148d1 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -255,12 +255,12 @@ message CertificateProviderPluginInstance { } // String matcher for subject alternative names, to match both type and value of the SAN. -message StringSanMatcher { +message SubjectAltNameMatcher { enum SanType { EMAIL = 0; DNS = 1; URI = 2; - IP_ADD = 3; + IP_ADDRESS = 3; } SanType san_type = 1 [(validate.rules).enum = {defined_only: true}]; @@ -268,14 +268,6 @@ message StringSanMatcher { type.matcher.v3.StringMatcher matcher = 2; } -message SubjectAltNameMatcher { - oneof matcher { - StringSanMatcher string_matcher = 1; - - config.core.v3.TypedExtensionConfig typed_config = 2; - } -} - // [#next-free-field: 15] message CertificateValidationContext { option (udpa.annotations.versioning).previous_message_type = @@ -306,8 +298,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 @@ -421,21 +413,20 @@ message CertificateValidationContext { // // .. code-block:: yaml // - // match_subject_alt_names_with_type: - // string_matcher: - // san_type: DNS_ID - // matcher: - // 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 SubjectAltNameMatcher match_subject_alt_names_with_type = 14; + repeated SubjectAltNameMatcher match_typed_subject_alt_names = 14; - // This field is deprecated in favor of ref:`match_subject_alt_names_with_type - // ` + // 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"]; 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..813b63ea7f06 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,8 @@ 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..17ed6342d760 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: URI + 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..85e3f8c7e4fd 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-42 :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-42 :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: 47 + :lines: 47-71 :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 c13492aaca47..d82f9c8f72e2 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -56,6 +56,7 @@ New Features * thrift_proxy: add upstream metrics to show decoding errors and whether exception is from local or remote, e.g. ``cluster.cluster_name.thrift.upstream_resp_exception_remote``. * 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 subset lb when using request or route metadata. +* tls: added support for :ref:`generic string matcher ` for subject alternative names. * udp: add support for multiple listener filters. * upstream: added the ability to :ref:`configure max connection duration ` for upstream clusters. * vcl_socket_interface: added VCL socket interface extension for fd.io VPP integration to :ref:`contrib images `. This can be enabled via :ref:`VCL ` configuration. @@ -66,3 +67,7 @@ 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 `. +* The `match_subject_alt_names` field in :ref:`Certificate Validation Context + ` + has been deprecated in favor of the :ref:`match_typed_subject_alt_names + ` field. 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/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 6ccebb1cf583..dedb41900b03 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -57,22 +57,36 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( std::vector CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config) { - if (!config.match_subject_alt_names_with_type().empty() && + if (!config.match_typed_subject_alt_names().empty() && !config.match_subject_alt_names().empty()) { - throw EnvoyException("SAN-based verification using both match_subject_alt_names_with_type and " + 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_subject_alt_names_with_type().begin(), - config.match_subject_alt_names_with_type().end()); + 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 backwards compatible san matcher configs. + // creating a matcher for each supported type. for (auto& matcher : config.match_subject_alt_names()) { subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().mutable_typed_config()->mutable_typed_config()->PackFrom( - matcher); - subject_alt_name_matchers.back().mutable_typed_config()->mutable_name()->assign( - "envoy.san_matchers.backward_compatible_san_matcher"); + subject_alt_name_matchers.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + *subject_alt_name_matchers.back().mutable_matcher() = matcher; + + subject_alt_name_matchers.emplace_back(); + subject_alt_name_matchers.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI); + *subject_alt_name_matchers.back().mutable_matcher() = matcher; + + subject_alt_name_matchers.emplace_back(); + subject_alt_name_matchers.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL); + *subject_alt_name_matchers.back().mutable_matcher() = matcher; + + subject_alt_name_matchers.emplace_back(); + subject_alt_name_matchers.back().set_san_type( + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS); + *subject_alt_name_matchers.back().mutable_matcher() = matcher; } return subject_alt_name_matchers; } 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 17d0ba856d48..0dd7bc6186fd 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -148,17 +148,7 @@ int DefaultCertValidator::initializeSslContexts(std::vector contexts, if (!cert_validation_config->subjectAltNameMatchers().empty()) { for (const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher : cert_validation_config->subjectAltNameMatchers()) { - if (matcher.has_string_matcher()) { - subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher.string_matcher())); - } else { - auto const factory = - Envoy::Config::Utility::getAndCheckFactory( - matcher.typed_config(), true); - if (factory != nullptr) { - subject_alt_name_matchers_.emplace_back( - factory->createSanMatcher(matcher.typed_config())); - } - } + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher)); } verify_mode = verify_mode_validation_context; } diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index ee4547d2b13b..ef37fa7ea33b 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -14,41 +14,28 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -Envoy::Ssl::SanMatcherPtr BackwardsCompatibleSanMatcherFactory::createSanMatcher( - const envoy::config::core::v3::TypedExtensionConfig& config) { - ASSERT(config.typed_config().Is()); - envoy::type::matcher::v3::StringMatcher string_matcher; - MessageUtil::unpackTo(config.typed_config(), string_matcher); - return Envoy::Ssl::SanMatcherPtr{std::make_unique(string_matcher)}; -} - -bool BackwardsCompatibleSanMatcher::match(const GENERAL_NAME* general_name) const { - return DefaultCertValidator::verifySubjectAltName(general_name, matcher_); -} - Envoy::Ssl::SanMatcherPtr createStringSanMatcher( - envoy::extensions::transport_sockets::tls::v3::StringSanMatcher const& matcher) { + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher) { switch (matcher.san_type()) { - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::DNS: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::EMAIL: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } - case envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::IP_ADD: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: { return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; } default: - ASSERT(true, - "Invalid san type for envoy::extensions::transport_sockets::tls::v3::StringSanMatcher"); + ASSERT(true, "Invalid san type for " + "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); return Envoy::Ssl::SanMatcherPtr(); } } -REGISTER_FACTORY(BackwardsCompatibleSanMatcherFactory, Envoy::Ssl::SanMatcherFactory); } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index ee82d1ac0b97..4e1f3f7d9c8e 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -40,36 +40,9 @@ using EmailSanMatcher = StringSanMatcher; using UriSanMatcher = StringSanMatcher; using IpAddSanMatcher = StringSanMatcher; -class BackwardsCompatibleSanMatcher : public Envoy::Ssl::SanMatcher { - -public: - bool match(const GENERAL_NAME* general_name) const override; - - ~BackwardsCompatibleSanMatcher() override = default; - - BackwardsCompatibleSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) - : matcher_(matcher) {} - -private: - Matchers::StringMatcherImpl matcher_; -}; - -class BackwardsCompatibleSanMatcherFactory : public Envoy::Ssl::SanMatcherFactory { -public: - ~BackwardsCompatibleSanMatcherFactory() override = default; - Envoy::Ssl::SanMatcherPtr - createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig& config) override; - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); - } - std::string name() const override { return "envoy.san_matchers.backward_compatible_san_matcher"; } -}; - -Envoy::Ssl ::SanMatcherPtr createBackwardsCompatibleSanMatcher( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher const& matcher); - Envoy::Ssl::SanMatcherPtr createStringSanMatcher( - envoy::extensions::transport_sockets::tls::v3::StringSanMatcher const& matcher); + const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher); + } // namespace Tls } // namespace TransportSockets } // namespace Extensions 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 cd957c1d9815..7852eb28ec55 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 @@ -39,20 +39,11 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC if (!config->subjectAltNameMatchers().empty()) { for (const auto& matcher : config->subjectAltNameMatchers()) { - if (matcher.has_string_matcher() && - matcher.string_matcher().san_type() == - envoy::extensions::transport_sockets::tls::v3::StringSanMatcher::URI) { + 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 - subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher.string_matcher())); - } else { - auto const factory = - Envoy::Config::Utility::getAndCheckFactory( - matcher.typed_config(), true); - if (factory != nullptr) { - subject_alt_name_matchers_.emplace_back( - factory->createSanMatcher(matcher.typed_config())); - } + subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher)); } } } @@ -240,16 +231,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()) { - // This check can be removed when match_subject_alt_names is removed from - // envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext. - if (general_name->type == GEN_URI) { - for (const auto& config_san_matcher : subject_alt_name_matchers_) { - if (config_san_matcher->match(general_name)) { - return true; - } + for (const auto& config_san_matcher : subject_alt_name_matchers_) { + if (config_san_matcher->match(general_name)) { + return true; } } } diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 2713d7bdf803..6314ec229d26 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -696,12 +696,31 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(ca_cert)), cvc_config.caCert()); // Verify that repeated fields are concatenated. - EXPECT_EQ(2, cvc_config.subjectAltNameMatchers().size()); - envoy::type::matcher::v3::StringMatcher string_matcher; - cvc_config.subjectAltNameMatchers()[0].typed_config().typed_config().UnpackTo(&string_matcher); - EXPECT_EQ("first san", string_matcher.exact()); - cvc_config.subjectAltNameMatchers()[1].typed_config().typed_config().UnpackTo(&string_matcher); - EXPECT_EQ("second san", string_matcher.exact()); + EXPECT_EQ(8, cvc_config.subjectAltNameMatchers().size()); + 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("first san", cvc_config.subjectAltNameMatchers()[1].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, + cvc_config.subjectAltNameMatchers()[1].san_type()); + EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[2].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, + cvc_config.subjectAltNameMatchers()[2].san_type()); + EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[3].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS, + cvc_config.subjectAltNameMatchers()[3].san_type()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[4].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, + cvc_config.subjectAltNameMatchers()[4].san_type()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[5].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, + cvc_config.subjectAltNameMatchers()[5].san_type()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[6].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, + cvc_config.subjectAltNameMatchers()[6].san_type()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[7].matcher().exact()); + EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS, + cvc_config.subjectAltNameMatchers()[7].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/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc index 295f8129cdcf..e917de8dc3ce 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 @@ -32,38 +32,22 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSMatched) { matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw([^.]*\.example.com)raw")); std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } -TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSTypeMatched) { +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( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); - subject_alt_name_matchers.clear(); subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(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( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); -} - -TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameWildcardDNSTypeMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir " "}}/test/extensions/transport_sockets/tls/test_data/san_multiple_dns_cert.pem")); @@ -76,19 +60,6 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameWildcardDNSTypeMatched) { } TEST(DefaultCertValidatorTest, TestMultiLevelMatch) { - // san_multiple_dns_cert matches *.example.com - 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("foo.api.example.com"); - std::vector subject_alt_name_matchers; - subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); -} - -TEST(DefaultCertValidatorTest, TestMultiLevelTypeMatch) { // san_multiple_dns_cert matches *.example.com bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir " @@ -119,17 +90,6 @@ TEST(DefaultCertValidatorTest, TestVerifySubjectAltMultiDomain) { } 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(R"raw(spiffe://lyft.com/[^/]*-team)raw")); - std::vector subject_alt_name_matchers; - subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); -} - -TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURITypeMatched) { 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; @@ -138,10 +98,6 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURITypeMatched) { subject_alt_name_matchers.push_back( Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); - subject_alt_name_matchers.clear(); - subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } TEST(DefaultCertValidatorTest, TestVerifySubjectAltNameNotMatched) { @@ -153,17 +109,6 @@ TEST(DefaultCertValidatorTest, TestVerifySubjectAltNameNotMatched) { } 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(R"raw([^.]*\.example\.net)raw")); - std::vector subject_alt_name_matchers; - subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); -} - -TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotTypeMatched) { 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; @@ -189,38 +134,6 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithSANMatcher) { /*CertificateValidationContextConfig=*/nullptr, stats, Event::GlobalTimeSystem().timeSystem()); - 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 san_matchers; - san_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(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( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); - // Verify the certificate with incorrect SAN exact matcher. - EXPECT_EQ(default_validator->verifyCertificate(cert.get(), /*verify_san_list=*/{}, - invalid_san_matchers), - Envoy::Ssl::ClientValidationStatus::Failed); - EXPECT_EQ(stats.fail_verify_san_.value(), 1); -} - -TEST(DefaultCertValidatorTest, TestCertificateVerificationWithTypedSANMatcher) { - Stats::TestUtil::TestStore test_store; - SslStats stats = generateSslStats(test_store); - // Create the default validator object. - auto default_validator = - std::make_unique( - /*CertificateValidationContextConfig=*/nullptr, stats, - Event::GlobalTimeSystem().timeSystem()); - 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; diff --git a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc index d8df3ae681ba..94cf5e7063fa 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/factory_test.cc @@ -1,8 +1,6 @@ #include -#include "source/common/config/utility.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" #include "test/extensions/transport_sockets/tls/cert_validator/test_common.h" @@ -24,22 +22,6 @@ TEST(FactoryTest, TestGetCertValidatorName) { EXPECT_EQ(custom_config.name(), getCertValidatorName(config.get())); } -TEST(FactoryTest, BackwardsCompatibleSanMatcherFactoryTest) { - // Create proto for BackwardsCompatibleSanMatcher. - envoy::config::core::v3::TypedExtensionConfig typed_config = {}; - typed_config.set_name("envoy.san_matchers.backward_compatible_san_matcher"); - envoy::type::matcher::v3::StringMatcher matcher; - typed_config.set_allocated_typed_config(new ProtobufWkt::Any()); - typed_config.mutable_typed_config()->PackFrom(matcher); - - // Create san matcher. - Envoy::Ssl::SanMatcherFactory* factory = - Envoy::Config::Utility::getAndCheckFactory(typed_config, true); - ASSERT_NE(factory, nullptr); - Envoy::Ssl::SanMatcherPtr san_matcher = factory->createSanMatcher(typed_config); - EXPECT_NE(dynamic_cast(san_matcher.get()), nullptr); -} - } // namespace Tls } // namespace TransportSockets } // namespace Extensions 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 40241d7153d1..c865b9f585f4 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 @@ -67,9 +67,24 @@ class TestSPIFFEValidator : public testing::Test { san_matchers_.clear(); for (auto& matcher : san_matchers) { san_matchers_.emplace_back(); - san_matchers_.back().mutable_typed_config()->mutable_typed_config()->PackFrom(matcher); - san_matchers_.back().mutable_typed_config()->mutable_name()->assign( - "envoy.san_matchers.backward_compatible_san_matcher"); + 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; } }; From 0d880c06809495f4a3990806da0858b5300a6a01 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 27 Oct 2021 12:26:05 +0000 Subject: [PATCH 15/32] Fix integration test. Signed-off-by: Pradeep Rao --- test/integration/ssl_utility.cc | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) 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); From 7970de4c8d314f020bef1f31886781c442d9ef65 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 27 Oct 2021 17:21:23 +0000 Subject: [PATCH 16/32] Fix doc issues. Signed-off-by: Pradeep Rao --- .../tls/v3/tls_spiffe_validator_config.proto | 3 +-- docs/root/start/quick-start/securing.rst | 8 ++++---- docs/root/version_history/current.rst | 7 ++----- 3 files changed, 7 insertions(+), 11 deletions(-) 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 813b63ea7f06..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,8 +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_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. +// - :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/docs/root/start/quick-start/securing.rst b/docs/root/start/quick-start/securing.rst index 85e3f8c7e4fd..35ff830450a6 100644 --- a/docs/root/start/quick-start/securing.rst +++ b/docs/root/start/quick-start/securing.rst @@ -122,7 +122,7 @@ and specify a mutually trusted certificate authority: :language: yaml :linenos: :lineno-start: 27 - :lines: 27-42 + :lines: 27-41 :emphasize-lines: 6, 8-10 :caption: :download:`envoy-demo-tls-client-auth.yaml <_include/envoy-demo-tls-client-auth.yaml>` @@ -135,7 +135,7 @@ similar to validating upstream certificates :ref:`described above ` @@ -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: 47 - :lines: 47-71 + :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 fd87b2175a93..33a61f1cff34 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -58,7 +58,7 @@ New Features * thrift_proxy: add upstream metrics to show decoding errors and whether exception is from local or remote, e.g. ``cluster.cluster_name.thrift.upstream_resp_exception_remote``. * 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 subset lb when using request or route metadata. -* tls: added support for :ref:`generic string matcher ` for subject alternative names. +* tls: added support for :ref:`match_typed_subject_alt_names ` for subject alternative names. * transport_socket: added :ref:`envoy.transport_sockets.tcp_stats ` which generates additional statistics gathered from the OS TCP stack. * udp: add support for multiple listener filters. * upstream: added the ability to :ref:`configure max connection duration ` for upstream clusters. @@ -70,8 +70,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 `. -* The `match_subject_alt_names` field in :ref:`Certificate Validation Context - ` - has been deprecated in favor of the :ref:`match_typed_subject_alt_names - ` field. +* 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 `. From d5e2ec493f712fbd5ad198c324127dc18aa81950 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 2 Nov 2021 15:35:52 +0000 Subject: [PATCH 17/32] Address CI failures. Signed-off-by: Pradeep Rao --- envoy/ssl/BUILD | 1 + .../common/secret/secret_manager_impl_test.cc | 30 ++++++++++++++++ test/config/utility.cc | 4 +-- test/config/utility.h | 7 ++-- .../cert_validator/default_validator_test.cc | 11 ++++++ .../spiffe_validator_integration_test.cc | 28 ++++++++++++--- .../spiffe_validator_integration_test.h | 3 +- test/integration/ads_integration.cc | 5 ++- test/integration/xfcc_integration_test.cc | 34 ++++++++++++++----- 9 files changed, 104 insertions(+), 19 deletions(-) diff --git a/envoy/ssl/BUILD b/envoy/ssl/BUILD index 5132e0067c14..704c92889f39 100644 --- a/envoy/ssl/BUILD +++ b/envoy/ssl/BUILD @@ -27,6 +27,7 @@ envoy_cc_library( envoy_cc_library( name = "context_config_interface", hdrs = ["context_config.h"], + external_deps = ["ssl"], deps = [ ":certificate_validation_context_config_interface", ":handshaker_interface", diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index a73f143ba861..b0982600d9b3 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -1128,6 +1128,36 @@ name: "abc.com" EnvoyException, "Failed to load private key provider: test"); } +// Test that we don't allow specification of both typed and untyped matchers for +// sans. +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_typed_subject_alt_names: + - san_type: DNS + matcher: + exact: "foo.example" + match_subject_alt_names: + exact: "foo.example" + )EOF"; + TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); + secret_manager->addStaticSecret(secret_config); + + EXPECT_THROW_WITH_MESSAGE( + Ssl::CertificateValidationContextConfigImpl cvc_config( + *secret_manager->findStaticCertificateValidationContextProvider("abc.com")->secret(), + *api_), + EnvoyException, + "SAN-based verification using both match_typed_subject_alt_names and " + "the deprecated match_subject_alt_names is not allowed"); +} + } // namespace } // namespace Secret } // namespace Envoy diff --git a/test/config/utility.cc b/test/config/utility.cc index 46f50346201b..f3778d8411ea 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1195,8 +1195,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 bc60c0cc8154..058a00c14923 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/default_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc index e917de8dc3ce..bd6004f3f630 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 @@ -176,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( + Envoy::Ssl::SanMatcherPtr{std::make_unique(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/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/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/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); }); From 4af6264a5c6353d7edfb91295159a40746926e53 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 2 Nov 2021 20:14:56 +0000 Subject: [PATCH 18/32] Address CI failures. Signed-off-by: Pradeep Rao --- envoy/ssl/BUILD | 6 ++++-- test/config/integration/server.yaml | 1 - test/config/integration/server_unix_listener.yaml | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/envoy/ssl/BUILD b/envoy/ssl/BUILD index 704c92889f39..21612ea08196 100644 --- a/envoy/ssl/BUILD +++ b/envoy/ssl/BUILD @@ -27,7 +27,6 @@ envoy_cc_library( envoy_cc_library( name = "context_config_interface", hdrs = ["context_config.h"], - external_deps = ["ssl"], deps = [ ":certificate_validation_context_config_interface", ":handshaker_interface", @@ -58,7 +57,10 @@ envoy_cc_library( envoy_cc_library( name = "certificate_validation_context_config_interface", hdrs = ["certificate_validation_context_config.h"], - external_deps = ["abseil_optional"], + external_deps = [ + "abseil_optional", + "ssl", + ], deps = [ "//source/common/common:matchers_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", diff --git a/test/config/integration/server.yaml b/test/config/integration/server.yaml index c874d5a09afe..e35a6acbf0c8 100644 --- a/test/config/integration/server.yaml +++ b/test/config/integration/server.yaml @@ -151,7 +151,6 @@ stats_sinks: typed_config: "@type": type.googleapis.com/envoy.config.metrics.v3.StatsdSink tcp_cluster_name: statsd -watchdog: {} layered_runtime: layers: - name: root diff --git a/test/config/integration/server_unix_listener.yaml b/test/config/integration/server_unix_listener.yaml index 50301659febc..b711d5a77c62 100644 --- a/test/config/integration/server_unix_listener.yaml +++ b/test/config/integration/server_unix_listener.yaml @@ -37,7 +37,6 @@ static_resources: port_value: 0 dns_lookup_family: V4_ONLY cluster_manager: {} -watchdog: {} admin: access_log: - name: envoy.access_loggers.file From b1b026105dee963f7fde35c3e1d7269c776704ba Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 3 Nov 2021 18:37:25 +0000 Subject: [PATCH 19/32] Add test for san matcher config Signed-off-by: Pradeep Rao --- .../tls/cert_validator/san_matcher_config.cc | 20 +++++++-------- .../tls/cert_validator/BUILD | 10 ++++++++ .../cert_validator/san_matcher_config_test.cc | 25 +++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index ef37fa7ea33b..8f1e745cdf9a 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -16,22 +16,22 @@ namespace Tls { Envoy::Ssl::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 == + 3); + switch (matcher.san_type()) { - case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; - } - case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; - } - case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; - } - case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: { + case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; - } default: - ASSERT(true, "Invalid san type for " - "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); + RELEASE_ASSERT(true, "Invalid san type for " + "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); return Envoy::Ssl::SanMatcherPtr(); } } diff --git a/test/extensions/transport_sockets/tls/cert_validator/BUILD b/test/extensions/transport_sockets/tls/cert_validator/BUILD index 7fd2c97084a1..1ef9b50f83c2 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/test/extensions/transport_sockets/tls/cert_validator/BUILD @@ -57,3 +57,13 @@ envoy_cc_test_library( "//test/test_common:utility_lib", ], ) + +envoy_cc_test( + name = "san_matcher_config_test", + srcs = [ + "san_matcher_config_test.cc", + ], + deps = [ + "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib", + ], +) diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc new file mode 100644 index 000000000000..14384cfa081c --- /dev/null +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -0,0 +1,25 @@ +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +// Verify we handle an invalid san type enum. +TEST(SanMatcherConfigTest, TestInvalidSanType) { + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; + san_matcher.mutable_matcher()->set_exact("foo.example"); + san_matcher.set_san_type( + envoy::extensions::transport_sockets::tls::v3:: + SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); + auto matcher = createStringSanMatcher(san_matcher); + EXPECT_EQ(matcher.get(), nullptr); +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy From 947da602166d17a9566ef1bbbfe3f2d5de5b03f2 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 4 Nov 2021 14:14:22 +0000 Subject: [PATCH 20/32] Address review comments. Signed-off-by: Pradeep Rao --- .../tls/cert_validator/san_matcher_config.h | 2 +- .../cert_validator/san_matcher_config_test.cc | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index 4e1f3f7d9c8e..a9abf937d74a 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -32,7 +32,7 @@ template class StringSanMatcher : public Envoy::Ssl::San StringSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) : matcher_(matcher) {} private: - Matchers::StringMatcherImpl matcher_; + const Matchers::StringMatcherImpl matcher_; }; using DnsSanMatcher = StringSanMatcher; diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 14384cfa081c..929cc1311047 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -8,17 +8,35 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -// Verify we handle an invalid san type enum. +// Verify that we handle an invalid san type enum. TEST(SanMatcherConfigTest, TestInvalidSanType) { envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type( envoy::extensions::transport_sockets::tls::v3:: SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); - auto matcher = createStringSanMatcher(san_matcher); + const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); EXPECT_EQ(matcher.get(), nullptr); } +// 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_SanType_MIN; + san_type <= + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher_SanType_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); + const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); + EXPECT_NE(matcher.get(), nullptr); + } +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions From c0113a95b6da355825875f9b4e6ed3e17a6b4a44 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 4 Nov 2021 14:42:49 +0000 Subject: [PATCH 21/32] Address review comment. Signed-off-by: Pradeep Rao --- ...tificate_validation_context_config_impl.cc | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 48ac0d2fb0ae..bb51025a62ea 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -66,26 +66,18 @@ CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( 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 (auto& matcher : config.match_subject_alt_names()) { - subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_san_type( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); - *subject_alt_name_matchers.back().mutable_matcher() = matcher; - - subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_san_type( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI); - *subject_alt_name_matchers.back().mutable_matcher() = matcher; - - subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_san_type( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL); - *subject_alt_name_matchers.back().mutable_matcher() = matcher; - - subject_alt_name_matchers.emplace_back(); - subject_alt_name_matchers.back().set_san_type( - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS); - *subject_alt_name_matchers.back().mutable_matcher() = matcher; + for (const envoy::type::matcher::v3::StringMatcher& matcher : config.match_subject_alt_names()) { + static const std::vector< + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType> + 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; } From d0e365b968d928666c4402f97f013338cd29ce0b Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 4 Nov 2021 17:00:46 +0000 Subject: [PATCH 22/32] Fix format issue. Signed-off-by: Pradeep Rao --- .../tls/cert_validator/san_matcher_config_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 929cc1311047..d41ad2648f02 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -23,9 +23,9 @@ TEST(SanMatcherConfigTest, TestInvalidSanType) { 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_SanType_MIN; + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType_MIN; san_type <= - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher_SanType_SanType_MAX; + 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))) { From 14b6c0f30ba4c2cf415e4c28e21d9fc862ea5cae Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Fri, 5 Nov 2021 22:42:18 +0000 Subject: [PATCH 23/32] Address feedback. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 18 ++++++++---- .../tls/cert_validator/san_matcher_config.cc | 2 +- .../tls/cert_validator/BUILD | 2 ++ .../cert_validator/san_matcher_config_test.cc | 29 ++++++++++++++++++- 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 60015b9d25c6..eaa13cdbda22 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -254,17 +254,23 @@ message CertificateProviderPluginInstance { string certificate_name = 2; } -// String matcher for subject alternative names, to match both type and value of the SAN. +// 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. See corresponding names and indices from the spec for the enums in the + // comments next to the values below. enum SanType { - EMAIL = 0; - DNS = 1; - URI = 2; - IP_ADDRESS = 3; + SAN_TYPE_UNSPECIFIED = 0; // Default, invalid value. + EMAIL = 1; // rfc822Name [1] + DNS = 2; // dNSName [2] + URI = 3; // uniformResourceIdentifier [6] + IP_ADDRESS = 4; // iPAdress [7] } - SanType san_type = 1 [(validate.rules).enum = {defined_only: true}]; + // Speficification of type of SAN. + SanType san_type = 1 [(validate.rules).enum = {in: [1, 2, 3, 4]}]; + // Matcher for SAN value. type.matcher.v3.StringMatcher matcher = 2; } diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index 8f1e745cdf9a..68e3a6efcc87 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -18,7 +18,7 @@ Envoy::Ssl::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 == - 3); + 4); switch (matcher.san_type()) { case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: diff --git a/test/extensions/transport_sockets/tls/cert_validator/BUILD b/test/extensions/transport_sockets/tls/cert_validator/BUILD index 1ef9b50f83c2..3984b62779fd 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/test/extensions/transport_sockets/tls/cert_validator/BUILD @@ -64,6 +64,8 @@ envoy_cc_test( "san_matcher_config_test.cc", ], deps = [ + "//source/common/protobuf:utility_lib", "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index d41ad2648f02..178a61d1b90a 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -1,5 +1,12 @@ +#include "envoy/extensions/transport_sockets/tls/v3/common.pb.validate.h" + #include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#include "source/common/protobuf/message_validator_impl.h" +#include "source/common/protobuf/utility.h" + +#include "test/test_common/utility.h" + #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -33,10 +40,30 @@ TEST(SanMatcherConfigTest, TestValidSanType) { san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type(san_type); const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); - EXPECT_NE(matcher.get(), nullptr); + if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: + SAN_TYPE_UNSPECIFIED) { + // Unspecified san type is not a valid for creating a string san matcher. + EXPECT_EQ(matcher.get(), nullptr); + } else { + 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 From b4d8863823030d426b1e4e9a6ffaa447936e7f3a Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 8 Nov 2021 14:34:30 +0000 Subject: [PATCH 24/32] Fix spelling. Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index eaa13cdbda22..4659a42e7322 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -264,10 +264,10 @@ message SubjectAltNameMatcher { EMAIL = 1; // rfc822Name [1] DNS = 2; // dNSName [2] URI = 3; // uniformResourceIdentifier [6] - IP_ADDRESS = 4; // iPAdress [7] + IP_ADDRESS = 4; // iPAddress [7] } - // Speficification of type of SAN. + // Specification of type of SAN. SanType san_type = 1 [(validate.rules).enum = {in: [1, 2, 3, 4]}]; // Matcher for SAN value. From e8c36bbae3cad54ca16096684d3ee384d9990a89 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 8 Nov 2021 16:28:28 +0000 Subject: [PATCH 25/32] Fix format issues. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 17 ++++++++--------- .../transport_sockets/tls/cert_validator/BUILD | 1 + .../cert_validator/san_matcher_config_test.cc | 3 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 4659a42e7322..2810e6a0eb0c 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -257,18 +257,17 @@ message CertificateProviderPluginInstance { // 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. See corresponding names and indices from the spec for the enums in the - // comments next to the values below. + // against. enum SanType { - SAN_TYPE_UNSPECIFIED = 0; // Default, invalid value. - EMAIL = 1; // rfc822Name [1] - DNS = 2; // dNSName [2] - URI = 3; // uniformResourceIdentifier [6] - IP_ADDRESS = 4; // iPAddress [7] + SAN_TYPE_UNSPECIFIED = 0; + EMAIL = 1; + DNS = 2; + URI = 3; + IP_ADDRESS = 4; } - // Specification of type of SAN. - SanType san_type = 1 [(validate.rules).enum = {in: [1, 2, 3, 4]}]; + // Specification of type of SAN. Note that the default enum value is an invalid choice. + SanType san_type = 1 [(validate.rules).enum = {in: 1 in: 2 in: 3 in: 4}]; // Matcher for SAN value. type.matcher.v3.StringMatcher matcher = 2; diff --git a/test/extensions/transport_sockets/tls/cert_validator/BUILD b/test/extensions/transport_sockets/tls/cert_validator/BUILD index 3984b62779fd..d4cf7da6ac14 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/test/extensions/transport_sockets/tls/cert_validator/BUILD @@ -67,5 +67,6 @@ envoy_cc_test( "//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/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 178a61d1b90a..47305d2a6386 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -1,9 +1,8 @@ #include "envoy/extensions/transport_sockets/tls/v3/common.pb.validate.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.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_config.h" #include "test/test_common/utility.h" From d94cc3e9716045ad99fa317556df6c349e8f431c Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 8 Nov 2021 22:45:16 +0000 Subject: [PATCH 26/32] Address comments. Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 2810e6a0eb0c..82af946124ef 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -270,7 +270,7 @@ message SubjectAltNameMatcher { SanType san_type = 1 [(validate.rules).enum = {in: 1 in: 2 in: 3 in: 4}]; // Matcher for SAN value. - type.matcher.v3.StringMatcher matcher = 2; + type.matcher.v3.StringMatcher matcher = 2 [(validate.rules).message = {required: true}]; } // [#next-free-field: 16] From d4dd0739127c2bf518e602d68a9d6c1ab08ed96a Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 10 Nov 2021 15:42:58 +0000 Subject: [PATCH 27/32] Address feedback. Signed-off-by: Pradeep Rao --- api/envoy/extensions/transport_sockets/tls/v3/common.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 82af946124ef..4bbcc408d807 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -267,7 +267,7 @@ message SubjectAltNameMatcher { } // Specification of type of SAN. Note that the default enum value is an invalid choice. - SanType san_type = 1 [(validate.rules).enum = {in: 1 in: 2 in: 3 in: 4}]; + 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}]; From e034bafb7f60bef311fcdfb2e8dba67055b57c66 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 10 Nov 2021 17:08:28 +0000 Subject: [PATCH 28/32] Address feedback. Signed-off-by: Pradeep Rao --- .../tls/cert_validator/san_matcher_config.cc | 4 +--- .../cert_validator/san_matcher_config_test.cc | 16 ++-------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index 68e3a6efcc87..df5eb3dde413 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -30,9 +30,7 @@ Envoy::Ssl::SanMatcherPtr createStringSanMatcher( case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; default: - RELEASE_ASSERT(true, "Invalid san type for " - "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); - return Envoy::Ssl::SanMatcherPtr(); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 47305d2a6386..64004c116982 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -14,17 +14,6 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -// Verify that we handle an invalid san type enum. -TEST(SanMatcherConfigTest, TestInvalidSanType) { - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; - san_matcher.mutable_matcher()->set_exact("foo.example"); - san_matcher.set_san_type( - envoy::extensions::transport_sockets::tls::v3:: - SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); - const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); - EXPECT_EQ(matcher.get(), nullptr); -} - // Verify that we get a valid string san matcher for all valid san types. TEST(SanMatcherConfigTest, TestValidSanType) { // Iterate over all san type enums. @@ -38,12 +27,11 @@ TEST(SanMatcherConfigTest, TestValidSanType) { envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type(san_type); - const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: SAN_TYPE_UNSPECIFIED) { - // Unspecified san type is not a valid for creating a string san matcher. - EXPECT_EQ(matcher.get(), nullptr); + continue; } else { + const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); EXPECT_NE(matcher.get(), nullptr); // Verify that the message is valid. TestUtility::validate(san_matcher); From de0525a435859a9377bcdf937aef4f598ebb5999 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 11 Nov 2021 14:37:02 +0000 Subject: [PATCH 29/32] Undo e034baf to fix coverage. Signed-off-by: Pradeep Rao --- .../tls/cert_validator/san_matcher_config.cc | 4 +++- .../cert_validator/san_matcher_config_test.cc | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index df5eb3dde413..68e3a6efcc87 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -30,7 +30,9 @@ Envoy::Ssl::SanMatcherPtr createStringSanMatcher( case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; default: - NOT_REACHED_GCOVR_EXCL_LINE; + RELEASE_ASSERT(true, "Invalid san type for " + "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); + return Envoy::Ssl::SanMatcherPtr(); } } diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 64004c116982..47305d2a6386 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -14,6 +14,17 @@ namespace Extensions { namespace TransportSockets { namespace Tls { +// Verify that we handle an invalid san type enum. +TEST(SanMatcherConfigTest, TestInvalidSanType) { + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; + san_matcher.mutable_matcher()->set_exact("foo.example"); + san_matcher.set_san_type( + envoy::extensions::transport_sockets::tls::v3:: + SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); + const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); + EXPECT_EQ(matcher.get(), nullptr); +} + // Verify that we get a valid string san matcher for all valid san types. TEST(SanMatcherConfigTest, TestValidSanType) { // Iterate over all san type enums. @@ -27,11 +38,12 @@ TEST(SanMatcherConfigTest, TestValidSanType) { envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type(san_type); + const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: SAN_TYPE_UNSPECIFIED) { - continue; + // Unspecified san type is not a valid for creating a string san matcher. + EXPECT_EQ(matcher.get(), nullptr); } else { - const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); EXPECT_NE(matcher.get(), nullptr); // Verify that the message is valid. TestUtility::validate(san_matcher); From 36226ea243efae12253ec4a7b023cdbe364b8fb3 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 16 Nov 2021 20:53:31 +0000 Subject: [PATCH 30/32] Address feedback. Signed-off-by: Pradeep Rao --- configs/envoy_double_proxy.template.yaml | 2 +- docs/root/version_history/current.rst | 2 +- envoy/ssl/BUILD | 6 +-- .../certificate_validation_context_config.h | 24 ----------- ...tificate_validation_context_config_impl.cc | 4 +- .../tls/cert_validator/default_validator.cc | 7 ++-- .../tls/cert_validator/default_validator.h | 10 ++--- .../tls/cert_validator/san_matcher_config.cc | 17 +++++--- .../tls/cert_validator/san_matcher_config.h | 30 ++++++------- .../cert_validator/spiffe/spiffe_validator.cc | 3 +- .../cert_validator/spiffe/spiffe_validator.h | 3 +- .../common/secret/secret_manager_impl_test.cc | 30 ------------- test/config/integration/server.yaml | 1 + .../integration/server_unix_listener.yaml | 1 + .../cert_validator/default_validator_test.cc | 42 +++++++++---------- .../cert_validator/san_matcher_config_test.cc | 4 +- .../tls/context_impl_test.cc | 29 +++++++++++++ 17 files changed, 98 insertions(+), 117 deletions(-) diff --git a/configs/envoy_double_proxy.template.yaml b/configs/envoy_double_proxy.template.yaml index 17ed6342d760..56f4d17ad201 100644 --- a/configs/envoy_double_proxy.template.yaml +++ b/configs/envoy_double_proxy.template.yaml @@ -191,7 +191,7 @@ static_resources: trusted_ca: filename: certs/cacert.pem match_typed_subject_alt_names: - - san_type: URI + - san_type: DNS matcher: exact: "collector-grpc.lightstep.com" flags_path: "/etc/envoy/flags" diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index d50956814aca..547650b70944 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -73,7 +73,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. +* 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 `. * transport_socket: added :ref:`envoy.transport_sockets.tcp_stats ` which generates additional statistics gathered from the OS TCP stack. * udp: add support for multiple listener filters. diff --git a/envoy/ssl/BUILD b/envoy/ssl/BUILD index 21612ea08196..3862d837cf75 100644 --- a/envoy/ssl/BUILD +++ b/envoy/ssl/BUILD @@ -57,13 +57,9 @@ envoy_cc_library( envoy_cc_library( name = "certificate_validation_context_config_interface", hdrs = ["certificate_validation_context_config.h"], - external_deps = [ - "abseil_optional", - "ssl", - ], + external_deps = ["abseil_optional"], deps = [ "//source/common/common:matchers_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/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 7d58285dc8a7..d331c45ce254 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -6,39 +6,15 @@ #include "envoy/api/api.h" #include "envoy/common/pure.h" -#include "envoy/config/core/v3/extension.pb.h" -#include "envoy/config/typed_config.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" -#include "openssl/x509v3.h" namespace Envoy { namespace Ssl { -/** 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 SanMatcherFactory : public Config::TypedFactory { -public: - ~SanMatcherFactory() override = default; - - virtual SanMatcherPtr - createSanMatcher(const envoy::config::core::v3::TypedExtensionConfig& config) PURE; - - std::string category() const override { return "envoy.san_matchers"; } -}; - class CertificateValidationContextConfig { public: virtual ~CertificateValidationContextConfig() = default; diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index bb51025a62ea..376f2b65a085 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -67,8 +67,8 @@ CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( // 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 const std::vector< - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType> + 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, 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 83cd10a79b3d..a7c5f8cebf90 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -26,7 +26,6 @@ #include "source/common/stats/utility.h" #include "source/extensions/transport_sockets/tls/cert_validator/cert_validator.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" #include "source/extensions/transport_sockets/tls/cert_validator/utility.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "source/extensions/transport_sockets/tls/utility.h" @@ -234,7 +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()) { @@ -304,13 +303,13 @@ bool DefaultCertValidator::verifySubjectAltName( } 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 (auto& config_san_matcher : subject_alt_name_matchers) { + 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 4316e699e1bc..7e8c0405d86d 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.h +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.h @@ -19,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_config.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "absl/synchronization/mutex.h" @@ -56,7 +57,7 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable& verify_san_list, - const std::vector& subject_alt_name_matchers); + const std::vector& subject_alt_name_matchers); /** * Verifies certificate hash for pinning. The hash is a hex-encoded SHA-256 of the DER-encoded @@ -98,9 +99,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_; @@ -110,7 +110,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_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc index 68e3a6efcc87..4a00e014ddc7 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc @@ -14,7 +14,12 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -Envoy::Ssl::SanMatcherPtr createStringSanMatcher( +bool StringSanMatcher::match(const GENERAL_NAME* general_name) const { + return general_name->type == general_name_type_ && + DefaultCertValidator::verifySubjectAltName(general_name, matcher_); +} + +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 == @@ -22,17 +27,17 @@ Envoy::Ssl::SanMatcherPtr createStringSanMatcher( switch (matcher.san_type()) { case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS: - return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + return SanMatcherPtr{std::make_unique(GEN_DNS, matcher.matcher())}; case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL: - return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + return SanMatcherPtr{std::make_unique(GEN_EMAIL, matcher.matcher())}; case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI: - return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + return SanMatcherPtr{std::make_unique(GEN_URI, matcher.matcher())}; case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: - return Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher.matcher())}; + return SanMatcherPtr{std::make_unique(GEN_IPADD, matcher.matcher())}; default: RELEASE_ASSERT(true, "Invalid san type for " "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); - return Envoy::Ssl::SanMatcherPtr(); + return SanMatcherPtr(); } } diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h index a9abf937d74a..5c2141f4b570 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h @@ -10,7 +10,6 @@ #include "source/common/common/hash.h" #include "source/common/common/matchers.h" #include "source/common/protobuf/protobuf.h" -#include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" #include "source/extensions/transport_sockets/tls/utility.h" #include "openssl/x509v3.h" @@ -20,27 +19,30 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -template class StringSanMatcher : public Envoy::Ssl::SanMatcher { +/** Interface to verify if there is a match in a list of subject alternative + * names. + */ +class SanMatcher { public: - bool match(const GENERAL_NAME* general_name) const override { - return general_name->type == general_name_type && - DefaultCertValidator::verifySubjectAltName(general_name, matcher_); - } + virtual bool match(GENERAL_NAME const*) const PURE; + virtual ~SanMatcher() = default; +}; - ~StringSanMatcher() override = default; +using SanMatcherPtr = std::unique_ptr; - StringSanMatcher(envoy::type::matcher::v3::StringMatcher matcher) : matcher_(matcher) {} +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_; }; -using DnsSanMatcher = StringSanMatcher; -using EmailSanMatcher = StringSanMatcher; -using UriSanMatcher = StringSanMatcher; -using IpAddSanMatcher = StringSanMatcher; - -Envoy::Ssl::SanMatcherPtr createStringSanMatcher( +SanMatcherPtr createStringSanMatcher( const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher); } // namespace Tls 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 7852eb28ec55..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 @@ -12,7 +12,6 @@ #include "source/common/protobuf/message_validator_impl.h" #include "source/common/stats/symbol_table_impl.h" #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" #include "source/extensions/transport_sockets/tls/cert_validator/utility.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "source/extensions/transport_sockets/tls/utility.h" @@ -43,6 +42,8 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC 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)); } } 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 dd519f04efeb..272003530943 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_config.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "openssl/ssl.h" @@ -67,7 +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/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index b0982600d9b3..a73f143ba861 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -1128,36 +1128,6 @@ name: "abc.com" EnvoyException, "Failed to load private key provider: test"); } -// Test that we don't allow specification of both typed and untyped matchers for -// sans. -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_typed_subject_alt_names: - - san_type: DNS - matcher: - exact: "foo.example" - match_subject_alt_names: - exact: "foo.example" - )EOF"; - TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); - - EXPECT_THROW_WITH_MESSAGE( - Ssl::CertificateValidationContextConfigImpl cvc_config( - *secret_manager->findStaticCertificateValidationContextProvider("abc.com")->secret(), - *api_), - EnvoyException, - "SAN-based verification using both match_typed_subject_alt_names and " - "the deprecated match_subject_alt_names is not allowed"); -} - } // namespace } // namespace Secret } // namespace Envoy diff --git a/test/config/integration/server.yaml b/test/config/integration/server.yaml index e35a6acbf0c8..c874d5a09afe 100644 --- a/test/config/integration/server.yaml +++ b/test/config/integration/server.yaml @@ -151,6 +151,7 @@ stats_sinks: typed_config: "@type": type.googleapis.com/envoy.config.metrics.v3.StatsdSink tcp_cluster_name: statsd +watchdog: {} layered_runtime: layers: - name: root diff --git a/test/config/integration/server_unix_listener.yaml b/test/config/integration/server_unix_listener.yaml index b711d5a77c62..50301659febc 100644 --- a/test/config/integration/server_unix_listener.yaml +++ b/test/config/integration/server_unix_listener.yaml @@ -37,6 +37,7 @@ static_resources: port_value: 0 dns_lookup_family: V4_ONLY cluster_manager: {} +watchdog: {} admin: access_log: - name: envoy.access_loggers.file 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 bd6004f3f630..61da0ac8800b 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 @@ -30,9 +30,9 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameDNSMatched) { "{{ 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; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -41,9 +41,9 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameIncorrectTypeMatched) { "{{ 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; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -53,9 +53,9 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameWildcardDNSMatched) { "}}/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; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -66,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; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -94,9 +94,9 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameURIMatched) { "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem")); envoy::type::matcher::v3::StringMatcher matcher; matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw(spiffe://lyft.com/[^/]*-team)raw")); - std::vector subject_alt_name_matchers; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); EXPECT_TRUE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -113,15 +113,15 @@ TEST(DefaultCertValidatorTest, TestMatchSubjectAltNameNotMatched) { "{{ 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\.net)raw")); - std::vector subject_alt_name_matchers; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_IPADD, matcher)}); subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_URI, matcher)}); subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_EMAIL, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } @@ -138,17 +138,17 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithSANMatcher) { "{{ 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 san_matchers; - san_matchers.push_back(Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + 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; + std::vector invalid_san_matchers; invalid_san_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + 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), @@ -181,9 +181,9 @@ TEST(DefaultCertValidatorTest, NoSanInCert) { "{{ 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; + std::vector subject_alt_name_matchers; subject_alt_name_matchers.push_back( - Envoy::Ssl::SanMatcherPtr{std::make_unique(matcher)}); + SanMatcherPtr{std::make_unique(GEN_DNS, matcher)}); EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc index 47305d2a6386..1ff1af87a982 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc @@ -21,7 +21,7 @@ TEST(SanMatcherConfigTest, TestInvalidSanType) { san_matcher.set_san_type( envoy::extensions::transport_sockets::tls::v3:: SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); - const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); + const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); EXPECT_EQ(matcher.get(), nullptr); } @@ -38,7 +38,7 @@ TEST(SanMatcherConfigTest, TestValidSanType) { envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type(san_type); - const Envoy::Ssl::SanMatcherPtr matcher = createStringSanMatcher(san_matcher); + const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: SAN_TYPE_UNSPECIFIED) { // Unspecified san type is not a valid for creating a string san matcher. diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 0967e77d25b0..5f4abd7e88b1 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1769,6 +1769,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"); +} + // Subclass ContextImpl so we can instantiate directly from tests, despite the // constructor being protected. class TestContextImpl : public ContextImpl { From 05fb9b01fdd17479a134bec3a68011482e1982f5 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 18 Nov 2021 17:17:54 +0000 Subject: [PATCH 31/32] Address feedback. Signed-off-by: Pradeep Rao --- .../tls/cert_validator/BUILD | 4 +- .../tls/cert_validator/default_validator.cc | 12 ------ .../tls/cert_validator/default_validator.h | 6 +-- .../{san_matcher_config.cc => san_matcher.cc} | 20 ++++++---- .../{san_matcher_config.h => san_matcher.h} | 0 .../cert_validator/spiffe/spiffe_validator.h | 2 +- test/common/secret/sds_api_test.cc | 34 +++++----------- .../common/secret/secret_manager_impl_test.cc | 37 +++++++++++++++++ test/common/upstream/upstream_impl_test.cc | 40 +++++++++++++------ .../tls/cert_validator/BUILD | 4 +- .../cert_validator/default_validator_test.cc | 2 +- ...her_config_test.cc => san_matcher_test.cc} | 18 ++------- .../tls/context_impl_test.cc | 6 ++- .../transport_sockets/tls/ssl_socket_test.cc | 30 +++++++++----- .../listener_manager_impl_quic_only_test.cc | 40 +++++++++++++------ 15 files changed, 151 insertions(+), 104 deletions(-) rename source/extensions/transport_sockets/tls/cert_validator/{san_matcher_config.cc => san_matcher.cc} (70%) rename source/extensions/transport_sockets/tls/cert_validator/{san_matcher_config.h => san_matcher.h} (100%) rename test/extensions/transport_sockets/tls/cert_validator/{san_matcher_config_test.cc => san_matcher_test.cc} (75%) diff --git a/source/extensions/transport_sockets/tls/cert_validator/BUILD b/source/extensions/transport_sockets/tls/cert_validator/BUILD index a8bea3139578..93f745f8faa3 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/source/extensions/transport_sockets/tls/cert_validator/BUILD @@ -13,14 +13,14 @@ envoy_cc_library( srcs = [ "default_validator.cc", "factory.cc", - "san_matcher_config.cc", + "san_matcher.cc", "utility.cc", ], hdrs = [ "cert_validator.h", "default_validator.h", "factory.h", - "san_matcher_config.h", + "san_matcher.h", "utility.h", ], external_deps = [ 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 a7c5f8cebf90..93bbe8b17a04 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -290,18 +290,6 @@ bool DefaultCertValidator::verifySubjectAltName(X509* cert, return false; } -bool DefaultCertValidator::verifySubjectAltName( - const GENERAL_NAME* general_name, - Matchers::StringMatcherImpl const& matcher) { - // 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); -} - bool DefaultCertValidator::matchSubjectAltName( X509* cert, const std::vector& subject_alt_name_matchers) { bssl::UniquePtr san_names( 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 7e8c0405d86d..64f52bc38728 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.h +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.h @@ -19,7 +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_config.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" @@ -89,10 +89,6 @@ class DefaultCertValidator : public CertValidator, Logger::Loggable& subject_alt_names); - static bool verifySubjectAltName( - const GENERAL_NAME* general_name, - Matchers::StringMatcherImpl const& matcher); - /** * Performs subjectAltName matching with the provided matchers. * @param ssl the certificate to verify diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc similarity index 70% rename from source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc rename to source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc index 4a00e014ddc7..4be1c64ec096 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc @@ -1,4 +1,4 @@ -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include @@ -7,7 +7,7 @@ #include "envoy/registry/registry.h" #include "envoy/ssl/certificate_validation_context_config.h" -#include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" +#include "source/extensions/transport_sockets/tls/utility.h" namespace Envoy { namespace Extensions { @@ -15,8 +15,16 @@ namespace TransportSockets { namespace Tls { bool StringSanMatcher::match(const GENERAL_NAME* general_name) const { - return general_name->type == general_name_type_ && - DefaultCertValidator::verifySubjectAltName(general_name, matcher_); + 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( @@ -35,9 +43,7 @@ SanMatcherPtr createStringSanMatcher( case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS: return SanMatcherPtr{std::make_unique(GEN_IPADD, matcher.matcher())}; default: - RELEASE_ASSERT(true, "Invalid san type for " - "envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher"); - return SanMatcherPtr(); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h b/source/extensions/transport_sockets/tls/cert_validator/san_matcher.h similarity index 100% rename from source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h rename to source/extensions/transport_sockets/tls/cert_validator/san_matcher.h 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 272003530943..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,7 +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_config.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include "source/extensions/transport_sockets/tls/stats.h" #include "openssl/ssl.h" diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 6314ec229d26..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; @@ -696,31 +702,13 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { EXPECT_EQ(TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(ca_cert)), cvc_config.caCert()); // Verify that repeated fields are concatenated. - EXPECT_EQ(8, cvc_config.subjectAltNameMatchers().size()); + EXPECT_EQ(2, cvc_config.subjectAltNameMatchers().size()); 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("first san", cvc_config.subjectAltNameMatchers()[1].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, - cvc_config.subjectAltNameMatchers()[1].san_type()); - EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[2].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, - cvc_config.subjectAltNameMatchers()[2].san_type()); - EXPECT_EQ("first san", cvc_config.subjectAltNameMatchers()[3].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS, - cvc_config.subjectAltNameMatchers()[3].san_type()); - EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[4].matcher().exact()); + EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[1].matcher().exact()); EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS, - cvc_config.subjectAltNameMatchers()[4].san_type()); - EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[5].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::URI, - cvc_config.subjectAltNameMatchers()[5].san_type()); - EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[6].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::EMAIL, - cvc_config.subjectAltNameMatchers()[6].san_type()); - EXPECT_EQ("second san", cvc_config.subjectAltNameMatchers()[7].matcher().exact()); - EXPECT_EQ(envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::IP_ADDRESS, - cvc_config.subjectAltNameMatchers()[7].san_type()); + 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/extensions/transport_sockets/tls/cert_validator/BUILD b/test/extensions/transport_sockets/tls/cert_validator/BUILD index d4cf7da6ac14..9d880d51fa05 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/BUILD +++ b/test/extensions/transport_sockets/tls/cert_validator/BUILD @@ -59,9 +59,9 @@ envoy_cc_test_library( ) envoy_cc_test( - name = "san_matcher_config_test", + name = "san_matcher_test", srcs = [ - "san_matcher_config_test.cc", + "san_matcher_test.cc", ], deps = [ "//source/common/protobuf:utility_lib", 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 61da0ac8800b..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,7 +2,7 @@ #include #include "source/extensions/transport_sockets/tls/cert_validator/default_validator.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.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" diff --git a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc similarity index 75% rename from test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc rename to test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc index 1ff1af87a982..b7729de8d41e 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/san_matcher_config_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc @@ -2,7 +2,7 @@ #include "source/common/protobuf/message_validator_impl.h" #include "source/common/protobuf/utility.h" -#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher_config.h" +#include "source/extensions/transport_sockets/tls/cert_validator/san_matcher.h" #include "test/test_common/utility.h" @@ -14,17 +14,6 @@ namespace Extensions { namespace TransportSockets { namespace Tls { -// Verify that we handle an invalid san type enum. -TEST(SanMatcherConfigTest, TestInvalidSanType) { - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; - san_matcher.mutable_matcher()->set_exact("foo.example"); - san_matcher.set_san_type( - envoy::extensions::transport_sockets::tls::v3:: - SubjectAltNameMatcher_SanType_SubjectAltNameMatcher_SanType_INT_MIN_SENTINEL_DO_NOT_USE_); - const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); - EXPECT_EQ(matcher.get(), nullptr); -} - // Verify that we get a valid string san matcher for all valid san types. TEST(SanMatcherConfigTest, TestValidSanType) { // Iterate over all san type enums. @@ -38,12 +27,11 @@ TEST(SanMatcherConfigTest, TestValidSanType) { envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher san_matcher; san_matcher.mutable_matcher()->set_exact("foo.example"); san_matcher.set_san_type(san_type); - const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); if (san_type == envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher:: SAN_TYPE_UNSPECIFIED) { - // Unspecified san type is not a valid for creating a string san matcher. - EXPECT_EQ(matcher.get(), nullptr); + continue; } else { + const SanMatcherPtr matcher = createStringSanMatcher(san_matcher); EXPECT_NE(matcher.get(), nullptr); // Verify that the message is valid. TestUtility::validate(san_matcher); diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 0daec66a9971..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 " 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/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: From 6ceb50d160ea84a0cd7b0e889e013d2fc65db3fb Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 18 Nov 2021 20:00:15 +0000 Subject: [PATCH 32/32] Modify coverage to account for unreached line. Signed-off-by: Pradeep Rao --- test/per_file_coverage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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