Add UHV config to strip URL fragment

Signed-off-by: Yan Avlasov <yavlasov@google.com>

api/envoy/extensions/http/header_validators/envoy_default/v3/header_validator.proto

@@ -144,4 +144,8 @@ message HeaderValidatorConfig {
   // Action to take when a client request with a header name containing underscore characters is received.
   // If this setting is not specified, the value defaults to ALLOW.
   HeadersWithUnderscoresAction headers_with_underscores_action = 4;
+
+  // Allow requests with fragment in URL path and strip the fragment before request processing.
+  // By default Envoy rejects requests with fragment in URL path.
+  bool strip_fragment_from_path = 5;
 }

envoy/http/header_validator_errors.h

@@ -20,6 +20,7 @@ struct UhvResponseCodeDetailValues {
   const std::string EmptyHeaderName = "uhv.empty_header_name";
   const std::string InvalidPseudoHeader = "uhv.invalid_pseudo_header";
   const std::string InvalidHostDeprecatedUserInfo = "uhv.invalid_host_deprecated_user_info";
+  const std::string FragmentInUrlPath = "uhv.fragment_in_url_path";
 };
 
 using UhvResponseCodeDetail = ConstSingleton<UhvResponseCodeDetailValues>;

source/common/http/conn_manager_impl.cc

@@ -1006,7 +1006,10 @@ bool ConnectionManagerImpl::ActiveStream::validateHeaders() {
         resetStream();
       } else {
         sendLocalReply(response_code, "", modify_headers, grpc_status, failure_details);
-        if (!response_encoder_->streamErrorOnInvalidHttpMessage()) {
+        // Pre UHV HCM did not respect stream_error_on_invalid_http_message
+        // and only sent 400 when :path contained fragment
+        if (!response_encoder_->streamErrorOnInvalidHttpMessage() &&
+            failure_details != UhvResponseCodeDetail::get().FragmentInUrlPath) {
           connection_manager_.handleCodecError(failure_details);
         }
       }

source/common/runtime/runtime_features.cc

@@ -50,6 +50,7 @@ RUNTIME_GUARD(envoy_reloadable_features_http2_validate_authority_with_quiche);
 RUNTIME_GUARD(envoy_reloadable_features_http_allow_partial_urls_in_referer);
 RUNTIME_GUARD(envoy_reloadable_features_http_ext_auth_failure_mode_allow_header_add);
 RUNTIME_GUARD(envoy_reloadable_features_http_filter_avoid_reentrant_local_reply);
+// Delay deprecation and decommission until UHV is enabled.
 RUNTIME_GUARD(envoy_reloadable_features_http_reject_path_with_fragment);
 RUNTIME_GUARD(envoy_reloadable_features_ignore_optional_option_from_hcm_for_route_config);
 RUNTIME_GUARD(envoy_reloadable_features_immediate_response_use_filter_mutation_rule);

source/extensions/filters/network/http_connection_manager/config.cc

@@ -155,6 +155,8 @@ Http::HeaderValidatorFactoryPtr createHeaderValidatorFactory(
         static_cast<::envoy::extensions::http::header_validators::envoy_default::v3::
                         HeaderValidatorConfig::HeadersWithUnderscoresAction>(
             config.common_http_protocol_options().headers_with_underscores_action()));
+    uhv_config.set_strip_fragment_from_path(!Runtime::runtimeFeatureEnabled(
+        "envoy.reloadable_features.http_reject_path_with_fragment"));
     legacy_header_validator_config.set_name("default_envoy_uhv_from_legacy_settings");
     legacy_header_validator_config.mutable_typed_config()->PackFrom(uhv_config);
   }

source/extensions/http/header_validators/envoy_default/header_validator.cc

