diff --git a/envoy/ssl/certificate_validation_context_config.h b/envoy/ssl/certificate_validation_context_config.h index 4098c0b7369c0..544215a73daea 100644 --- a/envoy/ssl/certificate_validation_context_config.h +++ b/envoy/ssl/certificate_validation_context_config.h @@ -40,11 +40,6 @@ class CertificateValidationContextConfig { */ virtual const std::string& certificateRevocationListPath() const PURE; - /** - * @return The subject alt names to be verified, if enabled. - */ - virtual const std::vector& verifySubjectAltNameList() const PURE; - /** * @return The subject alt name matchers to be verified, if enabled. */ diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 4782127e99ecc..40dc20f6ef3a3 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -22,9 +22,6 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( certificate_revocation_list_path_( Config::DataSource::getPath(config.crl()) .value_or(certificate_revocation_list_.empty() ? EMPTY_STRING : INLINE_STRING)), - verify_subject_alt_name_list_( - config.hidden_envoy_deprecated_verify_subject_alt_name().begin(), - config.hidden_envoy_deprecated_verify_subject_alt_name().end()), subject_alt_name_matchers_(config.match_subject_alt_names().begin(), config.match_subject_alt_names().end()), verify_certificate_hash_list_(config.verify_certificate_hash().begin(), @@ -44,7 +41,7 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( throw EnvoyException(fmt::format("Failed to load CRL from {} without trusted CA", certificateRevocationListPath())); } - if (!subject_alt_name_matchers_.empty() || !verify_subject_alt_name_list_.empty()) { + if (!subject_alt_name_matchers_.empty()) { throw EnvoyException("SAN-based verification of peer certificates without " "trusted CA is insecure and not allowed"); } diff --git a/source/common/ssl/certificate_validation_context_config_impl.h b/source/common/ssl/certificate_validation_context_config_impl.h index 56765baa74342..7cf045a351841 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.h +++ b/source/common/ssl/certificate_validation_context_config_impl.h @@ -24,9 +24,6 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte const std::string& certificateRevocationListPath() const final { return certificate_revocation_list_path_; } - const std::vector& verifySubjectAltNameList() const override { - return verify_subject_alt_name_list_; - } const std::vector& subjectAltNameMatchers() const override { return subject_alt_name_matchers_; @@ -56,7 +53,6 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte const std::string ca_cert_path_; const std::string certificate_revocation_list_; const std::string certificate_revocation_list_path_; - const std::vector verify_subject_alt_name_list_; const std::vector subject_alt_name_matchers_; const std::vector verify_certificate_hash_list_; const std::vector verify_certificate_spki_list_; diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc index 335bb2a8c9a86..6650a021ebaf8 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc @@ -29,14 +29,8 @@ Cluster::Cluster( // support these parameters dynamically in the future. This is not an exhaustive list of // parameters that don't make sense but should be the most obvious ones that a user might set // in error. - if (!cluster.hidden_envoy_deprecated_tls_context().sni().empty() || - !cluster.hidden_envoy_deprecated_tls_context() - .common_tls_context() - .validation_context() - .hidden_envoy_deprecated_verify_subject_alt_name() - .empty()) { - throw EnvoyException( - "dynamic_forward_proxy cluster cannot configure 'sni' or 'verify_subject_alt_name'"); + if (!cluster.hidden_envoy_deprecated_tls_context().sni().empty()) { + throw EnvoyException("dynamic_forward_proxy cluster cannot configure 'sni'"); } } 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 cf4463f6e5a5e..6f3c8edea9f7e 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -143,11 +143,6 @@ int DefaultCertValidator::initializeSslContexts(std::vector contexts, const Envoy::Ssl::CertificateValidationContextConfig* cert_validation_config = config_; if (cert_validation_config != nullptr) { - if (!cert_validation_config->verifySubjectAltNameList().empty()) { - verify_subject_alt_name_list_ = cert_validation_config->verifySubjectAltNameList(); - verify_mode = verify_mode_validation_context; - } - if (!cert_validation_config->subjectAltNameMatchers().empty()) { for (const envoy::type::matcher::v3::StringMatcher& matcher : cert_validation_config->subjectAltNameMatchers()) { @@ -204,13 +199,12 @@ int DefaultCertValidator::doVerifyCertChain( } } - Envoy::Ssl::ClientValidationStatus validated = verifyCertificate( - &leaf_cert, - transport_socket_options && - !transport_socket_options->verifySubjectAltNameListOverride().empty() - ? transport_socket_options->verifySubjectAltNameListOverride() - : verify_subject_alt_name_list_, - subject_alt_name_matchers_); + Envoy::Ssl::ClientValidationStatus validated = + verifyCertificate(&leaf_cert, + transport_socket_options != nullptr + ? transport_socket_options->verifySubjectAltNameListOverride() + : std::vector{}, + subject_alt_name_matchers_); if (ssl_extended_info) { if (ssl_extended_info->certificateValidationStatus() == @@ -381,12 +375,6 @@ void DefaultCertValidator::updateDigestForSessionId(bssl::ScopedEVP_MD_CTX& md, rc = EVP_DigestUpdate(md.get(), hash_buffer, hash_length); RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or("")); - - // verify_subject_alt_name_list_ can only be set with a ca_cert - for (const std::string& name : verify_subject_alt_name_list_) { - rc = EVP_DigestUpdate(md.get(), name.data(), name.size()); - RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or("")); - } } for (const auto& hash : verify_certificate_hash_list_) { 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 03a037ef33df7..fb7e7acab1f4e 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.h +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.h @@ -116,7 +116,6 @@ class DefaultCertValidator : public CertValidator { std::vector subject_alt_name_matchers_; std::vector> verify_certificate_hash_list_; std::vector> verify_certificate_spki_list_; - std::vector verify_subject_alt_name_list_; bool verify_trusted_ca_{false}; }; diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index 30837b4fddc05..efcd16337872f 100644 --- a/test/common/quic/envoy_quic_proof_source_test.cc +++ b/test/common/quic/envoy_quic_proof_source_test.cc @@ -61,8 +61,6 @@ class TestGetProofCallback : public quic::ProofSource::Callback { ON_CALL(cert_validation_ctx_config_, certificateRevocationListPath()) .WillByDefault(ReturnRef(path_string)); const std::vector empty_string_list; - ON_CALL(cert_validation_ctx_config_, verifySubjectAltNameList()) - .WillByDefault(ReturnRef(empty_string_list)); const std::vector san_matchers; ON_CALL(cert_validation_ctx_config_, subjectAltNameMatchers()) .WillByDefault(ReturnRef(san_matchers)); diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 49dd9696e2fb8..442e9b867f961 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -57,8 +57,6 @@ class EnvoyQuicProofVerifierTest : public testing::Test { .WillRepeatedly(ReturnRef(empty_string_)); EXPECT_CALL(cert_validation_ctx_config_, certificateRevocationListPath()) .WillRepeatedly(ReturnRef(path_string_)); - EXPECT_CALL(cert_validation_ctx_config_, verifySubjectAltNameList()) - .WillRepeatedly(ReturnRef(empty_string_list_)); EXPECT_CALL(cert_validation_ctx_config_, subjectAltNameMatchers()) .WillRepeatedly(ReturnRef(san_matchers_)); EXPECT_CALL(cert_validation_ctx_config_, verifyCertificateHashList()) diff --git a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc index 0544523a3be1f..2f0550321a85a 100644 --- a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc +++ b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc @@ -254,34 +254,8 @@ connect_timeout: 0.25s filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" )EOF"); - EXPECT_THROW_WITH_MESSAGE( - createCluster(yaml_config, false), EnvoyException, - "dynamic_forward_proxy cluster cannot configure 'sni' or 'verify_subject_alt_name'"); -} - -// Verify that using 'verify_subject_alt_name' causes a failure. -TEST_F(ClusterFactoryTest, DEPRECATED_FEATURE_TEST(InvalidVerifySubjectAltName)) { - TestDeprecatedV2Api _deprecated_v2_api; - const std::string yaml_config = TestEnvironment::substitute(R"EOF( -name: name -connect_timeout: 0.25s -cluster_type: - name: dynamic_forward_proxy - typed_config: - "@type": type.googleapis.com/envoy.config.cluster.dynamic_forward_proxy.v2alpha.ClusterConfig - dns_cache_config: - name: foo -tls_context: - common_tls_context: - validation_context: - trusted_ca: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - verify_subject_alt_name: [api.lyft.com] -)EOF"); - - EXPECT_THROW_WITH_MESSAGE( - createCluster(yaml_config, false), EnvoyException, - "dynamic_forward_proxy cluster cannot configure 'sni' or 'verify_subject_alt_name'"); + EXPECT_THROW_WITH_MESSAGE(createCluster(yaml_config, false), EnvoyException, + "dynamic_forward_proxy cluster cannot configure 'sni'"); } TEST_F(ClusterFactoryTest, InvalidUpstreamHttpProtocolOptions) { 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 c0cc7f28425fd..b958f17272075 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/test_common.h +++ b/test/extensions/transport_sockets/tls/cert_validator/test_common.h @@ -47,9 +47,6 @@ class TestCertificateValidationContextConfig const std::string& certificateRevocationListPath() const final { CONSTRUCT_ON_FIRST_USE(std::string, ""); } - const std::vector& verifySubjectAltNameList() const override { - CONSTRUCT_ON_FIRST_USE(std::vector, {}); - } const std::vector& subjectAltNameMatchers() const override { return san_matchers_; diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 94cf360a3e706..43ad275fce277 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -156,7 +156,6 @@ 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&, verifySubjectAltNameList, (), (const)); MOCK_METHOD(const std::vector&, subjectAltNameMatchers, (), (const)); MOCK_METHOD(const std::vector&, verifyCertificateHashList, (), (const));