[9.1.x] Backport HTTP Validations (#9016)

* Fail fast on HTTP/2 header validation (#9009)

(cherry picked from commit eaef5e8)

Conflicts:
	proxy/http2/HTTP2.cc

* Restrict unknown scheme of HTTP/2 request (#9010)

Strictly following RFC 3986 Section 3.1

```
scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
```

(cherry picked from commit c56f872)

Conflicts:
	proxy/http2/HTTP2.cc

* Add control char check in MIME Parser (#9011)

(cherry picked from commit 2f363d9)

Conflicts:
	tests/gold_tests/headers/good_request_after_bad.test.py
	tests/gold_tests/logging/gold/field-json-test.gold
	tests/gold_tests/logging/log-field-json.test.py

* Add content length mismatch check on handling HEADERS frame and CONTINUATION frame (#9012)

* Add content length mismatch check on handling HEADERS frame and CONTINUATION frame

* Correct error class of HTTP/2 malformed requests

(cherry picked from commit e921228)

* Ignore POST request case from a check for background fill (#9013)

(cherry picked from commit 1f3e111)

Co-authored-by: Masakazu Kitajo <maskit@apache.org>

include/tscore/ParseRules.h

@@ -128,7 +128,7 @@ class ParseRules
   static CTypeResult is_empty(char c);            // wslfcr,#
   static CTypeResult is_alnum(char c);            // 0-9,A-Z,a-z
   static CTypeResult is_space(char c);            // ' ' HT,VT,NP,CR,LF
-  static CTypeResult is_control(char c);          // 0-31 127
+  static CTypeResult is_control(char c);          // 0x00-0x08, 0x0a-0x1f, 0x7f
   static CTypeResult is_mime_sep(char c);         // ()<>,;\"/[]?{} \t
   static CTypeResult is_http_field_name(char c);  // not : or mime_sep except for @
   static CTypeResult is_http_field_value(char c); // not CR, LF, comma, or "
@@ -667,14 +667,24 @@ ParseRules::is_space(char c)
 #endif
 }
 
+/**
+   Return true if @c is a control char except HTAB(0x09) and SP(0x20).
+   If you need to check @c is HTAB or SP, use `ParseRules::is_ws`.
+ */
 inline CTypeResult
 ParseRules::is_control(char c)
 {
 #ifndef COMPILE_PARSE_RULES
   return (parseRulesCType[(unsigned char)c] & is_control_BIT);
 #else
-  if (((unsigned char)c) < 32 || ((unsigned char)c) == 127)
+  if (c == CHAR_HT || c == CHAR_SP) {
+    return false;
+  }
+
+  if (((unsigned char)c) < 0x20 || c == 0x7f) {
     return true;
+  }
+
   return false;
 #endif
 }

proxy/hdrs/MIME.cc

@@ -2611,6 +2611,21 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char
 
     int field_name_wks_idx = hdrtoken_tokenize(field_name.data(), field_name.size());
 
+    if (field_name_wks_idx < 0) {
+      for (auto i : field_name) {
+        if (ParseRules::is_control(i)) {
+          return PARSE_RESULT_ERROR;
+        }
+      }
+    }
+
+    // RFC 9110 Section 5.5. Field Values
+    for (char i : field_value) {
+      if (ParseRules::is_control(i)) {
+        return PARSE_RESULT_ERROR;
+      }
+    }
+
     ///////////////////////////////////////////
     // build and insert the new field object //
     ///////////////////////////////////////////

proxy/hdrs/MIME.h

@@ -170,7 +170,7 @@ struct MIMEField {
   int value_get_comma_list(StrList *list) const;
 
   void name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length);
-  bool name_is_valid() const;
+  bool name_is_valid(uint32_t invalid_char_bits = is_control_BIT) const;
 
   void value_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length);
   void value_set_int(HdrHeap *heap, MIMEHdrImpl *mh, int32_t value);
@@ -182,7 +182,7 @@ struct MIMEField {
   // Other separators (e.g. ';' in Set-cookie/Cookie) are also possible
   void value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length, bool prepend_comma = false,
                     const char separator = ',');
-  bool value_is_valid() const;
+  bool value_is_valid(uint32_t invalid_char_bits = is_control_BIT) const;
   int has_dups() const;
 };
 
@@ -835,13 +835,13 @@ MIMEField::name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length
   -------------------------------------------------------------------------*/
 
 inline bool
-MIMEField::name_is_valid() const
+MIMEField::name_is_valid(uint32_t invalid_char_bits) const
 {
   const char *name;
   int length;
 
   for (name = name_get(&length); length > 0; length--) {
-    if (ParseRules::is_control(name[length - 1])) {
+    if (ParseRules::is_type(name[length - 1], invalid_char_bits)) {
       return false;
     }
   }
@@ -944,13 +944,13 @@ MIMEField::value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int l
   -------------------------------------------------------------------------*/
 
 inline bool
-MIMEField::value_is_valid() const
+MIMEField::value_is_valid(uint32_t invalid_char_bits) const
 {
   const char *value;
   int length;
 
   for (value = value_get(&length); length > 0; length--) {
-    if (ParseRules::is_control(value[length - 1])) {
+    if (ParseRules::is_type(value[length - 1], invalid_char_bits)) {
       return false;
     }
   }

proxy/hdrs/URL.cc

@@ -114,6 +114,36 @@ validate_host_name(std::string_view addr)
   return std::all_of(addr.begin(), addr.end(), &is_host_char);
 }
 
+/**
+   Checks if the (un-well-known) scheme is valid
+
+   RFC 3986 Section 3.1
+   These are the valid characters in a scheme:
+     scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+   return an error if there is another character in the scheme
+*/
+bool
+validate_scheme(std::string_view scheme)
+{
+  if (scheme.empty()) {
+    return false;
+  }
+
+  if (!ParseRules::is_alpha(scheme[0])) {
+    return false;
+  }
+
+  for (size_t i = 0; i < scheme.size(); ++i) {
+    const char &c = scheme[i];
+
+    if (!(ParseRules::is_alnum(c) != 0 || c == '+' || c == '-' || c == '.')) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
 
@@ -1147,19 +1177,9 @@ url_parse_scheme(HdrHeap *heap, URLImpl *url, const char **start, const char *en
 
         if (!(scheme_wks_idx > 0 && hdrtoken_wks_to_token_type(scheme_wks) == HDRTOKEN_TYPE_SCHEME)) {
           // Unknown scheme, validate the scheme
-
-          // RFC 3986 Section 3.1
-          // These are the valid characters in a scheme:
-          //   scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
-          // return an error if there is another character in the scheme
-          if (!ParseRules::is_alpha(*scheme_start)) {
+          if (!validate_scheme({scheme_start, static_cast<size_t>(scheme_end - scheme_start)})) {
             return PARSE_RESULT_ERROR;
           }
-          for (cur = scheme_start + 1; cur < scheme_end; ++cur) {
-            if (!(ParseRules::is_alnum(*cur) != 0 || *cur == '+' || *cur == '-' || *cur == '.')) {
-              return PARSE_RESULT_ERROR;
-            }
-          }
         }
         url_scheme_set(heap, url, scheme_start, scheme_wks_idx, scheme_end - scheme_start, copy_strings_p);
       }

proxy/hdrs/URL.h

@@ -157,6 +157,8 @@ extern int URL_LEN_MMST;
 
 /* Public */
 bool validate_host_name(std::string_view addr);
+bool validate_scheme(std::string_view scheme);
+
 void url_init();
 
 URLImpl *url_create(HdrHeap *heap);

proxy/hdrs/unit_tests/test_Hdrs.cc

@@ -530,6 +530,55 @@ TEST_CASE("HdrTest", "[proxy][hdrtest]")
   mime_init();
   http_init();
 
+  SECTION("Field Char Check")
+  {
+    static struct {
+      std::string_view line;
+      ParseResult expected;
+    } test_cases[] = {
+      ////
+      // Field Name
+      {"Content-Length: 10\r\n", PARSE_RESULT_CONT},
+      {"Content-Length\x0b: 10\r\n", PARSE_RESULT_ERROR},
+      ////
+      // Field Value
+      // SP
+      {"Content-Length: 10\r\n", PARSE_RESULT_CONT},
+      {"Foo: ab cd\r\n", PARSE_RESULT_CONT},
+      // HTAB
+      {"Foo: ab\td/cd\r\n", PARSE_RESULT_CONT},
+      // VCHAR
+      {"Foo: ab\x21/cd\r\n", PARSE_RESULT_CONT},
+      {"Foo: ab\x7e/cd\r\n", PARSE_RESULT_CONT},
+      // DEL
+      {"Foo: ab\x7f/cd\r\n", PARSE_RESULT_ERROR},
+      // obs-text
+      {"Foo: ab\x80/cd\r\n", PARSE_RESULT_CONT},
+      {"Foo: ab\xff/cd\r\n", PARSE_RESULT_CONT},
+      // control char
+      {"Content-Length: 10\x0b\r\n", PARSE_RESULT_ERROR},
+      {"Content-Length:\x0b 10\r\n", PARSE_RESULT_ERROR},
+      {"Foo: ab\x1d/cd\r\n", PARSE_RESULT_ERROR},
+    };
+
+    MIMEHdr hdr;
+    MIMEParser parser;
+    mime_parser_init(&parser);
+
+    for (const auto &t : test_cases) {
+      mime_parser_clear(&parser);
+
+      const char *start = t.line.data();
+      const char *end   = start + t.line.size();
+
+      int r = hdr.parse(&parser, &start, end, false, false, false);
+      if (r != t.expected) {
+        std::printf("Expected %s is %s, but not", t.line.data(), t.expected == PARSE_RESULT_ERROR ? "invalid" : "valid");
+        CHECK(false);
+      }
+    }
+  }
+
   SECTION("Test parse date")
   {
     static struct {

proxy/hdrs/unit_tests/test_URL.cc

@@ -53,6 +53,27 @@ TEST_CASE("ValidateURL", "[proxy][validurl]")
   }
 }
 
+TEST_CASE("Validate Scheme", "[proxy][validscheme]")
+{
+  static const struct {
+    std::string_view text;
+    bool valid;
+  } scheme_test_cases[] = {{"http", true},      {"https", true},      {"example", true},    {"example.", true},
+                           {"example++", true}, {"example--.", true}, {"++example", false}, {"--example", false},
+                           {".example", false}, {"example://", false}};
+
+  for (auto i : scheme_test_cases) {
+    // it's pretty hard to debug with
+    //     CHECK(validate_scheme(i.text) == i.valid);
+
+    std::string_view text = i.text;
+    if (validate_scheme(text) != i.valid) {
+      std::printf("Validation of scheme: \"%s\", expected %s, but not\n", text.data(), (i.valid ? "true" : "false"));
+      CHECK(false);
+    }
+  }
+}
+
 namespace UrlImpl
 {
 bool url_is_strictly_compliant(const char *start, const char *end);

proxy/http/HttpTunnel.h

@@ -528,12 +528,14 @@ HttpTunnel::has_consumer_besides_client() const
       continue;
     }
 
-    if (consumer.vc_type == HT_HTTP_CLIENT) {
-      res = false;
+    switch (consumer.vc_type) {
+    case HT_HTTP_CLIENT:
       continue;
-    } else {
-      res = true;
-      break;
+    case HT_HTTP_SERVER:
+      // ignore uploading data to servers
+      continue;
+    default:
+      return true;
     }
   }
 

proxy/http2/HTTP2.cc

@@ -426,11 +426,21 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
 
   if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_REQUEST) {
     // :scheme
-    if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME); field != nullptr && field->value_is_valid()) {
+    if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME);
+        field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
       int scheme_len;
       const char *scheme = field->value_get(&scheme_len);
+      const char *scheme_wks;
+
+      int scheme_wks_idx = hdrtoken_tokenize(scheme, scheme_len, &scheme_wks);
+
+      if (!(scheme_wks_idx > 0 && hdrtoken_wks_to_token_type(scheme_wks) == HDRTOKEN_TYPE_SCHEME)) {
+        // unkown scheme, validate the scheme
+        if (!validate_scheme({scheme, static_cast<size_t>(scheme_len)})) {
+          return PARSE_RESULT_ERROR;
+        }
+      }
 
-      int scheme_wks_idx = hdrtoken_tokenize(scheme, scheme_len);
       url_scheme_set(headers->m_heap, headers->m_http->u.req.m_url_impl, scheme, scheme_wks_idx, scheme_len, true);
 
       headers->field_delete(field);
@@ -440,7 +450,7 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
 
     // :authority
     if (MIMEField *field = headers->field_find(HTTP2_VALUE_AUTHORITY, HTTP2_LEN_AUTHORITY);
-        field != nullptr && field->value_is_valid()) {
+        field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
       int authority_len;
       const char *authority = field->value_get(&authority_len);
 
@@ -452,7 +462,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
     }
 
     // :path
-    if (MIMEField *field = headers->field_find(HTTP2_VALUE_PATH, HTTP2_LEN_PATH); field != nullptr && field->value_is_valid()) {
+    if (MIMEField *field = headers->field_find(HTTP2_VALUE_PATH, HTTP2_LEN_PATH);
+        field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
       int path_len;
       const char *path = field->value_get(&path_len);
 
@@ -470,7 +481,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
     }
 
     // :method
-    if (MIMEField *field = headers->field_find(HTTP2_VALUE_METHOD, HTTP2_LEN_METHOD); field != nullptr && field->value_is_valid()) {
+    if (MIMEField *field = headers->field_find(HTTP2_VALUE_METHOD, HTTP2_LEN_METHOD);
+        field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
       int method_len;
       const char *method = field->value_get(&method_len);
 
@@ -500,7 +512,7 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
   // Check validity of all names and values
   MIMEFieldIter iter;
   for (auto *mf = headers->iter_get_first(&iter); mf != nullptr; mf = headers->iter_get_next(&iter)) {
-    if (!mf->name_is_valid() || !mf->value_is_valid()) {
+    if (!mf->name_is_valid(is_control_BIT | is_ws_BIT) || !mf->value_is_valid()) {
       return PARSE_RESULT_ERROR;
     }
   }

proxy/http2/Http2ConnectionState.cc

@@ -140,7 +140,7 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
       return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
     }
     if (!stream->payload_length_is_valid()) {
-      return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
+      return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
                         "recv data bad payload length");
     }
 
@@ -385,6 +385,12 @@ rcv_headers_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
       }
     }
 
+    // Check Content-Length & payload length when END_STREAM flag is true
+    if (stream->recv_end_stream && !stream->payload_length_is_valid()) {
+      return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
+                        "recv data bad payload length");
+    }
+
     // Set up the State Machine
     if (!empty_request) {
       SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
@@ -944,6 +950,12 @@ rcv_continuation_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
       }
     }
 
+    // Check Content-Length & payload length when END_STREAM flag is true
+    if (stream->recv_end_stream && !stream->payload_length_is_valid()) {
+      return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
+                        "recv data bad payload length");
+    }
+
     // Set up the State Machine
     SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
     stream->mark_milestone(Http2StreamMilestone::START_TXN);

proxy/http2/Http2Stream.cc

@@ -233,7 +233,11 @@ Http2Stream::send_request(Http2ConnectionState &cstate)
   this->_http_sm_id = this->_sm->sm_id;
 
   // Convert header to HTTP/1.1 format
-  http2_convert_header_from_2_to_1_1(&_req_header);
+  if (http2_convert_header_from_2_to_1_1(&_req_header) == PARSE_RESULT_ERROR) {
+    // There's no way to cause Bad Request directly at this time.
+    // Set an invalid method so it causes an error later.
+    _req_header.method_set("\xffVOID", 1);
+  }
 
   // Write header to a buffer.  Borrowing logic from HttpSM::write_header_into_buffer.
   // Seems like a function like this ought to be in HTTPHdr directly