From 8fa4717fc2e202ee179c8dacfc91e7362d651c43 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Sun, 24 Nov 2024 22:45:13 +0000 Subject: [PATCH] Hardening: unify header validation across HTTP versions (#7343) * hardening: unify header validation across HTTP versions This is meant to prevent request smuggling attacks over different HTTP versions. * docs++, reject transfer-encoding for HTTP/1.0 * [autofix.ci] apply automated fixes * tests++ --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 + mitmproxy/net/http/http1/__init__.py | 2 - mitmproxy/net/http/http1/read.py | 100 +++----------- mitmproxy/net/http/validate.py | 126 ++++++++++++++++++ mitmproxy/proxy/layers/http/__init__.py | 63 ++++++++- mitmproxy/proxy/layers/http/_http1.py | 9 +- test/mitmproxy/net/http/http1/test_read.py | 50 +------ test/mitmproxy/net/http/test_validate.py | 100 ++++++++++++++ test/mitmproxy/proxy/layers/http/test_http.py | 49 +++++-- 9 files changed, 353 insertions(+), 148 deletions(-) create mode 100644 mitmproxy/net/http/validate.py create mode 100644 test/mitmproxy/net/http/test_validate.py diff --git a/CHANGELOG.md b/CHANGELOG.md index eca8afe45..3ec49f258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ ([#7228](https://github.com/mitmproxy/mitmproxy/pull/7228), @fatanugraha) - Add a `tun` proxy mode that creates a virtual network device on Linux for transparent proxying. ([#7278](https://github.com/mitmproxy/mitmproxy/pull/7278), @mhils) +- Implement stricter validation of HTTP headers to harden against request smuggling attacks. + ([#7345](https://github.com/mitmproxy/mitmproxy/issues/7345), @mhils) - Fix a bug where mitmproxy would incorrectly report that TLS 1.0 and 1.1 are not supported with the current OpenSSL build. ([#7241](https://github.com/mitmproxy/mitmproxy/pull/7241), @mhils) diff --git a/mitmproxy/net/http/http1/__init__.py b/mitmproxy/net/http/http1/__init__.py index 022e03fa2..01c543afb 100644 --- a/mitmproxy/net/http/http1/__init__.py +++ b/mitmproxy/net/http/http1/__init__.py @@ -7,14 +7,12 @@ from .read import connection_close from .read import expected_http_body_size from .read import read_request_head from .read import read_response_head -from .read import validate_headers __all__ = [ "read_request_head", "read_response_head", "connection_close", "expected_http_body_size", - "validate_headers", "assemble_request", "assemble_request_head", "assemble_response", diff --git a/mitmproxy/net/http/http1/read.py b/mitmproxy/net/http/http1/read.py index 774dc83f1..f0876d677 100644 --- a/mitmproxy/net/http/http1/read.py +++ b/mitmproxy/net/http/http1/read.py @@ -1,11 +1,13 @@ import re import time +import typing from collections.abc import Iterable from mitmproxy.http import Headers from mitmproxy.http import Request from mitmproxy.http import Response from mitmproxy.net.http import url +from mitmproxy.net.http import validate def get_header_tokens(headers, key): @@ -42,40 +44,6 @@ def connection_close(http_version, headers): ) -# https://datatracker.ietf.org/doc/html/rfc7230#section-3.2: Header fields are tokens. -# "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA -_valid_header_name = re.compile(rb"^[!#$%&'*+\-.^_`|~0-9a-zA-Z]+$") - - -def validate_headers(headers: Headers) -> None: - """ - Validate headers to avoid request smuggling attacks. Raises a ValueError if they are malformed. - """ - - te_found = False - cl_found = False - - for name, value in headers.fields: - if not _valid_header_name.match(name): - raise ValueError( - f"Received an invalid header name: {name!r}. Invalid header names may introduce " - f"request smuggling vulnerabilities. Disable the validate_inbound_headers option " - f"to skip this security check." - ) - - name_lower = name.lower() - te_found = te_found or name_lower == b"transfer-encoding" - cl_found = cl_found or name_lower == b"content-length" - - if te_found and cl_found: - raise ValueError( - "Received both a Transfer-Encoding and a Content-Length header, " - "refusing as recommended in RFC 7230 Section 3.3.3. " - "See https://github.com/mitmproxy/mitmproxy/issues/4799 for details. " - "Disable the validate_inbound_headers option to skip this security check." - ) - - def expected_http_body_size( request: Request, response: Response | None = None ) -> int | None: @@ -87,7 +55,7 @@ def expected_http_body_size( - -1, if all data should be read until end of stream. Raises: - ValueError, if the content length header is invalid + ValueError, if the content-length or transfer-encoding header is invalid """ # Determine response size according to http://tools.ietf.org/html/rfc7230#section-3.3, which is inlined below. if not response: @@ -137,39 +105,20 @@ def expected_http_body_size( # remove the received Content-Length field prior to forwarding such # a message downstream. # - if "transfer-encoding" in headers: - # we should make sure that there isn't also a content-length header. - # this is already handled in validate_headers. - - te: str = headers["transfer-encoding"] - if not te.isascii(): - # guard against .lower() transforming non-ascii to ascii - raise ValueError(f"Invalid transfer encoding: {te!r}") - te = te.lower().strip("\t ") - te = re.sub(r"[\t ]*,[\t ]*", ",", te) - if te in ( - "chunked", - "compress,chunked", - "deflate,chunked", - "gzip,chunked", - ): - return None - elif te in ( - "compress", - "deflate", - "gzip", - "identity", - ): - if response: - return -1 - else: - raise ValueError( - f"Invalid request transfer encoding, message body cannot be determined reliably." - ) - else: - raise ValueError( - f"Unknown transfer encoding: {headers['transfer-encoding']!r}" - ) + if te_str := headers.get("transfer-encoding"): + te = validate.parse_transfer_encoding(te_str) + match te: + case "chunked" | "compress,chunked" | "deflate,chunked" | "gzip,chunked": + return None + case "compress" | "deflate" | "gzip" | "identity": + if response: + return -1 + else: + raise ValueError( + "Invalid request transfer encoding, message body cannot be determined reliably." + ) + case other: # pragma: no cover + typing.assert_never(other) # 4. If a message is received without Transfer-Encoding and with # either multiple Content-Length header fields having differing @@ -190,19 +139,8 @@ def expected_http_body_size( # the recipient times out before the indicated number of octets are # received, the recipient MUST consider the message to be # incomplete and close the connection. - if "content-length" in headers: - sizes = headers.get_all("content-length") - different_content_length_headers = any(x != sizes[0] for x in sizes) - if different_content_length_headers: - raise ValueError(f"Conflicting Content-Length headers: {sizes!r}") - try: - size = int(sizes[0]) - except ValueError: - raise ValueError(f"Invalid Content-Length header: {sizes[0]!r}") - if size < 0: - raise ValueError(f"Negative Content-Length header: {sizes[0]!r}") - return size - + if cl := headers.get("content-length"): + return validate.parse_content_length(cl) # 6. If this is a request message and none of the above are true, then # the message body length is zero (no message body is present). if not response: diff --git a/mitmproxy/net/http/validate.py b/mitmproxy/net/http/validate.py new file mode 100644 index 000000000..bc5c140e8 --- /dev/null +++ b/mitmproxy/net/http/validate.py @@ -0,0 +1,126 @@ +import logging +import re +import typing + +from mitmproxy.http import Message +from mitmproxy.http import Response + +logger = logging.getLogger(__name__) + +# https://datatracker.ietf.org/doc/html/rfc7230#section-3.2: Header fields are tokens. +# "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA +_valid_header_name = re.compile(rb"^[!#$%&'*+\-.^_`|~0-9a-zA-Z]+$") + +_valid_content_length = re.compile(rb"^(?:0|[1-9][0-9]*)$") +_valid_content_length_str = re.compile(r"^(?:0|[1-9][0-9]*)$") + +# https://datatracker.ietf.org/doc/html/rfc9112#section-6.1: +# > A sender MUST NOT apply the chunked transfer coding more than once to a message body (i.e., chunking an already +# > chunked message is not allowed). If any transfer coding other than chunked is applied to a request's content, the +# > sender MUST apply chunked as the final transfer coding to ensure that the message is properly framed. If any +# > transfer coding other than chunked is applied to a response's content, the sender MUST either apply chunked as the +# > final transfer coding or terminate the message by closing the connection. +# +# The RFC technically still allows for fun encodings, we are a bit stricter and only accept a known subset by default. +TransferEncoding = typing.Literal[ + "chunked", + "compress,chunked", + "deflate,chunked", + "gzip,chunked", + "compress", + "deflate", + "gzip", + "identity", +] +_HTTP_1_1_TRANSFER_ENCODINGS = frozenset(typing.get_args(TransferEncoding)) + + +def parse_content_length(value: str | bytes) -> int: + """Parse a content-length header value, or raise a ValueError if it is invalid.""" + if isinstance(value, str): + valid = bool(_valid_content_length_str.match(value)) + else: + valid = bool(_valid_content_length.match(value)) + if not valid: + raise ValueError(f"invalid content-length header: {value!r}") + return int(value) + + +def parse_transfer_encoding(value: str | bytes) -> TransferEncoding: + """Parse a transfer-encoding header value, or raise a ValueError if it is invalid or unknown.""" + # guard against .lower() transforming non-ascii to ascii + if not value.isascii(): + raise ValueError(f"invalid transfer-encoding header: {value!r}") + if isinstance(value, str): + te = value + else: + te = value.decode() + te = te.lower() + te = re.sub(r"[\t ]*,[\t ]*", ",", te) + if te not in _HTTP_1_1_TRANSFER_ENCODINGS: + raise ValueError(f"unknown transfer-encoding header: {value!r}") + return typing.cast(TransferEncoding, te) + + +def validate_headers(message: Message) -> None: + """ + Validate HTTP message headers to avoid request smuggling attacks. + + Raises a ValueError if they are malformed. + """ + + te = [] + cl = [] + + for name, value in message.headers.fields: + if not _valid_header_name.match(name): + raise ValueError(f"invalid header name: {name!r}") + match name.lower(): + case b"transfer-encoding": + te.append(value) + case b"content-length": + cl.append(value) + + if te and cl: + # > A server MAY reject a request that contains both Content-Length and Transfer-Encoding or process such a + # > request in accordance with the Transfer-Encoding alone. + + # > A sender MUST NOT send a Content-Length header field in any message that contains a Transfer-Encoding header + # > field. + raise ValueError( + "message with both transfer-encoding and content-length headers" + ) + elif te: + if len(te) > 1: + raise ValueError(f"multiple transfer-encoding headers: {te!r}") + # > Transfer-Encoding was added in HTTP/1.1. It is generally assumed that implementations advertising only + # > HTTP/1.0 support will not understand how to process transfer-encoded content, and that an HTTP/1.0 message + # > received with a Transfer-Encoding is likely to have been forwarded without proper handling of the chunked + # > transfer coding in transit. + # + # > A client MUST NOT send a request containing Transfer-Encoding unless it knows the server will handle + # > HTTP/1.1 requests (or later minor revisions); such knowledge might be in the form of specific user + # > configuration or by remembering the version of a prior received response. A server MUST NOT send a response + # > containing Transfer-Encoding unless the corresponding request indicates HTTP/1.1 (or later minor revisions). + + # > A server MUST NOT send a Transfer-Encoding header field in any response with a status code of 1xx + # > (Informational) or 204 (No Content). + te_disallowed = not message.is_http11 or ( + isinstance(message, Response) + and (100 <= message.status_code <= 199 or message.status_code == 204) + ) + if te_disallowed: + raise ValueError( + f"Unexpected HTTP transfer encoding: {message.http_version!r}" + ) + parse_transfer_encoding(te[0]) + elif cl: + # > If a message is received without Transfer-Encoding and with an invalid Content-Length header field, then the + # > message framing is invalid and the recipient MUST treat it as an unrecoverable error, unless the field value + # > can be successfully parsed as a comma-separated list (Section 5.6.1 of [HTTP]), all values in the list are + # > valid, and all values in the list are the same (in which case, the message is processed with that single + # > value used as the Content-Length field value). + # We are stricter here and reject comma-separated lists. + if len(cl) > 1: + raise ValueError(f"multiple content-length headers: {cl!r}") + parse_content_length(cl[0]) diff --git a/mitmproxy/proxy/layers/http/__init__.py b/mitmproxy/proxy/layers/http/__init__.py index 6bf80b90e..9ec64d41d 100644 --- a/mitmproxy/proxy/layers/http/__init__.py +++ b/mitmproxy/proxy/layers/http/__init__.py @@ -51,6 +51,7 @@ from mitmproxy.net import server_spec from mitmproxy.net.http import status_codes from mitmproxy.net.http import url from mitmproxy.net.http.http1 import expected_http_body_size +from mitmproxy.net.http.validate import validate_headers from mitmproxy.proxy import commands from mitmproxy.proxy import events from mitmproxy.proxy import layer @@ -72,7 +73,9 @@ class HTTPMode(enum.Enum): upstream = 3 -def validate_request(mode: HTTPMode, request: http.Request) -> str | None: +def validate_request( + mode: HTTPMode, request: http.Request, validate_inbound_headers: bool +) -> str | None: if request.scheme not in ("http", "https", ""): return f"Invalid request scheme: {request.scheme}" if mode is HTTPMode.transparent and request.method == "CONNECT": @@ -80,6 +83,14 @@ def validate_request(mode: HTTPMode, request: http.Request) -> str | None: f"mitmproxy received an HTTP CONNECT request even though it is not running in regular/upstream mode. " f"This usually indicates a misconfiguration, please see the mitmproxy mode documentation for details." ) + if validate_inbound_headers: + try: + validate_headers(request) + except ValueError as e: + return ( + f"Received {e} from client, refusing to prevent request smuggling attacks. " + "Disable the validate_inbound_headers option to skip this security check." + ) return None @@ -204,10 +215,8 @@ class HttpStream(layer.Layer): self.flow.request = event.request self.flow.live = True - if err := validate_request(self.mode, self.flow.request): - self.flow.response = http.Response.make(502, str(err)) - self.client_state = self.state_errored - return (yield from self.send_response()) + if (yield from self.check_invalid(True)): + return if self.flow.request.method == "CONNECT": return (yield from self.handle_connect()) @@ -401,6 +410,8 @@ class HttpStream(layer.Layer): if not event.end_stream and (yield from self.check_body_size(False)): return + if (yield from self.check_invalid(False)): + return yield HttpResponseHeadersHook(self.flow) if (yield from self.check_killed(True)): @@ -622,6 +633,48 @@ class HttpStream(layer.Layer): yield from self.handle_event(ResponseData(self.stream_id, body_buf)) return False + def check_invalid(self, request: bool) -> layer.CommandGenerator[bool]: + err: str | None = None + if request: + err = validate_request( + self.mode, + self.flow.request, + self.context.options.validate_inbound_headers, + ) + elif self.context.options.validate_inbound_headers: + assert self.flow.response is not None + try: + validate_headers(self.flow.response) + except ValueError as e: + err = ( + f"Received {e} from server, refusing to prevent request smuggling attacks. " + "Disable the validate_inbound_headers option to skip this security check." + ) + + if err: + self.flow.error = flow.Error(err) + + if request: + # flow has not been seen yet, register it. + yield HttpRequestHeadersHook(self.flow) + else: + # immediately kill of server connection + yield commands.CloseConnection(self.flow.server_conn) + yield HttpErrorHook(self.flow) + yield SendHttp( + ResponseProtocolError( + self.stream_id, + err, + status_codes.BAD_REQUEST if request else status_codes.BAD_GATEWAY, + ), + self.context.client, + ) + self.flow.live = False + self.client_state = self.server_state = self.state_errored + return True + else: + return False + def check_killed(self, emit_error_hook: bool) -> layer.CommandGenerator[bool]: killed_by_us = ( self.flow.error and self.flow.error.msg == flow.Error.KILLED_MESSAGE diff --git a/mitmproxy/proxy/layers/http/_http1.py b/mitmproxy/proxy/layers/http/_http1.py index a4932b8cd..75838d0b7 100644 --- a/mitmproxy/proxy/layers/http/_http1.py +++ b/mitmproxy/proxy/layers/http/_http1.py @@ -264,12 +264,13 @@ class Http1Server(Http1Connection): yield commands.SendData(self.conn, b"0\r\n\r\n") yield from self.mark_done(response=True) elif isinstance(event, ResponseProtocolError): + if not (self.conn.state & ConnectionState.CAN_WRITE): + return if not self.response and event.code != status_codes.NO_RESPONSE: yield commands.SendData( self.conn, make_error_response(event.code, event.message) ) - if self.conn.state & ConnectionState.CAN_WRITE: - yield commands.CloseConnection(self.conn) + yield commands.CloseConnection(self.conn) else: raise AssertionError(f"Unexpected event: {event}") @@ -283,8 +284,6 @@ class Http1Server(Http1Connection): self.request = http1.read_request_head( [bytes(x) for x in request_head] ) - if self.context.options.validate_inbound_headers: - http1.validate_headers(self.request.headers) expected_body_size = http1.expected_http_body_size(self.request) except ValueError as e: yield commands.SendData(self.conn, make_error_response(400, str(e))) @@ -406,8 +405,6 @@ class Http1Client(Http1Connection): self.response = http1.read_response_head( [bytes(x) for x in response_head] ) - if self.context.options.validate_inbound_headers: - http1.validate_headers(self.response.headers) expected_size = http1.expected_http_body_size( self.request, self.response ) diff --git a/test/mitmproxy/net/http/http1/test_read.py b/test/mitmproxy/net/http/http1/test_read.py index 09441e96c..d2d391d0a 100644 --- a/test/mitmproxy/net/http/http1/test_read.py +++ b/test/mitmproxy/net/http/http1/test_read.py @@ -9,7 +9,6 @@ from mitmproxy.net.http.http1.read import expected_http_body_size from mitmproxy.net.http.http1.read import get_header_tokens from mitmproxy.net.http.http1.read import read_request_head from mitmproxy.net.http.http1.read import read_response_head -from mitmproxy.net.http.http1.read import validate_headers from mitmproxy.test.tutils import treq from mitmproxy.test.tutils import tresp @@ -64,22 +63,6 @@ def test_read_response_head(): assert r.content is None -def test_validate_headers(): - # both content-length and chunked (possible request smuggling) - with pytest.raises( - ValueError, - match="Received both a Transfer-Encoding and a Content-Length header", - ): - validate_headers( - Headers(transfer_encoding="chunked", content_length="42"), - ) - - with pytest.raises(ValueError, match="Received an invalid header name"): - validate_headers( - Headers([(b"content-length ", b"42")]), - ) - - def test_expected_http_body_size(): # Expect: 100-continue assert ( @@ -120,19 +103,19 @@ def test_expected_http_body_size(): ) is None ) - with pytest.raises(ValueError, match="Invalid transfer encoding"): + with pytest.raises(ValueError, match="invalid transfer-encoding header"): expected_http_body_size( treq( headers=Headers(transfer_encoding="chun\u212aed") ), # "chunKed".lower() == "chunked" ) - with pytest.raises(ValueError, match="Unknown transfer encoding"): + with pytest.raises(ValueError, match="unknown transfer-encoding header"): expected_http_body_size( treq( headers=Headers(transfer_encoding="chun ked") ), # "chunKed".lower() == "chunked" ) - with pytest.raises(ValueError, match="Unknown transfer encoding"): + with pytest.raises(ValueError, match="unknown transfer-encoding header"): expected_http_body_size( treq(headers=Headers(transfer_encoding="qux")), ) @@ -150,12 +133,11 @@ def test_expected_http_body_size(): ) # explicit length - for val in (b"foo", b"-7"): - with pytest.raises(ValueError): - expected_http_body_size(treq(headers=Headers(content_length=val))) assert expected_http_body_size(treq(headers=Headers(content_length="42"))) == 42 - # multiple content-length headers with same value - assert ( + # invalid lengths + with pytest.raises(ValueError): + expected_http_body_size(treq(headers=Headers(content_length=b"foo"))) + with pytest.raises(ValueError): expected_http_body_size( treq( headers=Headers( @@ -163,24 +145,6 @@ def test_expected_http_body_size(): ) ) ) - == 42 - ) - # multiple content-length headers with conflicting value - with pytest.raises(ValueError, match="Conflicting Content-Length headers"): - expected_http_body_size( - treq( - headers=Headers( - [(b"content-length", b"42"), (b"content-length", b"45")] - ) - ) - ) - - # non-int content-length - with pytest.raises(ValueError, match="Invalid Content-Length header"): - expected_http_body_size(treq(headers=Headers([(b"content-length", b"NaN")]))) - # negative content-length - with pytest.raises(ValueError, match="Negative Content-Length header"): - expected_http_body_size(treq(headers=Headers([(b"content-length", b"-1")]))) # no length assert expected_http_body_size(treq(headers=Headers())) == 0 diff --git a/test/mitmproxy/net/http/test_validate.py b/test/mitmproxy/net/http/test_validate.py new file mode 100644 index 000000000..b118f50d7 --- /dev/null +++ b/test/mitmproxy/net/http/test_validate.py @@ -0,0 +1,100 @@ +import pytest + +from mitmproxy.http import Headers +from mitmproxy.http import Response +from mitmproxy.net.http.validate import parse_content_length +from mitmproxy.net.http.validate import parse_transfer_encoding +from mitmproxy.net.http.validate import validate_headers + + +def test_parse_content_length_ok(): + assert parse_content_length("0") == 0 + assert parse_content_length("42") == 42 + assert parse_content_length(b"0") == 0 + assert parse_content_length(b"42") == 42 + + +@pytest.mark.parametrize( + "cl", ["NaN", "", " ", "-1", "+1", "0x42", "010", "foo", "1, 1"] +) +def test_parse_content_length_invalid(cl): + with pytest.raises(ValueError, match="invalid content-length"): + parse_content_length(cl) + with pytest.raises(ValueError, match="invalid content-length"): + parse_content_length(cl.encode()) + + +def test_parse_transfer_encoding_ok(): + assert parse_transfer_encoding(b"chunked") == "chunked" + assert parse_transfer_encoding("chunked") == "chunked" + assert parse_transfer_encoding("gzip,chunked") == "gzip,chunked" + assert parse_transfer_encoding("gzip, chunked") == "gzip,chunked" + + +@pytest.mark.parametrize( + "te", + [ + "unknown", + "chunked,chunked", + "chunked,gzip", + "", + "chunKed", + "chun ked", + ], +) +def test_parse_transfer_encoding_invalid(te): + with pytest.raises(ValueError, match="transfer-encoding"): + parse_transfer_encoding(te) + with pytest.raises(ValueError, match="transfer-encoding"): + parse_transfer_encoding(te.encode()) + + +def test_validate_headers_ok(): + validate_headers( + Response.make(headers=Headers(content_length="42")), + ) + + +@pytest.mark.parametrize( + "headers", + [ + pytest.param( + Headers(transfer_encoding="chunked", content_length="42"), id="cl.te" + ), + pytest.param(Headers([(b"content-length ", b"42")]), id="whitespace-key"), + pytest.param(Headers([(b"content-length", b"42 ")]), id="whitespace-value"), + pytest.param(Headers(content_length="-42"), id="negative-cl"), + pytest.param(Headers(content_length="042"), id="zero-prefixed-cl"), + pytest.param(Headers(transfer_encoding="unknown"), id="unknown-te"), + pytest.param( + Headers([(b"content-length", b"42"), (b"content-length", b"43")]), + id="multi-cl", + ), + pytest.param( + Headers([(b"transfer-encoding", b""), (b"transfer-encoding", b"chunked")]), + id="multi-te", + ), + ], +) +def test_validate_headers_invalid(headers: Headers): + resp = Response.make() + resp.headers = ( + headers # update manually as Response.make() fixes content-length headers. + ) + with pytest.raises(ValueError): + validate_headers(resp) + + +def test_validate_headers_te_forbidden(): + te_headers = Headers(transfer_encoding="chunked") + resp = Response.make(headers=te_headers) + resp.headers = te_headers + resp.http_version = "HTTP/1.0" + + with pytest.raises(ValueError): + validate_headers(resp) + + resp = Response.make(status_code=204) + resp.headers = te_headers + with pytest.raises(ValueError): + validate_headers(Response.make(headers=te_headers, status_code=204)) diff --git a/test/mitmproxy/proxy/layers/http/test_http.py b/test/mitmproxy/proxy/layers/http/test_http.py index f55162ea1..5854a0f76 100644 --- a/test/mitmproxy/proxy/layers/http/test_http.py +++ b/test/mitmproxy/proxy/layers/http/test_http.py @@ -969,12 +969,11 @@ def test_proxy_chain(tctx, strategy): playbook >> reply_next_layer(lambda ctx: http.HttpLayer(ctx, HTTPMode.transparent)) playbook << SendData( tctx.client, - b"HTTP/1.1 502 Bad Gateway\r\n" - b"content-length: 198\r\n" - b"\r\n" - b"mitmproxy received an HTTP CONNECT request even though it is not running in regular/upstream mode. " - b"This usually indicates a misconfiguration, please see the mitmproxy mode documentation for details.", + BytesMatching( + b"mitmproxy received an HTTP CONNECT request even though it is not running in regular/upstream mode." + ), ) + playbook << CloseConnection(tctx.client) assert playbook @@ -1519,7 +1518,7 @@ def test_request_smuggling(tctx): << SendData( tctx.client, BytesMatching( - b"Received both a Transfer-Encoding and a Content-Length header" + b"Disable the validate_inbound_headers option to skip this security check" ), ) << CloseConnection(tctx.client) @@ -1536,7 +1535,34 @@ def test_request_smuggling_whitespace(tctx): b"Host: example.com\r\n" b"Content-Length : 42\r\n\r\n", ) - << SendData(tctx.client, BytesMatching(b"Received an invalid header name")) + << SendData(tctx.client, BytesMatching(b"invalid header name")) + << CloseConnection(tctx.client) + ) + + +def test_request_smuggling_response(tctx): + """Test that we reject response smuggling""" + server = Placeholder(Server) + assert ( + Playbook(http.HttpLayer(tctx, HTTPMode.regular), hooks=False) + >> DataReceived( + tctx.client, + b"GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n", + ) + << OpenConnection(server) + >> reply(None) + << SendData(server, b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n") + >> DataReceived( + server, + b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nContent-Length: 42\r\n\r\n", + ) + << CloseConnection(server) + << SendData( + tctx.client, + BytesMatching( + b"Disable the validate_inbound_headers option to skip this security check" + ), + ) << CloseConnection(tctx.client) ) @@ -1573,12 +1599,13 @@ def test_request_smuggling_te_te(tctx): "Transfer-Encoding: chunKed\r\n\r\n" ).encode(), ) # note the non-standard "K" - << SendData(tctx.client, BytesMatching(b"Invalid transfer encoding")) + << SendData(tctx.client, BytesMatching(b"invalid transfer-encoding header")) << CloseConnection(tctx.client) ) -def test_invalid_content_length(tctx): +@pytest.mark.parametrize("cl", [b"NaN", b"-1"]) +def test_invalid_content_length(tctx, cl): """Test that we still trigger flow hooks for requests with semantic errors""" flow = Placeholder(HTTPFlow) assert ( @@ -1588,10 +1615,10 @@ def test_invalid_content_length(tctx): ( b"GET http://example.com/ HTTP/1.1\r\n" b"Host: example.com\r\n" - b"Content-Length: NaN\r\n\r\n" + b"Content-Length: " + cl + b"\r\n\r\n" ), ) - << SendData(tctx.client, BytesMatching(b"Invalid Content-Length header")) + << SendData(tctx.client, BytesMatching(b"invalid content-length header")) << CloseConnection(tctx.client) << http.HttpRequestHeadersHook(flow) >> reply()