Merge pull request #3113 from pajod/patch-security

Fix numerous message parsing issues (v2)

Author: benoitc

SECURITY.md

@@ -0,0 +1,22 @@
+# Security Policy
+
+## Reporting a Vulnerability
+
+**Please note that public Github issues are open for everyone to see!**
+
+If you believe you are found a problem in Gunicorn software, examples or documentation, we encourage you to send your report privately via email, or via Github using the *Report a vulnerability* button in the [Security](https://github.com/benoitc/gunicorn/security) section.
+
+## Supported Releases
+
+At this time, **only the latest release** receives any security attention whatsoever.
+
+| Version | Status          |
+| ------- | ------------------ |
+| latest release  | :white_check_mark: |
+| 21.2.0  | :x: |
+| 20.0.0  | :x: |
+| < 20.0  | :x: |
+
+## Python Versions
+
+Gunicorn runs on Python 3.7+, we *highly recommend* the latest release of a [supported series](https://devguide.python.org/version/) and will not prioritize issues exclusively affecting in EoL environments.

docs/source/2023-news.rst

@@ -2,6 +2,28 @@
 Changelog - 2023
 ================
 
+22.0.0 - TBDTBDTBD
+==================
+
+- fix numerous security vulnerabilites in HTTP parser (closing some request smuggling vectors)
+- parsing additional requests is no longer attempted past unsupported request framing
+- on HTTP versions < 1.1 support for chunked transfer is refused (only used in exploits)
+- requests conflicting configured or passed SCRIPT_NAME now produce a verbose error
+- Trailer fields are no longer inspected for headers indicating secure scheme
+
+** Breaking changes **
+
+- the limitations on valid characters in the HTTP method have been bounded to Internet Standards
+- requests specifying unsupported transfer coding (order) are refused by default (rare)
+- HTTP methods are no longer casefolded by default (IANA method registry contains none affacted)
+- HTTP methods containing the number sign (#) are no longer accepted by default (rare)
+- HTTP versions < 1.0 or >= 2.0 are no longer accepted by default (rare, only HTTP/1.1 is supported)
+- HTTP versions consisting of multiple digits or containing a prefix/suffix are no longer accepted
+- HTTP header field names Gunicorn cannot safely map to variables are silently dropped, as in other software
+- HTTP headers with empty field name are refused by default (no legitimate use cases, used in exploits)
+- requests with both Transfer-Encoding and Content-Length are refused by default (such a message might indicate an attempt to perform request smuggling)
+- empty transfer codings are no longer permitted (reportedly seen with really old & broken proxies)
+
 21.2.0 - 2023-07-19
 ===================
 

gunicorn/config.py

@@ -2254,5 +2254,131 @@ class StripHeaderSpaces(Setting):
         This is known to induce vulnerabilities and is not compliant with the HTTP/1.1 standard.
         See https://portswigger.net/research/http-desync-attacks-request-smuggling-reborn.
 
-        Use with care and only if necessary.
+        Use with care and only if necessary. May be removed in a future version.
+
+        .. versionadded:: 20.0.1
+        """
+
+
+class PermitUnconventionalHTTPMethod(Setting):
+    name = "permit_unconventional_http_method"
+    section = "Server Mechanics"
+    cli = ["--permit-unconventional-http-method"]
+    validator = validate_bool
+    action = "store_true"
+    default = False
+    desc = """\
+        Permit HTTP methods not matching conventions, such as IANA registration guidelines
+
+        This permits request methods of length less than 3 or more than 20,
+        methods with lowercase characters or methods containing the # character.
+        HTTP methods are case sensitive by definition, and merely uppercase by convention.
+
+        This option is provided to diagnose backwards-incompatible changes.
+
+        Use with care and only if necessary. May be removed in a future version.
+
+        .. versionadded:: 22.0.0
+        """
+
+
+class PermitUnconventionalHTTPVersion(Setting):
+    name = "permit_unconventional_http_version"
+    section = "Server Mechanics"
+    cli = ["--permit-unconventional-http-version"]
+    validator = validate_bool
+    action = "store_true"
+    default = False
+    desc = """\
+        Permit HTTP version not matching conventions of 2023
+
+        This disables the refusal of likely malformed request lines.
+        It is unusual to specify HTTP 1 versions other than 1.0 and 1.1.
+
+        This option is provided to diagnose backwards-incompatible changes.
+        Use with care and only if necessary. May be removed in a future version.
+
+        .. versionadded:: 22.0.0
+        """
+
+
+class CasefoldHTTPMethod(Setting):
+    name = "casefold_http_method"
+    section = "Server Mechanics"
+    cli = ["--casefold-http-method"]
+    validator = validate_bool
+    action = "store_true"
+    default = False
+    desc = """\
+         Transform received HTTP methods to uppercase
+
+         HTTP methods are case sensitive by definition, and merely uppercase by convention.
+
+         This option is provided because previous versions of gunicorn defaulted to this behaviour.
+
+         Use with care and only if necessary. May be removed in a future version.
+
+         .. versionadded:: 22.0.0
+         """
+
+
+def validate_header_map_behaviour(val):
+    # FIXME: refactor all of this subclassing stdlib argparse
+
+    if val is None:
+        return
+
+    if not isinstance(val, str):
+        raise TypeError("Invalid type for casting: %s" % val)
+    if val.lower().strip() == "drop":
+        return "drop"
+    elif val.lower().strip() == "refuse":
+        return "refuse"
+    elif val.lower().strip() == "dangerous":
+        return "dangerous"
+    else:
+        raise ValueError("Invalid header map behaviour: %s" % val)
+
+
+class HeaderMap(Setting):
+    name = "header_map"
+    section = "Server Mechanics"
+    cli = ["--header-map"]
+    validator = validate_header_map_behaviour
+    default = "drop"
+    desc = """\
+        Configure how header field names are mapped into environ
+
+        Headers containing underscores are permitted by RFC9110,
+        but gunicorn joining headers of different names into
+        the same environment variable will dangerously confuse applications as to which is which.
+
+        The safe default ``drop`` is to silently drop headers that cannot be unambiguously mapped.
+        The value ``refuse`` will return an error if a request contains *any* such header.
+        The value ``dangerous`` matches the previous, not advisabble, behaviour of mapping different
+        header field names into the same environ name.
+
+        Use with care and only if necessary and after considering if your problem could
+        instead be solved by specifically renaming or rewriting only the intended headers
+        on a proxy in front of Gunicorn.
+
+        .. 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
         """

gunicorn/http/body.py

@@ -51,7 +51,7 @@ def parse_trailers(self, unreader, data):
         if done:
             unreader.unread(buf.getvalue()[2:])
             return b""
-        self.req.trailers = self.req.parse_headers(buf.getvalue()[:idx])
+        self.req.trailers = self.req.parse_headers(buf.getvalue()[:idx], from_trailer=True)
         unreader.unread(buf.getvalue()[idx + 4:])
 
     def parse_chunked(self, unreader):
@@ -85,11 +85,13 @@ def parse_chunk_size(self, unreader, data=None):
         data = buf.getvalue()
         line, rest_chunk = data[:idx], data[idx + 2:]
 
-        chunk_size = line.split(b";", 1)[0].strip()
-        try:
-            chunk_size = int(chunk_size, 16)
-        except ValueError:
+        # RFC9112 7.1.1: BWS before chunk-ext - but ONLY then
+        chunk_size, *chunk_ext = line.split(b";", 1)
+        if chunk_ext:
+            chunk_size = chunk_size.rstrip(b" \t")
+        if any(n not in b"0123456789abcdefABCDEF" for n in chunk_size):
             raise InvalidChunkSize(chunk_size)
+        chunk_size = int(chunk_size, 16)
 
         if chunk_size == 0:
             try:

gunicorn/http/errors.py

@@ -22,6 +22,15 @@ def __str__(self):
         return "No more data after: %r" % self.buf
 
 
+class ConfigurationProblem(ParseException):
+    def __init__(self, info):
+        self.info = info
+        self.code = 500
+
+    def __str__(self):
+        return "Configuration problem: %s" % self.info
+
+
 class InvalidRequestLine(ParseException):
     def __init__(self, req):
         self.req = req
@@ -64,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