From 18eb29c7364bb5dafab1af3e4259e70b9a5fbfc3 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Thu, 28 Nov 2024 19:20:27 +0100 Subject: [PATCH] Improve transfer-encoding error messages, be more permissive if validate_inbound_headers is disabled (#7361) * improve error message for header validation * move transfer-encoding validation from http.http1.read to http.validate * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 ++ mitmproxy/net/http/http1/read.py | 10 +++---- mitmproxy/net/http/validate.py | 31 +++++++++++++++------ mitmproxy/proxy/layers/http/__init__.py | 2 +- test/mitmproxy/net/http/http1/test_read.py | 4 --- test/mitmproxy/net/http/test_validate.py | 32 ++++++++++++++++------ 6 files changed, 53 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 927e254bb..cc21e5b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ ([#7355](https://github.com/mitmproxy/mitmproxy/pull/7355), @nneonneo) - Fix a bug where the mitmproxy UI would crash on negative durations. ([#7358](https://github.com/mitmproxy/mitmproxy/pull/7358), @mhils) +- Allow HTTP transfer encodings with read-until-EOF semantics in requests if `validate_inbound_headers` is disabled. + ([#7361](https://github.com/mitmproxy/mitmproxy/pull/7361), @mhils) - Fix a bug in windows management in mitmproxy TUI whereby the help window does not appear if "?" is pressed within the overlay ([#6500](https://github.com/mitmproxy/mitmproxy/pull/6500), @emanuele-em) diff --git a/mitmproxy/net/http/http1/read.py b/mitmproxy/net/http/http1/read.py index f0876d677..29edcc485 100644 --- a/mitmproxy/net/http/http1/read.py +++ b/mitmproxy/net/http/http1/read.py @@ -111,12 +111,10 @@ def expected_http_body_size( 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." - ) + # These values are valid for responses only (not requests), which is ensured in + # mitmproxy.net.http.validate. Here we strive for maximum compatibility with + # weird clients, assuming validate_inbound_headers=false. + return -1 case other: # pragma: no cover typing.assert_never(other) diff --git a/mitmproxy/net/http/validate.py b/mitmproxy/net/http/validate.py index bc5c140e8..7a333d279 100644 --- a/mitmproxy/net/http/validate.py +++ b/mitmproxy/net/http/validate.py @@ -3,6 +3,7 @@ import re import typing from mitmproxy.http import Message +from mitmproxy.http import Request from mitmproxy.http import Response logger = logging.getLogger(__name__) @@ -102,18 +103,32 @@ def validate_headers(message: Message) -> None: # > 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). - + if not message.is_http11: + raise ValueError( + f"unexpected HTTP transfer-encoding {te[0]!r} for {message.http_version}" + ) # > 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: + if isinstance(message, Response) and ( + 100 <= message.status_code <= 199 or message.status_code == 204 + ): raise ValueError( - f"Unexpected HTTP transfer encoding: {message.http_version!r}" + f"unexpected HTTP transfer-encoding {te[0]!r} for response with status code {message.status_code}" ) - parse_transfer_encoding(te[0]) + # > If a Transfer-Encoding header field is present in a request and the chunked transfer coding is not the final + # > encoding, the message body length cannot be determined reliably; the server MUST respond with the 400 (Bad + # > Request) status code and then close the connection. + te_parsed = parse_transfer_encoding(te[0]) + match te_parsed: + case "chunked" | "compress,chunked" | "deflate,chunked" | "gzip,chunked": + pass + case "compress" | "deflate" | "gzip" | "identity": + if isinstance(message, Request): + raise ValueError( + f"unexpected HTTP transfer-encoding {te_parsed!r} for request" + ) + case other: # pragma: no cover + typing.assert_never(other) 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 diff --git a/mitmproxy/proxy/layers/http/__init__.py b/mitmproxy/proxy/layers/http/__init__.py index 9ec64d41d..d4f37bf56 100644 --- a/mitmproxy/proxy/layers/http/__init__.py +++ b/mitmproxy/proxy/layers/http/__init__.py @@ -658,7 +658,7 @@ class HttpStream(layer.Layer): # flow has not been seen yet, register it. yield HttpRequestHeadersHook(self.flow) else: - # immediately kill of server connection + # immediately kill server connection yield commands.CloseConnection(self.flow.server_conn) yield HttpErrorHook(self.flow) yield SendHttp( diff --git a/test/mitmproxy/net/http/http1/test_read.py b/test/mitmproxy/net/http/http1/test_read.py index d2d391d0a..43a8be895 100644 --- a/test/mitmproxy/net/http/http1/test_read.py +++ b/test/mitmproxy/net/http/http1/test_read.py @@ -120,10 +120,6 @@ def test_expected_http_body_size(): treq(headers=Headers(transfer_encoding="qux")), ) # transfer-encoding: gzip - with pytest.raises(ValueError, match="Invalid request transfer encoding"): - expected_http_body_size( - treq(headers=Headers(transfer_encoding="gzip")), - ) assert ( expected_http_body_size( treq(), diff --git a/test/mitmproxy/net/http/test_validate.py b/test/mitmproxy/net/http/test_validate.py index b118f50d7..2ef8f3d9e 100644 --- a/test/mitmproxy/net/http/test_validate.py +++ b/test/mitmproxy/net/http/test_validate.py @@ -1,6 +1,7 @@ import pytest from mitmproxy.http import Headers +from mitmproxy.http import Request from mitmproxy.http import Response from mitmproxy.net.http.validate import parse_content_length from mitmproxy.net.http.validate import parse_transfer_encoding @@ -53,6 +54,11 @@ def test_validate_headers_ok(): validate_headers( Response.make(headers=Headers(content_length="42")), ) + validate_headers( + Request.make( + "POST", "https://example.com", headers=Headers(transfer_encoding="chunked") + ), + ) @pytest.mark.parametrize( @@ -63,8 +69,7 @@ def test_validate_headers_ok(): ), 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(content_length="-42"), id="invalid-cl"), pytest.param(Headers(transfer_encoding="unknown"), id="unknown-te"), pytest.param( Headers([(b"content-length", b"42"), (b"content-length", b"43")]), @@ -85,16 +90,25 @@ def test_validate_headers_invalid(headers: Headers): 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 +def test_validate_headers_te_forbidden_http10(): + resp = Response.make(headers=Headers(transfer_encoding="chunked")) resp.http_version = "HTTP/1.0" with pytest.raises(ValueError): validate_headers(resp) - resp = Response.make(status_code=204) - resp.headers = te_headers + +def test_validate_headers_te_forbidden_204(): + resp = Response.make(headers=Headers(transfer_encoding="chunked"), status_code=204) + with pytest.raises(ValueError): - validate_headers(Response.make(headers=te_headers, status_code=204)) + validate_headers(resp) + + +def test_validate_headers_te_forbidden_identity_request(): + req = Request.make( + "POST", "https://example.com", headers=Headers(transfer_encoding="identity") + ) + + with pytest.raises(ValueError): + validate_headers(req)