From 85a2cea9845393632325d2c1026592ec99b7050e Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Fri, 25 Mar 2022 15:42:16 +0000 Subject: [PATCH 1/7] Ignore non typed san matchers when both typed and non typed san matchers are provided in config. Signed-off-by: Pradeep Rao --- ...tificate_validation_context_config_impl.cc | 10 +++---- .../tls/context_impl_test.cc | 28 +++++++++++++------ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index 376f2b65a085e..cf405b40234ff 100644 --- a/source/common/ssl/certificate_validation_context_config_impl.cc +++ b/source/common/ssl/certificate_validation_context_config_impl.cc @@ -56,14 +56,14 @@ 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()) { + 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 597cd4dc12d89..a80b42f4f0bdb 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,34 @@ 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"); + ServerContextConfigImpl server_context_config(tls_context, factory_context_); + EXPECT_EQ(server_context_config.certificateValidationContext()->subjectAltNameMatchers().size(), + 1); + EXPECT_EQ( + server_context_config.certificateValidationContext()->subjectAltNameMatchers()[0].san_type(), + envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); + EXPECT_EQ(server_context_config.certificateValidationContext() + ->subjectAltNameMatchers()[0] + .matcher() + .exact(), + "foo1.example"); } TEST_F(ServerContextConfigImplTest, Pkcs12LoadFailureBothPkcs12AndMethod) { From c3300cc5e18334192c0d6983be5b2459f7bd5a38 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Mon, 28 Mar 2022 19:53:44 +0000 Subject: [PATCH 2/7] Address feedback. Signed-off-by: Pradeep Rao --- .../transport_sockets/tls/v3/common.proto | 6 +++++- docs/root/version_history/current.rst | 1 + ...tificate_validation_context_config_impl.cc | 9 +++++++++ .../tls/context_impl_test.cc | 20 +++++++++++++------ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/tls/v3/common.proto b/api/envoy/extensions/transport_sockets/tls/v3/common.proto index 9bdd80a39e807..b5d45033f8f06 100644 --- a/api/envoy/extensions/transport_sockets/tls/v3/common.proto +++ b/api/envoy/extensions/transport_sockets/tls/v3/common.proto @@ -448,8 +448,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 5a3a81b9f5f1d..95bcf74c39847 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -39,6 +39,7 @@ Minor Behavior Changes * sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. ++* tls: If both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. * tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. Bug Fixes diff --git a/source/common/ssl/certificate_validation_context_config_impl.cc b/source/common/ssl/certificate_validation_context_config_impl.cc index cf405b40234ff..45206e025ed9b 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 { @@ -62,6 +64,13 @@ CertificateValidationContextConfigImpl::getSubjectAltNameMatchers( // 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 diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index a80b42f4f0bdb..2aeca582863b5 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1960,13 +1960,21 @@ TEST_F(ServerContextConfigImplTest, DeprecatedSanMatcher) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context); - ServerContextConfigImpl server_context_config(tls_context, factory_context_); - EXPECT_EQ(server_context_config.certificateValidationContext()->subjectAltNameMatchers().size(), - 1); + 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.certificateValidationContext()->subjectAltNameMatchers()[0].san_type(), - envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::DNS); - EXPECT_EQ(server_context_config.certificateValidationContext() + 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(), From 9ef7193d535e730add06b4dfcd7e723777d90c1d Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 29 Mar 2022 14:11:40 +0000 Subject: [PATCH 3/7] Fix doc build failure. Signed-off-by: Pradeep Rao --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index d20e8f7d3fa74..0c99b206c15ec 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -39,7 +39,7 @@ Minor Behavior Changes * sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. -+* tls: If both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. +* tls: If both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. * tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. Bug Fixes From a47153f4765028652c7376bd333587051a75aa5e Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Tue, 29 Mar 2022 15:10:04 +0000 Subject: [PATCH 4/7] Fix format. Signed-off-by: Pradeep Rao --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index d2f78021becb0..dc102e72ff57b 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -41,7 +41,7 @@ Minor Behavior Changes * sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. -* tls: If both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. +* tls: if both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. * tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. Bug Fixes From 1e5c76210f868dfd8f97292425b2b94e774e72f0 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 20 Apr 2022 18:34:46 +0000 Subject: [PATCH 5/7] Fix format. Signed-off-by: Pradeep Rao --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 170e2766cd1ce..300b405b7c6fe 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -41,8 +41,8 @@ Minor Behavior Changes * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. * tls: if both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. -* tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. * tls: removed SHA-1 cipher suites from the server-side defaults. +* tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. Bug Fixes --------- From 508fc0ceeead66a7b8ec35a83ad1403259f6ff82 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 21 Apr 2022 15:19:38 +0000 Subject: [PATCH 6/7] Modify doc. Signed-off-by: Pradeep Rao --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 300b405b7c6fe..b7b2e57cbb7fb 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -40,7 +40,7 @@ Minor Behavior Changes * sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. -* tls: if both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. +* 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. * tls: removed SHA-1 cipher suites from the server-side defaults. * tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. From c1177fef5e6b0060344a1d53a3798b93eaebaacd Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 21 Apr 2022 16:46:13 +0000 Subject: [PATCH 7/7] Fix bad merge Signed-off-by: Pradeep Rao --- docs/root/version_history/current.rst | 30 --------------------------- 1 file changed, 30 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index b7b2e57cbb7fb..14eef8417ebc6 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -11,38 +11,8 @@ Minor Behavior Changes ---------------------- *Changes that may cause incompatibilities for some users, but should not for most* -* access_log: log all header values in the grpc access log. -* build: ``VERSION`` and ``API_VERSION`` have been renamed to ``VERSION.txt`` and ``API_VERSION.txt`` respectively to avoid conflicts with the C++ ```` header. -* config: warning messages for protobuf unknown fields now contain ancestors for easier troubleshooting. -* cryptomb: remove RSA PKCS1 v1.5 padding support. -* decompressor: decompressor does not duplicate ``accept-encoding`` header values anymore. This behavioral change can be reverted by setting runtime guard ``envoy.reloadable_features.append_to_accept_content_encoding_only_once`` to false. -* dynamic_forward_proxy: if a DNS resolution fails, failing immediately with a specific resolution error, rather than finishing up all local filters and failing to select an upstream host. -* ecds: changed to use ``http_filter`` stat prefix as the metrics root for ECDS subscriptions. This behavior can be temporarily reverted by setting ``envoy.reloadable_features.top_level_ecds_stats`` to false. -* ext_authz: added requested server name in ext_authz network filter for auth review. -* ext_authz: forward :ref:`typed_filter_metadata ` selected by :ref:`typed_metadata_context_namespaces ` to external auth service. -* file: changed disk based files to truncate files which are not being appended to. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.append_or_truncate`` to false. -* grpc: flip runtime guard ``envoy.reloadable_features.enable_grpc_async_client_cache`` to be default enabled. async grpc client created through getOrCreateRawAsyncClient will be cached by default. -* health_checker: exposing ``initial_metadata`` to GrpcHealthCheck in a way similar to ``request_headers_to_add`` of HttpHealthCheck. -* http: avoiding delay-close for HTTP/1.0 responses framed by connection: close as well as HTTP/1.1 if the request is fully read. This means for responses to such requests, the FIN will be sent immediately after the response. This behavior can be temporarily reverted by setting ``envoy.reloadable_features.skip_delay_close`` to false. If clients are are seen to be receiving sporadic partial responses and flipping this flag fixes it, please notify the project immediately. -* http: changed the http status code to 504 from 408 if the request timeouts after the request is completed. This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.override_request_timeout_by_gateway_timeout`` to false. -* http: lazy disable downstream connection reading in the HTTP/1 codec to reduce unnecessary system calls. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.http1_lazy_read_disable`` to false. -* http: now the max concurrent streams of http2 connection can not only be adjusted down according to the SETTINGS frame but also can be adjusted up, of course, it can not exceed the configured upper bounds. This fix is guarded by ``envoy.reloadable_features.http2_allow_capacity_increase_by_settings``. -* http: respecting ``content-type`` in :ref:`headers_to_add ` even when the response body is modified. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.allow_adding_content_type_in_local_replies`` to false. -* http: when writing custom filters, `injectEncodedDataToFilterChain` and `injectDecodedDataToFilterChain` now trigger sending of headers if they were not yet sent due to `StopIteration`. Previously, calling one of the inject functions in that state would trigger an assertion. See issue #19891 for more details. -* http: when writing custom filters, ``injectEncodedDataToFilterChain`` and ``injectDecodedDataToFilterChain`` now trigger sending of headers if they were not yet sent due to ``StopIteration``. Previously, calling one of the inject functions in that state would trigger an assertion. See issue #19891 for more details. -* listener: the :ref:`ipv4_compat ` flag can only be set on Ipv6 address and Ipv4-mapped Ipv6 address. A runtime guard is added ``envoy.reloadable_features.strict_check_on_ipv4_compat`` and the default is true. -* network: add a new ConnectionEvent ``ConnectedZeroRtt`` which may be raised by QUIC connections to allow early data to be sent before the handshake finishes. This event is ignored at callsites which is only reachable for TCP connections in the Envoy core code. Any extensions which depend on ConnectionEvent enum value should audit their usage of it to make sure this new event is handled appropriately. -* perf: ssl contexts are now tracked without scan based garbage collection and greatly improved the performance on secret update. -* ratelimit: the :ref:`header_value_match ` support custom descriptor_key. -* router: record upstream request timeouts for all the cases and not just for those requests which are awaiting headers. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.do_not_await_headers_on_upstream_timeout_to_emit_stats`` to false. -* runtime: deprecated runtime flags set via configuration files or xDS will now ENVOY_BUG, rather than silently resulting in unexpected behavior on the data plane by no longer applying removed code paths. -* runtime: removed global runtime as Envoy default. This behavioral change can be reverted by setting runtime guard ``envoy.restart_features.no_runtime_singleton`` to false. -* sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. -* sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. -* stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. * 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. * tls: removed SHA-1 cipher suites from the server-side defaults. -* tracing: set tracing error tag for grpc non-ok response code only when it is a upstream error. Client error will not be tagged as a grpc error. This fix is guarded by ``envoy.reloadable_features.update_grpc_response_error_tag``. Bug Fixes ---------