UHV: harmonize check for %00 with legacy path normalization

envoy/http/header_validator_errors.h

@@ -22,6 +22,7 @@ struct UhvResponseCodeDetailValues {
   const std::string InvalidHostDeprecatedUserInfo = "uhv.invalid_host_deprecated_user_info";
   const std::string FragmentInUrlPath = "uhv.fragment_in_url_path";
   const std::string EscapedSlashesInPath = "uhv.escaped_slashes_in_url_path";
+  const std::string Percent00InPath = "uhv.percent_00_in_url_path";
 };
 
 using UhvResponseCodeDetail = ConstSingleton<UhvResponseCodeDetailValues>;

source/common/http/conn_manager_impl.cc

@@ -973,6 +973,17 @@ uint32_t ConnectionManagerImpl::ActiveStream::localPort() {
   return ip->port();
 }
 
+namespace {
+bool streamErrorOnlyErrors(absl::string_view error_details) {
+  // Pre UHV HCM did not respect stream_error_on_invalid_http_message
+  // and only sent 400 for specific errors.
+  // TODO(#28555): make these errors respect the stream_error_on_invalid_http_message
+  return error_details == UhvResponseCodeDetail::get().FragmentInUrlPath ||
+         error_details == UhvResponseCodeDetail::get().EscapedSlashesInPath ||
+         error_details == UhvResponseCodeDetail::get().Percent00InPath;
+}
+} // namespace
+
 bool ConnectionManagerImpl::ActiveStream::validateHeaders() {
   if (header_validator_) {
     auto validation_result = header_validator_->validateRequestHeaders(*request_headers_);
@@ -1017,12 +1028,8 @@ bool ConnectionManagerImpl::ActiveStream::validateHeaders() {
         resetStream();
       } else {
         sendLocalReply(response_code, "", modify_headers, grpc_status, failure_details);
-        // Pre UHV HCM did not respect stream_error_on_invalid_http_message
-        // and only sent 400 when :path contained fragment or encoded slashes
-        // TODO(#28555): make these errors respect the stream_error_on_invalid_http_message
         if (!response_encoder_->streamErrorOnInvalidHttpMessage() &&
-            failure_details != UhvResponseCodeDetail::get().FragmentInUrlPath &&
-            failure_details != UhvResponseCodeDetail::get().EscapedSlashesInPath) {
+            !streamErrorOnlyErrors(failure_details)) {
           connection_manager_.handleCodecError(failure_details);
         }
       }

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

@@ -11,9 +11,22 @@ namespace EnvoyDefault {
 struct ConfigOverrides {
   ConfigOverrides() = default;
   ConfigOverrides(const Envoy::Runtime::Snapshot& snapshot)
-      : preserve_url_encoded_case_(
+      : reject_percent_00_(snapshot.getBoolean("envoy.uhv.reject_percent_00", true)),
+        preserve_url_encoded_case_(
             snapshot.getBoolean("envoy.uhv.preserve_url_encoded_case", true)) {}
 
+  // This flag enables check for the %00 sequence in the URL path. If this sequence is
+  // found request is rejected as invalid. This check requires path normalization to be
+  // enabled to occur.
+  // https://datatracker.ietf.org/doc/html/rfc3986#section-2.1 allows %00 sequence, and
+  // this check is implemented for backward compatibility with legacy path normalization
+  // only.
+  //
+  // This option currently is `true` by default and can be overridden using the
+  // "envoy.uhv.reject_percent_00" runtime value. Note that the default value
+  // will be changed to `false` in the future to make it RFC compliant.
+  const bool reject_percent_00_{true};
+
   // This flag enables preservation of the case of percent-encoded triplets in URL path for
   // compatibility with legacy path normalization.
   // https://datatracker.ietf.org/doc/html/rfc3986#section-2.1 mandates that uppercase

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

@@ -621,6 +621,41 @@ HeaderValidator::sanitizeEncodedSlashes(::Envoy::Http::RequestHeaderMap& header_
   return PathNormalizer::PathNormalizationResult::success();
 }
 
+PathNormalizer::PathNormalizationResult
+HeaderValidator::transformUrlPath(::Envoy::Http::RequestHeaderMap& header_map) {
+  if (!config_.uri_path_normalization_options().skip_path_normalization()) {
+    auto path_result = path_normalizer_.normalizePathUri(header_map);
+    if (!path_result.ok()) {
+      return path_result;
+    }
+    auto percent_00_result = checkForPercent00InUrlPath(header_map);
+    if (!percent_00_result.ok()) {
+      return {PathNormalizer::PathNormalizationResult::Action::Reject, percent_00_result.details()};
+    }
+  } else {
+    // Path normalization includes sanitization of encoded slashes for performance reasons.
+    // If normalization is disabled, sanitize encoded slashes here
+    auto result = sanitizeEncodedSlashes(header_map);
+    if (!result.ok()) {
+      return result;
+    }
+  }
+  return PathNormalizer::PathNormalizationResult::success();
+}
+
+HeaderValidator::HeaderValueValidationResult
+HeaderValidator::checkForPercent00InUrlPath(const ::Envoy::Http::RequestHeaderMap& header_map) {
+  if (!header_map.Path() || !config_overrides_.reject_percent_00_) {
+    return HeaderValueValidationResult::success();
+  }
+  if (absl::StrContains(header_map.getPathValue(), "%00")) {
+    return {HeaderValueValidationResult::Action::Reject,
+            UhvResponseCodeDetail::get().Percent00InPath};
+  }
+
+  return HeaderValueValidationResult::success();
+}
+
 } // namespace EnvoyDefault
 } // namespace HeaderValidators
 } // namespace Http

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

@@ -177,6 +177,19 @@ class HeaderValidator {
   PathNormalizer::PathNormalizationResult
   sanitizeEncodedSlashes(::Envoy::Http::RequestHeaderMap& header_map);
 
+  /**
+   * Transform URL path according to configuration (i.e. apply path normalization).
+   */
+  PathNormalizer::PathNormalizationResult
+  transformUrlPath(::Envoy::Http::RequestHeaderMap& header_map);
+
+  /**
+   * Check for presence of %00 sequence based on configuration.
+   * Reject request if %00 sequence was found.
+   */
+  HeaderValueValidationResult
+  checkForPercent00InUrlPath(const ::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

@@ -309,18 +309,9 @@ ServerHttp1HeaderValidator::transformRequestHeaders(::Envoy::Http::RequestHeader
   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()) {
-      return path_result;
-    }
-  } else {
-    // Path normalization includes sanitization of encoded slashes for performance reasons.
-    // If normalization is disabled, sanitize encoded slashes here
-    auto result = sanitizeEncodedSlashes(header_map);
-    if (!result.ok()) {
-      return result;
-    }
+  auto path_result = transformUrlPath(header_map);
+  if (!path_result.ok()) {
+    return path_result;
   }
   return ::Envoy::Http::ServerHeaderValidator::RequestHeadersTransformationResult::success();
 }

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

@@ -430,18 +430,9 @@ ::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()) {
-      return path_result;
-    }
-  } else {
-    // Path normalization includes sanitization of encoded slashes for performance reasons.
-    // If normalization is disabled, sanitize encoded slashes here
-    auto result = sanitizeEncodedSlashes(header_map);
-    if (!result.ok()) {
-      return result;
-    }
+  auto path_result = transformUrlPath(header_map);
+  if (!path_result.ok()) {
+    return path_result;
   }
 
   // Transform H/2 extended CONNECT to H/1 UPGRADE, so that request processing always observes H/1

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

@@ -27,16 +27,16 @@ class HttpCommonValidationTest : public HeaderValidatorTest,
         typed_config;
     TestUtility::loadFromYaml(std::string(config_yaml), typed_config);
 
+    ConfigOverrides overrides(scoped_runtime_.loader().snapshot());
     if (GetParam() == Protocol::Http11) {
       return std::make_unique<ServerHttp1HeaderValidator>(typed_config, Protocol::Http11, stats_,
-                                                          overrides_);
+                                                          overrides);
     }
     return std::make_unique<ServerHttp2HeaderValidator>(typed_config, GetParam(), stats_,
-                                                        overrides_);
+                                                        overrides);
   }
 
   TestScopedRuntime scoped_runtime_;
