From 563d1ad181d622aabff3c7fd0a750be4ef619734 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 14 Jan 2021 11:54:10 -0500 Subject: [PATCH 1/2] dns: removing envoy.reloadable_features.fix_wildcard_matching #14644 Signed-off-by: Alyssa Wilk --- docs/root/version_history/current.rst | 1 + source/common/runtime/runtime_features.cc | 1 - .../transport_sockets/tls/context_impl.cc | 8 ++------ .../transport_sockets/tls/context_impl_test.cc | 17 ----------------- 4 files changed, 3 insertions(+), 24 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index ff3d5dc33333..bce7e2b9d454 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -31,6 +31,7 @@ Removed Config or Runtime *Normally occurs at the end of the* :ref:`deprecation period ` * access_logs: removed legacy unbounded access logs and runtime guard `envoy.reloadable_features.disallow_unbounded_access_logs`. +* dns: removed legacy buggy wildcard matching path and runtime guard `envoy.reloadable_features.fix_wildcard_matching`. * dynamic_forward_proxy: removed `envoy.reloadable_features.enable_dns_cache_circuit_breakers` and legacy code path. * http: removed legacy connection close behavior and runtime guard `envoy.reloadable_features.fixed_connection_close`. * http: removed legacy HTTP/1.1 error reporting path and runtime guard `envoy.reloadable_features.early_errors_via_hcm`. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index aec7aa601aa1..65366f421920 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -65,7 +65,6 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.consume_all_retry_headers", "envoy.reloadable_features.check_ocsp_policy", "envoy.reloadable_features.disable_tls_inspector_injection", - "envoy.reloadable_features.fix_wildcard_matching", "envoy.reloadable_features.hcm_stream_error_on_invalid_message", "envoy.reloadable_features.health_check.graceful_goaway_handling", "envoy.reloadable_features.http_default_alpn", diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index ee84dec354b9..f718e79ac4d3 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -757,12 +757,8 @@ bool ContextImpl::dnsNameMatch(const absl::string_view dns_name, const absl::str if (pattern_len > 1 && pattern[0] == '*' && pattern[1] == '.') { if (dns_name.length() > pattern_len - 1) { const size_t off = dns_name.length() - pattern_len + 1; - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.fix_wildcard_matching")) { - return dns_name.substr(0, off).find('.') == std::string::npos && - dns_name.substr(off, pattern_len - 1) == pattern.substr(1, pattern_len - 1); - } else { - return dns_name.substr(off, pattern_len - 1) == pattern.substr(1, pattern_len - 1); - } + return dns_name.substr(0, off).find('.') == std::string::npos && + dns_name.substr(off, pattern_len - 1) == pattern.substr(1, pattern_len - 1); } } diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 86571982f358..c30997212cee 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -114,23 +114,6 @@ TEST_F(SslContextImplTest, TestDnsNameMatching) { EXPECT_FALSE(ContextImpl::dnsNameMatch("lyft.com", "")); } -TEST_F(SslContextImplTest, TestDnsNameMatchingLegacy) { - TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.fix_wildcard_matching", "false"}}); - EXPECT_TRUE(ContextImpl::dnsNameMatch("lyft.com", "lyft.com")); - EXPECT_TRUE(ContextImpl::dnsNameMatch("a.lyft.com", "*.lyft.com")); - // Legacy behavior - EXPECT_TRUE(ContextImpl::dnsNameMatch("a.b.lyft.com", "*.lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("foo.test.com", "*.lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("lyft.com", "*.lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("alyft.com", "*.lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("alyft.com", "*lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("lyft.com", "*lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("", "*lyft.com")); - EXPECT_FALSE(ContextImpl::dnsNameMatch("lyft.com", "")); -} - TEST_F(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem")); From 45581de7ce7eeabc26ed10a63976b1787f2f00ed Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 20 Jan 2021 15:39:02 -0500 Subject: [PATCH 2/2] removing overlooked legacy tests Signed-off-by: Alyssa Wilk --- .../tls/context_impl_test.cc | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index c30997212cee..ab9f0a334ab0 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -155,20 +155,6 @@ TEST_F(SslContextImplTest, TestMultiLevelMatch) { EXPECT_FALSE(ContextImpl::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } -TEST_F(SslContextImplTest, TestMultiLevelMatchLegacy) { - TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.fix_wildcard_matching", "false"}}); - 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(Matchers::StringMatcherImpl(matcher)); - EXPECT_TRUE(ContextImpl::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); -} - TEST_F(SslContextImplTest, TestVerifySubjectAltNameURIMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem")); @@ -185,17 +171,6 @@ TEST_F(SslContextImplTest, TestVerifySubjectAltMultiDomain) { EXPECT_FALSE(ContextImpl::verifySubjectAltName(cert.get(), verify_subject_alt_name_list)); } -TEST_F(SslContextImplTest, TestVerifySubjectAltMultiDomainLegacy) { - TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.fix_wildcard_matching", "false"}}); - bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( - "{{ test_rundir " - "}}/test/extensions/transport_sockets/tls/test_data/san_multiple_dns_cert.pem")); - std::vector verify_subject_alt_name_list = {"https://a.www.example.com"}; - EXPECT_TRUE(ContextImpl::verifySubjectAltName(cert.get(), verify_subject_alt_name_list)); -} - TEST_F(SslContextImplTest, TestMatchSubjectAltNameURIMatched) { bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem"));