http: raise max_request_headers_kb limit to 8192 KiB (8MiB) from 96 KiB (#15921)

Commit Message:
Raise max configurable max_request_headers_kb limit to 8192 KiB (8MiB) from 96 KiB in http connection manager protobuf.

Additional Description:
Added/Updated relevant unit tests and integration tests. This change will allow increasing max_request_headers_kb to 8MiB from http connection manager's configuration. The change is mainly updating the limit in a validation check in the protobuf. Also, the old (merged) PR #5859 is read, no nghttp2 library-related issues are observed on raising the max_request_headers_kb beyond 96 KiB.
Risk Level: Low
Testing: Unit, Integration, Manual
Docs Changes: Inline in proto file.

Signed-off-by: Anirudha Singh <[email protected]>

Author: anirudhaps

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

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

api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto

@@ -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.

docs/root/version_history/current.rst

@@ -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<MockRequestDecoder> 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();

generated_api_shadow/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto

@@ -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);
   }
 

generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -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.