-  ConfigOverrides overrides_;
 };
 
 std::string protocolTestParamsToString(const ::testing::TestParamInfo<Protocol>& params) {
@@ -167,6 +167,28 @@ TEST_P(HttpCommonValidationTest, RejectEncodedSlashes) {
                              "uhv.escaped_slashes_in_url_path");
 }
 
+TEST_P(HttpCommonValidationTest, RejectPercent00ByDefault) {
+  ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"},
+                                                  {":method", "GET"},
+                                                  {":path", "/dir1%00dir2"},
+                                                  {":authority", "envoy.com"}};
+  auto uhv = createUhv(empty_config);
+  EXPECT_ACCEPT(uhv->validateRequestHeaders(headers));
+  EXPECT_REJECT_WITH_DETAILS(uhv->transformRequestHeaders(headers), "uhv.percent_00_in_url_path");
+}
+
+TEST_P(HttpCommonValidationTest, AllowPercent00WithOverride) {
+  scoped_runtime_.mergeValues({{"envoy.uhv.reject_percent_00", "false"}});
+  ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"},
+                                                  {":method", "GET"},
+                                                  {":path", "/dir1%00dir2"},
+                                                  {":authority", "envoy.com"}};
+  auto uhv = createUhv(empty_config);
+  EXPECT_ACCEPT(uhv->validateRequestHeaders(headers));
+  EXPECT_ACCEPT(uhv->transformRequestHeaders(headers));
+  EXPECT_EQ(headers.path(), "/dir1%00dir2");
+}
+
 } // namespace
 } // namespace EnvoyDefault
 } // namespace HeaderValidators

