diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 423ec11b8cd9f..9890ac1cc1eb8 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.core.v3; import "envoy/config/core/v3/extension.proto"; +import "envoy/type/matcher/v3/string.proto"; import "envoy/type/v3/percent.proto"; import "google/protobuf/duration.proto"; @@ -302,7 +303,7 @@ message HttpProtocolOptions { google.protobuf.UInt32Value max_requests_per_connection = 6; } -// [#next-free-field: 11] +// [#next-free-field: 12] message Http1ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.Http1ProtocolOptions"; @@ -414,6 +415,14 @@ message Http1ProtocolOptions { // ` // to reject custom methods. bool allow_custom_methods = 10 [(xds.annotations.v3.field_status).work_in_progress = true]; + + // Ignore HTTP/1.1 upgrade values matching any of the supplied matchers. + // + // .. note:: + // + // ``h2c`` upgrades are always removed for backwards compatibility, regardless of the + // value in this setting. + repeated type.matcher.v3.StringMatcher ignore_http_11_upgrade = 11; } message KeepaliveSettings { diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9ecf0d6e48ce5..119cc3a92cea3 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -13,5 +13,10 @@ removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` new_features: +- area: http + change: | + Added :ref:`ignore_http_11_upgrade + ` + to ignore HTTP/1.1 Upgrade values matching any of the supplied matchers. deprecated: diff --git a/envoy/http/BUILD b/envoy/http/BUILD index 0d4854c722b43..a97d5e9275939 100644 --- a/envoy/http/BUILD +++ b/envoy/http/BUILD @@ -50,6 +50,7 @@ envoy_cc_library( ":stream_reset_handler_interface", "//envoy/access_log:access_log_interface", "//envoy/buffer:buffer_interface", + "//envoy/common:matchers_interface", "//envoy/grpc:status", "//envoy/network:address_interface", "//envoy/stream_info:stream_info_interface", diff --git a/envoy/http/codec.h b/envoy/http/codec.h index 0297a8a32356a..75f3e127c9184 100644 --- a/envoy/http/codec.h +++ b/envoy/http/codec.h @@ -6,6 +6,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/buffer/buffer.h" +#include "envoy/common/matchers.h" #include "envoy/common/pure.h" #include "envoy/grpc/status.h" #include "envoy/http/header_formatter.h" @@ -497,6 +498,9 @@ struct Http1Settings { // headers set. By default such messages are rejected, but if option is enabled - Envoy will // remove Content-Length header and process message. bool allow_chunked_length_{false}; + // Remove HTTP/1.1 Upgrade header tokens matching any provided matcher. By default such + // messages are rejected + std::shared_ptr> ignore_upgrade_matchers_; enum class HeaderKeyFormat { // By default no formatting is performed, presenting all headers in lowercase (as Envoy diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD index 564dfd31f354e..6e6e62ccb25e7 100644 --- a/source/common/http/http1/BUILD +++ b/source/common/http/http1/BUILD @@ -94,6 +94,7 @@ envoy_cc_library( deps = [ "//envoy/http:codec_interface", "//envoy/protobuf:message_validator_interface", + "//source/common/common:matchers_lib", "//source/common/config:utility_lib", "//source/common/runtime:runtime_features_lib", "@com_google_absl//absl/types:optional", diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 684b4b7dc71bf..a9234731034f7 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -65,7 +65,12 @@ static constexpr uint32_t kMaxOutboundResponses = 2; using Http1ResponseCodeDetails = ConstSingleton; using Http1HeaderTypes = ConstSingleton; -const StringUtil::CaseUnorderedSet& caseUnorderdSetContainingUpgradeAndHttp2Settings() { +const StringUtil::CaseUnorderedSet& caseUnorderedSetContainingUpgrade() { + CONSTRUCT_ON_FIRST_USE(StringUtil::CaseUnorderedSet, + Http::Headers::get().ConnectionValues.Upgrade); +} + +const StringUtil::CaseUnorderedSet& caseUnorderedSetContainingUpgradeAndHttp2Settings() { CONSTRUCT_ON_FIRST_USE(StringUtil::CaseUnorderedSet, Http::Headers::get().ConnectionValues.Upgrade, Http::Headers::get().ConnectionValues.Http2Settings); @@ -846,24 +851,33 @@ StatusOr ConnectionImpl::onHeadersCompleteImpl() { RequestOrResponseHeaderMap& request_or_response_headers = requestOrResponseHeaders(); const Http::HeaderValues& header_values = Http::Headers::get(); if (Utility::isUpgrade(request_or_response_headers) && upgradeAllowed()) { + auto upgrade_value = request_or_response_headers.getUpgradeValue(); + const bool is_h2c = absl::EqualsIgnoreCase(upgrade_value, header_values.UpgradeValues.H2c); + // Ignore h2c upgrade requests until we support them. // See https://github.com/envoyproxy/envoy/issues/7161 for details. - if (absl::EqualsIgnoreCase(request_or_response_headers.getUpgradeValue(), - header_values.UpgradeValues.H2c)) { + // Upgrades are rejected unless ignore_http_11_upgrade is configured. + // See https://github.com/envoyproxy/envoy/issues/36305 for details. + if (is_h2c) { ENVOY_CONN_LOG(trace, "removing unsupported h2c upgrade headers.", connection_); request_or_response_headers.removeUpgrade(); - if (request_or_response_headers.Connection()) { - const auto& tokens_to_remove = caseUnorderdSetContainingUpgradeAndHttp2Settings(); - std::string new_value = StringUtil::removeTokens( - request_or_response_headers.getConnectionValue(), ",", tokens_to_remove, ","); - if (new_value.empty()) { - request_or_response_headers.removeConnection(); - } else { - request_or_response_headers.setConnection(new_value); - } - } + Utility::removeConnectionUpgrade(request_or_response_headers, + caseUnorderedSetContainingUpgradeAndHttp2Settings()); request_or_response_headers.remove(header_values.Http2Settings); - } else { + } else if (codec_settings_.ignore_upgrade_matchers_ != nullptr && + !codec_settings_.ignore_upgrade_matchers_->empty()) { + ENVOY_CONN_LOG(trace, "removing ignored upgrade headers.", connection_); + + Utility::removeUpgrade(request_or_response_headers, + *codec_settings_.ignore_upgrade_matchers_); + + if (!request_or_response_headers.Upgrade()) { + Utility::removeConnectionUpgrade(request_or_response_headers, + caseUnorderedSetContainingUpgrade()); + } + } + + if (Utility::isUpgrade(request_or_response_headers)) { ENVOY_CONN_LOG(trace, "codec entering upgrade mode.", connection_); handling_upgrade_ = true; } diff --git a/source/common/http/http1/settings.cc b/source/common/http/http1/settings.cc index e74af4f491354..2662ec9d57c47 100644 --- a/source/common/http/http1/settings.cc +++ b/source/common/http/http1/settings.cc @@ -2,6 +2,7 @@ #include "envoy/http/header_formatter.h" +#include "source/common/common/matchers.h" #include "source/common/config/utility.h" #include "source/common/runtime/runtime_features.h" @@ -10,6 +11,7 @@ namespace Http { namespace Http1 { Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config, + Server::Configuration::CommonFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) { Http1Settings ret; ret.allow_absolute_url_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, allow_absolute_url, true); @@ -19,6 +21,18 @@ Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOpt ret.enable_trailers_ = config.enable_trailers(); ret.allow_chunked_length_ = config.allow_chunked_length(); + if (!config.ignore_http_11_upgrade().empty()) { + std::vector matchers; + for (const auto& matcher : config.ignore_http_11_upgrade()) { + matchers.emplace_back( + std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + } + ret.ignore_upgrade_matchers_ = + std::make_shared>(std::move(matchers)); + } + if (config.header_key_format().has_proper_case_words()) { ret.header_key_format_ = Http1Settings::HeaderKeyFormat::ProperCase; } else if (config.header_key_format().has_stateful_formatter()) { @@ -45,10 +59,11 @@ Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOpt } Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config, + Server::Configuration::CommonFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor, const ProtobufWkt::BoolValue& hcm_stream_error, bool validate_scheme) { - Http1Settings ret = parseHttp1Settings(config, validation_visitor); + Http1Settings ret = parseHttp1Settings(config, context, validation_visitor); ret.validate_scheme_ = validate_scheme; if (config.has_override_stream_error_on_invalid_http_message()) { diff --git a/source/common/http/http1/settings.h b/source/common/http/http1/settings.h index baf2cb4553945..51ed573b426e1 100644 --- a/source/common/http/http1/settings.h +++ b/source/common/http/http1/settings.h @@ -3,6 +3,7 @@ #include "envoy/config/core/v3/protocol.pb.h" #include "envoy/http/codec.h" #include "envoy/protobuf/message_validator.h" +#include "envoy/server/factory_context.h" namespace Envoy { namespace Http { @@ -13,9 +14,11 @@ namespace Http1 { * envoy::config::core::v3::Http1ProtocolOptions config. */ Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config, + Server::Configuration::CommonFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor); Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config, + Server::Configuration::CommonFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor, const ProtobufWkt::BoolValue& hcm_stream_error, bool validate_scheme); diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 4b37d158f5789..e1687fdb4f334 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -649,6 +649,41 @@ bool Utility::isWebSocketUpgradeRequest(const RequestHeaderMap& headers) { Http::Headers::get().UpgradeValues.WebSocket)); } +void Utility::removeUpgrade(RequestOrResponseHeaderMap& headers, + const std::vector& matchers) { + if (headers.Upgrade()) { + std::vector tokens = + Envoy::StringUtil::splitToken(headers.getUpgradeValue(), ",", false, true); + + auto end = std::remove_if(tokens.begin(), tokens.end(), [&](absl::string_view token) { + return std::any_of( + matchers.begin(), matchers.end(), + [&token](const Matchers::StringMatcherPtr& matcher) { return matcher->match(token); }); + }); + + const std::string new_value = absl::StrJoin(tokens.begin(), end, ","); + + if (new_value.empty()) { + headers.removeUpgrade(); + } else { + headers.setUpgrade(new_value); + } + } +} + +void Utility::removeConnectionUpgrade(RequestOrResponseHeaderMap& headers, + StringUtil::CaseUnorderedSet tokens_to_remove) { + if (headers.Connection()) { + const std::string new_value = + StringUtil::removeTokens(headers.getConnectionValue(), ",", tokens_to_remove, ","); + if (new_value.empty()) { + headers.removeConnection(); + } else { + headers.setConnection(new_value); + } + } +} + Utility::PreparedLocalReplyPtr Utility::prepareLocalReply(const EncodeFunctions& encode_functions, const LocalReplyData& local_reply_data) { Code response_code = local_reply_data.response_code_; diff --git a/source/common/http/utility.h b/source/common/http/utility.h index eaee0c34fae7c..c1cef92ea4527 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -315,6 +315,20 @@ bool isH3UpgradeRequest(const RequestHeaderMap& headers); */ bool isWebSocketUpgradeRequest(const RequestHeaderMap& headers); +/** + * Removes tokens from `Upgrade` header matching one of the matchers. Removes the `Upgrade` + * header if result is empty. + */ +void removeUpgrade(RequestOrResponseHeaderMap& headers, + const std::vector& matchers); + +/** + * Removes `tokens_to_remove` from the `Connection` header, if present and part of a comma separated + * set of values. Removes the `Connection` header if it only contains `tokens_to_remove`. + */ +void removeConnectionUpgrade(RequestOrResponseHeaderMap& headers, + StringUtil::CaseUnorderedSet tokens_to_remove); + struct EncodeFunctions { // Function to modify locally generated response headers. std::function modify_headers_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 1dafc23d65b88..a27b938563de6 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -1032,7 +1032,7 @@ ClusterInfoImpl::generateTimeoutBudgetStats(Stats::Scope& scope, absl::StatusOr> createOptions(const envoy::config::cluster::v3::Cluster& config, std::shared_ptr&& options, - ProtobufMessage::ValidationVisitor& validation_visitor) { + Server::Configuration::ProtocolOptionsFactoryContext& factory_context) { if (options) { return std::move(options); } @@ -1056,7 +1056,8 @@ createOptions(const envoy::config::cluster::v3::Cluster& config, : absl::nullopt), config.protocol_selection() == envoy::config::cluster::v3::Cluster::USE_DOWNSTREAM_PROTOCOL, - config.has_http2_protocol_options(), validation_visitor); + config.has_http2_protocol_options(), factory_context.serverFactoryContext(), + factory_context.messageValidationVisitor()); RETURN_IF_NOT_OK_REF(options_or_error.status()); return options_or_error.value(); } @@ -1170,7 +1171,7 @@ ClusterInfoImpl::ClusterInfoImpl( createOptions(config, extensionProtocolOptionsTyped( "envoy.extensions.upstreams.http.v3.HttpProtocolOptions"), - factory_context.messageValidationVisitor()), + factory_context), std::shared_ptr)), tcp_protocol_options_(extensionProtocolOptionsTyped( "envoy.extensions.upstreams.tcp.v3.TcpProtocolOptions")), diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index f038ce870ae57..892ad9bfb25a8 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -371,8 +371,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( config.http3_protocol_options(), config.has_stream_error_on_invalid_http_message(), config.stream_error_on_invalid_http_message())), http1_settings_(Http::Http1::parseHttp1Settings( - config.http_protocol_options(), context.messageValidationVisitor(), - config.stream_error_on_invalid_http_message(), + config.http_protocol_options(), context.serverFactoryContext(), + context.messageValidationVisitor(), config.stream_error_on_invalid_http_message(), xff_num_trusted_hops_ == 0 && use_remote_address_)), max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( config, max_request_headers_kb, diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 8acda93047c8c..16fb8aef8fb70 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -206,12 +206,13 @@ ProtocolOptionsConfigImpl::createProtocolOptionsConfig( const envoy::config::core::v3::HttpProtocolOptions& common_options, const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2, + Server::Configuration::ServerFactoryContext& server_context, ProtobufMessage::ValidationVisitor& validation_visitor) { auto options_or_error = Http2::Utility::initializeAndValidateOptions(http2_options); RETURN_IF_NOT_OK_REF(options_or_error.status()); return std::shared_ptr(new ProtocolOptionsConfigImpl( http1_settings, options_or_error.value(), common_options, upstream_options, - use_downstream_protocol, use_http2, validation_visitor)); + use_downstream_protocol, use_http2, server_context, validation_visitor)); } ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( @@ -221,7 +222,7 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( absl::optional cache_options, Server::Configuration::ServerFactoryContext& server_context) : http1_settings_(Envoy::Http::Http1::parseHttp1Settings( - getHttpOptions(options), server_context.messageValidationVisitor())), + getHttpOptions(options), server_context, server_context.messageValidationVisitor())), http2_options_(std::move(http2_options)), http3_options_(getHttp3Options(options)), common_http_protocol_options_(options.common_http_protocol_options()), upstream_http_protocol_options_( @@ -244,8 +245,10 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( const envoy::config::core::v3::HttpProtocolOptions& common_options, const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2, + Server::Configuration::ServerFactoryContext& server_context, ProtobufMessage::ValidationVisitor& validation_visitor) - : http1_settings_(Envoy::Http::Http1::parseHttp1Settings(http1_settings, validation_visitor)), + : http1_settings_(Envoy::Http::Http1::parseHttp1Settings(http1_settings, server_context, + validation_visitor)), http2_options_(validated_http2_options), common_http_protocol_options_(common_options), upstream_http_protocol_options_(upstream_options), use_downstream_protocol_(use_downstream_protocol), use_http2_(use_http2) { diff --git a/source/extensions/upstreams/http/config.h b/source/extensions/upstreams/http/config.h index fcc49e722a3d4..75eabed5a5ca0 100644 --- a/source/extensions/upstreams/http/config.h +++ b/source/extensions/upstreams/http/config.h @@ -35,6 +35,7 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { const envoy::config::core::v3::HttpProtocolOptions& common_options, const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2, + Server::Configuration::ServerFactoryContext& server_context, ProtobufMessage::ValidationVisitor& validation_visitor); // Given the supplied cluster config, and protocol options configuration, @@ -74,6 +75,7 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { const envoy::config::core::v3::HttpProtocolOptions& common_options, const absl::optional upstream_options, bool use_downstream_protocol, bool use_http2, + Server::Configuration::ServerFactoryContext& server_context, ProtobufMessage::ValidationVisitor& validation_visitor); }; diff --git a/test/common/http/BUILD b/test/common/http/BUILD index f97c7d141c289..c3dfb5752f320 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -442,6 +442,7 @@ envoy_cc_test( "//source/common/http:utility_lib", "//source/common/network:address_lib", "//test/mocks/http:http_mocks", + "//test/mocks/server:factory_context_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", diff --git a/test/common/http/http1/BUILD b/test/common/http/http1/BUILD index 5a76eb737f947..d1c96c8618fbb 100644 --- a/test/common/http/http1/BUILD +++ b/test/common/http/http1/BUILD @@ -39,6 +39,7 @@ envoy_cc_test( "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/server:factory_context_mocks", "//test/mocks/server:overload_manager_mocks", "//test/mocks/stream_info:stream_info_mocks", "//test/mocks/thread_local:thread_local_mocks", diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 8687d4b12687e..c778b3a36c031 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -22,6 +22,7 @@ #include "test/mocks/buffer/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/factory_context.h" #include "test/mocks/server/overload_manager.h" #include "test/test_common/logging.h" #include "test/test_common/printers.h" @@ -2139,6 +2140,90 @@ TEST_P(Http1ServerConnectionImplTest, IgnoreUpgradeH2cCloseEtc) { expectHeadersTest(Protocol::Http11, true, buffer, expected_headers); } +TEST_P(Http1ServerConnectionImplTest, IgnoreSpecificTLSVersionUpgradeRequest) { + NiceMock context; + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_exact("TLS/1.2"); + std::vector matchers; + matchers.push_back( + std::make_unique>( + matcher, context)); + codec_settings_.ignore_upgrade_matchers_ = + std::make_shared>(std::move(matchers)); + + initialize(); + + TestRequestHeaderMapImpl expected_headers{ + {":authority", "www.somewhere.com"}, {":scheme", "http"}, {":path", "/"}, {":method", "GET"}}; + Buffer::OwnedImpl buffer("GET http://www.somewhere.com/ HTTP/1.1\r\nConnection: Upgrade\r\n" + "Upgrade: TLS/1.2\r\nHost: bah\r\n\r\n"); + expectHeadersTest(Protocol::Http11, true, buffer, expected_headers); +} + +TEST_P(Http1ServerConnectionImplTest, IgnorePrefixUpgradeRequest) { + NiceMock context; + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("TLS/"); + std::vector matchers; + matchers.push_back( + std::make_unique>( + matcher, context)); + codec_settings_.ignore_upgrade_matchers_ = + std::make_shared>(std::move(matchers)); + + initialize(); + + TestRequestHeaderMapImpl expected_headers{ + {":authority", "www.somewhere.com"}, {":scheme", "http"}, {":path", "/"}, {":method", "GET"}}; + Buffer::OwnedImpl buffer("GET http://www.somewhere.com/ HTTP/1.1\r\nConnection: Upgrade\r\n" + "Upgrade: TLS/1.1, TLS/1.2\r\nHost: bah\r\n\r\n"); + expectHeadersTest(Protocol::Http11, true, buffer, expected_headers); +} + +TEST_P(Http1ServerConnectionImplTest, PartialIgnoreUpgradeRequest) { + NiceMock context; + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_exact("TLS/1.2"); + std::vector matchers; + matchers.push_back( + std::make_unique>( + matcher, context)); + codec_settings_.ignore_upgrade_matchers_ = + std::make_shared>(std::move(matchers)); + + initialize(); + + InSequence sequence; + NiceMock decoder; + EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); + + TestRequestHeaderMapImpl expected_headers{ + {":path", "/"}, {":method", "GET"}, {"connection", "upgrade"}, {"upgrade", "TLS/1.1"}}; + EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), false)); + + Buffer::OwnedImpl buffer( + "GET / HTTP/1.1\r\nConnection: upgrade\r\nUpgrade: TLS/1.1, TLS/1.2\r\n\r\n"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); +} + +TEST_P(Http1ServerConnectionImplTest, NoIgnoreUpgradeRequest) { + initialize(); + + InSequence sequence; + NiceMock decoder; + EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); + + TestRequestHeaderMapImpl expected_headers{ + {":path", "/"}, {":method", "GET"}, {"connection", "upgrade"}, {"upgrade", "TLS/1.2"}}; + EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), false)); + ; + + Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\nConnection: upgrade\r\nUpgrade: TLS/1.2\r\n\r\n"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); +} + TEST_P(Http1ServerConnectionImplTest, UpgradeRequest) { initialize(); diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 69e63853b79e7..e84d987413702 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -15,6 +15,7 @@ #include "test/mocks/http/mocks.h" #include "test/mocks/protobuf/mocks.h" +#include "test/mocks/server/factory_context.h" #include "test/test_common/printers.h" #include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" @@ -235,6 +236,143 @@ TEST(HttpUtility, getResponseStatus) { EXPECT_EQ(200U, Utility::getResponseStatus(TestResponseHeaderMapImpl{{":status", "200"}})); } +TEST(HttpUtility, removeUpgrade) { + NiceMock context; + { + TestRequestHeaderMapImpl expected_headers = { + {":method", "GET"}, {"Upgrade", "foo1"}, {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo1, foo2"}, {"Connection", "keep-alive, Upgrade"}}; + + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("foo2"); + std::vector matchers; + matchers.push_back(std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + + { + TestRequestHeaderMapImpl expected_headers = {{":method", "GET"}, + {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo1, foo2"}, {"Connection", "keep-alive, Upgrade"}}; + + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("foo"); + std::vector matchers; + matchers.push_back(std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + + { + TestRequestHeaderMapImpl expected_headers = {{":method", "GET"}, + {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo1"}, {"Connection", "keep-alive, Upgrade"}}; + + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("foo1"); + std::vector matchers; + matchers.push_back(std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + + // Test with multiple matchers. + { + envoy::type::matcher::v3::StringMatcher matcher; + std::vector matchers; + matcher.set_exact("foo1"); + matchers.push_back(std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + matcher.set_exact("foo2"); + matchers.push_back(std::make_unique< + Envoy::Matchers::StringMatcherImpl>( + matcher, context)); + { + TestRequestHeaderMapImpl expected_headers = {{":method", "GET"}, + {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo1"}, {"Connection", "keep-alive, Upgrade"}}; + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + { + TestRequestHeaderMapImpl expected_headers = {{":method", "GET"}, + {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo2"}, {"Connection", "keep-alive, Upgrade"}}; + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + { + TestRequestHeaderMapImpl expected_headers = { + {":method", "GET"}, {"Upgrade", "foo3"}, {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo3"}, {"Connection", "keep-alive, Upgrade"}}; + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + { + TestRequestHeaderMapImpl expected_headers = { + {":method", "GET"}, {"Upgrade", "foo3"}, {"Connection", "keep-alive, Upgrade"}}; + TestRequestHeaderMapImpl converted_headers = {{":method", "GET"}, + {"Upgrade", "foo1, foo2, foo3"}, + {"Connection", "keep-alive, Upgrade"}}; + + Utility::removeUpgrade(converted_headers, matchers); + + ASSERT_EQ(converted_headers, expected_headers); + } + } +} + +TEST(HttpUtility, removeConnectionUpgrade) { + { + TestRequestHeaderMapImpl expected_headers = { + {":method", "GET"}, {"Upgrade", "foo"}, {"Connection", "keep-alive"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo"}, {"Connection", "keep-alive, Upgrade"}}; + StringUtil::CaseUnorderedSet tokens_to_remove{"upgrade"}; + + Utility::removeConnectionUpgrade(converted_headers, tokens_to_remove); + + ASSERT_EQ(converted_headers, expected_headers); + } + + { + TestRequestHeaderMapImpl expected_headers = {{":method", "GET"}, {"Upgrade", "foo"}}; + TestRequestHeaderMapImpl converted_headers = { + {":method", "GET"}, {"Upgrade", "foo"}, {"Connection", "Upgrade"}}; + StringUtil::CaseUnorderedSet tokens_to_remove{"upgrade"}; + + Utility::removeConnectionUpgrade(converted_headers, tokens_to_remove); + + ASSERT_EQ(converted_headers, expected_headers); + } +} + TEST(HttpUtility, isWebSocketUpgradeRequest) { EXPECT_FALSE(Utility::isWebSocketUpgradeRequest(TestRequestHeaderMapImpl{})); EXPECT_FALSE( @@ -655,88 +793,104 @@ TEST(HttpUtility, ValidateStreamErrorsWithHcm) { TEST(HttpUtility, ValidateStreamErrorConfigurationForHttp1) { envoy::config::core::v3::Http1ProtocolOptions http1_options; ProtobufWkt::BoolValue hcm_value; + NiceMock context; NiceMock validation_visitor; // nothing explicitly configured, default to false (i.e. default stream error behavior for HCM) - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .stream_error_on_invalid_http_message_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .stream_error_on_invalid_http_message_); // http1_options.stream_error overrides HCM.stream_error http1_options.mutable_override_stream_error_on_invalid_http_message()->set_value(true); hcm_value.set_value(false); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .stream_error_on_invalid_http_message_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .stream_error_on_invalid_http_message_); // http1_options.stream_error overrides HCM.stream_error (flip boolean value) http1_options.mutable_override_stream_error_on_invalid_http_message()->set_value(false); hcm_value.set_value(true); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .stream_error_on_invalid_http_message_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .stream_error_on_invalid_http_message_); http1_options.clear_override_stream_error_on_invalid_http_message(); // fallback to HCM.stream_error hcm_value.set_value(true); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .stream_error_on_invalid_http_message_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .stream_error_on_invalid_http_message_); // fallback to HCM.stream_error (flip boolean value) hcm_value.set_value(false); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .stream_error_on_invalid_http_message_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .stream_error_on_invalid_http_message_); } TEST(HttpUtility, UseBalsaParser) { envoy::config::core::v3::Http1ProtocolOptions http1_options; ProtobufWkt::BoolValue hcm_value; + NiceMock context; NiceMock validation_visitor; // If Http1ProtocolOptions::use_balsa_parser has no value set, then behavior is controlled by the // runtime flag. TestScopedRuntime scoped_runtime; scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "true"}}); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "false"}}); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); // Enable Balsa using Http1ProtocolOptions::use_balsa_parser. Runtime flag is ignored. http1_options.mutable_use_balsa_parser()->set_value(true); scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "true"}}); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "false"}}); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); // Disable Balsa using Http1ProtocolOptions::use_balsa_parser. Runtime flag is ignored. http1_options.mutable_use_balsa_parser()->set_value(false); scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "true"}}); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_use_balsa_parser", "false"}}); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .use_balsa_parser_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .use_balsa_parser_); } TEST(HttpUtility, AllowCustomMethods) { envoy::config::core::v3::Http1ProtocolOptions http1_options; ProtobufWkt::BoolValue hcm_value; + NiceMock context; NiceMock validation_visitor; EXPECT_FALSE(http1_options.allow_custom_methods()); - EXPECT_FALSE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .allow_custom_methods_); + EXPECT_FALSE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .allow_custom_methods_); http1_options.set_allow_custom_methods(true); - EXPECT_TRUE(Http1::parseHttp1Settings(http1_options, validation_visitor, hcm_value, false) - .allow_custom_methods_); + EXPECT_TRUE( + Http1::parseHttp1Settings(http1_options, context, validation_visitor, hcm_value, false) + .allow_custom_methods_); } TEST(HttpUtility, getLastAddressFromXFF) { diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 4676d6262eff9..98b6a8f61e13d 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -2142,6 +2142,45 @@ TEST_P(IntegrationTest, TestUpgradeHeaderInResponseWithTrailers) { EXPECT_NE(response->trailers(), nullptr); } +// With the default configuration, an upgrade request is rejected. +TEST_P(IntegrationTest, TestUpgradeRequestRejected) { + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":authority", "envoyproxy.io"}, {":path", "/test/long/url"}, + {":scheme", "http"}, {"connection", "Upgrade"}, {"upgrade", "TLS/1.3"}}; + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_EQ(response->headers().getStatusValue(), "403"); +} + +// With the default configuration, an upgrade request is rejected. +TEST_P(IntegrationTest, TestUpgradeRequestStripped) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + auto* matcher = hcm.mutable_http_protocol_options()->add_ignore_http_11_upgrade(); + matcher->set_prefix("TLS/"); + }); + + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":authority", "envoyproxy.io"}, {":path", "/test/long/url"}, + {":scheme", "http"}, {"connection", "Upgrade"}, {"upgrade", "TLS/1.3"}}; + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); + EXPECT_EQ(nullptr, upstream_request_->headers().Upgrade()); + EXPECT_EQ(nullptr, upstream_request_->headers().Connection()); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_EQ(response->headers().getStatusValue(), "200"); +} + TEST_P(IntegrationTest, ConnectWithNoBody) { config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&