From b45fafaedcfc27f65f8356f13961b03a602512c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 12:06:42 +0200 Subject: [PATCH 01/26] Allow ip ranges for FORWARDED_ALLOW_IPS --- tests/middleware/test_proxy_headers.py | 6 +++ uvicorn/middleware/proxy_headers.py | 72 ++++++++++++++++++-------- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 53a4e70db..95e1bd19f 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -41,8 +41,14 @@ async def app( # trusted proxy list (["127.0.0.1", "10.0.0.1"], "Remote: https://1.2.3.4:0"), ("127.0.0.1, 10.0.0.1", "Remote: https://1.2.3.4:0"), + # trusted proxy network + ("127.0.0.0/24, 10.0.0.1", "Remote: https://1.2.3.4:0"), # request from untrusted proxy ("192.168.0.1", "Remote: http://127.0.0.1:123"), + # request from untrusted proxy network + ("192.168.0.0/16", "Remote: http://127.0.0.1:123"), + # request from client running on proxy server + (["127.0.0.1", "1.2.3.4"], "Remote: https://1.2.3.4:0"), ], ) async def test_proxy_headers_trusted_hosts( diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 28277e1d6..64c03809d 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -8,7 +8,8 @@ https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Proxies """ -from typing import List, Optional, Tuple, Union, cast +import ipaddress +from typing import List, Optional, Tuple, Union, Set, cast from uvicorn._types import ( ASGI3Application, @@ -20,6 +21,50 @@ ) +def _parse_raw_hosts(value: str) -> List[str]: + return [item.strip() for item in value.split(",")] + + +class _TrustedHosts: + def __init__(self, trusted_hosts: Union[List[str], str]) -> None: + self.trusted_networks: Set[ipaddress.IPv4Network] = set() + self.trusted_literals: Set[str] = set() + + self.always_trust = trusted_hosts == "*" + + if not self.always_trust: + if isinstance(trusted_hosts, str): + trusted_hosts = _parse_raw_hosts(trusted_hosts) + for host in trusted_hosts: + try: + self.trusted_networks.add(ipaddress.IPv4Network(host)) + except ValueError: + self.trusted_literals.add(host) + + def __contains__(self, item: str): + if self.always_trust: + return True + + try: + ip = ipaddress.IPv4Address(item) + return any(ip in net for net in self.trusted_networks) + except ValueError: + return item in self.trusted_literals + + def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: + x_forwarded_for_hosts = _parse_raw_hosts(x_forwarded_for) + if self.always_trust: + return x_forwarded_for_hosts[0] + + host = None + for host in reversed(x_forwarded_for_hosts): + if host not in self: + return host + # The request came from a client on the proxy itself. Trust it. + if host in self: + return x_forwarded_for_hosts[0] + + class ProxyHeadersMiddleware: def __init__( self, @@ -27,23 +72,7 @@ def __init__( trusted_hosts: Union[List[str], str] = "127.0.0.1", ) -> None: self.app = app - if isinstance(trusted_hosts, str): - self.trusted_hosts = {item.strip() for item in trusted_hosts.split(",")} - else: - self.trusted_hosts = set(trusted_hosts) - self.always_trust = "*" in self.trusted_hosts - - def get_trusted_client_host( - self, x_forwarded_for_hosts: List[str] - ) -> Optional[str]: - if self.always_trust: - return x_forwarded_for_hosts[0] - - for host in reversed(x_forwarded_for_hosts): - if host not in self.trusted_hosts: - return host - - return None + self.trusted_hosts = _TrustedHosts(trusted_hosts) async def __call__( self, scope: "Scope", receive: "ASGIReceiveCallable", send: "ASGISendCallable" @@ -53,7 +82,7 @@ async def __call__( client_addr: Optional[Tuple[str, int]] = scope.get("client") client_host = client_addr[0] if client_addr else None - if self.always_trust or client_host in self.trusted_hosts: + if client_host in self.trusted_hosts: headers = dict(scope["headers"]) if b"x-forwarded-proto" in headers: @@ -74,10 +103,7 @@ async def __call__( # X-Forwarded-For header. We've lost the connecting client's port # information by now, so only include the host. x_forwarded_for = headers[b"x-forwarded-for"].decode("latin1") - x_forwarded_for_hosts = [ - item.strip() for item in x_forwarded_for.split(",") - ] - host = self.get_trusted_client_host(x_forwarded_for_hosts) + host = self.trusted_hosts.get_trusted_client_host(x_forwarded_for) port = 0 scope["client"] = (host, port) # type: ignore[arg-type] From 3caf2a17898a24fa8a4931d3bc10db76c2de8e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 14:16:43 +0200 Subject: [PATCH 02/26] Fix host for empty x-forwarded-for header --- tests/middleware/test_proxy_headers.py | 25 ++++++++++++++++++++++++- uvicorn/middleware/proxy_headers.py | 12 +++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 95e1bd19f..c51e43113 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -42,12 +42,14 @@ async def app( (["127.0.0.1", "10.0.0.1"], "Remote: https://1.2.3.4:0"), ("127.0.0.1, 10.0.0.1", "Remote: https://1.2.3.4:0"), # trusted proxy network + # https://github.com/encode/uvicorn/issues/1068#issuecomment-1004813267 ("127.0.0.0/24, 10.0.0.1", "Remote: https://1.2.3.4:0"), # request from untrusted proxy ("192.168.0.1", "Remote: http://127.0.0.1:123"), # request from untrusted proxy network ("192.168.0.0/16", "Remote: http://127.0.0.1:123"), - # request from client running on proxy server + # request from client running on proxy server itself + # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 (["127.0.0.1", "1.2.3.4"], "Remote: https://1.2.3.4:0"), ], ) @@ -148,3 +150,24 @@ async def websocket_app(scope, receive, send): async with websockets.client.connect(url, extra_headers=headers) as websocket: data = await websocket.recv() assert data == "wss://1.2.3.4:0" + + +@pytest.mark.anyio +async def test_proxy_headers_empty_x_forwarded_for() -> None: + # fallback to the default behavior if x-forwarded-for is an empty list for some reason + # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 + app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") + transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver" + ) as client: + headers = httpx.Headers( + { + "X-Forwarded-Proto": "https", + "X-Forwarded-For": "", + }, + encoding="latin-1", + ) + response = await client.get("/", headers=headers) + assert response.status_code == 200 + assert response.text == "Remote: https://1.2.3.4:8080" diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 64c03809d..e546486c1 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -29,7 +29,6 @@ class _TrustedHosts: def __init__(self, trusted_hosts: Union[List[str], str]) -> None: self.trusted_networks: Set[ipaddress.IPv4Network] = set() self.trusted_literals: Set[str] = set() - self.always_trust = trusted_hosts == "*" if not self.always_trust: @@ -37,6 +36,8 @@ def __init__(self, trusted_hosts: Union[List[str], str]) -> None: trusted_hosts = _parse_raw_hosts(trusted_hosts) for host in trusted_hosts: try: + # Try parsing the trusted host as an IPv4Network to allow checking a whole range. + # See https://github.com/encode/uvicorn/issues/1068#issuecomment-1004813267 self.trusted_networks.add(ipaddress.IPv4Network(host)) except ValueError: self.trusted_literals.add(host) @@ -61,6 +62,7 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: if host not in self: return host # The request came from a client on the proxy itself. Trust it. + # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 if host in self: return x_forwarded_for_hosts[0] @@ -104,7 +106,11 @@ async def __call__( # information by now, so only include the host. x_forwarded_for = headers[b"x-forwarded-for"].decode("latin1") host = self.trusted_hosts.get_trusted_client_host(x_forwarded_for) - port = 0 - scope["client"] = (host, port) # type: ignore[arg-type] + + # Host is None or an empty string if the x-forwarded-for header is empty. + # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 + if host: + port = 0 + scope["client"] = (host, port) # type: ignore[arg-type] return await self.app(scope, receive, send) From d721ef43b238b15c3ed1cef5a2e42d7e2457bae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 14:53:16 +0200 Subject: [PATCH 03/26] Add multi-host ip-range testcase, run scripts/lint --- tests/middleware/test_proxy_headers.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index c51e43113..3158b4da2 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -57,9 +57,7 @@ async def test_proxy_headers_trusted_hosts( trusted_hosts: Union[List[str], str], response_text: str ) -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: headers = {"X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4"} response = await client.get("/", headers=headers) @@ -85,15 +83,15 @@ async def test_proxy_headers_trusted_hosts( ), # should set first untrusted as remote address (["192.168.0.2", "127.0.0.1"], "Remote: https://10.0.2.1:0"), + # Mixed literals and networks + (["127.0.0.1", "10.0.0.0/8", "192.168.0.2"], "Remote: https://1.2.3.4:0"), ], ) async def test_proxy_headers_multiple_proxies( trusted_hosts: Union[List[str], str], response_text: str ) -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: headers = { "X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4, 10.0.2.1, 192.168.0.2", @@ -107,9 +105,7 @@ async def test_proxy_headers_multiple_proxies( @pytest.mark.anyio async def test_proxy_headers_invalid_x_forwarded_for() -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", @@ -158,9 +154,7 @@ async def test_proxy_headers_empty_x_forwarded_for() -> None: # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", From 271f7399cf94cfd4cc4f62b065f0e4f4228050b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 17:30:10 +0200 Subject: [PATCH 04/26] Re-run black with line-length 88 --- tests/middleware/test_proxy_headers.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 3158b4da2..fe6b0fca6 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -57,7 +57,9 @@ async def test_proxy_headers_trusted_hosts( trusted_hosts: Union[List[str], str], response_text: str ) -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: + async with httpx.AsyncClient( + app=app_with_middleware, base_url="http://testserver" + ) as client: headers = {"X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4"} response = await client.get("/", headers=headers) @@ -91,7 +93,9 @@ async def test_proxy_headers_multiple_proxies( trusted_hosts: Union[List[str], str], response_text: str ) -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: + async with httpx.AsyncClient( + app=app_with_middleware, base_url="http://testserver" + ) as client: headers = { "X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4, 10.0.2.1, 192.168.0.2", @@ -105,7 +109,9 @@ async def test_proxy_headers_multiple_proxies( @pytest.mark.anyio async def test_proxy_headers_invalid_x_forwarded_for() -> None: app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") - async with httpx.AsyncClient(app=app_with_middleware, base_url="http://testserver") as client: + async with httpx.AsyncClient( + app=app_with_middleware, base_url="http://testserver" + ) as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", @@ -154,7 +160,9 @@ async def test_proxy_headers_empty_x_forwarded_for() -> None: # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver" + ) as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", From bdd7977ed277b7b015ccd83767e94ca2bfe6dd74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 17:37:32 +0200 Subject: [PATCH 05/26] Wrap comments to fit in line-length --- uvicorn/middleware/proxy_headers.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index e546486c1..4bacae8e2 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -36,8 +36,9 @@ def __init__(self, trusted_hosts: Union[List[str], str]) -> None: trusted_hosts = _parse_raw_hosts(trusted_hosts) for host in trusted_hosts: try: - # Try parsing the trusted host as an IPv4Network to allow checking a whole range. - # See https://github.com/encode/uvicorn/issues/1068#issuecomment-1004813267 + # Try parsing the trusted host as an IPv4Network + # to allow checking a whole range. + # https://github.com/encode/uvicorn/issues/1068 self.trusted_networks.add(ipaddress.IPv4Network(host)) except ValueError: self.trusted_literals.add(host) @@ -107,8 +108,9 @@ async def __call__( x_forwarded_for = headers[b"x-forwarded-for"].decode("latin1") host = self.trusted_hosts.get_trusted_client_host(x_forwarded_for) - # Host is None or an empty string if the x-forwarded-for header is empty. - # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 + # Host is None or an empty string + # if the x-forwarded-for header is empty. + # See https://github.com/encode/uvicorn/issues/1068 if host: port = 0 scope["client"] = (host, port) # type: ignore[arg-type] From 064341df04efed097cb5a2f4906d512dff172600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 17:39:57 +0200 Subject: [PATCH 06/26] Another missed line-length violation --- tests/middleware/test_proxy_headers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index fe6b0fca6..a39d151f8 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -156,7 +156,7 @@ async def websocket_app(scope, receive, send): @pytest.mark.anyio async def test_proxy_headers_empty_x_forwarded_for() -> None: - # fallback to the default behavior if x-forwarded-for is an empty list for some reason + # fallback to the default behavior if x-forwarded-for is an empty list # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) From 97bfc18153b497e8d0b8198bb8b424eacf32ab12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20D=C3=BCggelin?= Date: Mon, 22 Aug 2022 17:46:00 +0200 Subject: [PATCH 07/26] Fix __contains__ annotations --- uvicorn/middleware/proxy_headers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 4bacae8e2..7376c2368 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -43,7 +43,7 @@ def __init__(self, trusted_hosts: Union[List[str], str]) -> None: except ValueError: self.trusted_literals.add(host) - def __contains__(self, item: str): + def __contains__(self, item: Optional[str]) -> bool: if self.always_trust: return True @@ -66,6 +66,7 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 if host in self: return x_forwarded_for_hosts[0] + return host class ProxyHeadersMiddleware: From e6280c9b6e353941551a89a93226325dc19d8295 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 16:53:07 +1100 Subject: [PATCH 08/26] Add support for IPv6 --- tests/middleware/test_proxy_headers.py | 282 ++++++++++++++++++++++++- uvicorn/middleware/proxy_headers.py | 98 ++++++--- 2 files changed, 354 insertions(+), 26 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index a39d151f8..bdff5cb2b 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -8,7 +8,7 @@ from tests.utils import run_server from uvicorn._types import ASGIReceiveCallable, ASGISendCallable, Scope from uvicorn.config import Config -from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware +from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware, _TrustedHosts if TYPE_CHECKING: from uvicorn.protocols.http.h11_impl import H11Protocol @@ -29,6 +29,286 @@ async def app( await response(scope, receive, send) +# Note: we vary the format here to also test some of the functionality +# of the _TrustedHosts.__init__ method. +_TRUSTED_NOTHING = [] +_TRUSTED_EVERYTHING = "*" +_TRUSTED_IPv4_ADDRESSES = "127.0.0.1, 10.0.0.1" +_TRUSTED_IPv4_NETWORKS = ["127.0.0.0/8", "10.0.0.0/8"] +_TRUSTED_IPv6_ADDRESSES = [ + "2001:db8::", + "2001:0db8:0001:0000:0000:0ab9:C0A8:0102", + "2001:db8:3333:4444:5555:6666:1.2.3.4", # This is a dual address + "::11.22.33.44", # This is a dual address +] +_TRUSTED_IPv6_NETWORKS = "2001:db8:abcd:0012::0/64" +_TRUSTED_LITERALS = "some-literal , unix:///foo/bar , /foo/bar" + + +@pytest.mark.parametrize( + ("init_hosts", "test_host", "expected"), + [ + ## Never Trust trust + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_EVERYTHING, "127.0.0.0", False), + (_TRUSTED_EVERYTHING, "127.0.0.1", False), + (_TRUSTED_EVERYTHING, "127.1.1.1", False), + (_TRUSTED_EVERYTHING, "127.255.255.255", False), + (_TRUSTED_EVERYTHING, "10.0.0.0", False), + (_TRUSTED_EVERYTHING, "10.0.0.1", False), + (_TRUSTED_EVERYTHING, "10.1.1.1", False), + (_TRUSTED_EVERYTHING, "10.255.255.255", False), + (_TRUSTED_EVERYTHING, "192.168.0.0", False), + (_TRUSTED_EVERYTHING, "192.168.0.1", False), + (_TRUSTED_EVERYTHING, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_EVERYTHING, "2001:db8::", False), + (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::0", False), + (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_EVERYTHING, "::", False), + (_TRUSTED_EVERYTHING, "::1", False), + ( + _TRUSTED_EVERYTHING, + "2001:db8:3333:4444:5555:6666:102:304", + False, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_EVERYTHING, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_EVERYTHING, "a:b:c:d::", False), + (_TRUSTED_EVERYTHING, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_EVERYTHING, "some-literal", False), + (_TRUSTED_EVERYTHING, "unix::///foo/bar", False), + (_TRUSTED_EVERYTHING, "/foo/bar", False), + (_TRUSTED_EVERYTHING, "*", False), + (_TRUSTED_EVERYTHING, "another-literal", False), + (_TRUSTED_EVERYTHING, "unix:///another/path", False), + (_TRUSTED_EVERYTHING, "/another/path", False), + ## Always trust + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_EVERYTHING, "127.0.0.0", True), + (_TRUSTED_EVERYTHING, "127.0.0.1", True), + (_TRUSTED_EVERYTHING, "127.1.1.1", True), + (_TRUSTED_EVERYTHING, "127.255.255.255", True), + (_TRUSTED_EVERYTHING, "10.0.0.0", True), + (_TRUSTED_EVERYTHING, "10.0.0.1", True), + (_TRUSTED_EVERYTHING, "10.1.1.1", True), + (_TRUSTED_EVERYTHING, "10.255.255.255", True), + (_TRUSTED_EVERYTHING, "192.168.0.0", True), + (_TRUSTED_EVERYTHING, "192.168.0.1", True), + (_TRUSTED_EVERYTHING, "1.1.1.1", True), + # Test IPv6 Addresses + (_TRUSTED_EVERYTHING, "2001:db8::", True), + (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::0", True), + (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::1:1", True), + (_TRUSTED_EVERYTHING, "::", True), + (_TRUSTED_EVERYTHING, "::1", True), + ( + _TRUSTED_EVERYTHING, + "2001:db8:3333:4444:5555:6666:102:304", + True, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_EVERYTHING, "::b16:212c", True), # aka ::11.22.33.44 + (_TRUSTED_EVERYTHING, "a:b:c:d::", True), + (_TRUSTED_EVERYTHING, "::a:b:c:d", True), + # Test Literals + (_TRUSTED_EVERYTHING, "some-literal", True), + (_TRUSTED_EVERYTHING, "unix::///foo/bar", True), + (_TRUSTED_EVERYTHING, "/foo/bar", True), + (_TRUSTED_EVERYTHING, "*", True), + (_TRUSTED_EVERYTHING, "another-literal", True), + (_TRUSTED_EVERYTHING, "unix:///another/path", True), + (_TRUSTED_EVERYTHING, "/another/path", True), + ## Trust IPv4 Addresses + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_IPv4_ADDRESSES, "127.0.0.0", False), + (_TRUSTED_IPv4_ADDRESSES, "127.0.0.1", True), + (_TRUSTED_IPv4_ADDRESSES, "127.1.1.1", False), + (_TRUSTED_IPv4_ADDRESSES, "127.255.255.255", False), + (_TRUSTED_IPv4_ADDRESSES, "10.0.0.0", False), + (_TRUSTED_IPv4_ADDRESSES, "10.0.0.1", True), + (_TRUSTED_IPv4_ADDRESSES, "10.1.1.1", False), + (_TRUSTED_IPv4_ADDRESSES, "10.255.255.255", False), + (_TRUSTED_IPv4_ADDRESSES, "192.168.0.0", False), + (_TRUSTED_IPv4_ADDRESSES, "192.168.0.1", False), + (_TRUSTED_IPv4_ADDRESSES, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_IPv4_ADDRESSES, "2001:db8::", False), + (_TRUSTED_IPv4_ADDRESSES, "2001:db8:abcd:0012::0", False), + (_TRUSTED_IPv4_ADDRESSES, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_IPv4_ADDRESSES, "::", False), + (_TRUSTED_IPv4_ADDRESSES, "::1", False), + ( + _TRUSTED_IPv4_ADDRESSES, + "2001:db8:3333:4444:5555:6666:102:304", + False, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_IPv4_ADDRESSES, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_IPv4_ADDRESSES, "a:b:c:d::", False), + (_TRUSTED_IPv4_ADDRESSES, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_IPv4_ADDRESSES, "some-literal", False), + (_TRUSTED_IPv4_ADDRESSES, "unix::///foo/bar", False), + (_TRUSTED_IPv4_ADDRESSES, "*", False), + (_TRUSTED_IPv4_ADDRESSES, "/foo/bar", False), + (_TRUSTED_IPv4_ADDRESSES, "another-literal", False), + (_TRUSTED_IPv4_ADDRESSES, "unix:///another/path", False), + (_TRUSTED_IPv4_ADDRESSES, "/another/path", False), + ## Trust IPv6 Addresses + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_IPv6_ADDRESSES, "127.0.0.0", False), + (_TRUSTED_IPv6_ADDRESSES, "127.0.0.1", False), + (_TRUSTED_IPv6_ADDRESSES, "127.1.1.1", False), + (_TRUSTED_IPv6_ADDRESSES, "127.255.255.255", False), + (_TRUSTED_IPv6_ADDRESSES, "10.0.0.0", False), + (_TRUSTED_IPv6_ADDRESSES, "10.0.0.1", False), + (_TRUSTED_IPv6_ADDRESSES, "10.1.1.1", False), + (_TRUSTED_IPv6_ADDRESSES, "10.255.255.255", False), + (_TRUSTED_IPv6_ADDRESSES, "192.168.0.0", False), + (_TRUSTED_IPv6_ADDRESSES, "192.168.0.1", False), + (_TRUSTED_IPv6_ADDRESSES, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_IPv6_ADDRESSES, "2001:db8::", True), + (_TRUSTED_IPv6_ADDRESSES, "2001:db8:abcd:0012::0", False), + (_TRUSTED_IPv6_ADDRESSES, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_IPv6_ADDRESSES, "::", False), + (_TRUSTED_IPv6_ADDRESSES, "::1", False), + ( + _TRUSTED_IPv6_ADDRESSES, + "2001:db8:3333:4444:5555:6666:102:304", + True, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_IPv6_ADDRESSES, "::b16:212c", True), # aka ::11.22.33.44 + (_TRUSTED_IPv6_ADDRESSES, "a:b:c:d::", False), + (_TRUSTED_IPv6_ADDRESSES, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_IPv6_ADDRESSES, "some-literal", False), + (_TRUSTED_IPv6_ADDRESSES, "unix::///foo/bar", False), + (_TRUSTED_IPv6_ADDRESSES, "*", False), + (_TRUSTED_IPv6_ADDRESSES, "/foo/bar", False), + (_TRUSTED_IPv6_ADDRESSES, "another-literal", False), + (_TRUSTED_IPv6_ADDRESSES, "unix:///another/path", False), + (_TRUSTED_IPv6_ADDRESSES, "/another/path", False), + ## Trust IPv4 Networks + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_IPv4_NETWORKS, "127.0.0.0", True), + (_TRUSTED_IPv4_NETWORKS, "127.0.0.1", True), + (_TRUSTED_IPv4_NETWORKS, "127.1.1.1", True), + (_TRUSTED_IPv4_NETWORKS, "127.255.255.255", True), + (_TRUSTED_IPv4_NETWORKS, "10.0.0.0", True), + (_TRUSTED_IPv4_NETWORKS, "10.0.0.1", True), + (_TRUSTED_IPv4_NETWORKS, "10.1.1.1", True), + (_TRUSTED_IPv4_NETWORKS, "10.255.255.255", True), + (_TRUSTED_IPv4_NETWORKS, "192.168.0.0", False), + (_TRUSTED_IPv4_NETWORKS, "192.168.0.1", False), + (_TRUSTED_IPv4_NETWORKS, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_IPv4_NETWORKS, "2001:db8::", False), + (_TRUSTED_IPv4_NETWORKS, "2001:db8:abcd:0012::0", False), + (_TRUSTED_IPv4_NETWORKS, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_IPv4_NETWORKS, "::", False), + (_TRUSTED_IPv4_NETWORKS, "::1", False), + ( + _TRUSTED_IPv4_NETWORKS, + "2001:db8:3333:4444:5555:6666:102:304", + False, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_IPv4_NETWORKS, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_IPv4_NETWORKS, "a:b:c:d::", False), + (_TRUSTED_IPv4_NETWORKS, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_IPv4_NETWORKS, "some-literal", False), + (_TRUSTED_IPv4_NETWORKS, "unix::///foo/bar", False), + (_TRUSTED_IPv4_NETWORKS, "*", False), + (_TRUSTED_IPv4_NETWORKS, "/foo/bar", False), + (_TRUSTED_IPv4_NETWORKS, "another-literal", False), + (_TRUSTED_IPv4_NETWORKS, "unix:///another/path", False), + (_TRUSTED_IPv4_NETWORKS, "/another/path", False), + ## Trust IPv6 Networks + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_IPv6_NETWORKS, "127.0.0.0", False), + (_TRUSTED_IPv6_NETWORKS, "127.0.0.1", False), + (_TRUSTED_IPv6_NETWORKS, "127.1.1.1", False), + (_TRUSTED_IPv6_NETWORKS, "127.255.255.255", False), + (_TRUSTED_IPv6_NETWORKS, "10.0.0.0", False), + (_TRUSTED_IPv6_NETWORKS, "10.0.0.1", False), + (_TRUSTED_IPv6_NETWORKS, "10.1.1.1", False), + (_TRUSTED_IPv6_NETWORKS, "10.255.255.255", False), + (_TRUSTED_IPv6_NETWORKS, "192.168.0.0", False), + (_TRUSTED_IPv6_NETWORKS, "192.168.0.1", False), + (_TRUSTED_IPv6_NETWORKS, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_IPv6_NETWORKS, "2001:db8::", False), + (_TRUSTED_IPv6_NETWORKS, "2001:db8:abcd:0012::0", True), + (_TRUSTED_IPv6_NETWORKS, "2001:db8:abcd:0012::1:1", True), + (_TRUSTED_IPv6_NETWORKS, "::", False), + (_TRUSTED_IPv6_NETWORKS, "::1", False), + ( + _TRUSTED_IPv6_NETWORKS, + "2001:db8:3333:4444:5555:6666:102:304", + False, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_IPv6_NETWORKS, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_IPv6_NETWORKS, "a:b:c:d::", False), + (_TRUSTED_IPv6_NETWORKS, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_IPv6_NETWORKS, "some-literal", False), + (_TRUSTED_IPv6_NETWORKS, "unix::///foo/bar", False), + (_TRUSTED_IPv6_NETWORKS, "*", False), + (_TRUSTED_IPv6_NETWORKS, "/foo/bar", False), + (_TRUSTED_IPv6_NETWORKS, "another-literal", False), + (_TRUSTED_IPv6_NETWORKS, "unix:///another/path", False), + (_TRUSTED_IPv6_NETWORKS, "/another/path", False), + ## Trust Literals + ## ----------------------------- + # Test IPv4 Addresses + (_TRUSTED_LITERALS, "127.0.0.0", False), + (_TRUSTED_LITERALS, "127.0.0.1", False), + (_TRUSTED_LITERALS, "127.1.1.1", False), + (_TRUSTED_LITERALS, "127.255.255.255", False), + (_TRUSTED_LITERALS, "10.0.0.0", False), + (_TRUSTED_LITERALS, "10.0.0.1", False), + (_TRUSTED_LITERALS, "10.1.1.1", False), + (_TRUSTED_LITERALS, "10.255.255.255", False), + (_TRUSTED_LITERALS, "192.168.0.0", False), + (_TRUSTED_LITERALS, "192.168.0.1", False), + (_TRUSTED_LITERALS, "1.1.1.1", False), + # Test IPv6 Addresses + (_TRUSTED_LITERALS, "2001:db8::", False), + (_TRUSTED_LITERALS, "2001:db8:abcd:0012::0", False), + (_TRUSTED_LITERALS, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_LITERALS, "::", False), + (_TRUSTED_LITERALS, "::1", False), + ( + _TRUSTED_LITERALS, + "2001:db8:3333:4444:5555:6666:102:304", + False, + ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 + (_TRUSTED_LITERALS, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_LITERALS, "a:b:c:d::", False), + (_TRUSTED_LITERALS, "::a:b:c:d", False), + # Test Literals + (_TRUSTED_LITERALS, "some-literal", True), + (_TRUSTED_LITERALS, "unix::///foo/bar", True), + (_TRUSTED_LITERALS, "*", False), + (_TRUSTED_LITERALS, "/foo/bar", True), + (_TRUSTED_LITERALS, "another-literal", False), + (_TRUSTED_LITERALS, "unix:///another/path", False), + (_TRUSTED_LITERALS, "/another/path", False), + ], +) +def test_forwarded_hosts( + init_hosts: Union[str, List[str]], test_host: str, expected: bool +) -> None: + trusted_hosts = _TrustedHosts(init_hosts) + assert test_host in trusted_hosts is expected + + @pytest.mark.anyio @pytest.mark.parametrize( ("trusted_hosts", "response_text"), diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 7376c2368..d41655ed1 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -1,15 +1,5 @@ -""" -This middleware can be used when a known proxy is fronting the application, -and is trusted to be properly setting the `X-Forwarded-Proto` and -`X-Forwarded-For` headers with the connecting client information. - -Modifies the `client` and `scheme` information so that they reference -the connecting client, rather that the connecting proxy. - -https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Proxies -""" import ipaddress -from typing import List, Optional, Tuple, Union, Set, cast +from typing import List, Optional, Set, Tuple, Union, cast from uvicorn._types import ( ASGI3Application, @@ -26,50 +16,108 @@ def _parse_raw_hosts(value: str) -> List[str]: class _TrustedHosts: + """Container for trusted hosts and networks""" + def __init__(self, trusted_hosts: Union[List[str], str]) -> None: - self.trusted_networks: Set[ipaddress.IPv4Network] = set() + self.always_trust: bool = trusted_hosts == "*" + self.trusted_literals: Set[str] = set() - self.always_trust = trusted_hosts == "*" + self.trusted_hosts: Set[ipaddress._BaseAddress] = set() + self.trusted_networks: Set[ipaddress._BaseNetwork] = set() + + # Notes: + # - We seperate hosts from literals as there are many ways to write + # an IPv6 Address so we need to compare by object. + # - We don't convert IP Address to single host networks (e.g. /32 / 128) as + # it more efficient to do an address lookup in a set than check for + # membership in each network. + # - We still allow literals as it might be possible that we receive a + # something that isn't an IP Address e.g. a unix socket. if not self.always_trust: if isinstance(trusted_hosts, str): trusted_hosts = _parse_raw_hosts(trusted_hosts) + for host in trusted_hosts: - try: - # Try parsing the trusted host as an IPv4Network - # to allow checking a whole range. - # https://github.com/encode/uvicorn/issues/1068 - self.trusted_networks.add(ipaddress.IPv4Network(host)) - except ValueError: - self.trusted_literals.add(host) + # Note: because we always convert invalid IP types to literals it + # is not possible for the user to know they provided a malformed IP + # type - this may lead to unexpected / difficult to debug behaviour. + + if "/" in host: + # Looks like a network + try: + self.trusted_networks.add(ipaddress.ip_network(host)) + except ValueError: + # Was not a valid IP Network + self.trusted_literals.add(host) + else: + try: + self.trusted_hosts.add(ipaddress.ip_address(host)) + except ValueError: + # Was not a valid IP Adress + self.trusted_literals.add(host) + return def __contains__(self, item: Optional[str]) -> bool: if self.always_trust: return True + if not item: + return False + try: - ip = ipaddress.IPv4Address(item) + ip = ipaddress.ip_address(item) + if ip in self.trusted_hosts: + return True return any(ip in net for net in self.trusted_networks) + except ValueError: return item in self.trusted_literals def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: + """Extract the client host from x_forwarded_for header + + In general this is the first "untrusted" host in the forwarded for list. + """ x_forwarded_for_hosts = _parse_raw_hosts(x_forwarded_for) + if self.always_trust: return x_forwarded_for_hosts[0] - host = None + host: Optional[str] = None + + # Note: each proxy appends to the list so check it in reverse order for host in reversed(x_forwarded_for_hosts): if host not in self: return host + # The request came from a client on the proxy itself. Trust it. # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 if host in self: return x_forwarded_for_hosts[0] + return host class ProxyHeadersMiddleware: + """Middleware for handling known proxy headers + + This middleware can be used when a known proxy is fronting the application, + and is trusted to be properly setting the `X-Forwarded-Proto` and + `X-Forwarded-For` headers with the connecting client information. + + Modifies the `client` and `scheme` information so that they reference + the connecting client, rather that the connecting proxy. + + References: + + - + - + """ + + # TODO: We should probably also support the Forwarded header: + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded + def __init__( self, app: "ASGI3Application", @@ -77,16 +125,16 @@ def __init__( ) -> None: self.app = app self.trusted_hosts = _TrustedHosts(trusted_hosts) + return async def __call__( self, scope: "Scope", receive: "ASGIReceiveCallable", send: "ASGISendCallable" ) -> None: if scope["type"] in ("http", "websocket"): - scope = cast(Union["HTTPScope", "WebSocketScope"], scope) + scope = cast(Union[HTTPScope, WebSocketScope], scope) client_addr: Optional[Tuple[str, int]] = scope.get("client") - client_host = client_addr[0] if client_addr else None - if client_host in self.trusted_hosts: + if client_addr and client_addr[0] in self.trusted_hosts: headers = dict(scope["headers"]) if b"x-forwarded-proto" in headers: From 61b93d0a416a2942e40893f7bae458bb86d9c4b9 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 17:19:45 +1100 Subject: [PATCH 09/26] Fix linting --- tests/middleware/test_proxy_headers.py | 2 +- uvicorn/middleware/proxy_headers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index bdff5cb2b..141596635 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -31,7 +31,7 @@ async def app( # Note: we vary the format here to also test some of the functionality # of the _TrustedHosts.__init__ method. -_TRUSTED_NOTHING = [] +_TRUSTED_NOTHING: List[str] = [] _TRUSTED_EVERYTHING = "*" _TRUSTED_IPv4_ADDRESSES = "127.0.0.1, 10.0.0.1" _TRUSTED_IPv4_NETWORKS = ["127.0.0.0/8", "10.0.0.0/8"] diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index d41655ed1..b85d8c09c 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -162,6 +162,6 @@ async def __call__( # See https://github.com/encode/uvicorn/issues/1068 if host: port = 0 - scope["client"] = (host, port) # type: ignore[arg-type] + scope["client"] = (host, port) return await self.app(scope, receive, send) From c5dc8741d859201af9ee635d10b9a5e3e9b1d006 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 17:47:29 +1100 Subject: [PATCH 10/26] Fix test cases --- tests/middleware/test_proxy_headers.py | 74 ++++++++++++++------------ 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 141596635..f742ce1ff 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -51,39 +51,40 @@ async def app( ## Never Trust trust ## ----------------------------- # Test IPv4 Addresses - (_TRUSTED_EVERYTHING, "127.0.0.0", False), - (_TRUSTED_EVERYTHING, "127.0.0.1", False), - (_TRUSTED_EVERYTHING, "127.1.1.1", False), - (_TRUSTED_EVERYTHING, "127.255.255.255", False), - (_TRUSTED_EVERYTHING, "10.0.0.0", False), - (_TRUSTED_EVERYTHING, "10.0.0.1", False), - (_TRUSTED_EVERYTHING, "10.1.1.1", False), - (_TRUSTED_EVERYTHING, "10.255.255.255", False), - (_TRUSTED_EVERYTHING, "192.168.0.0", False), - (_TRUSTED_EVERYTHING, "192.168.0.1", False), - (_TRUSTED_EVERYTHING, "1.1.1.1", False), + (_TRUSTED_NOTHING, "127.0.0.0", False), + (_TRUSTED_NOTHING, "127.0.0.1", False), + (_TRUSTED_NOTHING, "127.1.1.1", False), + (_TRUSTED_NOTHING, "127.255.255.255", False), + (_TRUSTED_NOTHING, "10.0.0.0", False), + (_TRUSTED_NOTHING, "10.0.0.1", False), + (_TRUSTED_NOTHING, "10.1.1.1", False), + (_TRUSTED_NOTHING, "10.255.255.255", False), + (_TRUSTED_NOTHING, "192.168.0.0", False), + (_TRUSTED_NOTHING, "192.168.0.1", False), + (_TRUSTED_NOTHING, "1.1.1.1", False), # Test IPv6 Addresses - (_TRUSTED_EVERYTHING, "2001:db8::", False), - (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::0", False), - (_TRUSTED_EVERYTHING, "2001:db8:abcd:0012::1:1", False), - (_TRUSTED_EVERYTHING, "::", False), - (_TRUSTED_EVERYTHING, "::1", False), + (_TRUSTED_NOTHING, "2001:db8::", False), + (_TRUSTED_NOTHING, "2001:db8:abcd:0012::0", False), + (_TRUSTED_NOTHING, "2001:db8:abcd:0012::1:1", False), + (_TRUSTED_NOTHING, "::", False), + (_TRUSTED_NOTHING, "::1", False), ( - _TRUSTED_EVERYTHING, + _TRUSTED_NOTHING, "2001:db8:3333:4444:5555:6666:102:304", False, ), # aka 2001:db8:3333:4444:5555:6666:1.2.3.4 - (_TRUSTED_EVERYTHING, "::b16:212c", False), # aka ::11.22.33.44 - (_TRUSTED_EVERYTHING, "a:b:c:d::", False), - (_TRUSTED_EVERYTHING, "::a:b:c:d", False), + (_TRUSTED_NOTHING, "::b16:212c", False), # aka ::11.22.33.44 + (_TRUSTED_NOTHING, "a:b:c:d::", False), + (_TRUSTED_NOTHING, "::a:b:c:d", False), # Test Literals - (_TRUSTED_EVERYTHING, "some-literal", False), - (_TRUSTED_EVERYTHING, "unix::///foo/bar", False), - (_TRUSTED_EVERYTHING, "/foo/bar", False), - (_TRUSTED_EVERYTHING, "*", False), - (_TRUSTED_EVERYTHING, "another-literal", False), - (_TRUSTED_EVERYTHING, "unix:///another/path", False), - (_TRUSTED_EVERYTHING, "/another/path", False), + (_TRUSTED_NOTHING, "some-literal", False), + (_TRUSTED_NOTHING, "unix:///foo/bar", False), + (_TRUSTED_NOTHING, "/foo/bar", False), + (_TRUSTED_NOTHING, "*", False), + (_TRUSTED_NOTHING, "another-literal", False), + (_TRUSTED_NOTHING, "unix:///another/path", False), + (_TRUSTED_NOTHING, "/another/path", False), + ## Always trust ## ----------------------------- # Test IPv4 Addresses @@ -114,12 +115,13 @@ async def app( (_TRUSTED_EVERYTHING, "::a:b:c:d", True), # Test Literals (_TRUSTED_EVERYTHING, "some-literal", True), - (_TRUSTED_EVERYTHING, "unix::///foo/bar", True), + (_TRUSTED_EVERYTHING, "unix:///foo/bar", True), (_TRUSTED_EVERYTHING, "/foo/bar", True), (_TRUSTED_EVERYTHING, "*", True), (_TRUSTED_EVERYTHING, "another-literal", True), (_TRUSTED_EVERYTHING, "unix:///another/path", True), (_TRUSTED_EVERYTHING, "/another/path", True), + ## Trust IPv4 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -150,12 +152,13 @@ async def app( (_TRUSTED_IPv4_ADDRESSES, "::a:b:c:d", False), # Test Literals (_TRUSTED_IPv4_ADDRESSES, "some-literal", False), - (_TRUSTED_IPv4_ADDRESSES, "unix::///foo/bar", False), + (_TRUSTED_IPv4_ADDRESSES, "unix:///foo/bar", False), (_TRUSTED_IPv4_ADDRESSES, "*", False), (_TRUSTED_IPv4_ADDRESSES, "/foo/bar", False), (_TRUSTED_IPv4_ADDRESSES, "another-literal", False), (_TRUSTED_IPv4_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv4_ADDRESSES, "/another/path", False), + ## Trust IPv6 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -186,12 +189,13 @@ async def app( (_TRUSTED_IPv6_ADDRESSES, "::a:b:c:d", False), # Test Literals (_TRUSTED_IPv6_ADDRESSES, "some-literal", False), - (_TRUSTED_IPv6_ADDRESSES, "unix::///foo/bar", False), + (_TRUSTED_IPv6_ADDRESSES, "unix:///foo/bar", False), (_TRUSTED_IPv6_ADDRESSES, "*", False), (_TRUSTED_IPv6_ADDRESSES, "/foo/bar", False), (_TRUSTED_IPv6_ADDRESSES, "another-literal", False), (_TRUSTED_IPv6_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv6_ADDRESSES, "/another/path", False), + ## Trust IPv4 Networks ## ----------------------------- # Test IPv4 Addresses @@ -222,12 +226,13 @@ async def app( (_TRUSTED_IPv4_NETWORKS, "::a:b:c:d", False), # Test Literals (_TRUSTED_IPv4_NETWORKS, "some-literal", False), - (_TRUSTED_IPv4_NETWORKS, "unix::///foo/bar", False), + (_TRUSTED_IPv4_NETWORKS, "unix:///foo/bar", False), (_TRUSTED_IPv4_NETWORKS, "*", False), (_TRUSTED_IPv4_NETWORKS, "/foo/bar", False), (_TRUSTED_IPv4_NETWORKS, "another-literal", False), (_TRUSTED_IPv4_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv4_NETWORKS, "/another/path", False), + ## Trust IPv6 Networks ## ----------------------------- # Test IPv4 Addresses @@ -258,12 +263,13 @@ async def app( (_TRUSTED_IPv6_NETWORKS, "::a:b:c:d", False), # Test Literals (_TRUSTED_IPv6_NETWORKS, "some-literal", False), - (_TRUSTED_IPv6_NETWORKS, "unix::///foo/bar", False), + (_TRUSTED_IPv6_NETWORKS, "unix:///foo/bar", False), (_TRUSTED_IPv6_NETWORKS, "*", False), (_TRUSTED_IPv6_NETWORKS, "/foo/bar", False), (_TRUSTED_IPv6_NETWORKS, "another-literal", False), (_TRUSTED_IPv6_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv6_NETWORKS, "/another/path", False), + ## Trust Literals ## ----------------------------- # Test IPv4 Addresses @@ -294,7 +300,7 @@ async def app( (_TRUSTED_LITERALS, "::a:b:c:d", False), # Test Literals (_TRUSTED_LITERALS, "some-literal", True), - (_TRUSTED_LITERALS, "unix::///foo/bar", True), + (_TRUSTED_LITERALS, "unix:///foo/bar", True), (_TRUSTED_LITERALS, "*", False), (_TRUSTED_LITERALS, "/foo/bar", True), (_TRUSTED_LITERALS, "another-literal", False), @@ -306,7 +312,7 @@ def test_forwarded_hosts( init_hosts: Union[str, List[str]], test_host: str, expected: bool ) -> None: trusted_hosts = _TrustedHosts(init_hosts) - assert test_host in trusted_hosts is expected + assert (test_host in trusted_hosts) is expected @pytest.mark.anyio From 4e6949421e5c5ac982b6496e178784407a6df115 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 18:24:29 +1100 Subject: [PATCH 11/26] Update docs, comments --- docs/deployment.md | 2 +- docs/settings.md | 2 +- tests/middleware/test_proxy_headers.py | 6 ---- uvicorn/main.py | 6 ++-- uvicorn/middleware/proxy_headers.py | 41 +++++++++++++++----------- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 58927d95e..166461c05 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -255,7 +255,7 @@ Using Nginx as a proxy in front of your Uvicorn processes may not be necessary, In managed environments such as `Heroku`, you won't typically need to configure Nginx, as your server processes will already be running behind load balancing proxies. The recommended configuration for proxying from Nginx is to use a UNIX domain socket between Nginx and whatever the process manager that is being used to run Uvicorn. -Note that when doing this you will need to run Uvicorn with `--forwarded-allow-ips='*'` to ensure that the domain socket is trusted as a source from which to proxy headers. +Note that when doing this you will need to run Uvicorn with `--forwarded-allow-ips='/path/to/socket'` to ensure that the domain socket is trusted as a source from which to proxy headers. When fronting the application with a proxy server you want to make sure that the proxy sets headers to ensure that the application can properly determine the client address of the incoming connection, and if the connection was over `http` or `https`. diff --git a/docs/settings.md b/docs/settings.md index 9c62460fe..77ae771e7 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -90,7 +90,7 @@ Note that WSGI mode always disables WebSocket support, as it is not supported by * `--root-path ` - Set the ASGI `root_path` for applications submounted below a given URL path. * `--proxy-headers` / `--no-proxy-headers` - Enable/Disable X-Forwarded-Proto, X-Forwarded-For, X-Forwarded-Port to populate remote address info. Defaults to enabled, but is restricted to only trusting connecting IPs in the `forwarded-allow-ips` configuration. -* `--forwarded-allow-ips` Comma separated list of IPs to trust with proxy headers. Defaults to the `$FORWARDED_ALLOW_IPS` environment variable if available, or '127.0.0.1'. A wildcard '*' means always trust. +* `--forwarded-allow-ips` Comma separated list of IP Addresses, IP Networks, or literals (e.g. UNIX Socket path) to trust with proxy headers. Defaults to the `$FORWARDED_ALLOW_IPS` environment variable if available, or '127.0.0.1'. The literal `'*'` means trust everything. * `--server-header` / `--no-server-header` - Enable/Disable default `Server` header. * `--date-header` / `--no-date-header` - Enable/Disable default `Date` header. diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index f742ce1ff..0c0df8bed 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -84,7 +84,6 @@ async def app( (_TRUSTED_NOTHING, "another-literal", False), (_TRUSTED_NOTHING, "unix:///another/path", False), (_TRUSTED_NOTHING, "/another/path", False), - ## Always trust ## ----------------------------- # Test IPv4 Addresses @@ -121,7 +120,6 @@ async def app( (_TRUSTED_EVERYTHING, "another-literal", True), (_TRUSTED_EVERYTHING, "unix:///another/path", True), (_TRUSTED_EVERYTHING, "/another/path", True), - ## Trust IPv4 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -158,7 +156,6 @@ async def app( (_TRUSTED_IPv4_ADDRESSES, "another-literal", False), (_TRUSTED_IPv4_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv4_ADDRESSES, "/another/path", False), - ## Trust IPv6 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -195,7 +192,6 @@ async def app( (_TRUSTED_IPv6_ADDRESSES, "another-literal", False), (_TRUSTED_IPv6_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv6_ADDRESSES, "/another/path", False), - ## Trust IPv4 Networks ## ----------------------------- # Test IPv4 Addresses @@ -232,7 +228,6 @@ async def app( (_TRUSTED_IPv4_NETWORKS, "another-literal", False), (_TRUSTED_IPv4_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv4_NETWORKS, "/another/path", False), - ## Trust IPv6 Networks ## ----------------------------- # Test IPv4 Addresses @@ -269,7 +264,6 @@ async def app( (_TRUSTED_IPv6_NETWORKS, "another-literal", False), (_TRUSTED_IPv6_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv6_NETWORKS, "/another/path", False), - ## Trust Literals ## ----------------------------- # Test IPv4 Addresses diff --git a/uvicorn/main.py b/uvicorn/main.py index fee6c5b4c..82a7cc43b 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -245,8 +245,10 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No "--forwarded-allow-ips", type=str, default=None, - help="Comma separated list of IPs to trust with proxy headers. Defaults to" - " the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'.", + help="Comma separated list of IP Addresses, IP Networks, or literals " + "(e.g. UNIX Socket path) to trust with proxy headers. Defaults to the " + "$FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. " + "The literal '*' means trust everything.", ) @click.option( "--root-path", diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index b85d8c09c..75705003b 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -81,22 +81,20 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: """ x_forwarded_for_hosts = _parse_raw_hosts(x_forwarded_for) + if not x_forwarded_for_hosts: + return None + if self.always_trust: return x_forwarded_for_hosts[0] - host: Optional[str] = None - - # Note: each proxy appends to the list so check it in reverse order + # Note: each proxy appends to the header list so check it in reverse order for host in reversed(x_forwarded_for_hosts): if host not in self: return host - # The request came from a client on the proxy itself. Trust it. + # All hosts are trusted meaning that the client was also a trusted proxy # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 - if host in self: - return x_forwarded_for_hosts[0] - - return host + return x_forwarded_for_hosts[0] class ProxyHeadersMiddleware: @@ -115,9 +113,6 @@ class ProxyHeadersMiddleware: - """ - # TODO: We should probably also support the Forwarded header: - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded - def __init__( self, app: "ASGI3Application", @@ -151,17 +146,29 @@ async def __call__( scope["scheme"] = x_forwarded_proto if b"x-forwarded-for" in headers: - # Determine the client address from the last trusted IP in the - # X-Forwarded-For header. We've lost the connecting client's port - # information by now, so only include the host. x_forwarded_for = headers[b"x-forwarded-for"].decode("latin1") host = self.trusted_hosts.get_trusted_client_host(x_forwarded_for) - # Host is None or an empty string - # if the x-forwarded-for header is empty. - # See https://github.com/encode/uvicorn/issues/1068 if host: + # If the x-forwarded-for header is empty then host is None or + # an empty string. + # Only set the client if we actually got something usable. + # See: https://github.com/encode/uvicorn/issues/1068 + + # Unless we can relaibly use x-forwarded-port (see below) then + # we will not have any port information so set it to 0. port = 0 scope["client"] = (host, port) + # if b"x-forwarded-port" in headers: + # ... + # TODO: Are we able to reliabily extract x-forwarded-port? + # https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-port + # If yes we should update the NGINX in docs/deployment.md + + # if b"forwarded" in headers: + # ... + # TODO: We should probably also support the Forwarded header: + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded + return await self.app(scope, receive, send) From 6f2810238cfdd717cd5acf8de689f5be650891a6 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 18:33:00 +1100 Subject: [PATCH 12/26] Update index's usage --- docs/index.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/index.md b/docs/index.md index 6ce92feb8..38a6892a5 100644 --- a/docs/index.md +++ b/docs/index.md @@ -163,10 +163,12 @@ Options: Enable/Disable default Server header. --date-header / --no-date-header Enable/Disable default Date header. - --forwarded-allow-ips TEXT Comma separated list of IPs to trust with - proxy headers. Defaults to the - $FORWARDED_ALLOW_IPS environment variable if - available, or '127.0.0.1'. + --forwarded-allow-ips TEXT Comma separated list of IP Addresses, IP + Networks, or literals (e.g. UNIX Socket + path) to trust with proxy headers. Defaults + to the $FORWARDED_ALLOW_IPS environment + variable if available, or '127.0.0.1'. The + literal '*' means trust everything. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or From e64ab818483839e516d45f81fea735ee23b0348c Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Thu, 25 Jan 2024 19:10:27 +1100 Subject: [PATCH 13/26] Fix more linting --- docs/deployment.md | 10 ++++++---- tests/middleware/test_proxy_headers.py | 7 +++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 166461c05..c4f2b8281 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -93,10 +93,12 @@ Options: Enable/Disable default Server header. --date-header / --no-date-header Enable/Disable default Date header. - --forwarded-allow-ips TEXT Comma separated list of IPs to trust with - proxy headers. Defaults to the - $FORWARDED_ALLOW_IPS environment variable if - available, or '127.0.0.1'. + --forwarded-allow-ips TEXT Comma separated list of IP Addresses, IP + Networks, or literals (e.g. UNIX Socket + path) to trust with proxy headers. Defaults + to the $FORWARDED_ALLOW_IPS environment + variable if available, or '127.0.0.1'. The + literal '*' means trust everything. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 0c0df8bed..b83461dd6 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -1,6 +1,7 @@ -from typing import TYPE_CHECKING, List, Type, Union +from typing import TYPE_CHECKING, List, Type, Union, cast import httpx +import httpx._transports.asgi import pytest import websockets.client @@ -438,7 +439,9 @@ async def websocket_app(scope, receive, send): async def test_proxy_headers_empty_x_forwarded_for() -> None: # fallback to the default behavior if x-forwarded-for is an empty list # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 - app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") + app_with_middleware = cast( + httpx._transports.asgi._ASGIApp, ProxyHeadersMiddleware(app, trusted_hosts="*") + ) transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) async with httpx.AsyncClient( transport=transport, base_url="http://testserver" From 819f72dbc4654b6ece328cc374127f7388873903 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Sun, 28 Jan 2024 20:59:29 +1100 Subject: [PATCH 14/26] Better unix socket support --- docs/deployment.md | 6 +++- docs/index.md | 4 +++ uvicorn/config.py | 8 ++++- uvicorn/main.py | 11 +++++++ uvicorn/middleware/proxy_headers.py | 47 ++++++++++++++++++++++++----- 5 files changed, 66 insertions(+), 10 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index c4f2b8281..a899e7fad 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -99,6 +99,10 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. + --forwarded-trust-literals Trust all literals in proxy headers. + Defaults to False. Useful when using + multiple proxies with UNIX Domain Sockets + for transport. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or @@ -257,7 +261,7 @@ Using Nginx as a proxy in front of your Uvicorn processes may not be necessary, In managed environments such as `Heroku`, you won't typically need to configure Nginx, as your server processes will already be running behind load balancing proxies. The recommended configuration for proxying from Nginx is to use a UNIX domain socket between Nginx and whatever the process manager that is being used to run Uvicorn. -Note that when doing this you will need to run Uvicorn with `--forwarded-allow-ips='/path/to/socket'` to ensure that the domain socket is trusted as a source from which to proxy headers. +Note that when doing this you will need to run Uvicorn with `--forwarded-allow-ips='unix:'` (or `--forwarded-trust-literals` to trust all literal "clients") to ensure that the domain socket is trusted as a source from which to proxy headers. When fronting the application with a proxy server you want to make sure that the proxy sets headers to ensure that the application can properly determine the client address of the incoming connection, and if the connection was over `http` or `https`. diff --git a/docs/index.md b/docs/index.md index 38a6892a5..ef54e3d62 100644 --- a/docs/index.md +++ b/docs/index.md @@ -169,6 +169,10 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. + --forwarded-trust-literals Trust all literals in proxy headers. + Defaults to False. Useful when using + multiple proxies with UNIX Domain Sockets + for transport. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or diff --git a/uvicorn/config.py b/uvicorn/config.py index b0dff4604..1ba52429e 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -208,6 +208,7 @@ def __init__( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, + forwarded_trust_literals: bool = False, root_path: str = "", limit_concurrency: int | None = None, limit_max_requests: int | None = None, @@ -349,6 +350,8 @@ def __init__( else: self.forwarded_allow_ips = forwarded_allow_ips + self.forwarded_trust_literals = forwarded_trust_literals + if self.reload and self.workers > 1: logger.warning('"workers" flag is ignored when reloading is enabled.') @@ -493,7 +496,10 @@ def load(self) -> None: self.loaded_app = MessageLoggerMiddleware(self.loaded_app) if self.proxy_headers: self.loaded_app = ProxyHeadersMiddleware( - self.loaded_app, trusted_hosts=self.forwarded_allow_ips + self.loaded_app, + trusted_hosts=self.forwarded_allow_ips, + trust_none_client=bool(self.uds), + trust_all_literal_clients=self.forwarded_trust_literals, ) self.loaded = True diff --git a/uvicorn/main.py b/uvicorn/main.py index 82a7cc43b..2eb688b56 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -250,6 +250,13 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No "$FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. " "The literal '*' means trust everything.", ) +@click.option( + "--forwarded-trust-literals", + is_flag=True, + default=False, + help="Trust all literals in proxy headers. Defaults to False. Useful when using " + "multiple proxies with UNIX Domain Sockets for transport.", +) @click.option( "--root-path", type=str, @@ -398,6 +405,7 @@ def main( server_header: bool, date_header: bool, forwarded_allow_ips: str, + forwarded_trust_literals: bool, root_path: str, limit_concurrency: int, backlog: int, @@ -447,6 +455,7 @@ def main( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, + forwarded_trust_literals=forwarded_trust_literals, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, @@ -499,6 +508,7 @@ def run( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, + forwarded_trust_literals: bool = False, root_path: str = "", limit_concurrency: int | None = None, backlog: int = 2048, @@ -551,6 +561,7 @@ def run( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, + forwarded_trust_literals=forwarded_trust_literals, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 75705003b..83c9bbc73 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -18,8 +18,13 @@ def _parse_raw_hosts(value: str) -> List[str]: class _TrustedHosts: """Container for trusted hosts and networks""" - def __init__(self, trusted_hosts: Union[List[str], str]) -> None: + def __init__( + self, + trusted_hosts: Union[List[str], str], + trust_all_literal_clients: bool = False, + ) -> None: self.always_trust: bool = trusted_hosts == "*" + self.trust_all_literal_clients = trust_all_literal_clients self.trusted_literals: Set[str] = set() self.trusted_hosts: Set[ipaddress._BaseAddress] = set() @@ -72,6 +77,8 @@ def __contains__(self, item: Optional[str]) -> bool: return any(ip in net for net in self.trusted_networks) except ValueError: + if self.trust_all_literal_clients: + return True return item in self.trusted_literals def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: @@ -96,6 +103,13 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 return x_forwarded_for_hosts[0] + def __str__(self) -> str: + return ( + f"{self.__class__.__name__}({self.trusted_hosts=}, " + f"{self.trusted_networks=}, {self.trusted_literals=}, " + f"{self.trust_all_literal_clients=}" + ) + class ProxyHeadersMiddleware: """Middleware for handling known proxy headers @@ -107,6 +121,16 @@ class ProxyHeadersMiddleware: Modifies the `client` and `scheme` information so that they reference the connecting client, rather that the connecting proxy. + Use `trust_none_client = True` to continue with proxy header handling when + the initial client is `None`. This does not affect the handling of client + adddresses in the proxy headers. If you are listening on a UNIX Domain Sockets + you will need to set this to `True` to enable proxy handling. + + Use `trust_all_literal_clients = True` to trust all non-IP clients in the header. + This is useful if you are using UNIX Domain Sockets to communicate between your + proxies and do not wish to list each literal or if you do not know the value of + each literal and startup. + References: - @@ -117,9 +141,13 @@ def __init__( self, app: "ASGI3Application", trusted_hosts: Union[List[str], str] = "127.0.0.1", + trust_none_client: bool = False, + trust_all_literal_clients: bool = False, ) -> None: self.app = app - self.trusted_hosts = _TrustedHosts(trusted_hosts) + self.trusted_hosts = _TrustedHosts(trusted_hosts, trust_all_literal_clients) + self.trust_none_client = trust_none_client + print(self.trusted_hosts) return async def __call__( @@ -129,9 +157,17 @@ async def __call__( scope = cast(Union[HTTPScope, WebSocketScope], scope) client_addr: Optional[Tuple[str, int]] = scope.get("client") - if client_addr and client_addr[0] in self.trusted_hosts: + if (client_addr is None and self.trust_none_client) or ( + client_addr is not None and client_addr[0] in self.trusted_hosts + ): headers = dict(scope["headers"]) + # if b"forwarded" in headers: + # ... + # TODO: We should probably also support the Forwarded header: + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded + # If we do it should take precedence over the x-forwarded-* headers. + if b"x-forwarded-proto" in headers: # Determine if the incoming request was http or https based on # the X-Forwarded-Proto header. @@ -166,9 +202,4 @@ async def __call__( # https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-port # If yes we should update the NGINX in docs/deployment.md - # if b"forwarded" in headers: - # ... - # TODO: We should probably also support the Forwarded header: - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded - return await self.app(scope, receive, send) From ccff4266866c92c55c0b9ce7d1737091073e6b75 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Mon, 29 Jan 2024 16:19:18 +1100 Subject: [PATCH 15/26] Expand proxy documentation --- docs/deployment.md | 46 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index a899e7fad..54898bec2 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -260,12 +260,9 @@ Using Nginx as a proxy in front of your Uvicorn processes may not be necessary, In managed environments such as `Heroku`, you won't typically need to configure Nginx, as your server processes will already be running behind load balancing proxies. -The recommended configuration for proxying from Nginx is to use a UNIX domain socket between Nginx and whatever the process manager that is being used to run Uvicorn. -Note that when doing this you will need to run Uvicorn with `--forwarded-allow-ips='unix:'` (or `--forwarded-trust-literals` to trust all literal "clients") to ensure that the domain socket is trusted as a source from which to proxy headers. +The recommended configuration for proxying from Nginx is to use a UNIX domain socket between Nginx and whatever the process manager that is being used to run Uvicorn. If using Uvicorn directly you can bind it to a UNIX domain socket using `uvicorn --uds /path/to/socket.sock <...>`. -When fronting the application with a proxy server you want to make sure that the proxy sets headers to ensure that the application can properly determine the client address of the incoming connection, and if the connection was over `http` or `https`. - -You should ensure that the `X-Forwarded-For` and `X-Forwarded-Proto` headers are set by the proxy, and that Uvicorn is run using the `--proxy-headers` setting. This ensures that the ASGI scope includes correct `client` and `scheme` information. +When running your application behind one or more proxies you will want to make sure that each proxy sets appropriate headers to ensure that your application can properly determine the client address of the incoming connection, and if the connection was over `http` or `https`. For more information see [Proxies and Forwarded Headers][#proxies-and-forwarded-headers] below. Here's how a simple Nginx configuration might look. This example includes setting proxy headers, and using a UNIX domain socket to communicate with the application server. @@ -343,3 +340,42 @@ $ gunicorn --keyfile=./key.pem --certfile=./cert.pem -k uvicorn.workers.UvicornW [nginx_websocket]: https://nginx.org/en/docs/http/websocket.html [letsencrypt]: https://letsencrypt.org/ [mkcert]: https://github.com/FiloSottile/mkcert + +## Proxies and Forwarded Headers + +When running an application behind one or more proxies, certain information about the request is lost. To avoid this most proxies will add headers containing this information for downstream servers to read. + +Uvicorn currently supports the following headers: + +- `X-Forwarded-For` ([MDN Reference](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For)) +- `X-Forwarded-Proto`([MDN Reference](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto)) + +Uvicorn can use these headers to correctly set the client and protocol in the request. However as anyone can set these headers you must configure which "clients" you will trust to have set them correctly. + +Uvicorn can be configured to trust IP Addresses (e.g. `127.0.0.1`), IP Networks (e.g. `10.100.0.0/16`), or Literals (e.g. `/path/to/socket.sock`). When running from CLI these are configured using `--forwarded-trust-ips`. + +!!! Warning: Only trust clients you can actually trust + Incorrectly trusting other clients can lead to malicious actors spoofing their apparent client address to your application. + +For more informations see [`ProxyHeadersMiddleware`](https://github.com/encode/uvicorn/blob/master/uvicorn/middleware/proxy_headers.py) + + +### UNIX Domain Sockets (UDS) + +Although it is common for UNIX Domain Sockets to be used for communicating between various HTTP servers, they can mess with some of the expected received values as they will be various non-address strings or missing values. + +For example: +- when NGINX itself is running behind a UDS it will add the literal `unix:` as the client in the `X-Forwarded-For` header. +- When Uvicorn is running behind a UDS the initial client will be `None`. + +In the case of Uvicorn, if it run using `--uds`, then it will automatically trust `None` as the initial client. + +If you are running Uvicorn behind multiple layers of proxies which use UDS, or you are not able to know the path of the sockets at initialisation, you can use the `--forwarded-trust-literals` setting to trust all literals without specifying them. You can still set `--forwarded-trust-ips` to trust other addresses and networks. + +!!! Warning: Malformed `X-Forwarded-For` values + `X-Forwarded-For` values that cannot be converted to an IP Address will be treated as literals when processing the header. This means that if a proxy adds a malformed value it will be treated as a literal, in combination with `--forwarded-trust-literals` this may result in unexpected values being trusted. + + +### Trust Everything + +Rather than specifying what to trust, you can instruct Uvicorn to trust all clients using the literal `"*"`. You should only set this when you know you can trust all values within the forwarded headers (e.g. because your proxies remove the existing headers before setting their own). From 10f1f10c81c3c2506a3069d328abd18f6f06aad5 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Mon, 29 Jan 2024 16:49:46 +1100 Subject: [PATCH 16/26] More docs and code comments --- docs/deployment.md | 2 ++ uvicorn/middleware/proxy_headers.py | 3 +++ 2 files changed, 5 insertions(+) diff --git a/docs/deployment.md b/docs/deployment.md index 54898bec2..419c2375f 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -358,7 +358,9 @@ Uvicorn can be configured to trust IP Addresses (e.g. `127.0.0.1`), IP Networks Incorrectly trusting other clients can lead to malicious actors spoofing their apparent client address to your application. For more informations see [`ProxyHeadersMiddleware`](https://github.com/encode/uvicorn/blob/master/uvicorn/middleware/proxy_headers.py) +### Client Port +Currently if the `ProxyHeadersMiddleware` is able to retrieve a trusted client value then the client's port will be set to `0`. This is because port information is lost when using these headers. ### UNIX Domain Sockets (UDS) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 83c9bbc73..7cbf32dc4 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -164,7 +164,9 @@ async def __call__( # if b"forwarded" in headers: # ... + # https://github.com/encode/uvicorn/discussions/2236 # TODO: We should probably also support the Forwarded header: + # https://datatracker.ietf.org/doc/html/rfc7239 # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded # If we do it should take precedence over the x-forwarded-* headers. @@ -198,6 +200,7 @@ async def __call__( # if b"x-forwarded-port" in headers: # ... + # https://github.com/encode/uvicorn/issues/1974 # TODO: Are we able to reliabily extract x-forwarded-port? # https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-port # If yes we should update the NGINX in docs/deployment.md From 39c794f681bbd40eaa74fb42257a54166750a35d Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Tue, 30 Jan 2024 02:54:35 +1100 Subject: [PATCH 17/26] Refactor common test cases, add cases for UDS --- tests/middleware/test_proxy_headers.py | 235 +++++++++++++++++-------- uvicorn/middleware/proxy_headers.py | 3 + 2 files changed, 162 insertions(+), 76 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index b83461dd6..c8b3b167a 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -1,4 +1,6 @@ -from typing import TYPE_CHECKING, List, Type, Union, cast +from __future__ import annotations + +from typing import TYPE_CHECKING, cast import httpx import httpx._transports.asgi @@ -18,21 +20,93 @@ from uvicorn.protocols.websockets.wsproto_impl import WSProtocol -async def app( +async def default_app( scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable, ) -> None: scheme = scope["scheme"] # type: ignore - host, port = scope["client"] # type: ignore - addr = "%s://%s:%d" % (scheme, host, port) - response = Response("Remote: " + addr, media_type="text/plain") + if (client := scope["client"]) is None: # type: ignore + client_addr = "NONE" + else: + host, port = client + client_addr = f"{host}:{port}" + + response = Response(f"{scheme}://{client_addr}", media_type="text/plain") await response(scope, receive, send) +class UdsMiddleware: + """Middleware that mimics ProxyHeadersMiddleware but first sets client to `None` + + Manually acts as if Uvicorn is running behind a UNIX Domain Socket + """ + + def __init__(self, *args, **kwargs) -> None: + self.app = ProxyHeadersMiddleware(*args, **kwargs) + return + + async def __call__( + self, + scope: Scope, + receive: ASGIReceiveCallable, + send: ASGISendCallable, + ) -> None: + if scope["type"] in ("http", "websocket"): + scope["client"] = None # type: ignore + return await self.app(scope, receive, send) + + +def make_httpx_client( + trusted_hosts: str | list[str], + client: tuple[str, int] = ("127.0.0.1", 123), + trust_none_client: bool = False, + trust_all_literal_clients: bool = False, + app_class: type = ProxyHeadersMiddleware, +) -> httpx.AsyncClient: + """Create async client for use in test cases + + Args: + trusted_hosts: trusted_hosts for proxy middleware + client: transport client to use + trust_none_client: trust_none_client for proxy middleware + trust_all_literal_clients: trust_all_literal_clients for proxy middleware + app_class: ASGI app to use with transport. + This is so that can use `UdsMiddleware`. + """ + + app = cast( + httpx._transports.asgi._ASGIApp, + app_class( + default_app, trusted_hosts, trust_none_client, trust_all_literal_clients + ), + ) + transport = httpx.ASGITransport(app=app, client=client) + return httpx.AsyncClient(transport=transport, base_url="http://testserver") + + +def make_x_headers(for_: str | None, proto: str | None = "https") -> dict[str, str]: + """Make X-Forwarded-* header dict + + Set any argument as `None` to exclude header. + + Args: + for_: forwarded for value + proto: forwarded proto value + """ + headers: dict[str, str] = {} + if for_ is not None: + headers["X-Forwarded-For"] = for_ + + if proto is not None: + headers["X-Forwarded-Proto"] = proto + + return headers + + # Note: we vary the format here to also test some of the functionality # of the _TrustedHosts.__init__ method. -_TRUSTED_NOTHING: List[str] = [] +_TRUSTED_NOTHING: list[str] = [] _TRUSTED_EVERYTHING = "*" _TRUSTED_IPv4_ADDRESSES = "127.0.0.1, 10.0.0.1" _TRUSTED_IPv4_NETWORKS = ["127.0.0.0/8", "10.0.0.0/8"] @@ -85,6 +159,7 @@ async def app( (_TRUSTED_NOTHING, "another-literal", False), (_TRUSTED_NOTHING, "unix:///another/path", False), (_TRUSTED_NOTHING, "/another/path", False), + (_TRUSTED_NOTHING, "", False), ## Always trust ## ----------------------------- # Test IPv4 Addresses @@ -121,6 +196,7 @@ async def app( (_TRUSTED_EVERYTHING, "another-literal", True), (_TRUSTED_EVERYTHING, "unix:///another/path", True), (_TRUSTED_EVERYTHING, "/another/path", True), + (_TRUSTED_EVERYTHING, "", True), ## Trust IPv4 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -157,6 +233,7 @@ async def app( (_TRUSTED_IPv4_ADDRESSES, "another-literal", False), (_TRUSTED_IPv4_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv4_ADDRESSES, "/another/path", False), + (_TRUSTED_IPv4_ADDRESSES, "", False), ## Trust IPv6 Addresses ## ----------------------------- # Test IPv4 Addresses @@ -193,6 +270,7 @@ async def app( (_TRUSTED_IPv6_ADDRESSES, "another-literal", False), (_TRUSTED_IPv6_ADDRESSES, "unix:///another/path", False), (_TRUSTED_IPv6_ADDRESSES, "/another/path", False), + (_TRUSTED_IPv6_ADDRESSES, "", False), ## Trust IPv4 Networks ## ----------------------------- # Test IPv4 Addresses @@ -229,6 +307,7 @@ async def app( (_TRUSTED_IPv4_NETWORKS, "another-literal", False), (_TRUSTED_IPv4_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv4_NETWORKS, "/another/path", False), + (_TRUSTED_IPv4_NETWORKS, "", False), ## Trust IPv6 Networks ## ----------------------------- # Test IPv4 Addresses @@ -265,6 +344,7 @@ async def app( (_TRUSTED_IPv6_NETWORKS, "another-literal", False), (_TRUSTED_IPv6_NETWORKS, "unix:///another/path", False), (_TRUSTED_IPv6_NETWORKS, "/another/path", False), + (_TRUSTED_IPv6_NETWORKS, "", False), ## Trust Literals ## ----------------------------- # Test IPv4 Addresses @@ -301,122 +381,123 @@ async def app( (_TRUSTED_LITERALS, "another-literal", False), (_TRUSTED_LITERALS, "unix:///another/path", False), (_TRUSTED_LITERALS, "/another/path", False), + (_TRUSTED_LITERALS, "", False), ], ) def test_forwarded_hosts( - init_hosts: Union[str, List[str]], test_host: str, expected: bool + init_hosts: str | list[str], test_host: str, expected: bool ) -> None: trusted_hosts = _TrustedHosts(init_hosts) assert (test_host in trusted_hosts) is expected +@pytest.mark.parametrize( + ("test_host", "expected"), + [ + ("127.0.0.1", True), + ("192.168.0.1", False), + ("::1", True), + ("ff::1", False), + ("some-literal", True), + ("1.2.34.5.6.7", True), # invalid ipv4 + (":::1", True), # invalip ipv6 + ("unix:", True), + ("", False), # empty string is not literal + ], +) +def test_forwarded_hosts_trust_literals(test_host: str, expected: bool) -> None: + trusted_hosts = _TrustedHosts("127.0.0.1, ::1, some-literal", True) + assert (test_host in trusted_hosts) is expected + + @pytest.mark.anyio @pytest.mark.parametrize( - ("trusted_hosts", "response_text"), + ("trusted_hosts", "expected"), [ # always trust - ("*", "Remote: https://1.2.3.4:0"), + ("*", "https://1.2.3.4:0"), # trusted proxy - ("127.0.0.1", "Remote: https://1.2.3.4:0"), - (["127.0.0.1"], "Remote: https://1.2.3.4:0"), + ("127.0.0.1", "https://1.2.3.4:0"), + (["127.0.0.1"], "https://1.2.3.4:0"), # trusted proxy list - (["127.0.0.1", "10.0.0.1"], "Remote: https://1.2.3.4:0"), - ("127.0.0.1, 10.0.0.1", "Remote: https://1.2.3.4:0"), + (["127.0.0.1", "10.0.0.1"], "https://1.2.3.4:0"), + ("127.0.0.1, 10.0.0.1", "https://1.2.3.4:0"), # trusted proxy network # https://github.com/encode/uvicorn/issues/1068#issuecomment-1004813267 - ("127.0.0.0/24, 10.0.0.1", "Remote: https://1.2.3.4:0"), + ("127.0.0.0/24, 10.0.0.1", "https://1.2.3.4:0"), # request from untrusted proxy - ("192.168.0.1", "Remote: http://127.0.0.1:123"), + ("192.168.0.1", "http://127.0.0.1:123"), # request from untrusted proxy network - ("192.168.0.0/16", "Remote: http://127.0.0.1:123"), + ("192.168.0.0/16", "http://127.0.0.1:123"), # request from client running on proxy server itself # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 - (["127.0.0.1", "1.2.3.4"], "Remote: https://1.2.3.4:0"), + (["127.0.0.1", "1.2.3.4"], "https://1.2.3.4:0"), ], ) async def test_proxy_headers_trusted_hosts( - trusted_hosts: Union[List[str], str], response_text: str + trusted_hosts: str | list[str], expected: str ) -> None: - app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: - headers = {"X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4"} - response = await client.get("/", headers=headers) + async with make_httpx_client(trusted_hosts) as client: + response = await client.get("/", headers=make_x_headers("1.2.3.4")) assert response.status_code == 200 - assert response.text == response_text + assert response.text == expected @pytest.mark.anyio @pytest.mark.parametrize( - ("trusted_hosts", "response_text"), + ("trusted_hosts", "expected"), [ # always trust - ("*", "Remote: https://1.2.3.4:0"), + ("*", "https://1.2.3.4:0"), # all proxies are trusted - ( - ["127.0.0.1", "10.0.2.1", "192.168.0.2"], - "Remote: https://1.2.3.4:0", - ), + (["127.0.0.1", "10.0.2.1", "192.168.0.2"], "https://1.2.3.4:0"), # order doesn't matter - ( - ["10.0.2.1", "192.168.0.2", "127.0.0.1"], - "Remote: https://1.2.3.4:0", - ), + (["10.0.2.1", "192.168.0.2", "127.0.0.1"], "https://1.2.3.4:0"), # should set first untrusted as remote address - (["192.168.0.2", "127.0.0.1"], "Remote: https://10.0.2.1:0"), + (["192.168.0.2", "127.0.0.1"], "https://10.0.2.1:0"), # Mixed literals and networks - (["127.0.0.1", "10.0.0.0/8", "192.168.0.2"], "Remote: https://1.2.3.4:0"), + (["127.0.0.1", "10.0.0.0/8", "192.168.0.2"], "https://1.2.3.4:0"), ], ) async def test_proxy_headers_multiple_proxies( - trusted_hosts: Union[List[str], str], response_text: str + trusted_hosts: str | list[str], expected: str ) -> None: - app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts=trusted_hosts) - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: - headers = { - "X-Forwarded-Proto": "https", - "X-Forwarded-For": "1.2.3.4, 10.0.2.1, 192.168.0.2", - } - response = await client.get("/", headers=headers) + async with make_httpx_client(trusted_hosts) as client: + response = await client.get( + "/", headers=make_x_headers("1.2.3.4, 10.0.2.1, 192.168.0.2") + ) assert response.status_code == 200 - assert response.text == response_text + assert response.text == expected @pytest.mark.anyio async def test_proxy_headers_invalid_x_forwarded_for() -> None: - app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") - async with httpx.AsyncClient( - app=app_with_middleware, base_url="http://testserver" - ) as client: + async with make_httpx_client("*") as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", - "X-Forwarded-For": "1.2.3.4, \xf0\xfd\xfd\xfd", + "X-Forwarded-For": "1.2.3.4, \xf0\xfd\xfd\xfd, unix:, ::1", }, encoding="latin-1", ) response = await client.get("/", headers=headers) assert response.status_code == 200 - assert response.text == "Remote: https://1.2.3.4:0" + assert response.text == "https://1.2.3.4:0" @pytest.mark.anyio async def test_proxy_headers_websocket_x_forwarded_proto( - ws_protocol_cls: "Type[WSProtocol | WebSocketProtocol]", - http_protocol_cls: "Type[H11Protocol | HttpToolsProtocol]", + ws_protocol_cls: type[WSProtocol | WebSocketProtocol], + http_protocol_cls: type[H11Protocol | HttpToolsProtocol], unused_tcp_port: int, ) -> None: async def websocket_app(scope, receive, send): scheme = scope["scheme"] host, port = scope["client"] - addr = "%s://%s:%d" % (scheme, host, port) await send({"type": "websocket.accept"}) - await send({"type": "websocket.send", "text": addr}) + await send({"type": "websocket.send", "text": f"{scheme}://{host}:{port}"}) app_with_middleware = ProxyHeadersMiddleware(websocket_app, trusted_hosts="*") config = Config( @@ -429,30 +510,32 @@ async def websocket_app(scope, receive, send): async with run_server(config): url = f"ws://127.0.0.1:{unused_tcp_port}" - headers = {"X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4"} - async with websockets.client.connect(url, extra_headers=headers) as websocket: + async with websockets.client.connect( + url, extra_headers=make_x_headers("1.2.3.4") + ) as websocket: data = await websocket.recv() assert data == "wss://1.2.3.4:0" +@pytest.mark.anyio +@pytest.mark.parametrize( + ("trust_none_client", "expected"), + [(False, "http://NONE"), (True, "https://1.2.3.4:0")], +) +async def test_proxy_headers_uds(trust_none_client: bool, expected: str) -> None: + async with make_httpx_client( + "10.0.0.0/8", trust_none_client=trust_none_client, app_class=UdsMiddleware + ) as client: + response = await client.get("/", headers=make_x_headers("1.2.3.4, 10.1.1.1")) + assert response.status_code == 200 + assert response.text == expected + + @pytest.mark.anyio async def test_proxy_headers_empty_x_forwarded_for() -> None: # fallback to the default behavior if x-forwarded-for is an empty list # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 - app_with_middleware = cast( - httpx._transports.asgi._ASGIApp, ProxyHeadersMiddleware(app, trusted_hosts="*") - ) - transport = httpx.ASGITransport(app=app_with_middleware, client=("1.2.3.4", 8080)) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver" - ) as client: - headers = httpx.Headers( - { - "X-Forwarded-Proto": "https", - "X-Forwarded-For": "", - }, - encoding="latin-1", - ) - response = await client.get("/", headers=headers) + async with make_httpx_client("*") as client: + response = await client.get("/", headers=make_x_headers("")) assert response.status_code == 200 - assert response.text == "Remote: https://1.2.3.4:8080" + assert response.text == "https://127.0.0.1:123" diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 7cbf32dc4..7a2e0e2e8 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -104,6 +104,9 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: return x_forwarded_for_hosts[0] def __str__(self) -> str: + if self.always_trust: + return f"{self.__class__.__name__}(always_trust=True)" + return ( f"{self.__class__.__name__}({self.trusted_hosts=}, " f"{self.trusted_networks=}, {self.trusted_literals=}, " From 28cf1ba8bc2f7e8b448826a8dcb2c0f20158e7f8 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Tue, 30 Jan 2024 02:58:15 +1100 Subject: [PATCH 18/26] Update settings docs --- docs/settings.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/settings.md b/docs/settings.md index 77ae771e7..5c2bb10a1 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -91,6 +91,7 @@ Note that WSGI mode always disables WebSocket support, as it is not supported by * `--proxy-headers` / `--no-proxy-headers` - Enable/Disable X-Forwarded-Proto, X-Forwarded-For, X-Forwarded-Port to populate remote address info. Defaults to enabled, but is restricted to only trusting connecting IPs in the `forwarded-allow-ips` configuration. * `--forwarded-allow-ips` Comma separated list of IP Addresses, IP Networks, or literals (e.g. UNIX Socket path) to trust with proxy headers. Defaults to the `$FORWARDED_ALLOW_IPS` environment variable if available, or '127.0.0.1'. The literal `'*'` means trust everything. +* `--forwarded-trust-literals` - Trust all literals in proxy headers. Defaults to False. Useful when using multiple proxies with UNIX Domain Sockets for transport. * `--server-header` / `--no-server-header` - Enable/Disable default `Server` header. * `--date-header` / `--no-date-header` - Enable/Disable default `Date` header. From 1b3eb6f0a6a7069c0b3f43cd809172fceaf48878 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Sat, 10 Feb 2024 22:16:17 +1100 Subject: [PATCH 19/26] Remove "better UDS handling" --- docs/deployment.md | 11 ----- docs/index.md | 4 -- docs/settings.md | 3 +- tests/middleware/test_proxy_headers.py | 65 +------------------------- uvicorn/config.py | 5 -- uvicorn/main.py | 11 ----- uvicorn/middleware/proxy_headers.py | 40 ++-------------- 7 files changed, 7 insertions(+), 132 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 419c2375f..2d3f576b0 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -99,10 +99,6 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. - --forwarded-trust-literals Trust all literals in proxy headers. - Defaults to False. Useful when using - multiple proxies with UNIX Domain Sockets - for transport. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or @@ -370,13 +366,6 @@ For example: - when NGINX itself is running behind a UDS it will add the literal `unix:` as the client in the `X-Forwarded-For` header. - When Uvicorn is running behind a UDS the initial client will be `None`. -In the case of Uvicorn, if it run using `--uds`, then it will automatically trust `None` as the initial client. - -If you are running Uvicorn behind multiple layers of proxies which use UDS, or you are not able to know the path of the sockets at initialisation, you can use the `--forwarded-trust-literals` setting to trust all literals without specifying them. You can still set `--forwarded-trust-ips` to trust other addresses and networks. - -!!! Warning: Malformed `X-Forwarded-For` values - `X-Forwarded-For` values that cannot be converted to an IP Address will be treated as literals when processing the header. This means that if a proxy adds a malformed value it will be treated as a literal, in combination with `--forwarded-trust-literals` this may result in unexpected values being trusted. - ### Trust Everything diff --git a/docs/index.md b/docs/index.md index ef54e3d62..38a6892a5 100644 --- a/docs/index.md +++ b/docs/index.md @@ -169,10 +169,6 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. - --forwarded-trust-literals Trust all literals in proxy headers. - Defaults to False. Useful when using - multiple proxies with UNIX Domain Sockets - for transport. --root-path TEXT Set the ASGI 'root_path' for applications submounted below a given URL path. --limit-concurrency INTEGER Maximum number of concurrent connections or diff --git a/docs/settings.md b/docs/settings.md index 5c2bb10a1..a4439c3d0 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -88,10 +88,9 @@ Note that WSGI mode always disables WebSocket support, as it is not supported by ## HTTP * `--root-path ` - Set the ASGI `root_path` for applications submounted below a given URL path. -* `--proxy-headers` / `--no-proxy-headers` - Enable/Disable X-Forwarded-Proto, X-Forwarded-For, X-Forwarded-Port to populate remote address info. Defaults to enabled, but is restricted to only trusting +* `--proxy-headers` / `--no-proxy-headers` - Enable/Disable X-Forwarded-Proto, X-Forwarded-For to populate remote address info. Defaults to enabled, but is restricted to only trusting connecting IPs in the `forwarded-allow-ips` configuration. * `--forwarded-allow-ips` Comma separated list of IP Addresses, IP Networks, or literals (e.g. UNIX Socket path) to trust with proxy headers. Defaults to the `$FORWARDED_ALLOW_IPS` environment variable if available, or '127.0.0.1'. The literal `'*'` means trust everything. -* `--forwarded-trust-literals` - Trust all literals in proxy headers. Defaults to False. Useful when using multiple proxies with UNIX Domain Sockets for transport. * `--server-header` / `--no-server-header` - Enable/Disable default `Server` header. * `--date-header` / `--no-date-header` - Enable/Disable default `Date` header. diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index c8b3b167a..26eb09081 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -36,50 +36,20 @@ async def default_app( await response(scope, receive, send) -class UdsMiddleware: - """Middleware that mimics ProxyHeadersMiddleware but first sets client to `None` - - Manually acts as if Uvicorn is running behind a UNIX Domain Socket - """ - - def __init__(self, *args, **kwargs) -> None: - self.app = ProxyHeadersMiddleware(*args, **kwargs) - return - - async def __call__( - self, - scope: Scope, - receive: ASGIReceiveCallable, - send: ASGISendCallable, - ) -> None: - if scope["type"] in ("http", "websocket"): - scope["client"] = None # type: ignore - return await self.app(scope, receive, send) - - def make_httpx_client( trusted_hosts: str | list[str], client: tuple[str, int] = ("127.0.0.1", 123), - trust_none_client: bool = False, - trust_all_literal_clients: bool = False, - app_class: type = ProxyHeadersMiddleware, ) -> httpx.AsyncClient: """Create async client for use in test cases Args: trusted_hosts: trusted_hosts for proxy middleware client: transport client to use - trust_none_client: trust_none_client for proxy middleware - trust_all_literal_clients: trust_all_literal_clients for proxy middleware - app_class: ASGI app to use with transport. - This is so that can use `UdsMiddleware`. """ app = cast( httpx._transports.asgi._ASGIApp, - app_class( - default_app, trusted_hosts, trust_none_client, trust_all_literal_clients - ), + ProxyHeadersMiddleware(default_app, trusted_hosts), ) transport = httpx.ASGITransport(app=app, client=client) return httpx.AsyncClient(transport=transport, base_url="http://testserver") @@ -391,25 +361,6 @@ def test_forwarded_hosts( assert (test_host in trusted_hosts) is expected -@pytest.mark.parametrize( - ("test_host", "expected"), - [ - ("127.0.0.1", True), - ("192.168.0.1", False), - ("::1", True), - ("ff::1", False), - ("some-literal", True), - ("1.2.34.5.6.7", True), # invalid ipv4 - (":::1", True), # invalip ipv6 - ("unix:", True), - ("", False), # empty string is not literal - ], -) -def test_forwarded_hosts_trust_literals(test_host: str, expected: bool) -> None: - trusted_hosts = _TrustedHosts("127.0.0.1, ::1, some-literal", True) - assert (test_host in trusted_hosts) is expected - - @pytest.mark.anyio @pytest.mark.parametrize( ("trusted_hosts", "expected"), @@ -517,20 +468,6 @@ async def websocket_app(scope, receive, send): assert data == "wss://1.2.3.4:0" -@pytest.mark.anyio -@pytest.mark.parametrize( - ("trust_none_client", "expected"), - [(False, "http://NONE"), (True, "https://1.2.3.4:0")], -) -async def test_proxy_headers_uds(trust_none_client: bool, expected: str) -> None: - async with make_httpx_client( - "10.0.0.0/8", trust_none_client=trust_none_client, app_class=UdsMiddleware - ) as client: - response = await client.get("/", headers=make_x_headers("1.2.3.4, 10.1.1.1")) - assert response.status_code == 200 - assert response.text == expected - - @pytest.mark.anyio async def test_proxy_headers_empty_x_forwarded_for() -> None: # fallback to the default behavior if x-forwarded-for is an empty list diff --git a/uvicorn/config.py b/uvicorn/config.py index 1ba52429e..8100933f9 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -208,7 +208,6 @@ def __init__( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, - forwarded_trust_literals: bool = False, root_path: str = "", limit_concurrency: int | None = None, limit_max_requests: int | None = None, @@ -350,8 +349,6 @@ def __init__( else: self.forwarded_allow_ips = forwarded_allow_ips - self.forwarded_trust_literals = forwarded_trust_literals - if self.reload and self.workers > 1: logger.warning('"workers" flag is ignored when reloading is enabled.') @@ -498,8 +495,6 @@ def load(self) -> None: self.loaded_app = ProxyHeadersMiddleware( self.loaded_app, trusted_hosts=self.forwarded_allow_ips, - trust_none_client=bool(self.uds), - trust_all_literal_clients=self.forwarded_trust_literals, ) self.loaded = True diff --git a/uvicorn/main.py b/uvicorn/main.py index 2eb688b56..82a7cc43b 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -250,13 +250,6 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No "$FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. " "The literal '*' means trust everything.", ) -@click.option( - "--forwarded-trust-literals", - is_flag=True, - default=False, - help="Trust all literals in proxy headers. Defaults to False. Useful when using " - "multiple proxies with UNIX Domain Sockets for transport.", -) @click.option( "--root-path", type=str, @@ -405,7 +398,6 @@ def main( server_header: bool, date_header: bool, forwarded_allow_ips: str, - forwarded_trust_literals: bool, root_path: str, limit_concurrency: int, backlog: int, @@ -455,7 +447,6 @@ def main( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, - forwarded_trust_literals=forwarded_trust_literals, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, @@ -508,7 +499,6 @@ def run( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, - forwarded_trust_literals: bool = False, root_path: str = "", limit_concurrency: int | None = None, backlog: int = 2048, @@ -561,7 +551,6 @@ def run( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, - forwarded_trust_literals=forwarded_trust_literals, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 7a2e0e2e8..64b0abc5e 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -21,10 +21,8 @@ class _TrustedHosts: def __init__( self, trusted_hosts: Union[List[str], str], - trust_all_literal_clients: bool = False, ) -> None: self.always_trust: bool = trusted_hosts == "*" - self.trust_all_literal_clients = trust_all_literal_clients self.trusted_literals: Set[str] = set() self.trusted_hosts: Set[ipaddress._BaseAddress] = set() @@ -77,8 +75,6 @@ def __contains__(self, item: Optional[str]) -> bool: return any(ip in net for net in self.trusted_networks) except ValueError: - if self.trust_all_literal_clients: - return True return item in self.trusted_literals def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: @@ -109,8 +105,8 @@ def __str__(self) -> str: return ( f"{self.__class__.__name__}({self.trusted_hosts=}, " - f"{self.trusted_networks=}, {self.trusted_literals=}, " - f"{self.trust_all_literal_clients=}" + f"{self.trusted_networks=}, {self.trusted_literals=}" + ")" ) @@ -124,16 +120,6 @@ class ProxyHeadersMiddleware: Modifies the `client` and `scheme` information so that they reference the connecting client, rather that the connecting proxy. - Use `trust_none_client = True` to continue with proxy header handling when - the initial client is `None`. This does not affect the handling of client - adddresses in the proxy headers. If you are listening on a UNIX Domain Sockets - you will need to set this to `True` to enable proxy handling. - - Use `trust_all_literal_clients = True` to trust all non-IP clients in the header. - This is useful if you are using UNIX Domain Sockets to communicate between your - proxies and do not wish to list each literal or if you do not know the value of - each literal and startup. - References: - @@ -144,13 +130,9 @@ def __init__( self, app: "ASGI3Application", trusted_hosts: Union[List[str], str] = "127.0.0.1", - trust_none_client: bool = False, - trust_all_literal_clients: bool = False, ) -> None: self.app = app - self.trusted_hosts = _TrustedHosts(trusted_hosts, trust_all_literal_clients) - self.trust_none_client = trust_none_client - print(self.trusted_hosts) + self.trusted_hosts = _TrustedHosts(trusted_hosts) return async def __call__( @@ -159,20 +141,11 @@ async def __call__( if scope["type"] in ("http", "websocket"): scope = cast(Union[HTTPScope, WebSocketScope], scope) client_addr: Optional[Tuple[str, int]] = scope.get("client") + client_host = client_addr[0] if client_addr else None - if (client_addr is None and self.trust_none_client) or ( - client_addr is not None and client_addr[0] in self.trusted_hosts - ): + if client_host in self.trusted_hosts: headers = dict(scope["headers"]) - # if b"forwarded" in headers: - # ... - # https://github.com/encode/uvicorn/discussions/2236 - # TODO: We should probably also support the Forwarded header: - # https://datatracker.ietf.org/doc/html/rfc7239 - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded - # If we do it should take precedence over the x-forwarded-* headers. - if b"x-forwarded-proto" in headers: # Determine if the incoming request was http or https based on # the X-Forwarded-Proto header. @@ -204,8 +177,5 @@ async def __call__( # if b"x-forwarded-port" in headers: # ... # https://github.com/encode/uvicorn/issues/1974 - # TODO: Are we able to reliabily extract x-forwarded-port? - # https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-port - # If yes we should update the NGINX in docs/deployment.md return await self.app(scope, receive, send) From fcaf9d2f1614b82e82f21bf0e0127e53a0ab1672 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Sat, 10 Feb 2024 22:40:44 +1100 Subject: [PATCH 20/26] Remove code leading to coverage failures --- uvicorn/middleware/proxy_headers.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 64b0abc5e..46230d361 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -99,16 +99,6 @@ def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: # See https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 return x_forwarded_for_hosts[0] - def __str__(self) -> str: - if self.always_trust: - return f"{self.__class__.__name__}(always_trust=True)" - - return ( - f"{self.__class__.__name__}({self.trusted_hosts=}, " - f"{self.trusted_networks=}, {self.trusted_literals=}" - ")" - ) - class ProxyHeadersMiddleware: """Middleware for handling known proxy headers From 899b4567e2871ec7c38cb7f26c95cbb98ac3fe06 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Tue, 5 Mar 2024 17:03:21 +1100 Subject: [PATCH 21/26] Apply suggestions from code review Co-authored-by: Marcelo Trylesinski --- uvicorn/middleware/proxy_headers.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 46230d361..7c029dfb9 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -111,7 +111,6 @@ class ProxyHeadersMiddleware: the connecting client, rather that the connecting proxy. References: - - - """ @@ -123,7 +122,6 @@ def __init__( ) -> None: self.app = app self.trusted_hosts = _TrustedHosts(trusted_hosts) - return async def __call__( self, scope: "Scope", receive: "ASGIReceiveCallable", send: "ASGISendCallable" @@ -159,13 +157,9 @@ async def __call__( # Only set the client if we actually got something usable. # See: https://github.com/encode/uvicorn/issues/1068 - # Unless we can relaibly use x-forwarded-port (see below) then - # we will not have any port information so set it to 0. + # We've lost the connecting client's port information by now, + # so only include the host. port = 0 scope["client"] = (host, port) - # if b"x-forwarded-port" in headers: - # ... - # https://github.com/encode/uvicorn/issues/1974 - return await self.app(scope, receive, send) From 772ff9b0e9b8f950994632acfa5df40abdcfde38 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Tue, 5 Mar 2024 18:13:23 +1100 Subject: [PATCH 22/26] Fixes, and more tests --- tests/middleware/test_proxy_headers.py | 49 ++++++++++++++++++-------- uvicorn/middleware/proxy_headers.py | 30 +++++++--------- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 121ba3fe5..8e9e824c6 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -354,9 +354,7 @@ def make_x_headers(for_: str | None, proto: str | None = "https") -> dict[str, s (_TRUSTED_LITERALS, "", False), ], ) -def test_forwarded_hosts( - init_hosts: str | list[str], test_host: str, expected: bool -) -> None: +def test_forwarded_hosts(init_hosts: str | list[str], test_host: str, expected: bool) -> None: trusted_hosts = _TrustedHosts(init_hosts) assert (test_host in trusted_hosts) is expected @@ -385,15 +383,38 @@ def test_forwarded_hosts( (["127.0.0.1", "1.2.3.4"], "https://1.2.3.4:0"), ], ) -async def test_proxy_headers_trusted_hosts( - trusted_hosts: str | list[str], expected: str -) -> None: +async def test_proxy_headers_trusted_hosts(trusted_hosts: str | list[str], expected: str) -> None: async with make_httpx_client(trusted_hosts) as client: response = await client.get("/", headers=make_x_headers("1.2.3.4")) assert response.status_code == 200 assert response.text == expected +@pytest.mark.anyio +@pytest.mark.parametrize( + ("header_args", "expected"), + [ + (("", ""), "http://127.0.0.1:123"), + (("", None), "http://127.0.0.1:123"), + (("", "asdf"), "http://127.0.0.1:123"), + ((" , ",), "https://127.0.0.1:123"), + ((", , ",), "https://127.0.0.1:123"), + ((" , 10.0.0.1",), "https://127.0.0.1:123"), + (("9.9.9.9 , , , 10.0.0.1",), "https://127.0.0.1:123"), + ((", , 9.9.9.9",), "https://9.9.9.9:0"), + ((", , 9.9.9.9, , ",), "https://127.0.0.1:123"), + ], +) +async def test_proxy_headers_trusted_hosts_malformed( + header_args: tuple[str | None, str | None] | tuple[str | None], + expected: str, +) -> None: + async with make_httpx_client("127.0.0.1, 10.0.0.0/8") as client: + response = await client.get("/", headers=make_x_headers(*header_args)) + assert response.status_code == 200 + assert response.text == expected + + @pytest.mark.anyio @pytest.mark.parametrize( ("trusted_hosts", "expected"), @@ -410,13 +431,9 @@ async def test_proxy_headers_trusted_hosts( (["127.0.0.1", "10.0.0.0/8", "192.168.0.2"], "https://1.2.3.4:0"), ], ) -async def test_proxy_headers_multiple_proxies( - trusted_hosts: str | list[str], expected: str -) -> None: +async def test_proxy_headers_multiple_proxies(trusted_hosts: str | list[str], expected: str) -> None: async with make_httpx_client(trusted_hosts) as client: - response = await client.get( - "/", headers=make_x_headers("1.2.3.4, 10.0.2.1, 192.168.0.2") - ) + response = await client.get("/", headers=make_x_headers("1.2.3.4, 10.0.2.1, 192.168.0.2")) assert response.status_code == 200 assert response.text == expected @@ -438,7 +455,7 @@ async def test_proxy_headers_invalid_x_forwarded_for() -> None: @pytest.mark.anyio @pytest.mark.parametrize( - "x_forwarded_proto,addr", + "forwarded_proto,expected", [ ("http", "ws://1.2.3.4:0"), ("https", "wss://1.2.3.4:0"), @@ -447,6 +464,8 @@ async def test_proxy_headers_invalid_x_forwarded_for() -> None: ], ) async def test_proxy_headers_websocket_x_forwarded_proto( + forwarded_proto: str, + expected: str, ws_protocol_cls: type[WSProtocol | WebSocketProtocol], http_protocol_cls: type[H11Protocol | HttpToolsProtocol], unused_tcp_port: int, @@ -471,10 +490,10 @@ async def websocket_app(scope: Scope, receive: ASGIReceiveCallable, send: ASGISe async with run_server(config): url = f"ws://127.0.0.1:{unused_tcp_port}" async with websockets.client.connect( - url, extra_headers=make_x_headers("1.2.3.4") + url, extra_headers=make_x_headers("1.2.3.4", forwarded_proto) ) as websocket: data = await websocket.recv() - assert data == "wss://1.2.3.4:0" + assert data == expected @pytest.mark.anyio diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index b67df1763..d56b5566d 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -1,12 +1,12 @@ from __future__ import annotations import ipaddress -from typing import Optional, Union, cast +from typing import Union, cast from uvicorn._types import ASGI3Application, ASGIReceiveCallable, ASGISendCallable, HTTPScope, Scope, WebSocketScope -def _parse_raw_hosts(value: str) -> List[str]: +def _parse_raw_hosts(value: str) -> list[str]: return [item.strip() for item in value.split(",")] @@ -15,7 +15,7 @@ class _TrustedHosts: def __init__( self, - trusted_hosts: Union[list[str], str], + trusted_hosts: list[str] | str, ) -> None: self.always_trust: bool = trusted_hosts == "*" @@ -56,7 +56,7 @@ def __init__( self.trusted_literals.add(host) return - def __contains__(self, item: Optional[str]) -> bool: + def __contains__(self, item: str | None) -> bool: if self.always_trust: return True @@ -72,16 +72,13 @@ def __contains__(self, item: Optional[str]) -> bool: except ValueError: return item in self.trusted_literals - def get_trusted_client_host(self, x_forwarded_for: str) -> Optional[str]: + def get_trusted_client_host(self, x_forwarded_for: str) -> str: """Extract the client host from x_forwarded_for header In general this is the first "untrusted" host in the forwarded for list. """ x_forwarded_for_hosts = _parse_raw_hosts(x_forwarded_for) - if not x_forwarded_for_hosts: - return None - if self.always_trust: return x_forwarded_for_hosts[0] @@ -121,28 +118,27 @@ def __init__( async def __call__(self, scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable) -> None: if scope["type"] in ("http", "websocket"): scope = cast(Union[HTTPScope, WebSocketScope], scope) - client_addr: Optional[tuple[str, int]] = scope.get("client") + client_addr: tuple[str, int] | None = scope.get("client") client_host = client_addr[0] if client_addr else None if client_host in self.trusted_hosts: headers = dict(scope["headers"]) if b"x-forwarded-proto" in headers: - # Determine if the incoming request was http or https based on - # the X-Forwarded-Proto header. x_forwarded_proto = headers[b"x-forwarded-proto"].decode("latin1").strip() - if scope["type"] == "websocket": - scope["scheme"] = x_forwarded_proto.replace("http", "ws") - else: - scope["scheme"] = x_forwarded_proto + + if x_forwarded_proto in {"http", "https", "ws", "wss"}: + if scope["type"] == "websocket": + scope["scheme"] = x_forwarded_proto.replace("http", "ws") + else: + scope["scheme"] = x_forwarded_proto if b"x-forwarded-for" in headers: x_forwarded_for = headers[b"x-forwarded-for"].decode("latin1") host = self.trusted_hosts.get_trusted_client_host(x_forwarded_for) if host: - # If the x-forwarded-for header is empty then host is None or - # an empty string. + # If the x-forwarded-for header is empty then host is an empty string. # Only set the client if we actually got something usable. # See: https://github.com/encode/uvicorn/issues/1068 From 22f8554cea854cf054c9f680aa64b00115a302c2 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Tue, 5 Mar 2024 23:37:17 +1100 Subject: [PATCH 23/26] Update uvicorn/config.py Co-authored-by: Marcelo Trylesinski --- uvicorn/config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/uvicorn/config.py b/uvicorn/config.py index 26f3fcf04..982c069b1 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -467,8 +467,7 @@ def load(self) -> None: self.loaded_app = MessageLoggerMiddleware(self.loaded_app) if self.proxy_headers: self.loaded_app = ProxyHeadersMiddleware( - self.loaded_app, - trusted_hosts=self.forwarded_allow_ips, + self.loaded_app, trusted_hosts=self.forwarded_allow_ips ) self.loaded = True From a25f8395456c824173ca544417d4793f5bed46fc Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Wed, 6 Mar 2024 00:01:19 +1100 Subject: [PATCH 24/26] Run ruff formatter --- uvicorn/_subprocess.py | 1 + uvicorn/_types.py | 1 + uvicorn/config.py | 4 +--- uvicorn/protocols/http/httptools_impl.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/uvicorn/_subprocess.py b/uvicorn/_subprocess.py index 5eb835327..5ac8ade47 100644 --- a/uvicorn/_subprocess.py +++ b/uvicorn/_subprocess.py @@ -2,6 +2,7 @@ Some light wrappers around Python's multiprocessing, to deal with cleanly starting child processes. """ + from __future__ import annotations import multiprocessing diff --git a/uvicorn/_types.py b/uvicorn/_types.py index 7546262a8..b94ffe033 100644 --- a/uvicorn/_types.py +++ b/uvicorn/_types.py @@ -27,6 +27,7 @@ (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. """ + from __future__ import annotations import sys diff --git a/uvicorn/config.py b/uvicorn/config.py index 982c069b1..3cad1d90f 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -466,9 +466,7 @@ def load(self) -> None: if logger.getEffectiveLevel() <= TRACE_LOG_LEVEL: self.loaded_app = MessageLoggerMiddleware(self.loaded_app) if self.proxy_headers: - self.loaded_app = ProxyHeadersMiddleware( - self.loaded_app, trusted_hosts=self.forwarded_allow_ips - ) + self.loaded_app = ProxyHeadersMiddleware(self.loaded_app, trusted_hosts=self.forwarded_allow_ips) self.loaded = True diff --git a/uvicorn/protocols/http/httptools_impl.py b/uvicorn/protocols/http/httptools_impl.py index 2950bd537..bc44f2821 100644 --- a/uvicorn/protocols/http/httptools_impl.py +++ b/uvicorn/protocols/http/httptools_impl.py @@ -38,8 +38,8 @@ ) from uvicorn.server import ServerState -HEADER_RE = re.compile(b'[\x00-\x1F\x7F()<>@,;:[]={} \t\\"]') -HEADER_VALUE_RE = re.compile(b"[\x00-\x1F\x7F]") +HEADER_RE = re.compile(b'[\x00-\x1f\x7f()<>@,;:[]={} \t\\"]') +HEADER_VALUE_RE = re.compile(b"[\x00-\x1f\x7f]") def _get_status_line(status_code: int) -> bytes: From b9e079de63e3902b3dd615028da78617386631b7 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Wed, 6 Mar 2024 13:33:18 +1100 Subject: [PATCH 25/26] Replace cast with type: ignore --- tests/middleware/test_proxy_headers.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 8e9e824c6..f972078d1 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING import httpx import httpx._transports.asgi @@ -47,11 +47,8 @@ def make_httpx_client( client: transport client to use """ - app = cast( - httpx._transports.asgi._ASGIApp, - ProxyHeadersMiddleware(default_app, trusted_hosts), - ) - transport = httpx.ASGITransport(app=app, client=client) + app = ProxyHeadersMiddleware(default_app, trusted_hosts) + transport = httpx.ASGITransport(app=app, client=client) # type: ignore return httpx.AsyncClient(transport=transport, base_url="http://testserver") From b5136dd0c14b6aaf9190b915e85a8f841ef7b355 Mon Sep 17 00:00:00 2001 From: Nicholas Hairs Date: Fri, 19 Apr 2024 17:59:07 +1000 Subject: [PATCH 26/26] remove make_x_headers --- tests/middleware/test_proxy_headers.py | 81 +++++++++++++------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index f972078d1..5f35d9a8d 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -20,6 +20,10 @@ from uvicorn.protocols.websockets.wsproto_impl import WSProtocol +X_FORWARDED_FOR = "X-Forwarded-For" +X_FORWARDED_PROTO = "X-Forwarded-Proto" + + async def default_app( scope: Scope, receive: ASGIReceiveCallable, @@ -52,25 +56,6 @@ def make_httpx_client( return httpx.AsyncClient(transport=transport, base_url="http://testserver") -def make_x_headers(for_: str | None, proto: str | None = "https") -> dict[str, str]: - """Make X-Forwarded-* header dict - - Set any argument as `None` to exclude header. - - Args: - for_: forwarded for value - proto: forwarded proto value - """ - headers: dict[str, str] = {} - if for_ is not None: - headers["X-Forwarded-For"] = for_ - - if proto is not None: - headers["X-Forwarded-Proto"] = proto - - return headers - - # Note: we vary the format here to also test some of the functionality # of the _TrustedHosts.__init__ method. _TRUSTED_NOTHING: list[str] = [] @@ -382,32 +367,40 @@ def test_forwarded_hosts(init_hosts: str | list[str], test_host: str, expected: ) async def test_proxy_headers_trusted_hosts(trusted_hosts: str | list[str], expected: str) -> None: async with make_httpx_client(trusted_hosts) as client: - response = await client.get("/", headers=make_x_headers("1.2.3.4")) + headers = { + X_FORWARDED_FOR: "1.2.3.4", + X_FORWARDED_PROTO: "https", + } + response = await client.get("/", headers=headers) assert response.status_code == 200 assert response.text == expected @pytest.mark.anyio @pytest.mark.parametrize( - ("header_args", "expected"), + ("forwarded_for", "forwarded_proto", "expected"), [ - (("", ""), "http://127.0.0.1:123"), - (("", None), "http://127.0.0.1:123"), - (("", "asdf"), "http://127.0.0.1:123"), - ((" , ",), "https://127.0.0.1:123"), - ((", , ",), "https://127.0.0.1:123"), - ((" , 10.0.0.1",), "https://127.0.0.1:123"), - (("9.9.9.9 , , , 10.0.0.1",), "https://127.0.0.1:123"), - ((", , 9.9.9.9",), "https://9.9.9.9:0"), - ((", , 9.9.9.9, , ",), "https://127.0.0.1:123"), + ("", "", "http://127.0.0.1:123"), + ("", None, "http://127.0.0.1:123"), + ("", "asdf", "http://127.0.0.1:123"), + (" , ", "https", "https://127.0.0.1:123"), + (", , ", "https", "https://127.0.0.1:123"), + (" , 10.0.0.1", "https", "https://127.0.0.1:123"), + ("9.9.9.9 , , , 10.0.0.1", "https", "https://127.0.0.1:123"), + (", , 9.9.9.9", "https", "https://9.9.9.9:0"), + (", , 9.9.9.9, , ", "https", "https://127.0.0.1:123"), ], ) async def test_proxy_headers_trusted_hosts_malformed( - header_args: tuple[str | None, str | None] | tuple[str | None], + forwarded_for: str, + forwarded_proto: str | None, expected: str, ) -> None: async with make_httpx_client("127.0.0.1, 10.0.0.0/8") as client: - response = await client.get("/", headers=make_x_headers(*header_args)) + headers = {X_FORWARDED_FOR: forwarded_for} + if forwarded_proto is not None: + headers[X_FORWARDED_PROTO] = forwarded_proto + response = await client.get("/", headers=headers) assert response.status_code == 200 assert response.text == expected @@ -430,7 +423,11 @@ async def test_proxy_headers_trusted_hosts_malformed( ) async def test_proxy_headers_multiple_proxies(trusted_hosts: str | list[str], expected: str) -> None: async with make_httpx_client(trusted_hosts) as client: - response = await client.get("/", headers=make_x_headers("1.2.3.4, 10.0.2.1, 192.168.0.2")) + headers = { + X_FORWARDED_FOR: "1.2.3.4, 10.0.2.1, 192.168.0.2", + X_FORWARDED_PROTO: "https", + } + response = await client.get("/", headers=headers) assert response.status_code == 200 assert response.text == expected @@ -440,8 +437,8 @@ async def test_proxy_headers_invalid_x_forwarded_for() -> None: async with make_httpx_client("*") as client: headers = httpx.Headers( { - "X-Forwarded-Proto": "https", - "X-Forwarded-For": "1.2.3.4, \xf0\xfd\xfd\xfd, unix:, ::1", + X_FORWARDED_FOR: "1.2.3.4, \xf0\xfd\xfd\xfd, unix:, ::1", + X_FORWARDED_PROTO: "https", }, encoding="latin-1", ) @@ -486,9 +483,11 @@ async def websocket_app(scope: Scope, receive: ASGIReceiveCallable, send: ASGISe async with run_server(config): url = f"ws://127.0.0.1:{unused_tcp_port}" - async with websockets.client.connect( - url, extra_headers=make_x_headers("1.2.3.4", forwarded_proto) - ) as websocket: + headers = { + X_FORWARDED_FOR: "1.2.3.4", + X_FORWARDED_PROTO: forwarded_proto, + } + async with websockets.client.connect(url, extra_headers=headers) as websocket: data = await websocket.recv() assert data == expected @@ -498,6 +497,10 @@ async def test_proxy_headers_empty_x_forwarded_for() -> None: # fallback to the default behavior if x-forwarded-for is an empty list # https://github.com/encode/uvicorn/issues/1068#issuecomment-855371576 async with make_httpx_client("*") as client: - response = await client.get("/", headers=make_x_headers("")) + headers = { + X_FORWARDED_FOR: "", + X_FORWARDED_PROTO: "https", + } + response = await client.get("/", headers=headers) assert response.status_code == 200 assert response.text == "https://127.0.0.1:123"