Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop untrusted values from trusted proxy headers #452

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,5 @@ Contributors
- Frank Krick, 2018-10-29

- Jonathan Vanasco, 2022-11-15

- Simon King, 2024-11-12
18 changes: 15 additions & 3 deletions src/waitress/proxy_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ def raise_for_multiple_values():
try:
forwarded_for = []

for forward_hop in environ["HTTP_X_FORWARDED_FOR"].split(","):
raw_forwarded_for = environ["HTTP_X_FORWARDED_FOR"].split(",")
for forward_hop in raw_forwarded_for:
forward_hop = forward_hop.strip()
forward_hop = undquote(forward_hop)

Expand All @@ -103,6 +104,9 @@ def raise_for_multiple_values():
client_addr = forwarded_for[0]

untrusted_headers.remove("X_FORWARDED_FOR")
environ["HTTP_X_FORWARDED_FOR"] = ",".join(
raw_forwarded_for[-trusted_proxy_count:]
).strip()
except Exception as ex:
raise MalformedProxyHeader(
"X-Forwarded-For", str(ex), environ["HTTP_X_FORWARDED_FOR"]
Expand All @@ -115,7 +119,8 @@ def raise_for_multiple_values():
try:
forwarded_host_multiple = []

for forward_host in environ["HTTP_X_FORWARDED_HOST"].split(","):
raw_forwarded_host = environ["HTTP_X_FORWARDED_HOST"].split(",")
for forward_host in raw_forwarded_host:
forward_host = forward_host.strip()
forward_host = undquote(forward_host)
forwarded_host_multiple.append(forward_host)
Expand All @@ -124,6 +129,9 @@ def raise_for_multiple_values():
forwarded_host = forwarded_host_multiple[0]

untrusted_headers.remove("X_FORWARDED_HOST")
environ["HTTP_X_FORWARDED_HOST"] = ",".join(
raw_forwarded_host[-trusted_proxy_count:]
).strip()
except Exception as ex:
raise MalformedProxyHeader(
"X-Forwarded-Host", str(ex), environ["HTTP_X_FORWARDED_HOST"]
Expand Down Expand Up @@ -163,8 +171,9 @@ def raise_for_multiple_values():
# If the Forwarded header exists, it gets priority
if forwarded:
proxies = []
raw_forwarded = forwarded.split(",")
try:
for forwarded_element in forwarded.split(","):
for forwarded_element in raw_forwarded:
# Remove whitespace that may have been introduced when
# appending a new entry
forwarded_element = forwarded_element.strip()
Expand Down Expand Up @@ -213,6 +222,9 @@ def raise_for_multiple_values():
raise MalformedProxyHeader("Forwarded", str(ex), environ["HTTP_FORWARDED"])

proxies = proxies[-trusted_proxy_count:]
environ["HTTP_FORWARDED"] = ",".join(
raw_forwarded[-trusted_proxy_count:]
).strip()

# Iterate backwards and fill in some values, the oldest entry that
# contains the information we expect is the one we use. We expect
Expand Down
21 changes: 21 additions & 0 deletions tests/test_proxy_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ def test_parse_proxy_headers_forwared_for_multiple(self):

environ = inner.environ
self.assertEqual(environ["REMOTE_ADDR"], "198.51.100.2")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")

def test_parse_forwarded_multiple_proxies_trust_only_two(self):
inner = DummyApp()
Expand All @@ -251,6 +252,10 @@ def test_parse_forwarded_multiple_proxies_trust_only_two(self):
self.assertEqual(environ["SERVER_NAME"], "example.com")
self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
self.assertEqual(environ["SERVER_PORT"], "8080")
self.assertEqual(
environ["HTTP_FORWARDED"],
"For=198.51.100.2;host=example.com:8080, For=203.0.113.1",
)

def test_parse_forwarded_multiple_proxies(self):
inner = DummyApp()
Expand Down Expand Up @@ -278,6 +283,11 @@ def test_parse_forwarded_multiple_proxies(self):
self.assertEqual(environ["HTTP_HOST"], "example.com:8443")
self.assertEqual(environ["SERVER_PORT"], "8443")
self.assertEqual(environ["wsgi.url_scheme"], "https")
self.assertEqual(
environ["HTTP_FORWARDED"],
'for="[2001:db8::1]:3821";host="example.com:8443";proto="https", '
'for=192.0.2.1;host="example.internal:8080"',
)

def test_parse_forwarded_multiple_proxies_minimal(self):
inner = DummyApp()
Expand All @@ -304,6 +314,10 @@ def test_parse_forwarded_multiple_proxies_minimal(self):
self.assertEqual(environ["HTTP_HOST"], "example.org")
self.assertEqual(environ["SERVER_PORT"], "443")
self.assertEqual(environ["wsgi.url_scheme"], "https")
self.assertEqual(
environ["HTTP_FORWARDED"],
'for="[2001:db8::1]";proto="https", for=192.0.2.1;host="example.org"',
)

def test_parse_proxy_headers_forwarded_host_with_port(self):
inner = DummyApp()
Expand Down Expand Up @@ -332,6 +346,7 @@ def test_parse_proxy_headers_forwarded_host_with_port(self):
self.assertEqual(environ["SERVER_NAME"], "example.com")
self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
self.assertEqual(environ["SERVER_PORT"], "8080")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")

def test_parse_proxy_headers_forwarded_host_without_port(self):
inner = DummyApp()
Expand Down Expand Up @@ -360,6 +375,7 @@ def test_parse_proxy_headers_forwarded_host_without_port(self):
self.assertEqual(environ["SERVER_NAME"], "example.com")
self.assertEqual(environ["HTTP_HOST"], "example.com")
self.assertEqual(environ["SERVER_PORT"], "80")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")

def test_parse_proxy_headers_forwarded_host_with_forwarded_port(self):
inner = DummyApp()
Expand Down Expand Up @@ -390,6 +406,7 @@ def test_parse_proxy_headers_forwarded_host_with_forwarded_port(self):
self.assertEqual(environ["SERVER_NAME"], "example.com")
self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
self.assertEqual(environ["SERVER_PORT"], "8080")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")

def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port(self):
inner = DummyApp()
Expand Down Expand Up @@ -420,6 +437,8 @@ def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port(self):
self.assertEqual(environ["SERVER_NAME"], "example.com")
self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
self.assertEqual(environ["SERVER_PORT"], "8080")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
self.assertEqual(environ["HTTP_X_FORWARDED_HOST"], "example.com, example.org")

def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port_limit_one_trusted(
self,
Expand Down Expand Up @@ -452,6 +471,8 @@ def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port_limit_o
self.assertEqual(environ["SERVER_NAME"], "example.org")
self.assertEqual(environ["HTTP_HOST"], "example.org:8080")
self.assertEqual(environ["SERVER_PORT"], "8080")
self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "203.0.113.1")
self.assertEqual(environ["HTTP_X_FORWARDED_HOST"], "example.org")

def test_parse_forwarded(self):
inner = DummyApp()
Expand Down