test/integration/default_header_validator_integration_test.cc

@@ -187,4 +187,55 @@ TEST_P(DownstreamUhvIntegrationTest, MalformedUrlEncodedTripletsAllowed) {
   ASSERT_TRUE(response->waitForEndStream());
 }
 
+// Without the `envoy.uhv.reject_percent_00` override UHV rejects requests with the %00
+// sequence.
+TEST_P(DownstreamUhvIntegrationTest, RejectPercent00) {
+  config_helper_.addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) -> void { hcm.mutable_normalize_path()->set_value(true); });
+  initialize();
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  // Start the request.
+  auto response = codec_client_->makeHeaderOnlyRequest(
+      Http::TestRequestHeaderMapImpl{{":method", "GET"},
+                                     {":path", "/path%00/to/something"},
+                                     {":scheme", "http"},
+                                     {":authority", "host"}});
+
+  ASSERT_TRUE(response->waitForEndStream());
+  ASSERT_TRUE(response->complete());
+  EXPECT_EQ("400", response->headers().getStatusValue());
+}
+
+TEST_P(DownstreamUhvIntegrationTest, UhvAllowsPercent00WithOverride) {
+  config_helper_.addRuntimeOverride("envoy.uhv.reject_percent_00", "false");
+  config_helper_.addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) -> void { hcm.mutable_normalize_path()->set_value(true); });
+  initialize();
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  // Start the request.
+  auto response = codec_client_->makeHeaderOnlyRequest(
+      Http::TestRequestHeaderMapImpl{{":method", "GET"},
+                                     {":path", "/path%00/to/something"},
+                                     {":scheme", "http"},
+                                     {":authority", "host"}});
+#ifdef ENVOY_ENABLE_UHV
+  waitForNextUpstreamRequest();
+
+  EXPECT_EQ(upstream_request_->headers().getPathValue(), "/path%00/to/something");
+
+  // Send a headers only response.
+  upstream_request_->encodeHeaders(default_response_headers_, true);
+  ASSERT_TRUE(response->waitForEndStream());
+#else
+  // In legacy mode %00 in URL path always causes request to be rejected
+  ASSERT_TRUE(response->waitForEndStream());
+  ASSERT_TRUE(response->complete());
+  EXPECT_EQ("400", response->headers().getStatusValue());
+#endif
+}
+
 } // namespace Envoy