Skip to content

Commit

Permalink
Drop untrusted values from trusted proxy headers
Browse files Browse the repository at this point in the history 
Headers such as X-Forwarded-For, X-Forwarded-Host and Forwarded can
contain more values than are actually trusted, leading to the
possibility that the downstream application could interpret those
headers differently to waitress.

This change rewrites the trusted headers so that they only contain the
values from the trusted proxies.
  • Loading branch information
@simonk52
simonk52 committed Nov 4, 2024
1 parent ae949bb commit d0f82c1
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 5 deletions.
26 changes: 21 additions & 5 deletions src/waitress/proxy_headers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def translate_proxy_headers(environ, start_response):
remote_peer = environ["REMOTE_ADDR"]
if trusted_proxy == "*" or remote_peer == trusted_proxy:
try:
untrusted_headers = parse_proxy_headers(
untrusted_headers, environ_rewrites = parse_proxy_headers(
environ,
trusted_proxy_count=trusted_proxy_count,
trusted_proxy_headers=trusted_proxy_headers,
Expand All @@ -55,6 +55,9 @@ def translate_proxy_headers(environ, start_response):
error = BadRequest(f'Header "{ex.header}" malformed.')
return error.wsgi_response(environ, start_response)

# Remove untrusted values from trusted headers
environ.update(environ_rewrites)

# Clear out the untrusted proxy headers
if clear_untrusted:
clear_untrusted_headers(
Expand All @@ -76,6 +79,7 @@ def parse_proxy_headers(
forwarded_host = forwarded_proto = forwarded_port = forwarded = ""
client_addr = None
untrusted_headers = set(PROXY_HEADERS)
environ_rewrites = {}

def raise_for_multiple_values():
raise ValueError("Unspecified behavior for multiple values found in header")
Expand All @@ -84,7 +88,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 +108,9 @@ def raise_for_multiple_values():
client_addr = forwarded_for[0]

untrusted_headers.remove("X_FORWARDED_FOR")
environ_rewrites["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 +123,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 +133,9 @@ def raise_for_multiple_values():
forwarded_host = forwarded_host_multiple[0]

untrusted_headers.remove("X_FORWARDED_HOST")
environ_rewrites["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 +175,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 +226,9 @@ def raise_for_multiple_values():
raise MalformedProxyHeader("Forwarded", str(ex), environ["HTTP_FORWARDED"])

proxies = proxies[-trusted_proxy_count:]
environ_rewrites["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 Expand Up @@ -300,7 +316,7 @@ def raise_for_multiple_values():
environ["REMOTE_ADDR"] = strip_brackets(client_addr.strip())
environ["REMOTE_HOST"] = environ["REMOTE_ADDR"]

return untrusted_headers
return untrusted_headers, environ_rewrites


def strip_brackets(addr):
Expand Down
21 changes: 21 additions & 0 deletions tests/test_proxy_headers.py
View file
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

0 comments on commit d0f82c1

Please sign in to comment.