http: modify authority validation to allow @ character (envoyproxy#35602

) Envoy uses the `authorityIsValid()` to validate the authority header for both H/1 and H/2 codecs. Previously Envoy used the nghttp2 validator and in envoyproxy#24943 this was changed to oghttp2's implementation. The two implementations differ in the way they handle the "@" character (nghttp2 allows it, and oghttp2 doesn't). According to the H/2 spec, the "@" character is not allowed as part of the authority header. However, for H/1 it is allowed as part of the "user-info@host:port" structure of the authority header. This PR changes the validator to be similar to the nghttp2 implemenation. The change can be temporarily dis In the future, when Envoy fully supports UHV (envoyproxy#10646), the H/1 and H/2 validation parts should be decoupled, and the oghttp2 authority validation can be used for H/2. --------- Signed-off-by: Adi Suissa-Peleg <[email protected]>
daixiang0 · Aug 19, 2024 · c618385 · c618385
1 parent 7a09bfb
commit c618385
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 0 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -55,6 +55,12 @@ minor_behavior_changes:
   change: |
     When Lua script executes httpCall, backpressure is exercised when receiving body from downstream client. This behavior can be reverted
     by setting the runtime guard ``envoy.reloadable_features.lua_flow_control_while_http_call`` to false.
+- area: http
+  change: |
+    Modified the authority header value validator to allow the same characters as oghttp2
+    plus the "@" character. This is compliant with nghttp2, and supports the HTTP/1 use-cases
+    that allow user-info@ as part of the authority. This behavior can be reverted by setting
+    the runtime guard ``envoy.reloadable_features.internal_authority_header_validator`` to false.
 - area: sni
   change: |
     When computing SNI and SAN value for the auto-sni and auto-san verification feature,

diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc
@@ -235,7 +235,96 @@ bool HeaderUtility::headerNameContainsUnderscore(const absl::string_view header_
   return header_name.find('_') != absl::string_view::npos;
 }
 
