You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2022/08/08 23:13:43 UTC
[trafficserver] 01/01: Fail fast on HTTP/2 header validation
This is an automated email from the ASF dual-hosted git repository.
masaori pushed a commit to branch asf-master-0809-1
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit def83057745b9802785eb76d9926178e1a863269
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Fri Jan 28 14:18:58 2022 +0900
Fail fast on HTTP/2 header validation
---
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 dc418fb56..ba9501d82 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;
};
@@ -972,13 +972,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;
}
}
@@ -1081,13 +1081,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 f1360ddc6..73cc4022e 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);
@@ -444,7 +445,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);
@@ -456,7 +457,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);
@@ -474,7 +476,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);
@@ -503,7 +506,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