diff --git a/VERSION b/VERSION index 4ea8ad87e6e4..4e00d0ac0791 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.14.3 +1.14.4 diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 47494f06cf29..f5e79c5471ae 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -1,6 +1,10 @@ Version history --------------- +1.14.4 (July 7, 2020) +===================== +* tls: fixed a bug where wilcard matching for "\*.foo.com" also matched domains of the form "a.b.foo.com". This behavior can be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_wildcard_matching` to false. + 1.14.3 (June 30, 2020) ====================== * buffer: fixed CVE-2020-12603 by avoiding fragmentation, and tracking of HTTP/2 data and control frames in the output buffer. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index a78868b3ff3c..68903c67d809 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -35,6 +35,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.new_http2_connection_pool_behavior", "envoy.deprecated_features.allow_deprecated_extension_names", "envoy.reloadable_features.ext_authz_http_service_enable_case_sensitive_string_matcher", + "envoy.reloadable_features.fix_wildcard_matching", }; // This is a section for officially sanctioned runtime features which are too diff --git a/source/extensions/transport_sockets/tls/BUILD b/source/extensions/transport_sockets/tls/BUILD index 748c7b99559f..93f44e4b15bb 100644 --- a/source/extensions/transport_sockets/tls/BUILD +++ b/source/extensions/transport_sockets/tls/BUILD @@ -109,6 +109,7 @@ envoy_cc_library( "//source/common/common:utility_lib", "//source/common/network:address_lib", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_lib", "//source/common/stats:symbol_table_lib", "//source/extensions/transport_sockets/tls/private_key:private_key_manager_lib", "@envoy_api//envoy/admin/v3:pkg_cc_proto", diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index 7292bba9b005..9d912b2d07bb 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -19,6 +19,7 @@ #include "common/common/utility.h" #include "common/network/address_impl.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_impl.h" #include "extensions/transport_sockets/tls/utility.h" @@ -696,7 +697,12 @@ bool ContextImpl::dnsNameMatch(const std::string& dns_name, const char* pattern) 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; - return dns_name.compare(off, pattern_len - 1, pattern + 1) == 0; + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.fix_wildcard_matching")) { + return dns_name.substr(0, off).find('.') == std::string::npos && + dns_name.compare(off, pattern_len - 1, pattern + 1) == 0; + } else { + return dns_name.compare(off, pattern_len - 1, pattern + 1) == 0; + } } } diff --git a/test/extensions/transport_sockets/tls/BUILD b/test/extensions/transport_sockets/tls/BUILD index 6e878a85d326..8fde920b07c7 100644 --- a/test/extensions/transport_sockets/tls/BUILD +++ b/test/extensions/transport_sockets/tls/BUILD @@ -83,6 +83,7 @@ envoy_cc_test( "//test/mocks/ssl:ssl_mocks", "//test/test_common:environment_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:test_runtime_lib", "@envoy_api//envoy/admin/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/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 2b6c67057c28..090ba932d468 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -24,6 +24,7 @@ #include "test/mocks/ssl/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -48,6 +49,23 @@ class SslContextImplTest : public SslCertsTest { TEST_F(SslContextImplTest, TestDnsNameMatching) { EXPECT_TRUE(ContextImpl::dnsNameMatch("lyft.com", "lyft.com")); EXPECT_TRUE(ContextImpl::dnsNameMatch("a.lyft.com", "*.lyft.com")); + EXPECT_FALSE(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, 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")); @@ -87,6 +105,32 @@ TEST_F(SslContextImplTest, TestMatchSubjectAltNameWildcardDNSMatched) { EXPECT_TRUE(ContextImpl::matchSubjectAltName(cert.get(), subject_alt_name_matchers)); } +TEST_F(SslContextImplTest, 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(Matchers::StringMatcherImpl(matcher)); + 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")); @@ -95,6 +139,25 @@ TEST_F(SslContextImplTest, TestVerifySubjectAltNameURIMatched) { EXPECT_TRUE(ContextImpl::verifySubjectAltName(cert.get(), verify_subject_alt_name_list)); } +TEST_F(SslContextImplTest, TestVerifySubjectAltMultiDomain) { + 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_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"));