+namespace {
+// This function validates the authority header for both HTTP/1 and HTTP/2.
+// Note the HTTP/1 spec allows "user-info@host:port" for the authority, whereas
+// the HTTP/2 spec only allows "host:port". Thus, this function permits all the
+// HTTP/2 valid characters (similar to oghttp2's implementation) and the "@" character.
+// Once UHV is used, this function should be removed, and the HTTP/1 and HTTP/2
+// authority validations should be different.
+bool check_authority_h1_h2(const absl::string_view header_value) {
+  static constexpr char ValidAuthorityChars[] = {
+      0 /* NUL  */, 0 /* SOH  */, 0 /* STX  */, 0 /* ETX  */,
+      0 /* EOT  */, 0 /* ENQ  */, 0 /* ACK  */, 0 /* BEL  */,
+      0 /* BS   */, 0 /* HT   */, 0 /* LF   */, 0 /* VT   */,
+      0 /* FF   */, 0 /* CR   */, 0 /* SO   */, 0 /* SI   */,
+      0 /* DLE  */, 0 /* DC1  */, 0 /* DC2  */, 0 /* DC3  */,
+      0 /* DC4  */, 0 /* NAK  */, 0 /* SYN  */, 0 /* ETB  */,
+      0 /* CAN  */, 0 /* EM   */, 0 /* SUB  */, 0 /* ESC  */,
+      0 /* FS   */, 0 /* GS   */, 0 /* RS   */, 0 /* US   */,
+      0 /* SPC  */, 1 /* !    */, 0 /* "    */, 0 /* #    */,
+      1 /* $    */, 1 /* %    */, 1 /* &    */, 1 /* '    */,
+      1 /* (    */, 1 /* )    */, 1 /* *    */, 1 /* +    */,
+      1 /* ,    */, 1 /* -    */, 1 /* . */,    0 /* /    */,
+      1 /* 0    */, 1 /* 1    */, 1 /* 2    */, 1 /* 3    */,
+      1 /* 4    */, 1 /* 5    */, 1 /* 6    */, 1 /* 7    */,
+      1 /* 8    */, 1 /* 9    */, 1 /* :    */, 1 /* ;    */,
+      0 /* <    */, 1 /* =    */, 0 /* >    */, 0 /* ?    */,
+      1 /* @    */, 1 /* A    */, 1 /* B    */, 1 /* C    */,
+      1 /* D    */, 1 /* E    */, 1 /* F    */, 1 /* G    */,
+      1 /* H    */, 1 /* I    */, 1 /* J    */, 1 /* K    */,
+      1 /* L    */, 1 /* M    */, 1 /* N    */, 1 /* O    */,
+      1 /* P    */, 1 /* Q    */, 1 /* R    */, 1 /* S    */,
+      1 /* T    */, 1 /* U    */, 1 /* V    */, 1 /* W    */,
+      1 /* X    */, 1 /* Y    */, 1 /* Z    */, 1 /* [    */,
+      0 /* \    */, 1 /* ]    */, 0 /* ^    */, 1 /* _    */,
+      0 /* `    */, 1 /* a    */, 1 /* b    */, 1 /* c    */,
+      1 /* d    */, 1 /* e    */, 1 /* f    */, 1 /* g    */,
+      1 /* h    */, 1 /* i    */, 1 /* j    */, 1 /* k    */,
+      1 /* l    */, 1 /* m    */, 1 /* n    */, 1 /* o    */,
+      1 /* p    */, 1 /* q    */, 1 /* r    */, 1 /* s    */,
+      1 /* t    */, 1 /* u    */, 1 /* v    */, 1 /* w    */,
+      1 /* x    */, 1 /* y    */, 1 /* z    */, 0 /* {    */,
+      0 /* |    */, 0 /* }    */, 1 /* ~    */, 0 /* DEL  */,
+      0 /* 0x80 */, 0 /* 0x81 */, 0 /* 0x82 */, 0 /* 0x83 */,
+      0 /* 0x84 */, 0 /* 0x85 */, 0 /* 0x86 */, 0 /* 0x87 */,
+      0 /* 0x88 */, 0 /* 0x89 */, 0 /* 0x8a */, 0 /* 0x8b */,
+      0 /* 0x8c */, 0 /* 0x8d */, 0 /* 0x8e */, 0 /* 0x8f */,
+      0 /* 0x90 */, 0 /* 0x91 */, 0 /* 0x92 */, 0 /* 0x93 */,
+      0 /* 0x94 */, 0 /* 0x95 */, 0 /* 0x96 */, 0 /* 0x97 */,
+      0 /* 0x98 */, 0 /* 0x99 */, 0 /* 0x9a */, 0 /* 0x9b */,
+      0 /* 0x9c */, 0 /* 0x9d */, 0 /* 0x9e */, 0 /* 0x9f */,
+      0 /* 0xa0 */, 0 /* 0xa1 */, 0 /* 0xa2 */, 0 /* 0xa3 */,
+      0 /* 0xa4 */, 0 /* 0xa5 */, 0 /* 0xa6 */, 0 /* 0xa7 */,
+      0 /* 0xa8 */, 0 /* 0xa9 */, 0 /* 0xaa */, 0 /* 0xab */,
+      0 /* 0xac */, 0 /* 0xad */, 0 /* 0xae */, 0 /* 0xaf */,
+      0 /* 0xb0 */, 0 /* 0xb1 */, 0 /* 0xb2 */, 0 /* 0xb3 */,
+      0 /* 0xb4 */, 0 /* 0xb5 */, 0 /* 0xb6 */, 0 /* 0xb7 */,
+      0 /* 0xb8 */, 0 /* 0xb9 */, 0 /* 0xba */, 0 /* 0xbb */,
+      0 /* 0xbc */, 0 /* 0xbd */, 0 /* 0xbe */, 0 /* 0xbf */,
+      0 /* 0xc0 */, 0 /* 0xc1 */, 0 /* 0xc2 */, 0 /* 0xc3 */,
+      0 /* 0xc4 */, 0 /* 0xc5 */, 0 /* 0xc6 */, 0 /* 0xc7 */,
+      0 /* 0xc8 */, 0 /* 0xc9 */, 0 /* 0xca */, 0 /* 0xcb */,
+      0 /* 0xcc */, 0 /* 0xcd */, 0 /* 0xce */, 0 /* 0xcf */,
+      0 /* 0xd0 */, 0 /* 0xd1 */, 0 /* 0xd2 */, 0 /* 0xd3 */,
+      0 /* 0xd4 */, 0 /* 0xd5 */, 0 /* 0xd6 */, 0 /* 0xd7 */,
+      0 /* 0xd8 */, 0 /* 0xd9 */, 0 /* 0xda */, 0 /* 0xdb */,
+      0 /* 0xdc */, 0 /* 0xdd */, 0 /* 0xde */, 0 /* 0xdf */,
+      0 /* 0xe0 */, 0 /* 0xe1 */, 0 /* 0xe2 */, 0 /* 0xe3 */,
+      0 /* 0xe4 */, 0 /* 0xe5 */, 0 /* 0xe6 */, 0 /* 0xe7 */,
+      0 /* 0xe8 */, 0 /* 0xe9 */, 0 /* 0xea */, 0 /* 0xeb */,
+      0 /* 0xec */, 0 /* 0xed */, 0 /* 0xee */, 0 /* 0xef */,
+      0 /* 0xf0 */, 0 /* 0xf1 */, 0 /* 0xf2 */, 0 /* 0xf3 */,
+      0 /* 0xf4 */, 0 /* 0xf5 */, 0 /* 0xf6 */, 0 /* 0xf7 */,
+      0 /* 0xf8 */, 0 /* 0xf9 */, 0 /* 0xfa */, 0 /* 0xfb */,
+      0 /* 0xfc */, 0 /* 0xfd */, 0 /* 0xfe */, 0 /* 0xff */
+  };
+
+  for (const uint8_t c : header_value) {
+    if (!ValidAuthorityChars[c]) {
+      return false;
+    }
+  }
+  return true;
+}
+} // namespace
+
 bool HeaderUtility::authorityIsValid(const absl::string_view header_value) {
+  if (Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.internal_authority_header_validator")) {
+    return check_authority_h1_h2(header_value);
+  }
+
 #ifdef ENVOY_NGHTTP2
   if (!Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.http2_validate_authority_with_quiche")) {

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -62,6 +62,7 @@ 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_http_route_connect_proxy_by_default);
+RUNTIME_GUARD(envoy_reloadable_features_internal_authority_header_validator);
 RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_remove_jwt_from_query_params);
 RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_validate_uri);
 RUNTIME_GUARD(envoy_reloadable_features_lua_flow_control_while_http_call);

