From d2ca84da30476957ed329c68a152cf3fe829dc8a Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Mon, 6 Sep 2021 02:34:09 -0400 Subject: [PATCH 1/7] router: added optional alt_header_name param to set an arbitrary header for populating SNI Signed-off-by: Rohit Agrawal --- api/envoy/config/core/v3/protocol.proto | 18 ++++-- docs/root/faq/configuration/sni.rst | 6 +- docs/root/version_history/current.rst | 1 + .../envoy/config/core/v3/protocol.proto | 18 ++++-- source/common/router/router.cc | 43 ++++++++++---- test/common/router/router_test.cc | 2 + .../proxy_filter_integration_test.cc | 41 ++++++++++++- .../http/router/auto_sni_integration_test.cc | 57 +++++++++++++------ 8 files changed, 143 insertions(+), 43 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 8f2347eb55179..7b3f0e4783d18 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -60,15 +60,23 @@ message UpstreamHttpProtocolOptions { "envoy.api.v2.core.UpstreamHttpProtocolOptions"; // Set transport socket `SNI `_ for new - // upstream connections based on the downstream HTTP host/authority header, as seen by the - // :ref:`router filter `. + // upstream connections based on the downstream HTTP host/authority header or any other arbitrary + // header when :ref:`alt_header_name ` + // is set, as seen by the :ref:`router filter `. bool auto_sni = 1; // Automatic validate upstream presented certificate for new upstream connections based on the - // downstream HTTP host/authority header, as seen by the - // :ref:`router filter `. - // This field is intended to set with `auto_sni` field. + // downstream HTTP host/authority header or any other arbitrary header when :ref:`alt_header_name ` + // is set, as seen by the :ref:`router filter `. + // This field is intended to be set with `auto_sni` field. bool auto_san_validation = 2; + + // An optional alternative to the host/authority header to be used for setting the SNI value. + // It should be a valid downstream HTTP header, as seen by the + // :ref:`router filter `. + // If unset, host/authority header will be used for populating the SNI. + // This field is intended to be set with `auto_sni` field. + string alt_header_name = 3; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/docs/root/faq/configuration/sni.rst b/docs/root/faq/configuration/sni.rst index 9b33302c595ec..29dac111b4ebb 100644 --- a/docs/root/faq/configuration/sni.rst +++ b/docs/root/faq/configuration/sni.rst @@ -70,8 +70,10 @@ How do I configure SNI for clusters? ==================================== For clusters, a fixed SNI can be set in :ref:`UpstreamTlsContext `. -To derive SNI from HTTP ``host`` or ``:authority`` header, turn on +To derive SNI from a downstream HTTP header like, ``host`` or ``:authority``, turn on :ref:`auto_sni ` to override the fixed SNI in -`UpstreamTlsContext`. If upstream will present certificates with the hostname in SAN, turn on +`UpstreamTlsContext`. A custom header other than the ``host`` or ``:authority`` can also be supplied using the optional +:ref:`alt_header_name ` field. +If upstream will present certificates with the hostname in SAN, turn on :ref:`auto_san_validation ` too. It still needs a trust CA in validation context in ``UpstreamTlsContext`` for trust anchor. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index c8e227d34b58d..feaed1ebc5f32 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -112,6 +112,7 @@ New Features * overload: add a new overload action that resets streams using a lot of memory. To enable the tracking of allocated bytes in buffers that a stream is using we need to configure the minimum threshold for tracking via:ref:`buffer_factory_config `. We have an overload action ``Envoy::Server::OverloadActionNameValues::ResetStreams`` that takes advantage of the tracking to reset the most expensive stream first. * rbac: added :ref:`destination_port_range ` for matching range of destination ports. * route config: added :ref:`dynamic_metadata ` for routing based on dynamic metadata. +* router: added an optional :ref:`alt_header_name ` to support setting SNI value from an arbitrary header other than host/authority. * sxg_filter: added filter to transform response to SXG package to :ref:`contrib images `. This can be enabled by setting :ref:`SXG ` configuration. * thrift_proxy: added support for :ref:`mirroring requests `. diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 8f2347eb55179..7b3f0e4783d18 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -60,15 +60,23 @@ message UpstreamHttpProtocolOptions { "envoy.api.v2.core.UpstreamHttpProtocolOptions"; // Set transport socket `SNI `_ for new - // upstream connections based on the downstream HTTP host/authority header, as seen by the - // :ref:`router filter `. + // upstream connections based on the downstream HTTP host/authority header or any other arbitrary + // header when :ref:`alt_header_name ` + // is set, as seen by the :ref:`router filter `. bool auto_sni = 1; // Automatic validate upstream presented certificate for new upstream connections based on the - // downstream HTTP host/authority header, as seen by the - // :ref:`router filter `. - // This field is intended to set with `auto_sni` field. + // downstream HTTP host/authority header or any other arbitrary header when :ref:`alt_header_name ` + // is set, as seen by the :ref:`router filter `. + // This field is intended to be set with `auto_sni` field. bool auto_san_validation = 2; + + // An optional alternative to the host/authority header to be used for setting the SNI value. + // It should be a valid downstream HTTP header, as seen by the + // :ref:`router filter `. + // If unset, host/authority header will be used for populating the SNI. + // This field is intended to be set with `auto_sni` field. + string alt_header_name = 3; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/source/common/router/router.cc b/source/common/router/router.cc index d1115c8a40df0..cce94b90e6e51 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -503,20 +503,39 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, const auto& upstream_http_protocol_options = cluster_->upstreamHttpProtocolOptions(); if (upstream_http_protocol_options.has_value()) { - const auto parsed_authority = Http::Utility::parseAuthority(headers.getHostValue()); - if (!parsed_authority.is_ip_address_ && upstream_http_protocol_options.value().auto_sni()) { - callbacks_->streamInfo().filterState()->setData( - Network::UpstreamServerName::key(), - std::make_unique(parsed_authority.host_), - StreamInfo::FilterState::StateType::Mutable); + std::string sni_value; + bool should_set_sni = true; + + // Check whether `alt_header_name` is specified. + const auto alt_header_name = upstream_http_protocol_options.value().alt_header_name(); + if (alt_header_name.empty()) { + // Default the SNI value to the Host/Authority header. + const auto parsed_authority = Http::Utility::parseAuthority(headers.getHostValue()); + should_set_sni = !parsed_authority.is_ip_address_; + sni_value = std::string(parsed_authority.host_); + } else { + // Use the header value from `alt_header_name` to set the SNI value. + const auto header_value = Http::HeaderUtility::getAllOfHeaderAsString( + headers, Http::LowerCaseString(alt_header_name)); + if (header_value.result().has_value()) { + sni_value = std::string(header_value.result().value()); + } } - if (upstream_http_protocol_options.value().auto_san_validation()) { - callbacks_->streamInfo().filterState()->setData( - Network::UpstreamSubjectAltNames::key(), - std::make_unique( - std::vector{std::string(parsed_authority.host_)}), - StreamInfo::FilterState::StateType::Mutable); + if (!sni_value.empty()) { + if (should_set_sni && upstream_http_protocol_options.value().auto_sni()) { + callbacks_->streamInfo().filterState()->setData( + Network::UpstreamServerName::key(), + std::make_unique(sni_value), + StreamInfo::FilterState::StateType::Mutable); + } + + if (upstream_http_protocol_options.value().auto_san_validation()) { + callbacks_->streamInfo().filterState()->setData( + Network::UpstreamSubjectAltNames::key(), + std::make_unique(std::vector{sni_value}), + StreamInfo::FilterState::StateType::Mutable); + } } } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 5963d3063a277..90ae361e36342 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -147,6 +147,7 @@ TEST_F(RouterTest, UpdateServerNameFilterState) { NiceMock stream_info; auto dummy_option = absl::make_optional(); dummy_option.value().set_auto_sni(true); + dummy_option.value().set_alt_header_name(":authority"); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(dummy_option)); ON_CALL(callbacks_.stream_info_, filterState()) @@ -178,6 +179,7 @@ TEST_F(RouterTest, UpdateServerNameFilterState) { TEST_F(RouterTest, UpdateSubjectAltNamesFilterState) { NiceMock stream_info; auto dummy_option = absl::make_optional(); + dummy_option.value().set_alt_header_name(":authority"); dummy_option.value().set_auto_san_validation(true); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(dummy_option)); diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index 6b6ff51010c10..7d79121fb348f 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -18,7 +18,8 @@ class ProxyFilterIntegrationTest : public testing::TestWithParammutable_validate_clusters()->set_value(false); }); + [alt_header_name]( + envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + hcm.mutable_route_config()->mutable_validate_clusters()->set_value(false); + }); // Setup the initial CDS cluster. cluster_.mutable_connect_timeout()->CopyFrom( @@ -64,6 +68,10 @@ name: dynamic_forward_proxy ConfigHelper::HttpProtocolOptions protocol_options; protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); + if (!alt_header_name.empty()) { + protocol_options.mutable_upstream_http_protocol_options()->set_alt_header_name( + alt_header_name); + } protocol_options.mutable_upstream_http_protocol_options()->set_auto_san_validation(true); protocol_options.mutable_explicit_http_config()->mutable_http_protocol_options(); ConfigHelper::setProtocolOptions(cluster_, protocol_options); @@ -279,6 +287,33 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTls) { checkSimpleRequestSuccess(0, 0, response.get()); } +// Verify that alt_header_name can be used along with auto_sni to set SNI from an arbitrary header. +TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithAltHeaderSni) { + upstream_tls_ = true; + initializeWithArgs(1024, 1024, "x-host"); + codec_client_ = makeHttpConnection(lookupPort("http")); + const Http::TestRequestHeaderMapImpl request_headers{ + {":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", + fmt::format("{}:{}", fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str(), + fake_upstreams_[0]->localAddress()->ip()->port())}, + {"x-host", "localhost"}}; + + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + waitForNextUpstreamRequest(); + + const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket = + dynamic_cast( + fake_upstream_connection_->connection().ssl().get()); + EXPECT_STREQ("localhost", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name)); + + upstream_request_->encodeHeaders(default_response_headers_, true); + ASSERT_TRUE(response->waitForEndStream()); + checkSimpleRequestSuccess(0, 0, response.get()); +} + TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithIpHost) { upstream_tls_ = true; initializeWithArgs(); diff --git a/test/extensions/filters/http/router/auto_sni_integration_test.cc b/test/extensions/filters/http/router/auto_sni_integration_test.cc index 25328614681fe..ab5d3f1861d86 100644 --- a/test/extensions/filters/http/router/auto_sni_integration_test.cc +++ b/test/extensions/filters/http/router/auto_sni_integration_test.cc @@ -18,24 +18,29 @@ class AutoSniIntegrationTest : public testing::TestWithParammutable_clusters()->at(0); - ConfigHelper::HttpProtocolOptions protocol_options; - protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); - ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), - protocol_options); - - envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; - auto* validation_context = - tls_context.mutable_common_tls_context()->mutable_validation_context(); - validation_context->mutable_trusted_ca()->set_filename( - TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); - cluster_config.mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); - cluster_config.mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); - }); + config_helper_.addConfigModifier( + [alt_header_name](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto& cluster_config = bootstrap.mutable_static_resources()->mutable_clusters()->at(0); + ConfigHelper::HttpProtocolOptions protocol_options; + protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); + if (!alt_header_name.empty()) { + protocol_options.mutable_upstream_http_protocol_options()->set_alt_header_name( + alt_header_name); + } + ConfigHelper::setProtocolOptions( + *bootstrap.mutable_static_resources()->mutable_clusters(0), protocol_options); + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + auto* validation_context = + tls_context.mutable_common_tls_context()->mutable_validation_context(); + validation_context->mutable_trusted_ca()->set_filename( + TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); + cluster_config.mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); + cluster_config.mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); + }); HttpIntegrationTest::initialize(); } @@ -83,6 +88,26 @@ TEST_P(AutoSniIntegrationTest, BasicAutoSniTest) { EXPECT_STREQ("localhost", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name)); } +TEST_P(AutoSniIntegrationTest, AutoSniWithAltHeaderNameTest) { + setup("x-host"); + codec_client_ = makeHttpConnection(lookupPort("http")); + const auto response_ = + sendRequestAndWaitForResponse(Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "localhost"}, + {"x-host", "custom"}}, + 0, default_response_headers_, 0); + + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response_->complete()); + + const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket = + dynamic_cast( + fake_upstream_connection_->connection().ssl().get()); + EXPECT_STREQ("custom", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name)); +} + TEST_P(AutoSniIntegrationTest, PassingNotDNS) { setup(); codec_client_ = makeHttpConnection(lookupPort("http")); From 6a65a1e63f46dfde0ef848f1252a82a8616047e9 Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Wed, 8 Sep 2021 17:19:29 -0400 Subject: [PATCH 2/7] Addressed Review Comments Signed-off-by: Rohit Agrawal --- api/envoy/config/core/v3/protocol.proto | 2 +- generated_api_shadow/envoy/config/core/v3/protocol.proto | 2 +- source/common/router/router.cc | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 7b3f0e4783d18..18a643fee55dd 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -76,7 +76,7 @@ message UpstreamHttpProtocolOptions { // :ref:`router filter `. // If unset, host/authority header will be used for populating the SNI. // This field is intended to be set with `auto_sni` field. - string alt_header_name = 3; + string alt_header_name = 3 [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME}]; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 7b3f0e4783d18..18a643fee55dd 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -76,7 +76,7 @@ message UpstreamHttpProtocolOptions { // :ref:`router filter `. // If unset, host/authority header will be used for populating the SNI. // This field is intended to be set with `auto_sni` field. - string alt_header_name = 3; + string alt_header_name = 3 [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME}]; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/source/common/router/router.cc b/source/common/router/router.cc index cce94b90e6e51..bce6ef32d5ccf 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -503,7 +503,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, const auto& upstream_http_protocol_options = cluster_->upstreamHttpProtocolOptions(); if (upstream_http_protocol_options.has_value()) { - std::string sni_value; + absl::string_view sni_value; bool should_set_sni = true; // Check whether `alt_header_name` is specified. @@ -533,7 +533,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, if (upstream_http_protocol_options.value().auto_san_validation()) { callbacks_->streamInfo().filterState()->setData( Network::UpstreamSubjectAltNames::key(), - std::make_unique(std::vector{sni_value}), + std::make_unique( + std::vector{std::string(sni_value).c_str()}), StreamInfo::FilterState::StateType::Mutable); } } From ecdb35eb59b358564f5aad52855a08833c34221d Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Thu, 9 Sep 2021 16:02:24 -0400 Subject: [PATCH 3/7] Addressed Comments Signed-off-by: Rohit Agrawal --- api/envoy/config/core/v3/protocol.proto | 13 ++-- docs/root/faq/configuration/sni.rst | 2 +- docs/root/version_history/current.rst | 2 +- .../envoy/config/core/v3/protocol.proto | 13 ++-- source/common/router/router.cc | 65 ++++++++++--------- test/common/router/router_test.cc | 4 +- .../proxy_filter_integration_test.cc | 13 ++-- .../http/router/auto_sni_integration_test.cc | 10 +-- 8 files changed, 65 insertions(+), 57 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 18a643fee55dd..93ac83718b5be 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -61,12 +61,12 @@ message UpstreamHttpProtocolOptions { // Set transport socket `SNI `_ for new // upstream connections based on the downstream HTTP host/authority header or any other arbitrary - // header when :ref:`alt_header_name ` + // header when :ref:`override_auto_sni_header ` // is set, as seen by the :ref:`router filter `. bool auto_sni = 1; // Automatic validate upstream presented certificate for new upstream connections based on the - // downstream HTTP host/authority header or any other arbitrary header when :ref:`alt_header_name ` + // downstream HTTP host/authority header or any other arbitrary header when :ref:`override_auto_sni_header ` // is set, as seen by the :ref:`router filter `. // This field is intended to be set with `auto_sni` field. bool auto_san_validation = 2; @@ -74,9 +74,12 @@ message UpstreamHttpProtocolOptions { // An optional alternative to the host/authority header to be used for setting the SNI value. // It should be a valid downstream HTTP header, as seen by the // :ref:`router filter `. - // If unset, host/authority header will be used for populating the SNI. - // This field is intended to be set with `auto_sni` field. - string alt_header_name = 3 [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME}]; + // If unset, host/authority header will be used for populating the SNI. If the specified header + // is not found or the value is empty, host/authority header will be used instead. + // This field is intended to be set with `auto_sni` and/or `auto_san_validation` fields. + // If none of these fields are set then setting this would be a no-op. + string override_auto_sni_header = 3 + [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME ignore_empty: true}]; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/docs/root/faq/configuration/sni.rst b/docs/root/faq/configuration/sni.rst index 29dac111b4ebb..e7bbdf1fb0b9f 100644 --- a/docs/root/faq/configuration/sni.rst +++ b/docs/root/faq/configuration/sni.rst @@ -73,7 +73,7 @@ For clusters, a fixed SNI can be set in :ref:`UpstreamTlsContext ` to override the fixed SNI in `UpstreamTlsContext`. A custom header other than the ``host`` or ``:authority`` can also be supplied using the optional -:ref:`alt_header_name ` field. +:ref:`override_auto_sni_header ` field. If upstream will present certificates with the hostname in SAN, turn on :ref:`auto_san_validation ` too. It still needs a trust CA in validation context in ``UpstreamTlsContext`` for trust anchor. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index feaed1ebc5f32..2f34303e92adb 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -112,7 +112,7 @@ New Features * overload: add a new overload action that resets streams using a lot of memory. To enable the tracking of allocated bytes in buffers that a stream is using we need to configure the minimum threshold for tracking via:ref:`buffer_factory_config `. We have an overload action ``Envoy::Server::OverloadActionNameValues::ResetStreams`` that takes advantage of the tracking to reset the most expensive stream first. * rbac: added :ref:`destination_port_range ` for matching range of destination ports. * route config: added :ref:`dynamic_metadata ` for routing based on dynamic metadata. -* router: added an optional :ref:`alt_header_name ` to support setting SNI value from an arbitrary header other than host/authority. +* router: added an optional :ref:`override_auto_sni_header ` to support setting SNI value from an arbitrary header other than host/authority. * sxg_filter: added filter to transform response to SXG package to :ref:`contrib images `. This can be enabled by setting :ref:`SXG ` configuration. * thrift_proxy: added support for :ref:`mirroring requests `. diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 18a643fee55dd..93ac83718b5be 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -61,12 +61,12 @@ message UpstreamHttpProtocolOptions { // Set transport socket `SNI `_ for new // upstream connections based on the downstream HTTP host/authority header or any other arbitrary - // header when :ref:`alt_header_name ` + // header when :ref:`override_auto_sni_header ` // is set, as seen by the :ref:`router filter `. bool auto_sni = 1; // Automatic validate upstream presented certificate for new upstream connections based on the - // downstream HTTP host/authority header or any other arbitrary header when :ref:`alt_header_name ` + // downstream HTTP host/authority header or any other arbitrary header when :ref:`override_auto_sni_header ` // is set, as seen by the :ref:`router filter `. // This field is intended to be set with `auto_sni` field. bool auto_san_validation = 2; @@ -74,9 +74,12 @@ message UpstreamHttpProtocolOptions { // An optional alternative to the host/authority header to be used for setting the SNI value. // It should be a valid downstream HTTP header, as seen by the // :ref:`router filter `. - // If unset, host/authority header will be used for populating the SNI. - // This field is intended to be set with `auto_sni` field. - string alt_header_name = 3 [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME}]; + // If unset, host/authority header will be used for populating the SNI. If the specified header + // is not found or the value is empty, host/authority header will be used instead. + // This field is intended to be set with `auto_sni` and/or `auto_san_validation` fields. + // If none of these fields are set then setting this would be a no-op. + string override_auto_sni_header = 3 + [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME ignore_empty: true}]; } // Configures the alternate protocols cache which tracks alternate protocols that can be used to diff --git a/source/common/router/router.cc b/source/common/router/router.cc index bce6ef32d5ccf..c8874c6923373 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -502,41 +502,42 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // Fetch a connection pool for the upstream cluster. const auto& upstream_http_protocol_options = cluster_->upstreamHttpProtocolOptions(); - if (upstream_http_protocol_options.has_value()) { - absl::string_view sni_value; - bool should_set_sni = true; - - // Check whether `alt_header_name` is specified. - const auto alt_header_name = upstream_http_protocol_options.value().alt_header_name(); - if (alt_header_name.empty()) { - // Default the SNI value to the Host/Authority header. - const auto parsed_authority = Http::Utility::parseAuthority(headers.getHostValue()); - should_set_sni = !parsed_authority.is_ip_address_; - sni_value = std::string(parsed_authority.host_); - } else { - // Use the header value from `alt_header_name` to set the SNI value. - const auto header_value = Http::HeaderUtility::getAllOfHeaderAsString( - headers, Http::LowerCaseString(alt_header_name)); - if (header_value.result().has_value()) { - sni_value = std::string(header_value.result().value()); + if (upstream_http_protocol_options.has_value() && + (upstream_http_protocol_options.value().auto_sni() || + upstream_http_protocol_options.value().auto_san_validation())) { + // Default the header to Host/Authority header. + absl::string_view header_value = headers.getHostValue(); + + // Check whether `override_auto_sni_header` is specified. + const auto override_auto_sni_header = + upstream_http_protocol_options.value().override_auto_sni_header(); + if (!override_auto_sni_header.empty()) { + // Use the header value from `override_auto_sni_header` to set the SNI value. + const auto overridden_header_value = Http::HeaderUtility::getAllOfHeaderAsString( + headers, Http::LowerCaseString(override_auto_sni_header)); + if (overridden_header_value.result().has_value() && + !overridden_header_value.result().value().empty()) { + header_value = overridden_header_value.result().value(); } } + const auto parsed_authority = Http::Utility::parseAuthority(header_value); + bool should_set_sni = !parsed_authority.is_ip_address_; + // `host_` returns a string_view so doing this should be safe. + absl::string_view sni_value = parsed_authority.host_; + + if (should_set_sni && upstream_http_protocol_options.value().auto_sni()) { + callbacks_->streamInfo().filterState()->setData( + Network::UpstreamServerName::key(), + std::make_unique(sni_value), + StreamInfo::FilterState::StateType::Mutable); + } - if (!sni_value.empty()) { - if (should_set_sni && upstream_http_protocol_options.value().auto_sni()) { - callbacks_->streamInfo().filterState()->setData( - Network::UpstreamServerName::key(), - std::make_unique(sni_value), - StreamInfo::FilterState::StateType::Mutable); - } - - if (upstream_http_protocol_options.value().auto_san_validation()) { - callbacks_->streamInfo().filterState()->setData( - Network::UpstreamSubjectAltNames::key(), - std::make_unique( - std::vector{std::string(sni_value).c_str()}), - StreamInfo::FilterState::StateType::Mutable); - } + if (upstream_http_protocol_options.value().auto_san_validation()) { + callbacks_->streamInfo().filterState()->setData( + Network::UpstreamSubjectAltNames::key(), + std::make_unique( + std::vector{std::string(sni_value)}), + StreamInfo::FilterState::StateType::Mutable); } } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 90ae361e36342..c4158aa478e98 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -147,7 +147,7 @@ TEST_F(RouterTest, UpdateServerNameFilterState) { NiceMock stream_info; auto dummy_option = absl::make_optional(); dummy_option.value().set_auto_sni(true); - dummy_option.value().set_alt_header_name(":authority"); + dummy_option.value().set_override_auto_sni_header(":authority"); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(dummy_option)); ON_CALL(callbacks_.stream_info_, filterState()) @@ -179,7 +179,7 @@ TEST_F(RouterTest, UpdateServerNameFilterState) { TEST_F(RouterTest, UpdateSubjectAltNamesFilterState) { NiceMock stream_info; auto dummy_option = absl::make_optional(); - dummy_option.value().set_alt_header_name(":authority"); + dummy_option.value().set_override_auto_sni_header(":authority"); dummy_option.value().set_auto_san_validation(true); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(dummy_option)); diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index 7d79121fb348f..ee69025669734 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -19,7 +19,7 @@ class ProxyFilterIntegrationTest : public testing::TestWithParammutable_validate_clusters()->set_value(false); @@ -68,9 +68,9 @@ name: dynamic_forward_proxy ConfigHelper::HttpProtocolOptions protocol_options; protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); - if (!alt_header_name.empty()) { - protocol_options.mutable_upstream_http_protocol_options()->set_alt_header_name( - alt_header_name); + if (!override_auto_sni_header.empty()) { + protocol_options.mutable_upstream_http_protocol_options()->set_override_auto_sni_header( + override_auto_sni_header); } protocol_options.mutable_upstream_http_protocol_options()->set_auto_san_validation(true); protocol_options.mutable_explicit_http_config()->mutable_http_protocol_options(); @@ -287,7 +287,8 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTls) { checkSimpleRequestSuccess(0, 0, response.get()); } -// Verify that alt_header_name can be used along with auto_sni to set SNI from an arbitrary header. +// Verify that `override_auto_sni_header` can be used along with auto_sni to set SNI from an +// arbitrary header. TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithAltHeaderSni) { upstream_tls_ = true; initializeWithArgs(1024, 1024, "x-host"); diff --git a/test/extensions/filters/http/router/auto_sni_integration_test.cc b/test/extensions/filters/http/router/auto_sni_integration_test.cc index ab5d3f1861d86..4c8307125d1a7 100644 --- a/test/extensions/filters/http/router/auto_sni_integration_test.cc +++ b/test/extensions/filters/http/router/auto_sni_integration_test.cc @@ -18,17 +18,17 @@ class AutoSniIntegrationTest : public testing::TestWithParammutable_clusters()->at(0); ConfigHelper::HttpProtocolOptions protocol_options; protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); - if (!alt_header_name.empty()) { - protocol_options.mutable_upstream_http_protocol_options()->set_alt_header_name( - alt_header_name); + if (!override_auto_sni_header.empty()) { + protocol_options.mutable_upstream_http_protocol_options()->set_override_auto_sni_header( + override_auto_sni_header); } ConfigHelper::setProtocolOptions( *bootstrap.mutable_static_resources()->mutable_clusters(0), protocol_options); From 4d18e75eb0978a15098fe92e85a18c59c4c6a2cf Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Thu, 9 Sep 2021 17:58:21 -0400 Subject: [PATCH 4/7] Added Tests Signed-off-by: Rohit Agrawal --- test/common/router/router_test.cc | 148 ++++++++++++++++++++---------- 1 file changed, 100 insertions(+), 48 deletions(-) diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index c4158aa478e98..fd9b13d0ee2d1 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -141,67 +141,119 @@ class RouterTest : public RouterTestBase { router_.onDestroy(); } + + void testAutoSniOptions( + absl::optional dummy_option, + Envoy::Http::TestRequestHeaderMapImpl* headers, std::string server_name = "host", + bool should_validate_san = false) { + NiceMock stream_info; + ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) + .WillByDefault(ReturnRef(dummy_option)); + ON_CALL(callbacks_.stream_info_, filterState()) + .WillByDefault(ReturnRef(stream_info.filterState())); + EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_, newStream(_, _)) + .WillOnce(Return(&cancellable_)); + stream_info.filterState()->setData(Network::UpstreamServerName::key(), + std::make_unique("dummy"), + StreamInfo::FilterState::StateType::Mutable); + expectResponseTimerCreate(); + + HttpTestUtility::addDefaultHeaders(*headers); + router_.decodeHeaders(*headers, true); + EXPECT_EQ(server_name, + stream_info.filterState() + ->getDataReadOnly(Network::UpstreamServerName::key()) + .value()); + if (should_validate_san) { + EXPECT_EQ(server_name, stream_info.filterState() + ->getDataReadOnly( + Network::UpstreamSubjectAltNames::key()) + .value()[0]); + } + EXPECT_CALL(cancellable_, cancel(_)); + router_.onDestroy(); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); + EXPECT_EQ(0U, + callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); + EXPECT_EQ(0U, + callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); + } }; -TEST_F(RouterTest, UpdateServerNameFilterState) { - NiceMock stream_info; +TEST_F(RouterTest, UpdateServerNameFilterStateWithoutHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + + auto headers = new Http::TestRequestHeaderMapImpl{}; + testAutoSniOptions(dummy_option, headers); +} + +TEST_F(RouterTest, UpdateServerNameFilterStateWithHostHeaderOverride) { auto dummy_option = absl::make_optional(); dummy_option.value().set_auto_sni(true); dummy_option.value().set_override_auto_sni_header(":authority"); - ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) - .WillByDefault(ReturnRef(dummy_option)); - ON_CALL(callbacks_.stream_info_, filterState()) - .WillByDefault(ReturnRef(stream_info.filterState())); - EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_, newStream(_, _)) - .WillOnce(Return(&cancellable_)); - stream_info.filterState()->setData(Network::UpstreamServerName::key(), - std::make_unique("dummy"), - StreamInfo::FilterState::StateType::Mutable); - expectResponseTimerCreate(); - Http::TestRequestHeaderMapImpl headers; + auto headers = new Http::TestRequestHeaderMapImpl{}; + testAutoSniOptions(dummy_option, headers); +} - HttpTestUtility::addDefaultHeaders(headers); - router_.decodeHeaders(headers, true); - EXPECT_EQ("host", - stream_info.filterState() - ->getDataReadOnly(Network::UpstreamServerName::key()) - .value()); - EXPECT_CALL(cancellable_, cancel(_)); - router_.onDestroy(); - EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); - EXPECT_EQ(0U, - callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); - EXPECT_EQ(0U, - callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); +TEST_F(RouterTest, UpdateServerNameFilterStateWithHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_override_auto_sni_header("x-host"); + + const auto server_name = "foo.bar"; + auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + testAutoSniOptions(dummy_option, headers, server_name); +} + +TEST_F(RouterTest, UpdateServerNameFilterStateWithEmptyValueHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_override_auto_sni_header("x-host"); + + auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", ""}}; + testAutoSniOptions(dummy_option, headers); +} + +TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithoutHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_auto_san_validation(true); + + auto headers = new Http::TestRequestHeaderMapImpl{}; + testAutoSniOptions(dummy_option, headers, "host", true); } -TEST_F(RouterTest, UpdateSubjectAltNamesFilterState) { - NiceMock stream_info; +TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithHostHeaderOverride) { auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_auto_san_validation(true); dummy_option.value().set_override_auto_sni_header(":authority"); + + auto headers = new Http::TestRequestHeaderMapImpl{}; + testAutoSniOptions(dummy_option, headers, "host", true); +} + +TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); dummy_option.value().set_auto_san_validation(true); - ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) - .WillByDefault(ReturnRef(dummy_option)); - ON_CALL(callbacks_.stream_info_, filterState()) - .WillByDefault(ReturnRef(stream_info.filterState())); - EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_, newStream(_, _)) - .WillOnce(Return(&cancellable_)); - expectResponseTimerCreate(); + dummy_option.value().set_override_auto_sni_header("x-host"); - Http::TestRequestHeaderMapImpl headers; + const auto server_name = "foo.bar"; + auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + testAutoSniOptions(dummy_option, headers, server_name, true); +} - HttpTestUtility::addDefaultHeaders(headers); - router_.decodeHeaders(headers, true); - EXPECT_EQ("host", stream_info.filterState() - ->getDataReadOnly( - Network::UpstreamSubjectAltNames::key()) - .value()[0]); - EXPECT_CALL(cancellable_, cancel(_)); - router_.onDestroy(); - EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); - EXPECT_EQ(0U, - callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); +TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithEmptyValueHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_auto_san_validation(true); + dummy_option.value().set_override_auto_sni_header("x-host"); + + auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", ""}}; + testAutoSniOptions(dummy_option, headers, "host", true); } TEST_F(RouterTest, RouteNotFound) { @@ -6152,7 +6204,7 @@ TEST_F(RouterTest, NotSetDynamicMaxStreamDurationIfZero) { return nullptr; })); - // The timer will not be created. + // The timer will not be created.over EXPECT_CALL(callbacks_.dispatcher_, createTimer_).Times(0); Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-stream-duration-ms", "0"}}; From f8193ea035126e6d789fce538b7a7439f154863b Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Thu, 9 Sep 2021 18:51:24 -0400 Subject: [PATCH 5/7] Add More Tests Signed-off-by: Rohit Agrawal --- test/common/router/router_test.cc | 23 +++++++++---- .../proxy_filter_integration_test.cc | 33 +++++++++++++++++-- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index fd9b13d0ee2d1..2c3b6fa0c4678 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -145,7 +145,7 @@ class RouterTest : public RouterTestBase { void testAutoSniOptions( absl::optional dummy_option, Envoy::Http::TestRequestHeaderMapImpl* headers, std::string server_name = "host", - bool should_validate_san = false) { + bool should_validate_san = false, std::string alt_server_name = "host") { NiceMock stream_info; ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(dummy_option)); @@ -165,10 +165,10 @@ class RouterTest : public RouterTestBase { ->getDataReadOnly(Network::UpstreamServerName::key()) .value()); if (should_validate_san) { - EXPECT_EQ(server_name, stream_info.filterState() - ->getDataReadOnly( - Network::UpstreamSubjectAltNames::key()) - .value()[0]); + EXPECT_EQ(alt_server_name, stream_info.filterState() + ->getDataReadOnly( + Network::UpstreamSubjectAltNames::key()) + .value()[0]); } EXPECT_CALL(cancellable_, cancel(_)); router_.onDestroy(); @@ -243,7 +243,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithHeaderOverride) { const auto server_name = "foo.bar"; auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; - testAutoSniOptions(dummy_option, headers, server_name, true); + testAutoSniOptions(dummy_option, headers, server_name, true, server_name); } TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithEmptyValueHeaderOverride) { @@ -256,6 +256,17 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithEmptyValueHeaderOverride) testAutoSniOptions(dummy_option, headers, "host", true); } +TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithIpHeaderOverride) { + auto dummy_option = absl::make_optional(); + dummy_option.value().set_auto_sni(true); + dummy_option.value().set_auto_san_validation(true); + dummy_option.value().set_override_auto_sni_header("x-host"); + + const auto server_name = "127.0.0.1"; + auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + testAutoSniOptions(dummy_option, headers, "dummy", true, server_name); +} + TEST_F(RouterTest, RouteNotFound) { EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound)); diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index ee69025669734..5e3056a451036 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -287,8 +287,8 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTls) { checkSimpleRequestSuccess(0, 0, response.get()); } -// Verify that `override_auto_sni_header` can be used along with auto_sni to set SNI from an -// arbitrary header. +// Verify that `override_auto_sni_header` can be used along with auto_sni to set +// SNI from an arbitrary header. TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithAltHeaderSni) { upstream_tls_ = true; initializeWithArgs(1024, 1024, "x-host"); @@ -315,6 +315,35 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithAltHeaderSni) { checkSimpleRequestSuccess(0, 0, response.get()); } +// Verify that if `override_auto_sni_header` point to an IP address then we don't +// populate the SNI but set the SAN. +TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithNoSniAndAltHeader) { + upstream_tls_ = true; + initializeWithArgs(1024, 1024, "custom-header"); + codec_client_ = makeHttpConnection(lookupPort("http")); + const Http::TestRequestHeaderMapImpl request_headers{ + {":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", + fmt::format("{}:{}", fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str(), + fake_upstreams_[0]->localAddress()->ip()->port())}, + {"custom-header", + fmt::format("{}", fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str())}}; + + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + waitForNextUpstreamRequest(); + + const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket = + dynamic_cast( + fake_upstream_connection_->connection().ssl().get()); + EXPECT_STREQ(nullptr, SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name)); + + upstream_request_->encodeHeaders(default_response_headers_, true); + ASSERT_TRUE(response->waitForEndStream()); + checkSimpleRequestSuccess(0, 0, response.get()); +} + TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithIpHost) { upstream_tls_ = true; initializeWithArgs(); From 1bc69d44ff16e784c0160b690e16e00631aea6c8 Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Thu, 9 Sep 2021 20:24:56 -0400 Subject: [PATCH 6/7] Remove Test (Hanging on IPv6) Signed-off-by: Rohit Agrawal --- test/common/router/router_test.cc | 24 +++++++-------- .../proxy_filter_integration_test.cc | 29 ------------------- 2 files changed, 12 insertions(+), 41 deletions(-) diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 2c3b6fa0c4678..68f69525abf4b 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -144,7 +144,7 @@ class RouterTest : public RouterTestBase { void testAutoSniOptions( absl::optional dummy_option, - Envoy::Http::TestRequestHeaderMapImpl* headers, std::string server_name = "host", + Envoy::Http::TestRequestHeaderMapImpl headers, std::string server_name = "host", bool should_validate_san = false, std::string alt_server_name = "host") { NiceMock stream_info; ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, upstreamHttpProtocolOptions()) @@ -158,8 +158,8 @@ class RouterTest : public RouterTestBase { StreamInfo::FilterState::StateType::Mutable); expectResponseTimerCreate(); - HttpTestUtility::addDefaultHeaders(*headers); - router_.decodeHeaders(*headers, true); + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); EXPECT_EQ(server_name, stream_info.filterState() ->getDataReadOnly(Network::UpstreamServerName::key()) @@ -184,7 +184,7 @@ TEST_F(RouterTest, UpdateServerNameFilterStateWithoutHeaderOverride) { auto dummy_option = absl::make_optional(); dummy_option.value().set_auto_sni(true); - auto headers = new Http::TestRequestHeaderMapImpl{}; + Http::TestRequestHeaderMapImpl headers{}; testAutoSniOptions(dummy_option, headers); } @@ -193,7 +193,7 @@ TEST_F(RouterTest, UpdateServerNameFilterStateWithHostHeaderOverride) { dummy_option.value().set_auto_sni(true); dummy_option.value().set_override_auto_sni_header(":authority"); - auto headers = new Http::TestRequestHeaderMapImpl{}; + Http::TestRequestHeaderMapImpl headers{}; testAutoSniOptions(dummy_option, headers); } @@ -203,7 +203,7 @@ TEST_F(RouterTest, UpdateServerNameFilterStateWithHeaderOverride) { dummy_option.value().set_override_auto_sni_header("x-host"); const auto server_name = "foo.bar"; - auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + Http::TestRequestHeaderMapImpl headers{{"x-host", server_name}}; testAutoSniOptions(dummy_option, headers, server_name); } @@ -212,7 +212,7 @@ TEST_F(RouterTest, UpdateServerNameFilterStateWithEmptyValueHeaderOverride) { dummy_option.value().set_auto_sni(true); dummy_option.value().set_override_auto_sni_header("x-host"); - auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", ""}}; + Http::TestRequestHeaderMapImpl headers{{"x-host", ""}}; testAutoSniOptions(dummy_option, headers); } @@ -221,7 +221,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithoutHeaderOverride) { dummy_option.value().set_auto_sni(true); dummy_option.value().set_auto_san_validation(true); - auto headers = new Http::TestRequestHeaderMapImpl{}; + Http::TestRequestHeaderMapImpl headers{}; testAutoSniOptions(dummy_option, headers, "host", true); } @@ -231,7 +231,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithHostHeaderOverride) { dummy_option.value().set_auto_san_validation(true); dummy_option.value().set_override_auto_sni_header(":authority"); - auto headers = new Http::TestRequestHeaderMapImpl{}; + Http::TestRequestHeaderMapImpl headers{}; testAutoSniOptions(dummy_option, headers, "host", true); } @@ -242,7 +242,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithHeaderOverride) { dummy_option.value().set_override_auto_sni_header("x-host"); const auto server_name = "foo.bar"; - auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + Http::TestRequestHeaderMapImpl headers{{"x-host", server_name}}; testAutoSniOptions(dummy_option, headers, server_name, true, server_name); } @@ -252,7 +252,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithEmptyValueHeaderOverride) dummy_option.value().set_auto_san_validation(true); dummy_option.value().set_override_auto_sni_header("x-host"); - auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", ""}}; + Http::TestRequestHeaderMapImpl headers{{"x-host", ""}}; testAutoSniOptions(dummy_option, headers, "host", true); } @@ -263,7 +263,7 @@ TEST_F(RouterTest, UpdateSubjectAltNamesFilterStateWithIpHeaderOverride) { dummy_option.value().set_override_auto_sni_header("x-host"); const auto server_name = "127.0.0.1"; - auto headers = new Http::TestRequestHeaderMapImpl{{"x-host", server_name}}; + Http::TestRequestHeaderMapImpl headers{{"x-host", server_name}}; testAutoSniOptions(dummy_option, headers, "dummy", true, server_name); } diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index 5e3056a451036..49c72aff1cbb4 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -315,35 +315,6 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithAltHeaderSni) { checkSimpleRequestSuccess(0, 0, response.get()); } -// Verify that if `override_auto_sni_header` point to an IP address then we don't -// populate the SNI but set the SAN. -TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithNoSniAndAltHeader) { - upstream_tls_ = true; - initializeWithArgs(1024, 1024, "custom-header"); - codec_client_ = makeHttpConnection(lookupPort("http")); - const Http::TestRequestHeaderMapImpl request_headers{ - {":method", "POST"}, - {":path", "/test/long/url"}, - {":scheme", "http"}, - {":authority", - fmt::format("{}:{}", fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str(), - fake_upstreams_[0]->localAddress()->ip()->port())}, - {"custom-header", - fmt::format("{}", fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str())}}; - - auto response = codec_client_->makeHeaderOnlyRequest(request_headers); - waitForNextUpstreamRequest(); - - const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket = - dynamic_cast( - fake_upstream_connection_->connection().ssl().get()); - EXPECT_STREQ(nullptr, SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name)); - - upstream_request_->encodeHeaders(default_response_headers_, true); - ASSERT_TRUE(response->waitForEndStream()); - checkSimpleRequestSuccess(0, 0, response.get()); -} - TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithIpHost) { upstream_tls_ = true; initializeWithArgs(); From a0aeb8520171e251e5281d74cbd25bf6567fb147 Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Mon, 13 Sep 2021 11:58:32 -0400 Subject: [PATCH 7/7] Fixed Typo Signed-off-by: Rohit Agrawal --- test/common/router/router_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 68f69525abf4b..20ae437c6380e 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -6215,7 +6215,7 @@ TEST_F(RouterTest, NotSetDynamicMaxStreamDurationIfZero) { return nullptr; })); - // The timer will not be created.over + // The timer will not be created. EXPECT_CALL(callbacks_.dispatcher_, createTimer_).Times(0); Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-stream-duration-ms", "0"}};