diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index eda87d9408d5..e566278600ab 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -209,7 +209,7 @@ message AlternateProtocolsCacheOptions { repeated string canonical_suffixes = 5; } -// [#next-free-field: 7] +// [#next-free-field: 8] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.HttpProtocolOptions"; @@ -259,11 +259,28 @@ message HttpProtocolOptions { // `. google.protobuf.Duration max_connection_duration = 3; - // The maximum number of headers. If unconfigured, the default - // maximum number of request headers allowed is 100. Requests that exceed this limit will receive - // a 431 response for HTTP/1.x and cause a stream reset for HTTP/2. + // The maximum number of headers (request headers if configured on HttpConnectionManager, + // response headers when configured on a cluster). + // If unconfigured, the default maximum number of headers allowed is 100. + // Downstream requests that exceed this limit will receive a 431 response for HTTP/1.x and cause a stream + // reset for HTTP/2. + // Upstream responses that exceed this limit will result in a 503 response. google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}]; + // The maximum size of response headers. + // If unconfigured, the default is 60 KiB, except for HTTP/1 response headers which have a default + // of 80KiB. + // Responses that exceed this limit will result in a 503 response. + // In Envoy, this setting is only valid when configured on an upstream cluster, not on the + // :ref:`HTTP Connection Manager + // `. + // + // Note: currently some protocol codecs impose limits on the maximum size of a single header: + // HTTP/2 (when using nghttp2) limits a single header to around 100kb. + // HTTP/3 limits a single header to around 1024kb. + google.protobuf.UInt32Value max_response_headers_kb = 7 + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; + // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. google.protobuf.Duration max_stream_duration = 4; diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 4cbbbc20d3fb..3d438ae87881 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -494,6 +494,10 @@ message HttpConnectionManager { // The maximum request headers size for incoming connections. // If unconfigured, the default max request headers allowed is 60 KiB. // Requests that exceed this limit will receive a 431 response. + // + // Note: currently some protocol codecs impose limits on the maximum size of a single header: + // HTTP/2 (when using nghttp2) limits a single header to around 100kb. + // HTTP/3 limits a single header to around 1024kb. google.protobuf.UInt32Value max_request_headers_kb = 29 [(validate.rules).uint32 = {lte: 8192 gt: 0}]; diff --git a/changelogs/current.yaml b/changelogs/current.yaml index dc5877b45824..cb7fb236024a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -292,6 +292,10 @@ new_features: change: | Added full feature absl::FormatTime() support to the DateFormatter. This allows the timepoint formatters (like ``%START_TIME%``) to use ``%E#S``, ``%E*S``, ``%E#f`` and ``%E*f`` to format the subsecond part of the timepoint. +- area: http + change: | + Added configuration setting for the :ref:`maximum size of response headers + ` in responses. - area: http_11_proxy change: | Added the option to configure the transport socket via locality or endpoint metadata. diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index e307de9471f7..ca704d02fdb6 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -1068,6 +1068,11 @@ class ClusterInfo : public Http::FilterChainFactory { */ virtual uint32_t maxResponseHeadersCount() const PURE; + /** + * @return uint32_t the maximum total size of response headers in KB. + */ + virtual absl::optional maxResponseHeadersKb() const PURE; + /** * @return the human readable name of the cluster. */ diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 2f74974d983d..77fb57a814ea 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -282,13 +282,14 @@ CodecClientProd::CodecClientProd(CodecType type, Network::ClientConnectionPtr&& } codec_ = std::make_unique( *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(), - host->cluster().maxResponseHeadersCount(), proxied); + host->cluster().maxResponseHeadersKb(), host->cluster().maxResponseHeadersCount(), proxied); break; } case CodecType::HTTP2: codec_ = std::make_unique( *connection_, *this, host->cluster().http2CodecStats(), random_generator, - host->cluster().http2Options(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB, + host->cluster().http2Options(), + host->cluster().maxResponseHeadersKb().value_or(Http::DEFAULT_MAX_REQUEST_HEADERS_KB), host->cluster().maxResponseHeadersCount(), Http2::ProdNghttp2SessionFactory::get()); break; case CodecType::HTTP3: { @@ -296,7 +297,8 @@ CodecClientProd::CodecClientProd(CodecType type, Network::ClientConnectionPtr&& auto& quic_session = dynamic_cast(*connection_); codec_ = std::make_unique( quic_session, *this, host->cluster().http3CodecStats(), host->cluster().http3Options(), - Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount()); + host->cluster().maxResponseHeadersKb().value_or(Http::DEFAULT_MAX_REQUEST_HEADERS_KB), + host->cluster().maxResponseHeadersCount()); // Initialize the session after max request header size is changed in above http client // connection creation. quic_session.Initialize(); diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 5e054058a996..92fb58bb1a40 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -1428,9 +1428,11 @@ void ServerConnectionImpl::ActiveRequest::dumpState(std::ostream& os, int indent ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, CodecStats& stats, ConnectionCallbacks&, const Http1Settings& settings, + absl::optional max_response_headers_kb, const uint32_t max_response_headers_count, bool passing_through_proxy) - : ConnectionImpl(connection, stats, settings, MessageType::Response, MAX_RESPONSE_HEADERS_KB, + : ConnectionImpl(connection, stats, settings, MessageType::Response, + max_response_headers_kb.value_or(MAX_RESPONSE_HEADERS_KB), max_response_headers_count), owned_output_buffer_(connection.dispatcher().getWatermarkFactory().createBuffer( [&]() -> void { this->onBelowLowWatermark(); }, diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index 26e3bfe82d9f..875a9c6d99d6 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -590,6 +590,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl { public: ClientConnectionImpl(Network::Connection& connection, CodecStats& stats, ConnectionCallbacks& callbacks, const Http1Settings& settings, + absl::optional max_response_headers_kb, const uint32_t max_response_headers_count, bool passing_through_proxy = false); // Http::ClientConnection diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 501cc723a872..2d427e7d9e7a 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -23,6 +23,12 @@ #define PROTOBUF_GET_WRAPPED_OR_DEFAULT(message, field_name, default_value) \ ((message).has_##field_name() ? (message).field_name().value() : (default_value)) +// Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, return +// absl::nullopt. +#define PROTOBUF_GET_OPTIONAL_WRAPPED(message, field_name) \ + ((message).has_##field_name() ? absl::make_optional((message).field_name().value()) \ + : absl::nullopt) + // Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, throw // a EnvoyException. diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index bfee8e6b2baa..195d2ba59acc 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -1229,6 +1229,8 @@ ClusterInfoImpl::ClusterInfoImpl( http_protocol_options_->common_http_protocol_options_, max_headers_count, runtime_.snapshot().getInteger(Http::MaxResponseHeadersCountOverrideKey, Http::DEFAULT_MAX_HEADERS_COUNT))), + max_response_headers_kb_(PROTOBUF_GET_OPTIONAL_WRAPPED( + http_protocol_options_->common_http_protocol_options_, max_response_headers_kb)), type_(config.type()), drain_connections_on_host_removal_(config.ignore_health_on_host_removal()), connection_pool_per_downstream_connection_( diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 47d32468571d..a3d43f7b969e 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -913,6 +913,9 @@ class ClusterInfoImpl : public ClusterInfo, bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; } + absl::optional maxResponseHeadersKb() const override { + return max_response_headers_kb_; + } const std::string& name() const override { return name_; } const std::string& observabilityName() const override { if (observability_name_ != nullptr) { @@ -1126,6 +1129,7 @@ class ClusterInfoImpl : public ClusterInfo, // overhead via alignment const uint32_t per_connection_buffer_limit_bytes_; const uint32_t max_response_headers_count_; + const absl::optional max_response_headers_kb_; const envoy::config::cluster::v3::Cluster::DiscoveryType type_; const bool drain_connections_on_host_removal_ : 1; const bool connection_pool_per_downstream_connection_ : 1; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 563ece44c917..757c2439d422 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -443,6 +443,12 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( idle_timeout_ = absl::nullopt; } + if (config.common_http_protocol_options().has_max_response_headers_kb()) { + creation_status = absl::InvalidArgumentError( + fmt::format("Error: max_response_headers_kb cannot be set on http_connection_manager.")); + return; + } + if (config.strip_any_host_port() && config.strip_matching_host_port()) { creation_status = absl::InvalidArgumentError(fmt::format( "Error: Only one of `strip_matching_host_port` or `strip_any_host_port` can be set.")); diff --git a/test/common/http/codec_impl_fuzz_test.cc b/test/common/http/codec_impl_fuzz_test.cc index 8554be417b01..bca32e84542e 100644 --- a/test/common/http/codec_impl_fuzz_test.cc +++ b/test/common/http/codec_impl_fuzz_test.cc @@ -616,7 +616,7 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi } else { client = std::make_unique( client_connection, Http1::CodecStats::atomicGet(http1_stats, scope), client_callbacks, - client_http1settings, max_response_headers_count); + client_http1settings, max_request_headers_kb, max_response_headers_count); } if (http2) { diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 1fca99b17e7b..0dca1c8abc16 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -2544,7 +2544,8 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase { public: void initialize() { codec_ = std::make_unique( - connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_, + connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_kb_, + max_response_headers_count_, /* passing_through_proxy=*/false); } @@ -2564,6 +2565,7 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase { protected: Stats::TestUtil::TestStore store_; uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; + uint32_t max_response_headers_kb_{Http::Http1::MAX_RESPONSE_HEADERS_KB}; }; void Http1ClientConnectionImplTest::testClientAllowChunkedContentLength( @@ -2571,8 +2573,9 @@ void Http1ClientConnectionImplTest::testClientAllowChunkedContentLength( // Response validation is not implemented in UHV yet #ifndef ENVOY_ENABLE_UHV codec_settings_.allow_chunked_length_ = allow_chunked_length; - codec_ = std::make_unique( - connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_); + codec_ = std::make_unique(connection_, http1CodecStats(), callbacks_, + codec_settings_, max_response_headers_kb_, + max_response_headers_count_); NiceMock response_decoder; Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); @@ -3706,6 +3709,28 @@ TEST_P(Http1ClientConnectionImplTest, LargeResponseHeadersAccepted) { std::string long_header = "big: " + std::string(79 * 1024, 'q') + "\r\n"; buffer = Buffer::OwnedImpl(long_header); status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); +} + +// Tests that the size of response headers for HTTP/1 can be configured higher than the default of +// 80kB. +TEST_P(Http1ClientConnectionImplTest, LargeResponseHeadersAcceptedConfigurable) { + constexpr uint32_t size_limit_kb = 85; + max_response_headers_kb_ = size_limit_kb; + initialize(); + + NiceMock response_decoder; + Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); + TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok()); + + Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); + std::string long_header = "big: " + std::string((size_limit_kb - 1) * 1024, 'q') + "\r\n"; + buffer = Buffer::OwnedImpl(long_header); + status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); } // Regression test for CVE-2019-18801. Large method headers should not trigger @@ -3792,6 +3817,7 @@ TEST_P(Http1ClientConnectionImplTest, ManyResponseHeadersAccepted) { // Response already contains one header. buffer = Buffer::OwnedImpl(createHeaderOrTrailerFragment(150) + "\r\n"); status = codec_->dispatch(buffer); + EXPECT_TRUE(status.ok()); } TEST_P(Http1ClientConnectionImplTest, TestResponseSplit0) { @@ -3821,8 +3847,9 @@ TEST_P(Http1ClientConnectionImplTest, TestResponseSplitAllowChunkedLength100) { TEST_P(Http1ClientConnectionImplTest, VerifyResponseHeaderTrailerMapMaxLimits) { codec_settings_.allow_chunked_length_ = true; codec_settings_.enable_trailers_ = true; - codec_ = std::make_unique( - connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_); + codec_ = std::make_unique(connection_, http1CodecStats(), callbacks_, + codec_settings_, max_response_headers_kb_, + max_response_headers_count_); NiceMock response_decoder; Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); diff --git a/test/common/http/http1/http1_connection_fuzz_test.cc b/test/common/http/http1/http1_connection_fuzz_test.cc index 9dae7af6c7b9..37eb6fc64e4a 100644 --- a/test/common/http/http1/http1_connection_fuzz_test.cc +++ b/test/common/http/http1/http1_connection_fuzz_test.cc @@ -45,7 +45,7 @@ class Http1Harness { client_ = std::make_unique( mock_client_connection_, Http1::CodecStats::atomicGet(http1_stats_, *stats_store_.rootScope()), - mock_client_callbacks_, client_settings_, Http::DEFAULT_MAX_HEADERS_COUNT); + mock_client_callbacks_, client_settings_, absl::nullopt, Http::DEFAULT_MAX_HEADERS_COUNT); Status status = client_->dispatch(payload); } diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 5f39f9935a21..06fa2cf5c194 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -996,6 +996,27 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeaderCountConfigurable) { EXPECT_EQ(200, config.maxRequestHeadersCount()); } +// Check that max response header size is invalid on HCM. +TEST_F(HttpConnectionManagerConfigTest, MaxResponseHeaderKbInvalid) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + common_http_protocol_options: + max_response_headers_kb: 200 + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + &scoped_routes_config_provider_manager_, tracer_manager_, + filter_config_provider_manager_, creation_status_); + EXPECT_FALSE(creation_status_.ok()); +} + // Checking that default max_requests_per_connection is 0. TEST_F(HttpConnectionManagerConfigTest, DefaultMaxRequestPerConnection) { const std::string yaml_string = R"EOF( diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index d8bc506c181e..c640f452141b 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -266,6 +266,7 @@ IntegrationCodecClientPtr HttpIntegrationTest::makeHttpConnection(uint32_t port) IntegrationCodecClientPtr HttpIntegrationTest::makeRawHttpConnection( Network::ClientConnectionPtr&& conn, absl::optional http2_options, + absl::optional common_http_options, bool wait_till_connected) { std::shared_ptr cluster{new NiceMock()}; cluster->max_response_headers_count_ = 200; @@ -286,6 +287,10 @@ IntegrationCodecClientPtr HttpIntegrationTest::makeRawHttpConnection( cluster->http2_options_ = http2_options.value(); cluster->http1_settings_.enable_trailers_ = true; + if (common_http_options.has_value()) { + cluster->common_http_protocol_options_ = common_http_options.value(); + } + if (!disable_client_header_validation_) { cluster->header_validator_factory_ = IntegrationUtil::makeHeaderValidationFactory( ::envoy::extensions::http::header_validators::envoy_default::v3::HeaderValidatorConfig()); @@ -1448,7 +1453,72 @@ void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t count, } else { IntegrationStreamDecoderPtr response = codec_client_->makeHeaderOnlyRequest(big_headers); RELEASE_ASSERT(response->waitForEndStream(timeout), - fmt::format("unexpected timeout after ", timeout.count(), " ms")); + fmt::format("unexpected timeout after {}ms", timeout.count())); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + } + if (count > max_count) { + EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("too_many_headers")); + } +} + +void HttpIntegrationTest::testLargeResponseHeaders(uint32_t size, uint32_t count, uint32_t max_size, + uint32_t max_count, + std::chrono::milliseconds timeout) { + autonomous_upstream_ = true; + useAccessLog("%RESPONSE_CODE_DETAILS%"); + // `size` parameter dictates the size of each header that will be added to the response and + // `count` parameter is the number of headers to be added. The actual request byte size will + // exceed `size` due to the keys and other headers. The actual request header count will exceed + // `count` by four due to default headers. + + config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + ConfigHelper::HttpProtocolOptions protocol_options; + auto* http_protocol_options = protocol_options.mutable_common_http_protocol_options(); + http_protocol_options->mutable_max_response_headers_kb()->set_value(max_size); + http_protocol_options->mutable_max_headers_count()->set_value(max_count); + + ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), + protocol_options); + }); + + // This test is validating upstream response headers, but the test client will fail to receive the + // request from Envoy if its limits aren't increased. + envoy::config::core::v3::HttpProtocolOptions client_protocol_options; + client_protocol_options.mutable_max_response_headers_kb()->set_value(max_size); + client_protocol_options.mutable_max_headers_count()->set_value(max_count); + + Http::TestRequestHeaderMapImpl big_headers(default_response_headers_); + + // Already added four headers. + for (unsigned int i = 0; i < count; i++) { + big_headers.addCopy(std::to_string(i), std::string(size * 1024, 'a')); + } + + initialize(); + codec_client_ = makeRawHttpConnection(makeClientConnection(lookupPort("http")), absl::nullopt, + client_protocol_options); + reinterpret_cast(fake_upstreams_.front().get()) + ->setResponseHeaders(std::make_unique(big_headers)); + + if (size >= max_size || count > max_count) { + // header size includes keys too, so expect rejection when equal + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + auto response = std::move(encoder_decoder.second); + + if (downstream_protocol_ == Http::CodecType::HTTP1) { + ASSERT_TRUE(codec_client_->waitForDisconnect()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("431", response->headers().getStatusValue()); + } else { + ASSERT_TRUE(response->waitForReset()); + codec_client_->close(); + } + } else { + IntegrationStreamDecoderPtr response = + codec_client_->makeHeaderOnlyRequest(default_request_headers_); + RELEASE_ASSERT(response->waitForEndStream(timeout), + fmt::format("unexpected timeout after {}ms", timeout.count())); EXPECT_TRUE(response->complete()); EXPECT_EQ("200", response->headers().getStatusValue()); } diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 06f2dd15a65c..55adabba2eb4 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -158,10 +158,12 @@ class HttpIntegrationTest : public BaseIntegrationTest { IntegrationCodecClientPtr makeHttpConnection(uint32_t port); // Makes a http connection object without checking its connected state. - virtual IntegrationCodecClientPtr - makeRawHttpConnection(Network::ClientConnectionPtr&& conn, - absl::optional http2_options, - bool wait_till_connected = true); + virtual IntegrationCodecClientPtr makeRawHttpConnection( + Network::ClientConnectionPtr&& conn, + absl::optional http2_options, + absl::optional common_http_options = + absl::nullopt, + bool wait_till_connected = true); // Makes a downstream network connection object based on client codec version. Network::ClientConnectionPtr makeClientConnectionWithOptions( uint32_t port, const Network::ConnectionSocket::OptionsSharedPtr& options) override; @@ -271,13 +273,13 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testRouterUpstreamResponseBeforeRequestComplete(uint32_t status_code = 0); void testTwoRequests(bool force_network_backup = false); - void testLargeHeaders(Http::TestRequestHeaderMapImpl request_headers, - Http::TestRequestTrailerMapImpl request_trailers, uint32_t size, - uint32_t max_size); void testLargeRequestUrl(uint32_t url_size, uint32_t max_headers_size); void testLargeRequestHeaders(uint32_t size, uint32_t count, uint32_t max_size = 60, uint32_t max_count = 100, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + void testLargeResponseHeaders(uint32_t size, uint32_t count, uint32_t max_size = 60, + uint32_t max_count = 100, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); void testLargeRequestTrailers(uint32_t size, uint32_t max_size = 60); void testManyRequestHeaders(std::chrono::milliseconds time = TestUtility::DefaultTimeout); diff --git a/test/integration/overload_integration_test.cc b/test/integration/overload_integration_test.cc index d3a62cdb2ff9..d5e808a88832 100644 --- a/test/integration/overload_integration_test.cc +++ b/test/integration/overload_integration_test.cc @@ -403,6 +403,7 @@ TEST_P(OverloadScaledTimerIntegrationTest, HTTP3CloseIdleHttpConnectionsDuringHa test_server_->waitForGaugeGe("overload.envoy.overload_actions.reduce_timeouts.scale_percent", 50); // Create an HTTP connection without finishing the handshake. codec_client_ = makeRawHttpConnection(makeClientConnection((lookupPort("http"))), absl::nullopt, + absl::nullopt, /*wait_till_connected=*/false); EXPECT_FALSE(codec_client_->connected()); @@ -418,8 +419,9 @@ TEST_P(OverloadScaledTimerIntegrationTest, HTTP3CloseIdleHttpConnectionsDuringHa 100); // Create another HTTP connection without finishing handshake. - IntegrationCodecClientPtr codec_client2 = makeRawHttpConnection( - makeClientConnection((lookupPort("http"))), absl::nullopt, /*wait_till_connected=*/false); + IntegrationCodecClientPtr codec_client2 = + makeRawHttpConnection(makeClientConnection((lookupPort("http"))), absl::nullopt, + absl::nullopt, /*wait_till_connected=*/false); EXPECT_FALSE(codec_client2->connected()); // Advancing past the minimum time and wait for the proxy to notice and close both connections. timeSystem().advanceTimeWait(std::chrono::seconds(3)); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 3625b0b58d51..921e5ba80925 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -2380,11 +2380,52 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { testLargeRequestHeaders(100, 1, 8192, 100); } -TEST_P(DownstreamProtocolIntegrationTest, ManyLargeRequestHeadersAccepted) { +TEST_P(ProtocolIntegrationTest, ManyLargeRequestHeadersAccepted) { // Send 70 headers each of size 100 kB with limit 8192 kB (8 MB) and 100 headers. testLargeRequestHeaders(100, 70, 8192, 100, TestUtility::DefaultTimeout); } +namespace { +uint32_t adjustMaxSingleHeaderSizeForCodecLimits(uint32_t size, + const HttpProtocolTestParams& params) { + if (params.http2_implementation == Http2Impl::Nghttp2 && + (params.downstream_protocol == Http::CodecType::HTTP2 || + params.upstream_protocol == Http::CodecType::HTTP2)) { + // nghttp2 has a hard-coded, unconfigurable limit of 64k for a header in it's header + // decompressor, so this test will always fail when using that codec. + // Reduce the size so that it can pass and receive some test coverage. + return 100; + } else if (params.downstream_protocol == Http::CodecType::HTTP3 || + params.upstream_protocol == Http::CodecType::HTTP3) { + // QUICHE has a hard-coded limit of 1024KiB in it's QPACK decoder. + // Reduce the size so that it can pass and receive some test coverage. + return 1023; + } + + return size; +} +} // namespace + +// Test a single header of the maximum allowed size. +TEST_P(ProtocolIntegrationTest, VeryLargeRequestHeadersAccepted) { + uint32_t size = adjustMaxSingleHeaderSizeForCodecLimits(8191, GetParam()); + + testLargeRequestHeaders(size, 1, 8192, 100, TestUtility::DefaultTimeout); +} + +// Test a single header of the maximum allowed size. +TEST_P(ProtocolIntegrationTest, ManyLargeResponseHeadersAccepted) { + // Send 70 headers each of size 100 kB with limit 8192 kB (8 MB) and 100 headers. + testLargeResponseHeaders(100, 70, 8192, 100, TestUtility::DefaultTimeout); +} + +// Test a single header of the maximum allowed size. +TEST_P(ProtocolIntegrationTest, VeryLargeResponseHeadersAccepted) { + uint32_t size = adjustMaxSingleHeaderSizeForCodecLimits(8191, GetParam()); + + testLargeResponseHeaders(size, 1, 8192, 100, TestUtility::DefaultTimeout); +} + TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersRejected) { // Send 101 empty headers with limit 60 kB and 100 headers. testLargeRequestHeaders(0, 101, 60, 80); diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index efc3e6b9224e..8be734d4747b 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -251,12 +251,15 @@ class QuicHttpIntegrationTestBase : public HttpIntegrationTest { return session; } - IntegrationCodecClientPtr - makeRawHttpConnection(Network::ClientConnectionPtr&& conn, - absl::optional http2_options, - bool wait_till_connected = true) override { + IntegrationCodecClientPtr makeRawHttpConnection( + Network::ClientConnectionPtr&& conn, + absl::optional http2_options, + absl::optional common_http_options = + absl::nullopt, + bool wait_till_connected = true) override { ENVOY_LOG(debug, "Creating a new client {}", conn->connectionInfoProvider().localAddress()->asStringView()); + ASSERT(!common_http_options.has_value(), "Not implemented"); return makeRawHttp3Connection(std::move(conn), http2_options, wait_till_connected); } diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 380c7a7c8bcd..8177186b0b4e 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -96,6 +96,13 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, extensionProtocolOptions(_)).WillByDefault(Return(extension_protocol_options_)); ON_CALL(*this, maxResponseHeadersCount()) .WillByDefault(ReturnPointee(&max_response_headers_count_)); + ON_CALL(*this, maxResponseHeadersKb()).WillByDefault(Invoke([this]() -> absl::optional { + if (common_http_protocol_options_.has_max_response_headers_kb()) { + return common_http_protocol_options_.max_response_headers_kb().value(); + } else { + return absl::nullopt; + } + })); ON_CALL(*this, maxRequestsPerConnection()) .WillByDefault(ReturnPointee(&max_requests_per_connection_)); ON_CALL(*this, trafficStats()).WillByDefault(ReturnRef(traffic_stats_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 8f5a72973f49..978f9c5826aa 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -128,6 +128,7 @@ class MockClusterInfo : public ClusterInfo { (const)); MOCK_METHOD(bool, maintenanceMode, (), (const)); MOCK_METHOD(uint32_t, maxResponseHeadersCount, (), (const)); + MOCK_METHOD(absl::optional, maxResponseHeadersKb, (), (const)); MOCK_METHOD(uint64_t, maxRequestsPerConnection, (), (const)); MOCK_METHOD(const std::string&, name, (), (const)); MOCK_METHOD(const std::string&, observabilityName, (), (const));