Add strict parsing of chunked encoding

changelogs/current.yaml

@@ -126,6 +126,11 @@ minor_behavior_changes:
     and the caller does not want to manage shared pointer ownership. The new shared pointer methods
     are intended for use cases where the caller needs to keep the ownerships to the route, cluster,
     or virtual host.
+- area: http
+  change: |
+    Enable strict parsing of HTTP/1 chunked encoding. Strict parsing is disabled by default and can be
+    enabled by setting the ``envoy.reloadable_features.mcp_filter_use_new_metadata_namespace`` runtime
+    flag to ``false``.
 
 bug_fixes:
 # *Changes expected to improve the state of the world and are unlikely to have negative effects*

source/common/http/http1/balsa_parser.cc

@@ -153,6 +153,10 @@ BalsaParser::BalsaParser(MessageType type, ParserCallbacks* connection, size_t m
   http_validation_policy.require_content_length_if_body_required = false;
   http_validation_policy.disallow_invalid_header_characters_in_response = true;
   http_validation_policy.disallow_lone_cr_in_chunk_extension = true;
+
+  http_validation_policy.disallow_stray_data_after_chunk =
+      Runtime::runtimeFeatureEnabled("envoy.reloadable_features.strict_chunk_parsing");
+
   framer_.set_http_validation_policy(http_validation_policy);
 
   framer_.set_balsa_headers(&headers_);

source/common/runtime/runtime_features.cc

@@ -222,6 +222,8 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_http2_use_oghttp2);
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_dynamic_modules_strip_custom_stat_prefix);
 // TODO(haoyuewang): Flip true after prod testing.
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_disable_data_read_immediately);
+// TODO(yavlasov): Flip to true after prod testing.
+FALSE_RUNTIME_GUARD(envoy_reloadable_features_strict_chunk_parsing);
 
 // Block of non-boolean flags. Use of int flags is deprecated. Do not add more.
 ABSL_FLAG(uint64_t, re2_max_program_size_error_level, 100, ""); // NOLINT

test/integration/integration_test.cc

@@ -941,6 +941,38 @@ TEST_P(IntegrationTest, TestSmuggling) {
   }
 }
 
+TEST_P(IntegrationTest, TestInvalidChunkedEncoding) {
+  autonomous_upstream_ = true;
+  config_helper_.addRuntimeOverride("envoy.reloadable_features.strict_chunk_parsing", "true");
+#ifdef ENVOY_ENABLE_UHV
+  config_helper_.addRuntimeOverride("envoy.reloadable_features.enable_universal_header_validator",
+                                    "true");
+#endif
+  config_helper_.disableDelayClose();
+  initialize();
+  std::string response;
+  const std::string request = "GET / HTTP/1.1\r\nHost: host\r\ntransfer-encoding: chunked\r\n\r\n"
+                              "1\r\nAAAAAA\r\n0\r\n\r\nXXXXXX";
+  sendRawHttpAndWaitForResponse(lookupPort("http"), request.c_str(), &response, false);
+  EXPECT_THAT(response, StartsWith("HTTP/1.1 400 Bad Request\r\n"));
+}
+
+TEST_P(IntegrationTest, TestInvalidChunkedEncodingDefault) {
+  autonomous_upstream_ = true;
+#ifdef ENVOY_ENABLE_UHV
+  config_helper_.addRuntimeOverride("envoy.reloadable_features.enable_universal_header_validator",
+                                    "true");
+#endif
+  config_helper_.disableDelayClose();
+  initialize();
+  std::string response;
+  const std::string request =
+      "GET / HTTP/1.1\r\nHost: host\r\nConnection: close\r\ntransfer-encoding: chunked\r\n\r\n"
+      "1\r\nAAAAAA\r\n0\r\n\r\nXXXXXX";
+  sendRawHttpAndWaitForResponse(lookupPort("http"), request.c_str(), &response, false);
+  EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n"));
+}
+
 TEST_P(IntegrationTest, TestInvalidTransferEncoding) {
 #ifdef ENVOY_ENABLE_UHV
   config_helper_.addRuntimeOverride("envoy.reloadable_features.enable_universal_header_validator",