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>
This commit is contained in:
Maximilian Hils 2024-11-28 19:20:27 +01:00 committed by GitHub
parent 0442d21261
commit 18eb29c736
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 53 additions and 28 deletions

View file

@ -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)

View file

@ -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)

View file

@ -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

View file

@ -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(

View file

@ -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(),

View file

@ -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)