diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index c05032df21a4d..3e7a4dc17769c 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -299,10 +299,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The idle timeout for connections managed by the connection manager. The // idle timeout is defined as the period in which there are no active 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 66575c40b1ccc..9ede9a6454351 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 @@ -343,10 +343,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The stream idle timeout for connections managed by the connection manager. // If not specified, this defaults to 5 minutes. The default value was selected diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index b292548832e13..67cb8194415e0 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -346,10 +346,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The stream idle timeout for connections managed by the connection manager. // If not specified, this defaults to 5 minutes. The default value was selected diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 0fde709aaffee..eab32d4524dab 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -19,6 +19,7 @@ Bug Fixes --------- *Changes expected to improve the state of the world and are unlikely to have negative effects* +* http: raise max configurable max_request_headers_kb limit to 8192 KiB (8MiB) from 96 KiB in http connection manager. * validation: fix an issue that causes TAP sockets to panic during config validation mode. * xray: fix the default sampling 'rate' for AWS X-Ray tracer extension to be 5% as opposed to 50%. * zipkin: fix timestamp serializaiton in annotations. A prior bug fix exposed an issue with timestamps being serialized as strings. diff --git a/generated_api_shadow/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/generated_api_shadow/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index c05032df21a4d..3e7a4dc17769c 100644 --- a/generated_api_shadow/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/generated_api_shadow/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -299,10 +299,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The idle timeout for connections managed by the connection manager. The // idle timeout is defined as the period in which there are no active diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index c9c6ac3a55573..d9343fcb0fbb7 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -349,10 +349,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The stream idle timeout for connections managed by the connection manager. // If not specified, this defaults to 5 minutes. The default value was selected diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index b292548832e13..67cb8194415e0 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -346,10 +346,8 @@ 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. - // The max configurable limit is 96 KiB, based on current implementation - // constraints. google.protobuf.UInt32Value max_request_headers_kb = 29 - [(validate.rules).uint32 = {lte: 96 gt: 0}]; + [(validate.rules).uint32 = {lte: 8192 gt: 0}]; // The stream idle timeout for connections managed by the connection manager. // If not specified, this defaults to 5 minutes. The default value was selected diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 49ab6beb301ab..a05c13d82a0e6 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -48,6 +48,15 @@ std::string createHeaderFragment(int num_headers) { return headers; } +std::string createLargeHeaderFragment(int num_headers) { + // Create a header field with num_headers headers with each header of size ~64 KiB. + std::string headers; + for (int i = 0; i < num_headers; i++) { + headers += "header" + std::to_string(i) + ": " + std::string(64 * 1024, 'q') + "\r\n"; + } + return headers; +} + Buffer::OwnedImpl createBufferWithNByteSlices(absl::string_view input, size_t max_slice_size) { Buffer::OwnedImpl buffer; for (size_t offset = 0; offset < input.size(); offset += max_slice_size) { @@ -2858,6 +2867,12 @@ TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejected) { testRequestHeadersExceedLimit(long_string, "headers size exceeds limit", ""); } +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejectedBeyondMaxConfigurable) { + max_request_headers_kb_ = 8192; + std::string long_string = "big: " + std::string(8193 * 1024, 'q') + "\r\n"; + testRequestHeadersExceedLimit(long_string, "headers size exceeds limit", ""); +} + // Tests that the default limit for the number of request headers is 100. TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersRejected) { // Send a request with 101 headers. @@ -2894,6 +2909,36 @@ TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejected) { EXPECT_EQ("http1.headers_too_large", response_encoder->getStream().responseDetails()); } +TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejectedMaxConfigurable) { + max_request_headers_kb_ = 8192; + max_request_headers_count_ = 150; + initialize(); + + std::string exception_reason; + NiceMock decoder; + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"); + auto status = codec_->dispatch(buffer); + + std::string long_string = std::string(64 * 1024, 'q'); + for (int i = 0; i < 127; i++) { + buffer = Buffer::OwnedImpl(fmt::format("big: {}\r\n", long_string)); + status = codec_->dispatch(buffer); + } + // the 128th 64kb header should induce overflow + buffer = Buffer::OwnedImpl(fmt::format("big: {}\r\n", long_string)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + status = codec_->dispatch(buffer); + EXPECT_TRUE(isCodecProtocolError(status)); + EXPECT_EQ(status.message(), "headers size exceeds limit"); + EXPECT_EQ("http1.headers_too_large", response_encoder->getStream().responseDetails()); +} + // Tests that the 101th request header causes overflow with the default max number of request // headers. TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersSplitRejected) { @@ -2924,14 +2969,14 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersSplitRejected) { } TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersAccepted) { - max_request_headers_kb_ = 65; - std::string long_string = "big: " + std::string(64 * 1024, 'q') + "\r\n"; + max_request_headers_kb_ = 4096; + std::string long_string = "big: " + std::string(1024 * 1024, 'q') + "\r\n"; testRequestHeadersAccepted(long_string); } TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersAcceptedMaxConfigurable) { - max_request_headers_kb_ = 96; - std::string long_string = "big: " + std::string(95 * 1024, 'q') + "\r\n"; + max_request_headers_kb_ = 8192; + std::string long_string = "big: " + std::string(8191 * 1024, 'q') + "\r\n"; testRequestHeadersAccepted(long_string); } @@ -2942,6 +2987,12 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersAccepted) { testRequestHeadersAccepted(createHeaderFragment(150)); } +TEST_F(Http1ServerConnectionImplTest, ManyLargeRequestHeadersAccepted) { + max_request_headers_kb_ = 8192; + // Create a request with 64 headers, each header of size ~64 KiB. Total size ~4MB. + testRequestHeadersAccepted(createLargeHeaderFragment(64)); +} + // Tests that incomplete response headers of 80 kB header value fails. TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeValueRejected) { initialize(); diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index b6bdf85aa97e8..f6327caaa71bf 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -2176,15 +2176,14 @@ TEST_P(Http2CodecImplTest, ManyLargeRequestHeadersUnderPerHeaderLimit) { } TEST_P(Http2CodecImplTest, LargeRequestHeadersAtMaxConfigurable) { - // Raising the limit past this triggers some unexpected nghttp2 error. - // Further debugging required to increase past ~96 KiB. - max_request_headers_kb_ = 96; + max_request_headers_kb_ = 8192; + max_request_headers_count_ = 150; initialize(); TestRequestHeaderMapImpl request_headers; HttpTestUtility::addDefaultHeaders(request_headers); - std::string long_string = std::string(1024, 'q'); - for (int i = 0; i < 95; i++) { + std::string long_string = std::string(63 * 1024, 'q'); + for (int i = 0; i < 129; i++) { request_headers.addCopy(std::to_string(i), long_string); } 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 367f615749be0..89cd004b8dd96 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -687,7 +687,7 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbConfigured) { TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbMaxConfigurable) { const std::string yaml_string = R"EOF( stat_prefix: ingress_http - max_request_headers_kb: 96 + max_request_headers_kb: 8192 route_config: name: local_route http_filters: @@ -698,7 +698,7 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbMaxConfigurable) { date_provider_, route_config_provider_manager_, scoped_routes_config_provider_manager_, http_tracer_manager_, filter_config_provider_manager_); - EXPECT_EQ(96, config.maxRequestHeadersKb()); + EXPECT_EQ(8192, config.maxRequestHeadersKb()); } // Validated that an explicit zero stream idle timeout disables. diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 850c5c009494c..eca6948652f9f 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1576,11 +1576,25 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { testLargeRequestHeaders(95, 1, 60, 100); } +TEST_P(DownstreamProtocolIntegrationTest, VeryLargeRequestHeadersRejected) { + EXCLUDE_DOWNSTREAM_HTTP3 + EXCLUDE_UPSTREAM_HTTP3; + // Send one very large 2048 kB (2 MB) header with limit 1024 kB (1 MB) and 100 headers. + testLargeRequestHeaders(2048, 1, 1024, 100); +} + TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersAccepted) { EXCLUDE_DOWNSTREAM_HTTP3 EXCLUDE_UPSTREAM_HTTP3; // requires configurable header limits. - // Send one 95 kB header with limit 96 kB and 100 headers. - testLargeRequestHeaders(95, 1, 96, 100); + // Send one 100 kB header with limit 8192 kB (8 MB) and 100 headers. + testLargeRequestHeaders(100, 1, 8192, 100); +} + +TEST_P(DownstreamProtocolIntegrationTest, ManyLargeRequestHeadersAccepted) { + EXCLUDE_DOWNSTREAM_HTTP3 + EXCLUDE_UPSTREAM_HTTP3; + // Send 70 headers each of size 100 kB with limit 8192 kB (8 MB) and 100 headers. + testLargeRequestHeaders(100, 70, 8192, 100); } TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersRejected) {