Skip to content

Commit

Permalink
http: sanitizing x-envoy-original-path correctly
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <[email protected]>

Signed-off-by: Ryan Northey <[email protected]>
  • Loading branch information
alyssawilk authored and phlax committed Apr 4, 2023
1 parent be2fd62 commit bf6904b
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 0 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ bug_fixes:
change: |
reject pseudo headers violating RFC 9114. Specifically, pseudo-header fields with more than one value for the ``:method`` (non-``CONNECT``),
``:scheme``, and ``:path``; or pseudo-header fields after regular header fields; or undefined pseudo-headers.
- area: http
change: |
fixed a bug where ``x-envoy-original-path`` was not being sanitized when sent from untrusted users. This behavioral change can be temporarily reverted by setting ``envoy.reloadable_features.sanitize_original_path`` to false.
removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
Expand Down
3 changes: 3 additions & 0 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ void ConnectionManagerUtility::cleanInternalHeaders(
request_headers.removeEnvoyIpTags();
request_headers.removeEnvoyOriginalUrl();
request_headers.removeEnvoyHedgeOnPerTryTimeout();
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.sanitize_original_path")) {
request_headers.removeEnvoyOriginalPath();
}

for (const LowerCaseString& header : internal_only_headers) {
request_headers.remove(header);
Expand Down
1 change: 1 addition & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ RUNTIME_GUARD(envoy_reloadable_features_override_request_timeout_by_gateway_time
RUNTIME_GUARD(envoy_reloadable_features_postpone_h3_client_connect_to_next_loop);
RUNTIME_GUARD(envoy_reloadable_features_proxy_102_103);
RUNTIME_GUARD(envoy_reloadable_features_sanitize_http_header_referer);
RUNTIME_GUARD(envoy_reloadable_features_sanitize_original_path);
RUNTIME_GUARD(envoy_reloadable_features_service_sanitize_non_utf8_strings);
RUNTIME_GUARD(envoy_reloadable_features_skip_delay_close);
RUNTIME_GUARD(envoy_reloadable_features_strict_check_on_ipv4_compat);
Expand Down
2 changes: 2 additions & 0 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestUseRemote) {
{"x-envoy-expected-rq-timeout-ms", "10"},
{"x-envoy-ip-tags", "bar"},
{"x-envoy-original-url", "my_url"},
{"x-envoy-original-path", "/my_path"},
{"custom_header", "foo"}};

EXPECT_EQ((MutateRequestRet{"50.0.0.1:0", false, Tracing::Reason::NotTraceable}),
Expand All @@ -778,6 +779,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestUseRemote) {
EXPECT_FALSE(headers.has("x-envoy-expected-rq-timeout-ms"));
EXPECT_FALSE(headers.has("x-envoy-ip-tags"));
EXPECT_FALSE(headers.has("x-envoy-original-url"));
EXPECT_FALSE(headers.has("x-envoy-original-path"));
EXPECT_FALSE(headers.has("custom_header"));
}

Expand Down

0 comments on commit bf6904b

Please sign in to comment.