You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/08/10 17:04:30 UTC
[trafficserver] branch 9.2.x updated: Fail fast on HTTP/2 header validation (#9009)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 07de4d32a Fail fast on HTTP/2 header validation (#9009)
07de4d32a is described below
commit 07de4d32a7587ccd748a1365a0a3f19f15169070
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Aug 9 09:28:52 2022 +0900
Fail fast on HTTP/2 header validation (#9009)
Co-authored-by: Masakazu Kitajo <ma...@apache.org>
(cherry picked from commit eaef5e8d7a3f4e2540caec516bffc59fa22d2472)
---
proxy/hdrs/MIME.h | 12 ++++++------
proxy/http2/HTTP2.cc | 13 ++++++++-----
proxy/http2/Http2Stream.cc | 6 +++++-
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/proxy/hdrs/MIME.h b/proxy/hdrs/MIME.h
index 6474dd0e8..1dc9da4d1 100644
--- a/proxy/hdrs/MIME.h
+++ b/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;
};
@@ -836,13 +836,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;
}
}
@@ -945,13 +945,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;
}
}
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index df31bc932..5b263bbe7 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -430,7 +430,8 @@ 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;
@@ -453,7 +454,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);
@@ -465,7 +466,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);
@@ -483,7 +485,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);
@@ -512,7 +515,7 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
// Check validity of all names and values
for (auto &mf : *headers) {
- 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;
}
}
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index a3e37819d..d019d44ef 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -236,7 +236,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