From 5d15571fff7072055c946163e5547aa0c945a095 Mon Sep 17 00:00:00 2001 From: Simon King Date: Mon, 4 Nov 2024 17:57:11 +0000 Subject: [PATCH 1/2] Drop untrusted values from trusted proxy headers 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. --- src/waitress/proxy_headers.py | 18 +++++++++++++++--- tests/test_proxy_headers.py | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/waitress/proxy_headers.py b/src/waitress/proxy_headers.py index 652ca0bc..fff896a4 100644 --- a/src/waitress/proxy_headers.py +++ b/src/waitress/proxy_headers.py @@ -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) @@ -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"] @@ -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) @@ -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"] @@ -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() @@ -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 diff --git a/tests/test_proxy_headers.py b/tests/test_proxy_headers.py index 45f98785..8191cde1 100644 --- a/tests/test_proxy_headers.py +++ b/tests/test_proxy_headers.py @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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, @@ -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() From da38a2093cc90afb51dfe6599bea7cdefd0024eb Mon Sep 17 00:00:00 2001 From: Simon King Date: Tue, 12 Nov 2024 14:04:20 +0000 Subject: [PATCH 2/2] Sign CONTRIBUTORS.txt --- CONTRIBUTORS.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 462ea249..83e25c04 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -148,3 +148,5 @@ Contributors - Frank Krick, 2018-10-29 - Jonathan Vanasco, 2022-11-15 + +- Simon King, 2024-11-12