Skip to content

Commit

Permalink
silently drop or refuse header names w/ underscore
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pajod committed Dec 15, 2023
1 parent b284678 commit 72b8970
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 0 deletions.
44 changes: 44 additions & 0 deletions gunicorn/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
17 changes: 17 additions & 0 deletions gunicorn/http/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions gunicorn/http/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions tests/requests/invalid/040.http
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions tests/requests/invalid/040.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from gunicorn.http.errors import InvalidHeaderName
from gunicorn.config import Config

cfg = Config()
cfg.set("header_map", "refuse")

request = InvalidHeaderName
10 changes: 10 additions & 0 deletions tests/requests/invalid/chunked_07.http
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions tests/requests/invalid/chunked_07.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from gunicorn.http.errors import InvalidHeaderName
from gunicorn.config import Config

cfg = Config()
cfg.set("header_map", "refuse")

request = InvalidHeaderName
6 changes: 6 additions & 0 deletions tests/requests/valid/040.http
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions tests/requests/valid/040.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
request = {
"method": "GET",
"uri": uri("/keep/same/as?invalid/040"),
"version": (1, 0),
"headers": [
("CONTENT-LENGTH", "7")
],
"body": b'tricked'
}
6 changes: 6 additions & 0 deletions tests/requests/valid/040_compat.http
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions tests/requests/valid/040_compat.py
Original file line number Diff line number Diff line change
@@ -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'
}

0 comments on commit 72b8970

Please sign in to comment.