Merge pull request #3260 from benoitc/fix-te

don't tolerate wrong te headers

gunicorn/config.py

@@ -2369,30 +2369,3 @@ 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.
-
-        In any case, the connection is closed after the malformed request,
-        as it is unclear if and at which boundary additional requests start.
-
-        Use with care and only if necessary.
-        Temporary; will be changed or removed in a future version.
-
-        .. versionadded:: 22.0.0
-        .. versionchanged: 22.1.0
-           The newly added rejection of invalid and dangerous characters CR, LF and NUL in
-           header field values is also controlled with this setting. rfc9110 permits both
-           rejecting and SP-replacing. With this option set, Gunicorn passes the field value
-           unchanged. With this option unset, Gunicorn rejects the request.
-        """

gunicorn/http/message.py

@@ -123,10 +123,7 @@ def parse_headers(self, data, from_trailer=False):
             value = " ".join(value)
 
             if RFC9110_5_5_INVALID_AND_DANGEROUS.search(value):
-                if not self.cfg.tolerate_dangerous_framing:
-                    raise InvalidHeader(name)
-                # value = RFC9110_5_5_INVALID_AND_DANGEROUS.sub(" ", value)
-                self.force_close()
+                raise InvalidHeader(name)
 
             if header_length > self.limit_request_field_size > 0:
                 raise LimitRequestHeaders("limit request headers fields size")
@@ -172,48 +169,39 @@ def set_body_reader(self):
                     raise InvalidHeader("CONTENT-LENGTH", req=self)
                 content_length = value
             elif name == "TRANSFER-ENCODING":
-                if value.lower() == "chunked":
-                    # DANGER: transfer 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:
+                # T-E can be a list
+                # https://datatracker.ietf.org/doc/html/rfc9112#name-transfer-encoding
+                vals = [v.strip() for v in value.split(',')]
+                for val in vals:
+                    if val.lower() == "chunked":
+                        # DANGER: transfer codings stack, and stacked chunking is never intended
+                        if chunked:
+                            raise InvalidHeader("TRANSFER-ENCODING", req=self)
+                        chunked = True
+                    elif val.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 val.lower() in ('compress', 'deflate', 'gzip'):
+                        # chunked should be the last one
+                        if chunked:
+                            raise InvalidHeader("TRANSFER-ENCODING", req=self)
+                        self.force_close()
+                    else:
                         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)
+                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)
+                raise InvalidHeader("CONTENT-LENGTH", req=self)
             self.body = Body(ChunkedReader(self, self.unreader))
         elif content_length is not None:
             try:

tests/requests/invalid/chunked_03.py

@@ -1,2 +1,2 @@
-from gunicorn.http.errors import UnsupportedTransferCoding
-request = UnsupportedTransferCoding
+from gunicorn.http.errors import InvalidHeader 
+request = InvalidHeader 

tests/requests/invalid/chunked_06.py

@@ -1,2 +1,2 @@
-from gunicorn.http.errors import UnsupportedTransferCoding
-request = UnsupportedTransferCoding
+from gunicorn.http.errors import InvalidHeader 
+request = InvalidHeader 

tests/requests/valid/025.http

@@ -1,24 +1,10 @@
-POST /chunked_cont_h_at_first HTTP/1.1\r\n
+POST /chunked HTTP/1.1\r\n
+Transfer-Encoding: gzip\r\n
 Transfer-Encoding: chunked\r\n
 \r\n
-5; some; parameters=stuff\r\n
+5\r\n
 hello\r\n
-6 \t;\tblahblah; blah\r\n
+6\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
-\r\n
-PUT /ignored_after_dangerous_framing HTTP/1.1\r\n
-Content-Length: 3\r\n
-\r\n
-foo\r\n
-\r\n

tests/requests/valid/025.py

@@ -1,27 +1,10 @@
-from gunicorn.config import Config
-
-cfg = Config()
-cfg.set("tolerate_dangerous_framing", True)
-
-req1 = {
+request = {
     "method": "POST",
-    "uri": uri("/chunked_cont_h_at_first"),
+    "uri": uri("/chunked"),
     "version": (1, 1),
     "headers": [
-        ("TRANSFER-ENCODING", "chunked")
+        ('TRANSFER-ENCODING', 'gzip'),
+        ('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]

tests/requests/valid/025_line.http

@@ -0,0 +1,9 @@
+POST /chunked HTTP/1.1\r\n
+Transfer-Encoding: gzip,chunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+0\r\n
+\r\n

tests/requests/valid/025_line.py

@@ -0,0 +1,10 @@
+request = {
+    "method": "POST",
+    "uri": uri("/chunked"),
+    "version": (1, 1),
+    "headers": [
+        ('TRANSFER-ENCODING', 'gzip,chunked')
+
+    ],
+    "body": b"hello world"
+}