diff --git a/gunicorn/config.py b/gunicorn/config.py index 50baeb616..be9bb001c 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -2344,3 +2344,21 @@ class HeaderMap(Setting): .. versionadded:: 22.0.0 """ + + +class TolerateDangerousFraming(Setting): + name = "tolerate_dangerous_framing" + section = "Server Mechanics" + cli = ["--tolerate-dangerous-framing"] + validator = validate_bool + action = "store_true" + default = False + desc = """\ + Process requests with both Transfer-Encoding and Content-Length + + This is known to induce vulnerabilities, but not strictly forbidden by RFC9112. + + Use with care and only if necessary. May be removed in a future version. + + .. versionadded:: 22.0.0 + """ diff --git a/gunicorn/http/errors.py b/gunicorn/http/errors.py index 05abd1ab0..340f0473c 100644 --- a/gunicorn/http/errors.py +++ b/gunicorn/http/errors.py @@ -73,6 +73,15 @@ def __str__(self): return "Invalid HTTP header name: %r" % self.hdr +class UnsupportedTransferCoding(ParseException): + def __init__(self, hdr): + self.hdr = hdr + self.code = 501 + + def __str__(self): + return "Unsupported transfer coding: %r" % self.hdr + + class InvalidChunkSize(IOError): def __init__(self, data): self.data = data diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index 59a780f6c..75b36e330 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -12,6 +12,7 @@ InvalidHeader, InvalidHeaderName, NoMoreData, InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion, LimitRequestLine, LimitRequestHeaders, + UnsupportedTransferCoding, ) from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest from gunicorn.http.errors import InvalidSchemeHeaders @@ -39,6 +40,7 @@ def __init__(self, cfg, unreader, peer_addr): self.trailers = [] self.body = None self.scheme = "https" if cfg.is_ssl else "http" + self.must_close = False # set headers limits self.limit_request_fields = cfg.limit_request_fields @@ -58,6 +60,9 @@ def __init__(self, cfg, unreader, peer_addr): self.unreader.unread(unused) self.set_body_reader() + def force_close(self): + self.must_close = True + def parse(self, unreader): raise NotImplementedError() @@ -152,9 +157,47 @@ def set_body_reader(self): content_length = value elif name == "TRANSFER-ENCODING": if value.lower() == "chunked": + # DANGER: transer codings stack, and stacked chunking is never intended + if chunked: + raise InvalidHeader("TRANSFER-ENCODING", req=self) chunked = True + elif value.lower() == "identity": + # does not do much, could still plausibly desync from what the proxy does + # safe option: nuke it, its never needed + if chunked: + raise InvalidHeader("TRANSFER-ENCODING", req=self) + elif value.lower() == "": + # lacking security review on this case + # offer the option to restore previous behaviour, but refuse by default, for now + self.force_close() + if not self.cfg.tolerate_dangerous_framing: + raise UnsupportedTransferCoding(value) + # DANGER: do not change lightly; ref: request smuggling + # T-E is a list and we *could* support correctly parsing its elements + # .. but that is only safe after getting all the edge cases right + # .. for which no real-world need exists, so best to NOT open that can of worms + else: + self.force_close() + # even if parser is extended, retain this branch: + # the "chunked not last" case remains to be rejected! + raise UnsupportedTransferCoding(value) if chunked: + # two potentially dangerous cases: + # a) CL + TE (TE overrides CL.. only safe if the recipient sees it that way too) + # b) chunked HTTP/1.0 (always faulty) + if self.version < (1, 1): + # framing wonky, see RFC 9112 Section 6.1 + self.force_close() + if not self.cfg.tolerate_dangerous_framing: + raise InvalidHeader("TRANSFER-ENCODING", req=self) + if content_length is not None: + # we cannot be certain the message framing we understood matches proxy intent + # -> whatever happens next, remaining input must not be trusted + self.force_close() + # either processing or rejecting is permitted in RFC 9112 Section 6.1 + if not self.cfg.tolerate_dangerous_framing: + raise InvalidHeader("CONTENT-LENGTH", req=self) self.body = Body(ChunkedReader(self, self.unreader)) elif content_length is not None: try: @@ -173,6 +216,8 @@ def set_body_reader(self): self.body = Body(EOFReader(self.unreader)) def should_close(self): + if self.must_close: + return True for (h, v) in self.headers: if h == "CONNECTION": v = v.lower().strip(" \t") diff --git a/tests/requests/invalid/chunked_01.http b/tests/requests/invalid/chunked_01.http new file mode 100644 index 000000000..7a8e55d2e --- /dev/null +++ b/tests/requests/invalid/chunked_01.http @@ -0,0 +1,12 @@ +POST /chunked_w_underscore_chunk_size HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +\r\n +5\r\n +hello\r\n +6_0\r\n + world\r\n +0\r\n +\r\n +POST /after HTTP/1.1\r\n +Transfer-Encoding: identity\r\n +\r\n diff --git a/tests/requests/invalid/chunked_01.py b/tests/requests/invalid/chunked_01.py new file mode 100644 index 000000000..0571e1183 --- /dev/null +++ b/tests/requests/invalid/chunked_01.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidChunkSize +request = InvalidChunkSize diff --git a/tests/requests/invalid/chunked_02.http b/tests/requests/invalid/chunked_02.http new file mode 100644 index 000000000..9ae49e525 --- /dev/null +++ b/tests/requests/invalid/chunked_02.http @@ -0,0 +1,9 @@ +POST /chunked_with_prefixed_value HTTP/1.1\r\n +Content-Length: 12\r\n +Transfer-Encoding: \tchunked\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +\r\n diff --git a/tests/requests/invalid/chunked_02.py b/tests/requests/invalid/chunked_02.py new file mode 100644 index 000000000..1541eb70b --- /dev/null +++ b/tests/requests/invalid/chunked_02.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidHeader +request = InvalidHeader diff --git a/tests/requests/invalid/chunked_03.http b/tests/requests/invalid/chunked_03.http new file mode 100644 index 000000000..0bbbfe6ef --- /dev/null +++ b/tests/requests/invalid/chunked_03.http @@ -0,0 +1,8 @@ +POST /double_chunked HTTP/1.1\r\n +Transfer-Encoding: identity, chunked, identity, chunked\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +\r\n diff --git a/tests/requests/invalid/chunked_03.py b/tests/requests/invalid/chunked_03.py new file mode 100644 index 000000000..58a34600c --- /dev/null +++ b/tests/requests/invalid/chunked_03.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import UnsupportedTransferCoding +request = UnsupportedTransferCoding diff --git a/tests/requests/invalid/chunked_04.http b/tests/requests/invalid/chunked_04.http new file mode 100644 index 000000000..d47109e31 --- /dev/null +++ b/tests/requests/invalid/chunked_04.http @@ -0,0 +1,11 @@ +POST /chunked_twice HTTP/1.1\r\n +Transfer-Encoding: identity\r\n +Transfer-Encoding: chunked\r\n +Transfer-Encoding: identity\r\n +Transfer-Encoding: chunked\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +\r\n diff --git a/tests/requests/invalid/chunked_04.py b/tests/requests/invalid/chunked_04.py new file mode 100644 index 000000000..1541eb70b --- /dev/null +++ b/tests/requests/invalid/chunked_04.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidHeader +request = InvalidHeader diff --git a/tests/requests/invalid/chunked_05.http b/tests/requests/invalid/chunked_05.http new file mode 100644 index 000000000..014e85ac0 --- /dev/null +++ b/tests/requests/invalid/chunked_05.http @@ -0,0 +1,11 @@ +POST /chunked_HTTP_1.0 HTTP/1.0\r\n +Transfer-Encoding: chunked\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +0\r\n +Vary: *\r\n +Content-Type: text/plain\r\n +\r\n diff --git a/tests/requests/invalid/chunked_05.py b/tests/requests/invalid/chunked_05.py new file mode 100644 index 000000000..1541eb70b --- /dev/null +++ b/tests/requests/invalid/chunked_05.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidHeader +request = InvalidHeader diff --git a/tests/requests/invalid/chunked_06.http b/tests/requests/invalid/chunked_06.http new file mode 100644 index 000000000..ef70faab9 --- /dev/null +++ b/tests/requests/invalid/chunked_06.http @@ -0,0 +1,9 @@ +POST /chunked_not_last HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +Transfer-Encoding: gzip\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +\r\n diff --git a/tests/requests/invalid/chunked_06.py b/tests/requests/invalid/chunked_06.py new file mode 100644 index 000000000..58a34600c --- /dev/null +++ b/tests/requests/invalid/chunked_06.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import UnsupportedTransferCoding +request = UnsupportedTransferCoding diff --git a/tests/requests/invalid/chunked_08.http b/tests/requests/invalid/chunked_08.http new file mode 100644 index 000000000..8d4aaa6e9 --- /dev/null +++ b/tests/requests/invalid/chunked_08.http @@ -0,0 +1,9 @@ +POST /chunked_not_last HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +Transfer-Encoding: identity\r\n +\r\n +5\r\n +hello\r\n +6\r\n + world\r\n +\r\n diff --git a/tests/requests/invalid/chunked_08.py b/tests/requests/invalid/chunked_08.py new file mode 100644 index 000000000..1541eb70b --- /dev/null +++ b/tests/requests/invalid/chunked_08.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidHeader +request = InvalidHeader diff --git a/tests/requests/invalid/nonascii_01.http b/tests/requests/invalid/nonascii_01.http new file mode 100644 index 000000000..30d18cd6f --- /dev/null +++ b/tests/requests/invalid/nonascii_01.http @@ -0,0 +1,4 @@ +GETß /germans.. HTTP/1.1\r\n +Content-Length: 3\r\n +\r\n +ÄÄÄ diff --git a/tests/requests/invalid/nonascii_01.py b/tests/requests/invalid/nonascii_01.py new file mode 100644 index 000000000..0da10f426 --- /dev/null +++ b/tests/requests/invalid/nonascii_01.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidRequestMethod + +cfg = Config() +request = InvalidRequestMethod diff --git a/tests/requests/invalid/nonascii_02.http b/tests/requests/invalid/nonascii_02.http new file mode 100644 index 000000000..36a617039 --- /dev/null +++ b/tests/requests/invalid/nonascii_02.http @@ -0,0 +1,4 @@ +GETÿ /french.. HTTP/1.1\r\n +Content-Length: 3\r\n +\r\n +ÄÄÄ diff --git a/tests/requests/invalid/nonascii_02.py b/tests/requests/invalid/nonascii_02.py new file mode 100644 index 000000000..0da10f426 --- /dev/null +++ b/tests/requests/invalid/nonascii_02.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidRequestMethod + +cfg = Config() +request = InvalidRequestMethod diff --git a/tests/requests/invalid/nonascii_04.http b/tests/requests/invalid/nonascii_04.http new file mode 100644 index 000000000..be0b15661 --- /dev/null +++ b/tests/requests/invalid/nonascii_04.http @@ -0,0 +1,5 @@ +GET /french.. HTTP/1.1\r\n +Content-Lengthÿ: 3\r\n +Content-Length: 3\r\n +\r\n +ÄÄÄ diff --git a/tests/requests/invalid/nonascii_04.py b/tests/requests/invalid/nonascii_04.py new file mode 100644 index 000000000..d336fbc85 --- /dev/null +++ b/tests/requests/invalid/nonascii_04.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidHeaderName + +cfg = Config() +request = InvalidHeaderName diff --git a/tests/requests/invalid/prefix_01.http b/tests/requests/invalid/prefix_01.http new file mode 100644 index 000000000..f8bdeb352 --- /dev/null +++ b/tests/requests/invalid/prefix_01.http @@ -0,0 +1,2 @@ +GET\0PROXY /foo HTTP/1.1\r\n +\r\n diff --git a/tests/requests/invalid/prefix_01.py b/tests/requests/invalid/prefix_01.py new file mode 100644 index 000000000..86a0774e5 --- /dev/null +++ b/tests/requests/invalid/prefix_01.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidRequestMethod +request = InvalidRequestMethod \ No newline at end of file diff --git a/tests/requests/invalid/prefix_02.http b/tests/requests/invalid/prefix_02.http new file mode 100644 index 000000000..8a9b155c4 --- /dev/null +++ b/tests/requests/invalid/prefix_02.http @@ -0,0 +1,2 @@ +GET\0 /foo HTTP/1.1\r\n +\r\n diff --git a/tests/requests/invalid/prefix_02.py b/tests/requests/invalid/prefix_02.py new file mode 100644 index 000000000..86a0774e5 --- /dev/null +++ b/tests/requests/invalid/prefix_02.py @@ -0,0 +1,2 @@ +from gunicorn.http.errors import InvalidRequestMethod +request = InvalidRequestMethod \ No newline at end of file diff --git a/tests/requests/invalid/prefix_03.http b/tests/requests/invalid/prefix_03.http new file mode 100644 index 000000000..7803935cc --- /dev/null +++ b/tests/requests/invalid/prefix_03.http @@ -0,0 +1,4 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length: 0 1\r\n +\r\n +x diff --git a/tests/requests/invalid/prefix_03.py b/tests/requests/invalid/prefix_03.py new file mode 100644 index 000000000..95b0581ae --- /dev/null +++ b/tests/requests/invalid/prefix_03.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidHeader + +cfg = Config() +request = InvalidHeader diff --git a/tests/requests/invalid/prefix_04.http b/tests/requests/invalid/prefix_04.http new file mode 100644 index 000000000..712631c8f --- /dev/null +++ b/tests/requests/invalid/prefix_04.http @@ -0,0 +1,5 @@ +GET /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length: 3 1\r\n +\r\n +xyz +abc123 diff --git a/tests/requests/invalid/prefix_04.py b/tests/requests/invalid/prefix_04.py new file mode 100644 index 000000000..95b0581ae --- /dev/null +++ b/tests/requests/invalid/prefix_04.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidHeader + +cfg = Config() +request = InvalidHeader diff --git a/tests/requests/invalid/prefix_05.http b/tests/requests/invalid/prefix_05.http new file mode 100644 index 000000000..120b6577d --- /dev/null +++ b/tests/requests/invalid/prefix_05.http @@ -0,0 +1,4 @@ +GET: /stuff/here?foo=bar HTTP/1.1\r\n +Content-Length: 3\r\n +\r\n +xyz diff --git a/tests/requests/invalid/prefix_05.py b/tests/requests/invalid/prefix_05.py new file mode 100644 index 000000000..0da10f426 --- /dev/null +++ b/tests/requests/invalid/prefix_05.py @@ -0,0 +1,5 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidRequestMethod + +cfg = Config() +request = InvalidRequestMethod diff --git a/tests/requests/valid/025.http b/tests/requests/valid/025.http index 62267add0..f8d7fae2e 100644 --- a/tests/requests/valid/025.http +++ b/tests/requests/valid/025.http @@ -1,5 +1,4 @@ POST /chunked_cont_h_at_first HTTP/1.1\r\n -Content-Length: -1\r\n Transfer-Encoding: chunked\r\n \r\n 5; some; parameters=stuff\r\n @@ -16,4 +15,10 @@ Content-Length: -1\r\n hello\r\n 6; blahblah; blah\r\n world\r\n -0\r\n \ No newline at end of file +0\r\n +\r\n +PUT /ignored_after_dangerous_framing HTTP/1.1\r\n +Content-Length: 3\r\n +\r\n +foo\r\n +\r\n diff --git a/tests/requests/valid/025.py b/tests/requests/valid/025.py index 12ea9ab76..33f5845cb 100644 --- a/tests/requests/valid/025.py +++ b/tests/requests/valid/025.py @@ -1,9 +1,13 @@ +from gunicorn.config import Config + +cfg = Config() +cfg.set("tolerate_dangerous_framing", True) + req1 = { "method": "POST", "uri": uri("/chunked_cont_h_at_first"), "version": (1, 1), "headers": [ - ("CONTENT-LENGTH", "-1"), ("TRANSFER-ENCODING", "chunked") ], "body": b"hello world" diff --git a/tests/requests/valid/025compat.http b/tests/requests/valid/025compat.http new file mode 100644 index 000000000..828f6fb71 --- /dev/null +++ b/tests/requests/valid/025compat.http @@ -0,0 +1,18 @@ +POST /chunked_cont_h_at_first HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +\r\n +5; some; parameters=stuff\r\n +hello\r\n +6; blahblah; blah\r\n + world\r\n +0\r\n +\r\n +PUT /chunked_cont_h_at_last HTTP/1.1\r\n +Transfer-Encoding: chunked\r\n +Content-Length: -1\r\n +\r\n +5; some; parameters=stuff\r\n +hello\r\n +6; blahblah; blah\r\n + world\r\n +0\r\n diff --git a/tests/requests/valid/025compat.py b/tests/requests/valid/025compat.py new file mode 100644 index 000000000..33f5845cb --- /dev/null +++ b/tests/requests/valid/025compat.py @@ -0,0 +1,27 @@ +from gunicorn.config import Config + +cfg = Config() +cfg.set("tolerate_dangerous_framing", True) + +req1 = { + "method": "POST", + "uri": uri("/chunked_cont_h_at_first"), + "version": (1, 1), + "headers": [ + ("TRANSFER-ENCODING", "chunked") + ], + "body": b"hello world" +} + +req2 = { + "method": "PUT", + "uri": uri("/chunked_cont_h_at_last"), + "version": (1, 1), + "headers": [ + ("TRANSFER-ENCODING", "chunked"), + ("CONTENT-LENGTH", "-1"), + ], + "body": b"hello world" +} + +request = [req1, req2] diff --git a/tests/requests/valid/029.http b/tests/requests/valid/029.http index c8611dbd3..5d029dd91 100644 --- a/tests/requests/valid/029.http +++ b/tests/requests/valid/029.http @@ -1,6 +1,6 @@ GET /stuff/here?foo=bar HTTP/1.1\r\n -Transfer-Encoding: chunked\r\n Transfer-Encoding: identity\r\n +Transfer-Encoding: chunked\r\n \r\n 5\r\n hello\r\n diff --git a/tests/requests/valid/029.py b/tests/requests/valid/029.py index f25449d15..64d026604 100644 --- a/tests/requests/valid/029.py +++ b/tests/requests/valid/029.py @@ -7,8 +7,8 @@ "uri": uri("/stuff/here?foo=bar"), "version": (1, 1), "headers": [ + ('TRANSFER-ENCODING', 'identity'), ('TRANSFER-ENCODING', 'chunked'), - ('TRANSFER-ENCODING', 'identity') ], "body": b"hello" } diff --git a/tests/treq.py b/tests/treq.py index 05adb1461..aeaae151f 100644 --- a/tests/treq.py +++ b/tests/treq.py @@ -248,8 +248,10 @@ def test_req(sn, sz, mt): def check(self, cfg, sender, sizer, matcher): cases = self.expect[:] p = RequestParser(cfg, sender(), None) - for req in p: + parsed_request_idx = -1 + for parsed_request_idx, req in enumerate(p): self.same(req, sizer, matcher, cases.pop(0)) + assert len(self.expect) == parsed_request_idx + 1 assert not cases def same(self, req, sizer, matcher, exp):