@@ -446,10 +446,15 @@ HeaderValidator::validatePathHeaderCharacters(const HeaderString& value) {
   }
 
   if (is_valid && iter != end && *iter == '#') {
-    // Validate the fragment component of the URI
-    ++iter;
-    for (; iter != end && is_valid; ++iter) {
-      is_valid &= testCharInTable(::Envoy::Http::kUriQueryAndFragmentCharTable, *iter);
+    if (!config_.strip_fragment_from_path()) {
+      return {HeaderValueValidationResult::Action::Reject,
+              UhvResponseCodeDetail::get().FragmentInUrlPath};
+    } else {
+      // Validate the fragment component of the URI
+      ++iter;
+      for (; iter != end && is_valid; ++iter) {
+        is_valid &= testCharInTable(::Envoy::Http::kUriQueryAndFragmentCharTable, *iter);
+      }
     }
   }
 
@@ -571,6 +576,15 @@ void HeaderValidator::sanitizeHeadersWithUnderscores(::Envoy::Http::HeaderMap& h
   }
 }
 
+void HeaderValidator::sanitizePathWithFragment(::Envoy::Http::RequestHeaderMap& header_map) {
+  auto fragment_pos = header_map.getPathValue().find('#');
+  if (fragment_pos != absl::string_view::npos) {
+    ASSERT(config_.strip_fragment_from_path());
+    // Check runtime override and throw away fragment from URI path
+    header_map.setPath(header_map.getPathValue().substr(0, fragment_pos));
+  }
+}
+
 } // namespace EnvoyDefault
 } // namespace HeaderValidators
 } // namespace Http

source/extensions/http/header_validators/envoy_default/header_validator.h

