silently drop or refuse header names w/ underscore

Ambiguous mappings open a bottomless pit of "what is user input and what is proxy input" confusion.
Default to what everyone else has been doing for years now, silently drop.

see also https://nginx.org/r/underscores_in_headers

Author: pajod

gunicorn/config.py

@@ -2300,3 +2300,47 @@ class CasefoldHTTPMethod(Setting):
 
          .. 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
+        """

gunicorn/http/message.py

@@ -120,6 +120,23 @@ def parse_headers(self, data):
                     scheme_header = True
                     self.scheme = scheme
 
+            # ambiguous mapping allows fooling downstream, e.g. merging non-identical headers:
+            # X-Forwarded-For: 2001:db8::ha:cc:ed
+            # X_Forwarded_For: 127.0.0.1,::1
+            # HTTP_X_FORWARDED_FOR = 2001:db8::ha:cc:ed,127.0.0.1,::1
+            # Only modify after fixing *ALL* header transformations; network to wsgi env
+            if "_" in name:
+                if self.cfg.header_map == "dangerous":
+                    # as if we did not know we cannot safely map this
+                    pass
+                elif self.cfg.header_map == "drop":
+                    # almost as if it never had been there
+                    # but still counts against resource limits
+                    continue
+                else:
+                    # fail-safe fallthrough: refuse
+                    raise InvalidHeaderName(name)
+
             headers.append((name, value))
 
         return headers

gunicorn/http/wsgi.py

@@ -135,6 +135,8 @@ def create(req, sock, client, server, cfg):
             environ['CONTENT_LENGTH'] = hdr_value
             continue
 
+        # do not change lightly, this is a common source of security problems
+        # RFC9110 Section 17.10 discourages ambiguous or incomplete mappings
         key = 'HTTP_' + hdr_name.replace('-', '_')
         if key in environ:
             hdr_value = "%s,%s" % (environ[key], hdr_value)

tests/requests/invalid/040.http

@@ -0,0 +1,6 @@
+GET /keep/same/as?invalid/040 HTTP/1.0\r\n
+Transfer_Encoding: tricked\r\n
+Content-Length: 7\r\n
+Content_Length: -1E23\r\n
+\r\n
+tricked\r\n

tests/requests/invalid/040.py

@@ -0,0 +1,7 @@
+from gunicorn.http.errors import InvalidHeaderName
+from gunicorn.config import Config
+
+cfg = Config()
+cfg.set("header_map", "refuse")
+
+request = InvalidHeaderName

tests/requests/invalid/chunked_07.http

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

tests/requests/invalid/chunked_07.py

@@ -0,0 +1,7 @@
+from gunicorn.http.errors import InvalidHeaderName
+from gunicorn.config import Config
+
+cfg = Config()
+cfg.set("header_map", "refuse")
+
+request = InvalidHeaderName

tests/requests/valid/040.http

@@ -0,0 +1,6 @@
+GET /keep/same/as?invalid/040 HTTP/1.0\r\n
+Transfer_Encoding: tricked\r\n
+Content-Length: 7\r\n
+Content_Length: -1E23\r\n
+\r\n
+tricked\r\n

tests/requests/valid/040.py

@@ -0,0 +1,9 @@
+request = {
+    "method": "GET",
+    "uri": uri("/keep/same/as?invalid/040"),
+    "version": (1, 0),
+    "headers": [
+        ("CONTENT-LENGTH", "7")
+    ],
+    "body": b'tricked'
+}

tests/requests/valid/040_compat.http

@@ -0,0 +1,6 @@
+GET /keep/same/as?invalid/040 HTTP/1.0\r\n
+Transfer_Encoding: tricked\r\n
+Content-Length: 7\r\n
+Content_Length: -1E23\r\n
+\r\n
+tricked\r\n

tests/requests/valid/040_compat.py

@@ -0,0 +1,16 @@
+from gunicorn.config import Config
+
+cfg = Config()
+cfg.set("header_map", "dangerous")
+
+request = {
+    "method": "GET",
+    "uri": uri("/keep/same/as?invalid/040"),
+    "version": (1, 0),
+    "headers": [
+        ("TRANSFER_ENCODING", "tricked"),
+        ("CONTENT-LENGTH", "7"),
+        ("CONTENT_LENGTH", "-1E23"),
+    ],
+    "body": b'tricked'
+}