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:42 UTC
[trafficserver] branch asf-master-0809-1 created (now def830577)
This is an automated email from the ASF dual-hosted git repository.
masaori pushed a change to branch asf-master-0809-1
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
at def830577 Fail fast on HTTP/2 header validation
This branch includes the following new commits:
new def830577 Fail fast on HTTP/2 header validation
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[trafficserver] 01/01: Fail fast on HTTP/2 header validation
Posted by ma...@apache.org.
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