This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit f7b88e99cf34fbe8381b87c70219a2aa642c61f0 Author: Brian Neradt <[email protected]> AuthorDate: Fri Apr 3 09:43:59 2026 -0500 Reject malformed Host header ports (#13050) Reject malformed Host field authorities before ATS reuses them as request targets or cached host state. Shared Host parsing now requires host or host:port syntax, enforces bracketed IPv6 when a port is present, and rejects invalid or out-of-range ports. This patch adds http_parse_host_header, a new host parser function which also validates the required host header format. The patch also updates the various host header parsing logic blocks across ATS to use the new parser. Reported-by: Ankit Singh Fixes: #13049 (cherry picked from commit de776b0b2755f0875283d5d4ce959dfe16cafe9b) --- include/proxy/hdrs/HTTP.h | 8 + src/proxy/hdrs/HTTP.cc | 128 +++++++++--- src/proxy/hdrs/unit_tests/test_Hdrs.cc | 230 +++++++++++++++++++++ src/proxy/http/HttpSM.cc | 36 +--- src/proxy/http/HttpTransact.cc | 15 +- src/proxy/http/remap/UrlRewrite.cc | 20 +- .../gold_tests/headers/malformed_host_port.test.py | 24 +++ .../replays/malformed_host_port.replay.yaml | 84 ++++++++ 8 files changed, 457 insertions(+), 88 deletions(-) diff --git a/include/proxy/hdrs/HTTP.h b/include/proxy/hdrs/HTTP.h index dc2d7b61f7..dd69181120 100644 --- a/include/proxy/hdrs/HTTP.h +++ b/include/proxy/hdrs/HTTP.h @@ -421,7 +421,15 @@ ParseResult http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl bool must_copy_strings, bool eof, int strict_uri_parsing, size_t max_request_line_size, size_t max_hdr_field_size); ParseResult validate_hdr_request_target(int method_wks_idx, URLImpl *url); + +// This calls http_parse_host_header internally to parse the Host field value +// when present, so it enforces the same syntax rules and also validates the +// port number when specified. ParseResult validate_hdr_host(HTTPHdrImpl *hh); + +// This parses and validates the Host field value. +bool http_parse_host_header(std::string_view value, std::string_view &host, int &port, bool &has_port); + ParseResult validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh); ParseResult http_parser_parse_resp(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const char **start, const char *end, bool must_copy_strings, bool eof); diff --git a/src/proxy/hdrs/HTTP.cc b/src/proxy/hdrs/HTTP.cc index f0db0665e7..b67534d6de 100644 --- a/src/proxy/hdrs/HTTP.cc +++ b/src/proxy/hdrs/HTTP.cc @@ -1143,6 +1143,82 @@ validate_hdr_request_target(int method_wk_idx, URLImpl *url) return ret; } +bool +http_parse_host_header(std::string_view value, std::string_view &host, int &port, bool &has_port) +{ + swoc::TextView text{value}; + + host = {}; + port = 0; + has_port = false; + + text.ltrim_if(&ParseRules::is_ws); + text.rtrim_if(&ParseRules::is_ws); + if (text.empty()) { + return false; + } + + if ('[' == *text) { + // IPv6. + auto const close = text.find(']'); + if (close == swoc::TextView::npos) { + // No closing ']', invalid host. + return false; + } + + host = {text.data(), close + 1}; + text.remove_prefix(close + 1); + if (!text.empty()) { + if (':' != text.front()) { + // If there are more characters, the next one must be a colon for the port. + return false; + } + text.remove_prefix(1); + if (text.empty()) { + // Port is indicated but not provided. + return false; + } + has_port = true; + } + } else { + // IPv4 or hostname. + auto const first_colon = text.find(':'); + if (first_colon == swoc::TextView::npos) { + host = text; + } else { + if (text.find(':', first_colon + 1) != swoc::TextView::npos || first_colon == 0) { + // Only one colon is allowed, and it can't be the first character (empty host). + return false; + } + + host = {text.data(), first_colon}; + text.remove_prefix(first_colon + 1); + if (text.empty()) { + return false; + } + has_port = true; + } + } + + if (!validate_host_name(host)) { + return false; + } + + if (has_port) { + if (text.size() > 5 || !std::all_of(text.begin(), text.end(), &ParseRules::is_digit)) { + // Too many characters for a port, or non-digit characters in the port. + return false; + } + + port = ink_atoi(text.data(), static_cast<int>(text.size())); + if (port <= 0 || port > 65535) { + return false; + } + } + + return true; +} + ParseResult validate_hdr_host(HTTPHdrImpl *hh) { @@ -1151,26 +1227,14 @@ validate_hdr_host(HTTPHdrImpl *hh) if (host_field) { if (host_field->has_dups()) { ret = ParseResult::ERROR; // can't have more than 1 host field. + } else if (auto host{host_field->value_get()}; host.empty()) { + ret = ParseResult::ERROR; } else { - auto host{host_field->value_get()}; - std::string_view addr, port, rest; - if (0 == ats_ip_parse(host, &addr, &port, &rest)) { - if (!port.empty()) { - if (port.size() > 5) { - return ParseResult::ERROR; - } - int port_i = ink_atoi(port.data(), port.size()); - if (port_i >= 65536 || port_i <= 0) { - return ParseResult::ERROR; - } - } - if (!validate_host_name(addr)) { - return ParseResult::ERROR; - } - if (ParseResult::DONE == ret && !std::all_of(rest.begin(), rest.end(), &ParseRules::is_ws)) { - return ParseResult::ERROR; - } - } else { + std::string_view parsed_host; + int port = 0; + bool has_port = false; + + if (!http_parse_host_header(host, parsed_host, port, has_port)) { ret = ParseResult::ERROR; } } @@ -1649,21 +1713,17 @@ HTTPHdr::_fill_target_cache() const m_host_mime = nullptr; m_host_length = static_cast<int>(host.length()); } else { - std::string_view port; - std::tie(m_host_mime, host, port) = const_cast<HTTPHdr *>(this)->get_host_port_values(); - m_host_length = static_cast<int>(host.length()); - - if (m_host_mime != nullptr) { - m_port = 0; - if (!port.empty()) { - for (auto c : port) { - if (isdigit(c)) { - m_port = m_port * 10 + c - '0'; - } - } - } - m_port_in_header = (0 != m_port); - m_port = url_canonicalize_port(url->m_url_impl->m_url_type, m_port); + m_host_mime = const_cast<HTTPHdr *>(this)->field_find(static_cast<std::string_view>(MIME_FIELD_HOST)); + m_host_length = 0; + m_port = 0; + + if (m_host_mime != nullptr && http_parse_host_header(m_host_mime->value_get(), host, m_port, m_port_in_header)) { + m_host_length = static_cast<int>(host.length()); + m_port = url_canonicalize_port(url->m_url_impl->m_url_type, m_port); + } else { + m_host_mime = nullptr; + m_port = 0; + m_port_in_header = false; } } diff --git a/src/proxy/hdrs/unit_tests/test_Hdrs.cc b/src/proxy/hdrs/unit_tests/test_Hdrs.cc index fef0166d49..f318723b25 100644 --- a/src/proxy/hdrs/unit_tests/test_Hdrs.cc +++ b/src/proxy/hdrs/unit_tests/test_Hdrs.cc @@ -2468,3 +2468,233 @@ TEST_CASE("HdrTestHostCacheInvalidation", "[proxy][hdrtest]") req_hdr.destroy(); } } + +TEST_CASE("HdrHostHeaderParserAcceptsValidValues", "[proxy][hdrtest]") +{ + struct TestCase { + std::string_view value; + std::string_view expected_host; + int expected_port; + bool expected_has_port; + }; + + static const std::vector<TestCase> test_cases = { + {"example.com", "example.com", 0, false}, + {"example.com:80", "example.com", 80, true }, + {"localhost:65535", "localhost", 65535, true }, + {"127.0.0.1", "127.0.0.1", 0, false}, + {"127.0.0.1:8080", "127.0.0.1", 8080, true }, + {"[::1]", "[::1]", 0, false}, + {"[::1]:443", "[::1]", 443, true }, + {"[::1]:00080", "[::1]", 80, true }, + {"[2001:db8::1]:8443", "[2001:db8::1]", 8443, true }, + {"[fe80::1%25eth0]:444", "[fe80::1%25eth0]", 444, true }, + {" example.com:81 ", "example.com", 81, true }, + {" [::1]:444 ", "[::1]", 444, true }, + }; + + auto test_case = GENERATE(from_range(test_cases)); + CAPTURE(test_case.value, test_case.expected_host, test_case.expected_port, test_case.expected_has_port); + + std::string_view host; + int port = -1; + bool has_port = true; + + bool const valid = http_parse_host_header(test_case.value, host, port, has_port); + + REQUIRE(valid); + CHECK(host == test_case.expected_host); + CHECK(port == test_case.expected_port); + CHECK(has_port == test_case.expected_has_port); +} + +TEST_CASE("HdrHostHeaderParserRejectsInvalidValues", "[proxy][hdrtest]") +{ + static const std::vector<std::string_view> test_cases = { + ""sv, + " "sv, + ":8080"sv, + "example.com:"sv, + "example.com:-1"sv, + "example.com:0"sv, + "example.com:65536"sv, + "example.com:999999"sv, + "example.com:http"sv, + "example.com:80x"sv, + "example.com:80:90"sv, + "127.0.0.1:-1"sv, + "127.0.0.1:80:90"sv, + "::1"sv, + "::1:443"sv, + "[::1"sv, + "[::1]:"sv, + "[::1]:-1"sv, + "[::1]:0"sv, + "[::1]:65536"sv, + "[::1]:80:90"sv, + "[::1]extra"sv, + }; + + auto test_case = GENERATE(from_range(test_cases)); + CAPTURE(test_case); + + std::string_view host; + int port = -1; + bool has_port = true; + + bool const valid = http_parse_host_header(test_case, host, port, has_port); + + CHECK_FALSE(valid); +} + +TEST_CASE("HdrValidatesHostHeaderOnRequestParse", "[proxy][hdrtest]") +{ + struct TestCase { + std::string_view host_header; + ParseResult expected_result; + std::string_view expected_host; + int expected_port; + bool expected_port_in_header; + }; + + static const std::vector<TestCase> test_cases = { + {"example.com", ParseResult::DONE, "example.com", 0, false}, + {"example.com:8080", ParseResult::DONE, "example.com", 8080, true }, + {"localhost", ParseResult::DONE, "localhost", 0, false}, + {"127.0.0.1", ParseResult::DONE, "127.0.0.1", 0, false}, + {"127.0.0.1:81", ParseResult::DONE, "127.0.0.1", 81, true }, + {"[::1]", ParseResult::DONE, "[::1]", 0, false}, + {"[::1]:443", ParseResult::DONE, "[::1]", 443, true }, + {"[2001:db8::1]:8443", ParseResult::DONE, "[2001:db8::1]", 8443, true }, + {"test:8080:9090:1234", ParseResult::ERROR, ""sv, 0, false}, + {"example.com:", ParseResult::ERROR, ""sv, 0, false}, + {"example.com:-1", ParseResult::ERROR, ""sv, 0, false}, + {"example.com:65536", ParseResult::ERROR, ""sv, 0, false}, + {"example.com:http", ParseResult::ERROR, ""sv, 0, false}, + {"127.0.0.1:-1", ParseResult::ERROR, ""sv, 0, false}, + {"127.0.0.1:80:90", ParseResult::ERROR, ""sv, 0, false}, + {"::1", ParseResult::ERROR, ""sv, 0, false}, + {"[::1]:-1", ParseResult::ERROR, ""sv, 0, false}, + {"[::1]:80:90", ParseResult::ERROR, ""sv, 0, false}, + {"[::1]extra", ParseResult::ERROR, ""sv, 0, false}, + {"[::1", ParseResult::ERROR, ""sv, 0, false}, + {":8080", ParseResult::ERROR, ""sv, 0, false}, + {"", ParseResult::ERROR, ""sv, 0, false}, + }; + + auto test_case = GENERATE(from_range(test_cases)); + CAPTURE(test_case.host_header, test_case.expected_result, test_case.expected_host, test_case.expected_port, + test_case.expected_port_in_header); + + hdrtoken_init(); + url_init(); + mime_init(); + http_init(); + + auto parse_request = [](std::string_view host_header, HTTPHdr &req_hdr, HTTPParser &parser) { + std::string request = "GET / HTTP/1.1\r\nHost: " + std::string(host_header) + "\r\n\r\n"; + const char *start = request.data(); + const char *end = start + request.size(); + ParseResult err; + + do { + err = req_hdr.parse_req(&parser, &start, end, true); + } while (err == ParseResult::CONT); + + return err; + }; + + HTTPHdr req_hdr; + HTTPParser parser; + + http_parser_init(&parser); + req_hdr.create(HTTPType::REQUEST); + + ParseResult const err = parse_request(test_case.host_header, req_hdr, parser); + REQUIRE(err == test_case.expected_result); + + if (err == ParseResult::DONE) { + CHECK(req_hdr.host_get() == test_case.expected_host); + CHECK(req_hdr.port_get() == test_case.expected_port); + CHECK(req_hdr.is_port_in_header() == test_case.expected_port_in_header); + } + + http_parser_clear(&parser); + req_hdr.destroy(); +} + +TEST_CASE("HdrPromotesOnlyValidHostHeaderMutations", "[proxy][hdrtest]") +{ + struct TestCase { + std::string_view host_header; + std::string_view expected_host; + int expected_port; + bool expected_port_in_header; + bool expected_valid; + }; + + static const std::vector<TestCase> test_cases = { + {"rewritten.example.com", "rewritten.example.com", 0, false, true }, + {"rewritten.example.com:443", "rewritten.example.com", 443, true, true }, + {"127.0.0.1:81", "127.0.0.1", 81, true, true }, + {"[::1]", "[::1]", 0, false, true }, + {"[2001:db8::1]:8443", "[2001:db8::1]", 8443, true, true }, + {"test:8080:9090:1234", ""sv, 0, false, false}, + {"example.com:", ""sv, 0, false, false}, + {"example.com:-1", ""sv, 0, false, false}, + {"127.0.0.1:-1", ""sv, 0, false, false}, + {"::1", ""sv, 0, false, false}, + {"[::1]:-1", ""sv, 0, false, false}, + {"[::1]:80:90", ""sv, 0, false, false}, + {"[::1]extra", ""sv, 0, false, false}, + {":8080", ""sv, 0, false, false}, + }; + + auto test_case = GENERATE(from_range(test_cases)); + CAPTURE(test_case.host_header, test_case.expected_host, test_case.expected_port, test_case.expected_port_in_header, + test_case.expected_valid); + + hdrtoken_init(); + url_init(); + mime_init(); + http_init(); + + static constexpr std::string_view request = "GET / HTTP/1.1\r\n" + "Host: original.example.com:8080\r\n" + "\r\n"; + + HTTPHdr req_hdr; + HTTPParser parser; + const char *start = request.data(); + const char *end = start + request.size(); + + http_parser_init(&parser); + req_hdr.create(HTTPType::REQUEST); + + ParseResult err; + do { + err = req_hdr.parse_req(&parser, &start, end, true); + } while (err == ParseResult::CONT); + + REQUIRE(err == ParseResult::DONE); + + MIMEField *host_field = req_hdr.field_find("Host"sv); + REQUIRE(host_field != nullptr); + host_field->value_set(req_hdr.m_heap, req_hdr.m_mime, test_case.host_header); + + CHECK(req_hdr.host_get() == test_case.expected_host); + CHECK(req_hdr.port_get() == test_case.expected_port); + CHECK(req_hdr.is_port_in_header() == test_case.expected_port_in_header); + + req_hdr.set_url_target_from_host_field(req_hdr.url_get()); + CHECK(req_hdr.url_get()->host_get() == test_case.expected_host); + CHECK(req_hdr.url_get()->port_get_raw() == test_case.expected_port); + + if (!test_case.expected_valid) { + CHECK(req_hdr.host_get().empty()); + CHECK(req_hdr.url_get()->host_get().empty()); + } + + http_parser_clear(&parser); + req_hdr.destroy(); +} diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 3e0dc489ca..d38c5d43a8 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -4536,37 +4536,13 @@ HttpSM::do_remap_request(bool run_inline) if (host_field) { auto host_name{host_field->value_get()}; if (!host_name.empty()) { - int port = -1; - // Host header can contain port number, and if it does we need to set host and port separately to unmapped_url. - // If header value starts with '[', the value must contain an IPv6 address, and it may contain a port number as well. - if (host_name.starts_with("["sv)) { // IPv6 - host_name.remove_prefix(1); // Skip '[' - // If header value ends with ']', the value must only contain an IPv6 address (no port number). - if (host_name.ends_with("]"sv)) { // Without port number - host_name.remove_suffix(1); // Exclude ']' - } else { // With port number - for (int idx = host_name.length() - 1; idx > 0; idx--) { - if (host_name[idx] == ':') { - port = ink_atoi(host_name.data() + idx + 1, host_name.length() - (idx + 1)); - host_name = host_name.substr(0, idx); - break; - } - } - } - } else { // Anything else (Hostname or IPv4 address) - // If the value contains ':' where it does not have IPv6 address, there must be port number - if (const char *colon = static_cast<const char *>(memchr(host_name.data(), ':', host_name.length())); - colon == nullptr) { // Without port number - // Nothing to adjust. Entire value should be used as hostname. - } else { // With port number - port = ink_atoi(colon + 1, host_name.length() - ((colon + 1) - host_name.data())); - host_name = host_name.substr(0, colon - host_name.data()); - } - } + int port = 0; + bool has_port = false; - // Set values - t_state.unmapped_url.host_set(host_name); - if (port >= 0) { + if (http_parse_host_header(host_name, host_name, port, has_port)) { + t_state.unmapped_url.host_set(host_name); + } + if (has_port) { t_state.unmapped_url.port_set(port); } } diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index 46562e5ed7..cd1049d1a6 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -2186,16 +2186,15 @@ HttpTransact::DecideCacheLookup(State *s) // We could a) have 6000 alts (barf, puke, vomit) or b) use the original // host header in the url before doing all cache actions (lookups, writes, etc.) if (s->txn_conf->maintain_pristine_host_hdr) { - // So, the host header will have the original host header. - if (auto [field, host, port_sv]{incoming_request->get_host_port_values()}; field != nullptr) { - int port = 0; - if (!port_sv.empty()) { - s->cache_info.lookup_url->host_set(host); - port = ink_atoi(port_sv.data(), static_cast<int>(port_sv.length())); - } else { + if (auto host_field = incoming_request->field_find(static_cast<std::string_view>(MIME_FIELD_HOST)); host_field != nullptr) { + std::string_view host; + int port = 0; + bool has_port = false; + + if (http_parse_host_header(host_field->value_get(), host, port, has_port)) { s->cache_info.lookup_url->host_set(host); + s->cache_info.lookup_url->port_set(has_port ? port : 0); } - s->cache_info.lookup_url->port_set(port); } } ink_assert(s->cache_info.lookup_url->valid() == true); diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc index 6c7e3f48d6..72a1e32771 100644 --- a/src/proxy/http/remap/UrlRewrite.cc +++ b/src/proxy/http/remap/UrlRewrite.cc @@ -666,24 +666,12 @@ UrlRewrite::Remap_redirect(HTTPHdr *request_header, URL *redirect_url) if (host.empty() && reverse_proxy != 0) { // Server request. Use the host header to figure out where // it goes. Host header parsing is same as in ::Remap auto host_hdr{request_header->value_get(static_cast<std::string_view>(MIME_FIELD_HOST))}; + int parsed_port = 0; + bool has_port = false; - const char *tmp = static_cast<const char *>(memchr(host_hdr.data(), ':', host_hdr.length())); - - int host_len; - if (tmp == nullptr) { - host_len = static_cast<int>(host_hdr.length()); - } else { - host_len = tmp - host_hdr.data(); - request_port = ink_atoi(tmp + 1, static_cast<int>(host_hdr.length()) - host_len); - - // If atoi fails, try the default for the - // protocol - if (request_port == 0) { - request_port = request_url->port_get(); - } + if (http_parse_host_header(host_hdr, host, parsed_port, has_port)) { + request_port = has_port ? parsed_port : request_url->port_get(); } - - host = {host_hdr.data(), static_cast<std::string_view::size_type>(host_len)}; } // Temporary Redirects have precedence over Permanent Redirects // the rationale behind this is that network administrators might diff --git a/tests/gold_tests/headers/malformed_host_port.test.py b/tests/gold_tests/headers/malformed_host_port.test.py new file mode 100644 index 0000000000..b444d4e935 --- /dev/null +++ b/tests/gold_tests/headers/malformed_host_port.test.py @@ -0,0 +1,24 @@ +''' +Verify malformed Host headers with extra port separators are rejected. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Verify malformed Host headers with extra port separators are rejected. +''' + +Test.ATSReplayTest(replay_file="replays/malformed_host_port.replay.yaml") diff --git a/tests/gold_tests/headers/replays/malformed_host_port.replay.yaml b/tests/gold_tests/headers/replays/malformed_host_port.replay.yaml new file mode 100644 index 0000000000..0201f9b89c --- /dev/null +++ b/tests/gold_tests/headers/replays/malformed_host_port.replay.yaml @@ -0,0 +1,84 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: "1.0" + +autest: + description: 'Reject malformed Host headers with multiple port separators' + + server: + name: 'server' + + client: + name: 'client' + + ats: + name: 'ts' + + process_config: + enable_cache: false + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'http' + + remap_config: + - from: "/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/"; + +sessions: +- transactions: + - client-request: + method: GET + version: '1.1' + url: /malformed-host + headers: + fields: + - [Host, "example.com:8080:9090:1234"] + - [uuid, malformed-host] + + # This malformed request should be rejected by ATS before reaching origin. + # If ATS forwards it anyway, the origin will reply 200 and the test will + # fail because the proxy response is expected to be 400. + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "0"] + + proxy-response: + status: 400 + + - client-request: + method: GET + version: '1.1' + url: /healthy-afterward + headers: + fields: + - [Host, example.com] + - [uuid, healthy-afterward] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "0"] + + proxy-response: + status: 200
