From 603f5fb84899fd24a94fc869006ab3c985578e84 Mon Sep 17 00:00:00 2001 From: Boger Date: Tue, 17 Mar 2020 12:19:08 +0300 Subject: [PATCH 1/4] Fix ProxyHeadersMiddleware, now it grabs the first IP from `x-forwarded-for` Also rewrite a few tests for increasing coverage for edge cases --- uvicorn/middleware/proxy_headers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 05f4f70e1..7ca956053 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -34,11 +34,11 @@ async def __call__(self, scope, receive, send): scope["scheme"] = x_forwarded_proto.strip() if b"x-forwarded-for" in headers: - # Determine the client address from the last trusted IP in the + # Determine the client address from the first 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 = x_forwarded_for.split(",")[-1].strip() + host = x_forwarded_for.split(",")[0].strip() port = 0 scope["client"] = (host, port) From b4501724cd8a35113875c479924d1f57c6a8d6ff Mon Sep 17 00:00:00 2001 From: Boger Date: Sun, 22 Mar 2020 11:36:37 +0300 Subject: [PATCH 2/4] Add searching algorithm for the trusted client host in x-forwarded-for --- uvicorn/middleware/proxy_headers.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 7ca956053..36c7ebb7b 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -14,11 +14,21 @@ class ProxyHeadersMiddleware: def __init__(self, app, trusted_hosts="127.0.0.1"): self.app = app if isinstance(trusted_hosts, str): - self.trusted_hosts = [item.strip() for item in trusted_hosts.split(",")] + self.trusted_hosts = {item.strip() for item in trusted_hosts.split(",")} else: - self.trusted_hosts = trusted_hosts + self.trusted_hosts = set(trusted_hosts) self.always_trust = "*" in self.trusted_hosts + def get_trusted_client_host( + self, x_forwarded_for_hosts + ): # type: (List[str]) -> 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 + async def __call__(self, scope, receive, send): if scope["type"] in ("http", "websocket"): client_addr = scope.get("client") @@ -34,11 +44,14 @@ async def __call__(self, scope, receive, send): scope["scheme"] = x_forwarded_proto.strip() if b"x-forwarded-for" in headers: - # Determine the client address from the first trusted IP in the + # 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 = x_forwarded_for.split(",")[0].strip() + x_forwarded_for_hosts = [ + item.strip() for item in x_forwarded_for.split(",") + ] + host = self.get_trusted_client_host(x_forwarded_for_hosts) port = 0 scope["client"] = (host, port) From ce7a211d10787485d71c06695b56fb1eae00d692 Mon Sep 17 00:00:00 2001 From: Dima Boger Date: Sat, 13 Mar 2021 16:09:24 +0300 Subject: [PATCH 3/4] Fix issues after rebasing --- tests/middleware/test_proxy_headers.py | 69 +++++++++++++++++++++----- uvicorn/middleware/proxy_headers.py | 1 + 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index d20adcd88..6cf20db19 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -13,34 +13,79 @@ async def app(scope, receive, send): await response(scope, receive, send) -app = ProxyHeadersMiddleware(app, trusted_hosts="*") - - @pytest.mark.asyncio -async def test_proxy_headers(): - async with httpx.AsyncClient(app=app, base_url="http://testserver") as client: +@pytest.mark.parametrize( + ("trusted_hosts", "response_text"), + [ + # always trust + ("*", "Remote: 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"), + # 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"), + # request from untrusted proxy + ("192.168.0.1", "Remote: http://127.0.0.1:123"), + ], +) +async def test_proxy_headers_trusted_hosts(trusted_hosts, response_text): + 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) + assert response.status_code == 200 - assert response.text == "Remote: https://1.2.3.4:0" + assert response.text == response_text @pytest.mark.asyncio -async def test_proxy_headers_no_port(): - async with httpx.AsyncClient(app=app, base_url="http://testserver") as client: - headers = {"X-Forwarded-Proto": "https", "X-Forwarded-For": "1.2.3.4"} +@pytest.mark.parametrize( + ("trusted_hosts", "response_text"), + [ + # always trust + ("*", "Remote: 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", + ), + # order isn't matter + ( + ["10.0.2.1", "192.168.0.2", "127.0.0.1"], + "Remote: 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"), + ], +) +async def test_proxy_headers_multiple_proxies(trusted_hosts, response_text): + 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) + assert response.status_code == 200 - assert response.text == "Remote: https://1.2.3.4:0" + assert response.text == response_text @pytest.mark.asyncio async def test_proxy_headers_invalid_x_forwarded_for(): - async with httpx.AsyncClient(app=app, base_url="http://testserver") as client: + app_with_middleware = ProxyHeadersMiddleware(app, trusted_hosts="*") + async with httpx.AsyncClient( + app=app_with_middleware, base_url="http://testserver" + ) as client: headers = httpx.Headers( { "X-Forwarded-Proto": "https", - "X-Forwarded-For": "\xf0\xfd\xfd\xfd, 1.2.3.4", + "X-Forwarded-For": "1.2.3.4, \xf0\xfd\xfd\xfd", }, encoding="latin-1", ) diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 36c7ebb7b..23901bd73 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -8,6 +8,7 @@ https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Proxies """ +from typing import List class ProxyHeadersMiddleware: From 05f43a96b7962563738ee5caab89684eed0d79dd Mon Sep 17 00:00:00 2001 From: Dima Boger Date: Sat, 13 Mar 2021 16:45:33 +0300 Subject: [PATCH 4/4] Fix typo in comment Co-authored-by: euri10 --- 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 6cf20db19..aacf789d4 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -52,7 +52,7 @@ async def test_proxy_headers_trusted_hosts(trusted_hosts, response_text): ["127.0.0.1", "10.0.2.1", "192.168.0.2"], "Remote: https://1.2.3.4:0", ), - # order isn't matter + # order doesn't matter ( ["10.0.2.1", "192.168.0.2", "127.0.0.1"], "Remote: https://1.2.3.4:0",