diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 6068acbaa057c..8416a8a849075 100644 --- a/changelogs/current.yaml +++ b/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* diff --git a/source/common/http/http1/balsa_parser.cc b/source/common/http/http1/balsa_parser.cc index 33fa63cf303da..583db79fbfd44 100644 --- a/source/common/http/http1/balsa_parser.cc +++ b/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_); diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 760d6a01590ba..d065946110b6d 100644 --- a/source/common/runtime/runtime_features.cc +++ b/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 diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index b1e66223ebed6..68b09793930a2 100644 --- a/test/integration/integration_test.cc +++ b/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",