diff --git a/test/common/http/header_utility_test.cc b/test/common/http/header_utility_test.cc
@@ -1152,6 +1152,24 @@ TEST(HeaderIsValidTest, ValidHeaderValuesAreAccepted) {
 TEST(HeaderIsValidTest, AuthorityIsValid) {
   EXPECT_TRUE(HeaderUtility::authorityIsValid("strangebutlegal$-%&'"));
   EXPECT_FALSE(HeaderUtility::authorityIsValid("illegal{}"));
+  // Validate that the "@" character is allowed.
+  // TODO(adisuissa): Once the envoy.reloadable_features.internal_authority_header_validator
+  // runtime flag is deprecated, this test should only validate the assignment
+  // to "true".
+  {
+    TestScopedRuntime scoped_runtime;
+    scoped_runtime.mergeValues(
+        {{"envoy.reloadable_features.internal_authority_header_validator", "true"}});
+    EXPECT_TRUE(HeaderUtility::authorityIsValid("[email protected]'"));
+  }
+  {
+    TestScopedRuntime scoped_runtime;
+    scoped_runtime.mergeValues(
+        {{"envoy.reloadable_features.internal_authority_header_validator", "false"}});
+    // When the above is false, Envoy should use oghttp2's validator which will
+    // reject the "@" character.
+    EXPECT_FALSE(HeaderUtility::authorityIsValid("[email protected]'"));
+  }
 }
 
 TEST(HeaderIsValidTest, IsConnect) {