@@ -163,6 +163,12 @@ class HeaderValidator {
    */
   void sanitizeHeadersWithUnderscores(::Envoy::Http::HeaderMap& header_map);
 
+  /**
+   * Check if the :path header contains a fragment. If the fragment is found it is stripped from
+   * the :path.
+   */
+  void sanitizePathWithFragment(::Envoy::Http::RequestHeaderMap& header_map);
+
   const envoy::extensions::http::header_validators::envoy_default::v3::HeaderValidatorConfig
       config_;
   ::Envoy::Http::Protocol protocol_;

source/extensions/http/header_validators/envoy_default/http1_header_validator.cc

@@ -307,6 +307,7 @@ ::Envoy::Http::ServerHeaderValidator::RequestHeadersTransformationResult
 ServerHttp1HeaderValidator::transformRequestHeaders(::Envoy::Http::RequestHeaderMap& header_map) {
   sanitizeContentLength(header_map);
   sanitizeHeadersWithUnderscores(header_map);
+  sanitizePathWithFragment(header_map);
   if (!config_.uri_path_normalization_options().skip_path_normalization()) {
     auto path_result = path_normalizer_.normalizePathUri(header_map);
     if (!path_result.ok()) {

source/extensions/http/header_validators/envoy_default/http2_header_validator.cc

@@ -428,6 +428,7 @@ ValidationResult Http2HeaderValidator::validateResponseTrailers(
 ::Envoy::Http::ServerHeaderValidator::RequestHeadersTransformationResult
 ServerHttp2HeaderValidator::transformRequestHeaders(::Envoy::Http::RequestHeaderMap& header_map) {
   sanitizeHeadersWithUnderscores(header_map);
+  sanitizePathWithFragment(header_map);
   if (!config_.uri_path_normalization_options().skip_path_normalization()) {
     auto path_result = path_normalizer_.normalizePathUri(header_map);
     if (!path_result.ok()) {

test/extensions/filters/http/rbac/rbac_filter_integration_test.cc

@@ -530,6 +530,8 @@ TEST_P(RBACIntegrationTest, RouteOverride) {
 }
 
 TEST_P(RBACIntegrationTest, PathWithQueryAndFragmentWithOverride) {
+  // Allow client to send path fragment
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_CONFIG_WITH_PATH_EXACT_MATCH);
   config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment",
                                     "false");
@@ -559,6 +561,8 @@ TEST_P(RBACIntegrationTest, PathWithQueryAndFragmentWithOverride) {
 }
 
 TEST_P(RBACIntegrationTest, PathWithFragmentRejectedByDefault) {
+  // Prevent UHV in test client from stripping fragment
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_CONFIG_WITH_PATH_EXACT_MATCH);
   initialize();
 
@@ -582,6 +586,7 @@ TEST_P(RBACIntegrationTest, PathWithFragmentRejectedByDefault) {
 // This test ensures that the exact match deny rule is not affected by fragment and query
 // when Envoy is configured to strip both fragment and query.
 TEST_P(RBACIntegrationTest, DenyExactMatchIgnoresQueryAndFragment) {
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_CONFIG_DENY_WITH_PATH_EXACT_MATCH);
   config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment",
                                     "false");
@@ -920,6 +925,8 @@ TEST_P(RBACIntegrationTest, MatcherRouteOverride) {
 }
 
 TEST_P(RBACIntegrationTest, PathMatcherWithQueryAndFragmentWithOverride) {
+  // Allow client to send path fragment
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PATH_EXACT_MATCH);
   config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment",
                                     "false");
@@ -949,6 +956,8 @@ TEST_P(RBACIntegrationTest, PathMatcherWithQueryAndFragmentWithOverride) {
 }
 
 TEST_P(RBACIntegrationTest, PathMatcherWithFragmentRejectedByDefault) {
+  // Allow client to send path fragment
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_MATCHER_CONFIG_WITH_PATH_EXACT_MATCH);
   initialize();
 
@@ -972,6 +981,7 @@ TEST_P(RBACIntegrationTest, PathMatcherWithFragmentRejectedByDefault) {
 // This test ensures that the exact match deny rule is not affected by fragment and query
 // when Envoy is configured to strip both fragment and query.
 TEST_P(RBACIntegrationTest, MatcherDenyExactMatchIgnoresQueryAndFragment) {
+  disable_client_header_validation_ = true;
   config_helper_.prependFilter(RBAC_MATCHER_CONFIG_DENY_WITH_PATH_EXACT_MATCH);
   config_helper_.addRuntimeOverride("envoy.reloadable_features.http_reject_path_with_fragment",
                                     "false");

test/extensions/filters/network/http_connection_manager/config_test.cc

@@ -3079,6 +3079,7 @@ TEST_F(HttpConnectionManagerConfigTest, DefaultHeaderValidatorConfig) {
 #ifdef ENVOY_ENABLE_UHV
   EXPECT_NE(nullptr, config.makeHeaderValidator(Http::Protocol::Http2));
   EXPECT_FALSE(proto_config.restrict_http_methods());
+  EXPECT_FALSE(proto_config.strip_fragment_from_path());
   EXPECT_TRUE(proto_config.uri_path_normalization_options().skip_path_normalization());
   EXPECT_TRUE(proto_config.uri_path_normalization_options().skip_merging_slashes());
   EXPECT_EQ(proto_config.uri_path_normalization_options().path_with_escaped_slashes_action(),
@@ -3107,6 +3108,11 @@ TEST_F(HttpConnectionManagerConfigTest, TranslateLegacyConfigToDefaultHeaderVali
       "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
   )EOF";
 
+  // Make sure the http_reject_path_with_fragment runtime value is reflected in config
+  TestScopedRuntime scoped_runtime;
+  scoped_runtime.mergeValues(
+      {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}});
+
   ::envoy::extensions::http::header_validators::envoy_default::v3::HeaderValidatorConfig
       proto_config;
   DefaultHeaderValidatorFactoryConfigOverride factory(proto_config);
@@ -3121,6 +3127,7 @@ TEST_F(HttpConnectionManagerConfigTest, TranslateLegacyConfigToDefaultHeaderVali
                                      filter_config_provider_manager_);
 #ifdef ENVOY_ENABLE_UHV
   EXPECT_NE(nullptr, config.makeHeaderValidator(Http::Protocol::Http2));
+  EXPECT_TRUE(proto_config.strip_fragment_from_path());
   EXPECT_FALSE(proto_config.restrict_http_methods());
   EXPECT_FALSE(proto_config.uri_path_normalization_options().skip_path_normalization());
   EXPECT_FALSE(proto_config.uri_path_normalization_options().skip_merging_slashes());

test/extensions/http/header_validators/envoy_default/header_validator_test.cc

@@ -290,10 +290,18 @@ TEST_F(BaseHeaderValidatorTest, ValidatePathHeaderCharactersQuery) {
                              UhvResponseCodeDetail::get().InvalidUrl);
 }
 
-TEST_F(BaseHeaderValidatorTest, ValidatePathHeaderCharactersFragment) {
+TEST_F(BaseHeaderValidatorTest, PathWithFragmentRejectedByDefault) {
+  HeaderString invalid{"/root?x=1#fragment"};
+  auto uhv = createBase(empty_config);
+
+  EXPECT_REJECT_WITH_DETAILS(uhv->validatePathHeaderCharacters(invalid),
+                             UhvResponseCodeDetail::get().FragmentInUrlPath);
+}
+
+TEST_F(BaseHeaderValidatorTest, PathWithFragmentAllowedWhenConfigured) {
   HeaderString valid{"/root?x=1#fragment"};
   HeaderString invalid{"/root#frag|ment"};
-  auto uhv = createBase(empty_config);
+  auto uhv = createBase(fragment_in_path_allowed);
 
   EXPECT_ACCEPT(uhv->validatePathHeaderCharacters(valid));
   EXPECT_REJECT_WITH_DETAILS(uhv->validatePathHeaderCharacters(invalid),

test/extensions/http/header_validators/envoy_default/header_validator_test.h

@@ -58,6 +58,10 @@ class HeaderValidatorTest {
       skip_path_normalization: true
       path_with_escaped_slashes_action: UNESCAPE_AND_REDIRECT
     )EOF";
+
+  static constexpr absl::string_view fragment_in_path_allowed = R"EOF(
+    strip_fragment_from_path: true
+    )EOF";
 };
 
 } // namespace EnvoyDefault

test/extensions/http/header_validators/envoy_default/http_common_validation_test.cc

@@ -16,6 +16,7 @@ namespace {
 
 using ::Envoy::Http::Protocol;
 using ::Envoy::Http::TestRequestHeaderMapImpl;
+using ::Envoy::Http::UhvResponseCodeDetail;
 
 // This test suite runs the same tests against both H/1 and H/2 header validators.
 class HttpCommonValidationTest : public HeaderValidatorTest,
@@ -72,6 +73,29 @@ TEST_P(HttpCommonValidationTest, MalformedUrlEncodingRejectedWithOverride) {
   EXPECT_REJECT_WITH_DETAILS(uhv->transformRequestHeaders(headers), "uhv.invalid_url");
 }
 
+TEST_P(HttpCommonValidationTest, PathWithFragmentRejectedByDefault) {
+  TestRequestHeaderMapImpl headers{{":scheme", "https"},
+                                   {":path", "/path/with?query=and#fragment"},
+                                   {":authority", "envoy.com"},
+                                   {":method", "GET"}};
+  auto uhv = createUhv(empty_config);
+
+  EXPECT_REJECT_WITH_DETAILS(uhv->validateRequestHeaders(headers),
+                             UhvResponseCodeDetail::get().FragmentInUrlPath);
+}
+
+TEST_P(HttpCommonValidationTest, FragmentStrippedFromPathWhenConfigured) {
+  TestRequestHeaderMapImpl headers{{":scheme", "https"},
+                                   {":path", "/path/with?query=and#fragment"},
+                                   {":authority", "envoy.com"},
+                                   {":method", "GET"}};
+  auto uhv = createUhv(fragment_in_path_allowed);
+
+  EXPECT_ACCEPT(uhv->validateRequestHeaders(headers));
+  EXPECT_ACCEPT(uhv->transformRequestHeaders(headers));
+  EXPECT_EQ(headers.path(), "/path/with?query=and");
+}
+
 } // namespace
 } // namespace EnvoyDefault
 } // namespace HeaderValidators

test/integration/protocol_integration_test.cc

@@ -3990,6 +3990,8 @@ TEST_P(DownstreamProtocolIntegrationTest, BadRequest) {
 }
 
 TEST_P(DownstreamProtocolIntegrationTest, PathWithFragmentRejectedByDefault) {
+  // Prevent UHV in test client from stripping fragment
+  disable_client_header_validation_ = true;
   initialize();
 
   codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http"))));