diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 4ed80caa4190..aaede3b60b8b 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -449,8 +449,12 @@ message CertificateValidationContext { // `. repeated SubjectAltNameMatcher match_typed_subject_alt_names = 15; - // This field is deprecated in favor of ref:`match_typed_subject_alt_names + // This field is deprecated in favor of + // :ref:`match_typed_subject_alt_names + // `. + // Note that if both this field and :ref:`match_typed_subject_alt_names // ` + // are specified, the former (deprecated field) is ignored. repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index afa4c497b4fe..6e81bc2e08f5 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -9,6 +9,8 @@ Minor Behavior Changes ---------------------- *Changes that may cause incompatibilities for some users, but should not for most* +* tls: if both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. Previously, setting both fields would result in an error. + Bug Fixes --------- *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 376f2b65a085..45206e025ed9 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -10,6 +10,8 @@ #include "source/common/common/logger.h" #include "source/common/config/datasource.h" +#include "spdlog/spdlog.h" + namespace Envoy { namespace Ssl { @@ -56,14 +58,21 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl( std::vector CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config) { - if (!config.match_typed_subject_alt_names().empty() && - !config.match_subject_alt_names().empty()) { - throw EnvoyException("SAN-based verification using both match_typed_subject_alt_names and " - "the deprecated match_subject_alt_names is not allowed"); - } std::vector subject_alt_name_matchers(config.match_typed_subject_alt_names().begin(), config.match_typed_subject_alt_names().end()); + // If typed subject alt name matchers are provided in the config, don't check + // for the deprecated non-typed field. + if (!subject_alt_name_matchers.empty()) { + // Warn that we're ignoring the deprecated san matcher field, if both are + // specified. + if (!config.match_subject_alt_names().empty()) { + ENVOY_LOG_MISC(warn, + "Ignoring match_subject_alt_names as match_typed_subject_alt_names is also " + "specified, and the former is deprecated."); + } + return subject_alt_name_matchers; + } // 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()) { diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 597cd4dc12d8..2aeca582863b 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1932,8 +1932,8 @@ 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 that if both typed and untyped matchers for sans are specified, we +// ignore the untyped matchers. TEST_F(ServerContextConfigImplTest, DeprecatedSanMatcher) { envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; NiceMock context_manager; @@ -1943,22 +1943,42 @@ TEST_F(ServerContextConfigImplTest, DeprecatedSanMatcher) { const std::string yaml = R"EOF( common_tls_context: + tls_certificates: + - certificate_chain: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_key.pem" 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" + exact: "foo1.example" match_subject_alt_names: - exact: "foo.example" + exact: "foo2.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"); + absl::optional server_context_config; + EXPECT_LOG_CONTAINS("warning", + "Ignoring match_subject_alt_names as match_typed_subject_alt_names is also " + "specified, and the former is deprecated.", + server_context_config.emplace(tls_context, factory_context_)); + EXPECT_EQ( + server_context_config.value().certificateValidationContext()->subjectAltNameMatchers().size(), + 1); + EXPECT_EQ(server_context_config.value() + .certificateValidationContext() + ->subjectAltNameMatchers()[0] + .san_type(), + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + EXPECT_EQ(server_context_config.value() + .certificateValidationContext() + ->subjectAltNameMatchers()[0] + .matcher() + .exact(), + "foo1.example"); } TEST_F(ServerContextConfigImplTest, Pkcs12LoadFailureBothPkcs12AndMethod) {