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

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

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",

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:

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

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

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
                     )

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

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

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