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:18:15 UTC
[trafficserver] branch asf-master-0809-2 created (now 2424565f9)
This is an automated email from the ASF dual-hosted git repository.
masaori pushed a change to branch asf-master-0809-2
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
at 2424565f9 Restrict unknown scheme of HTTP/2 request
This branch includes the following new commits:
new 2424565f9 Restrict unknown scheme of HTTP/2 request
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: Restrict unknown scheme of HTTP/2 request
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-2
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2424565f96fe861e52f31e241ced87c73c5fe165
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Mar 29 14:37:16 2022 +0900
Restrict unknown scheme of HTTP/2 request
Strictly following RFC 3986 Section 3.1
```
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
```
---
proxy/hdrs/URL.cc | 42 +++++++++++++++++++++++++++++----------
proxy/hdrs/URL.h | 2 ++
proxy/hdrs/unit_tests/test_URL.cc | 21 ++++++++++++++++++++
proxy/http2/HTTP2.cc | 11 +++++++++-
4 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/proxy/hdrs/URL.cc b/proxy/hdrs/URL.cc
index 5614a9b9c..2896cdfb9 100644
--- a/proxy/hdrs/URL.cc
+++ b/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;
+}
+
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
@@ -1133,19 +1163,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->set_scheme(heap, scheme_start, scheme_wks_idx, scheme_end - scheme_start, copy_strings_p);
}
diff --git a/proxy/hdrs/URL.h b/proxy/hdrs/URL.h
index ac9d4a0d9..c9a1c75f5 100644
--- a/proxy/hdrs/URL.h
+++ b/proxy/hdrs/URL.h
@@ -185,6 +185,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);
diff --git a/proxy/hdrs/unit_tests/test_URL.cc b/proxy/hdrs/unit_tests/test_URL.cc
index a39f7c01b..115aeee5d 100644
--- a/proxy/hdrs/unit_tests/test_URL.cc
+++ b/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);
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index f1360ddc6..df31bc932 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -433,8 +433,17 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME); field != nullptr && field->value_is_valid()) {
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);
headers->m_http->u.req.m_url_impl->set_scheme(headers->m_heap, scheme, scheme_wks_idx, scheme_len, true);
headers->field_delete(field);