diff --git a/gunicorn/config.py b/gunicorn/config.py index 808a4b04..50baeb61 100644 --- a/gunicorn/config.py +++ b/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 + """ diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index 2aa021c8..59a780f6 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -120,6 +120,23 @@ class Message(object): 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 diff --git a/gunicorn/http/wsgi.py b/gunicorn/http/wsgi.py index bafed49e..6f3d9b68 100644 --- a/gunicorn/http/wsgi.py +++ b/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) diff --git a/tests/requests/invalid/040.http b/tests/requests/invalid/040.http new file mode 100644 index 00000000..be03929d --- /dev/null +++ b/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 diff --git a/tests/requests/invalid/040.py b/tests/requests/invalid/040.py new file mode 100644 index 00000000..643289fa --- /dev/null +++ b/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 diff --git a/tests/requests/invalid/chunked_07.http b/tests/requests/invalid/chunked_07.http new file mode 100644 index 00000000..a78db48b --- /dev/null +++ b/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 diff --git a/tests/requests/invalid/chunked_07.py b/tests/requests/invalid/chunked_07.py new file mode 100644 index 00000000..643289fa --- /dev/null +++ b/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 diff --git a/tests/requests/valid/040.http b/tests/requests/valid/040.http new file mode 100644 index 00000000..be03929d --- /dev/null +++ b/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 diff --git a/tests/requests/valid/040.py b/tests/requests/valid/040.py new file mode 100644 index 00000000..7c2243d9 --- /dev/null +++ b/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' +} diff --git a/tests/requests/valid/040_compat.http b/tests/requests/valid/040_compat.http new file mode 100644 index 00000000..be03929d --- /dev/null +++ b/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 diff --git a/tests/requests/valid/040_compat.py b/tests/requests/valid/040_compat.py new file mode 100644 index 00000000..5f13487c --- /dev/null +++ b/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' +}