From f439d0e6be4d053dcd71a7fc69068560b8510dd6 Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 21:45:45 -0300 Subject: [PATCH 1/7] Create setting/flag forwarded-port to enable/disable X-Forwarded-Port to populate remote address info (closes #1974) --- docs/deployment.md | 2 ++ docs/index.md | 2 ++ docs/settings.md | 1 + tests/middleware/test_proxy_headers.py | 22 +++++++++++++++++++++- uvicorn/config.py | 5 ++++- uvicorn/main.py | 6 ++++++ uvicorn/middleware/proxy_headers.py | 13 ++++++++++--- 7 files changed, 46 insertions(+), 5 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index e1854deff..85a3f368f 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -99,6 +99,8 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. + --forwarded-port / --no-forwarded-port + Enable/Disable X-Forwarded-Port handling. --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/index.md b/docs/index.md index 6ea346c0e..825954908 100644 --- a/docs/index.md +++ b/docs/index.md @@ -169,6 +169,8 @@ Options: to the $FORWARDED_ALLOW_IPS environment variable if available, or '127.0.0.1'. The literal '*' means trust everything. + --forwarded-port / --no-forwarded-port + Enable/Disable X-Forwarded-Port handling. --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 a4439c3d0..0db7dcb23 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 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-port` / `--no-forwarded-port` - Enable/Disable X-Forwarded-Port to populate remote address info. Defaults to disabled. * `--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 a2cbde775..435809b90 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -22,6 +22,7 @@ X_FORWARDED_FOR = "X-Forwarded-For" X_FORWARDED_PROTO = "X-Forwarded-Proto" +X_FORWARDED_PORT = "X-Forwarded-Port" async def default_app(scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable) -> None: @@ -39,6 +40,7 @@ async def default_app(scope: Scope, receive: ASGIReceiveCallable, send: ASGISend def make_httpx_client( trusted_hosts: str | list[str], client: tuple[str, int] = ("127.0.0.1", 123), + forwarded_port: bool = False, ) -> httpx.AsyncClient: """Create async client for use in test cases. @@ -47,7 +49,7 @@ def make_httpx_client( client: transport client to use """ - app = ProxyHeadersMiddleware(default_app, trusted_hosts) + app = ProxyHeadersMiddleware(default_app, trusted_hosts, forwarded_port) transport = httpx.ASGITransport(app=app, client=client) # type: ignore return httpx.AsyncClient(transport=transport, base_url="http://testserver") @@ -422,6 +424,24 @@ async def test_proxy_headers_multiple_proxies(trusted_hosts: str | list[str], ex assert response.text == expected +@pytest.mark.anyio +@pytest.mark.parametrize( + ("forwarded_port", "port", "expected"), + [ + (True, "443", 443), + (True, "1234", 1234), + (False, "1234", 0), + (False, "443", 0), + ] +) +async def test_proxy_headers_with_forwarded_port(forwarded_port, port, expected) -> None: + async with make_httpx_client("*", forwarded_port=forwarded_port) as client: + headers = {X_FORWARDED_FOR: "192.168.0.2", X_FORWARDED_PROTO: "https", X_FORWARDED_PORT: port} + response = await client.get("/", headers=headers) + assert response.status_code == 200 + assert response.text == f"https://192.168.0.2:{expected}" + + @pytest.mark.anyio async def test_proxy_headers_invalid_x_forwarded_for() -> None: async with make_httpx_client("*") as client: diff --git a/uvicorn/config.py b/uvicorn/config.py index 9aff8c968..d28f94e65 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -205,6 +205,7 @@ def __init__( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, + forwarded_port: bool = False, root_path: str = "", limit_concurrency: int | None = None, limit_max_requests: int | None = None, @@ -331,8 +332,10 @@ def __init__( self.forwarded_allow_ips: list[str] | str if forwarded_allow_ips is None: self.forwarded_allow_ips = os.environ.get("FORWARDED_ALLOW_IPS", "127.0.0.1") + self.forwarded_port = False else: self.forwarded_allow_ips = forwarded_allow_ips # pragma: full coverage + self.forwarded_port = forwarded_port if self.reload and self.workers > 1: logger.warning('"workers" flag is ignored when reloading is enabled.') @@ -467,7 +470,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, forwarded_port=self.forwarded_port) self.loaded = True diff --git a/uvicorn/main.py b/uvicorn/main.py index 43956622d..0af2b903a 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -245,6 +245,12 @@ 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-port/--no-forwarded-port", + is_flag=True, + default=True, + help="Enable/Disable X-Forwarded-Port to " "populate remote address info.", +) @click.option( "--root-path", type=str, diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index ce4fd8c01..6c4938635 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -20,9 +20,11 @@ class ProxyHeadersMiddleware: - """ - def __init__(self, app: ASGI3Application, trusted_hosts: list[str] | str = "127.0.0.1") -> None: + def __init__(self, app: ASGI3Application, trusted_hosts: list[str] | str = "127.0.0.1", + forwarded_port: bool = False) -> None: self.app = app self.trusted_hosts = _TrustedHosts(trusted_hosts) + self.forwarded_port = forwarded_port async def __call__(self, scope: Scope, receive: ASGIReceiveCallable, send: ASGISendCallable) -> None: if scope["type"] == "lifespan": @@ -53,8 +55,13 @@ async def __call__(self, scope: Scope, receive: ASGIReceiveCallable, send: ASGIS # See: https://github.com/encode/uvicorn/issues/1068 # We've lost the connecting client's port information by now, - # so only include the host. - port = 0 + # so unless X-Forwarded-Port is available and --forwarded-port is enabled, + # only include the host. + if self.forwarded_port and b"x-forwarded-port" in headers: + x_forwarded_port = headers[b"x-forwarded-port"].decode("latin1") + port = int(x_forwarded_port) + else: + port = 0 scope["client"] = (host, port) return await self.app(scope, receive, send) From 41f4638451c73b886acd191637e4733df537be4e Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 21:52:56 -0300 Subject: [PATCH 2/7] Fix code formatting --- tests/middleware/test_proxy_headers.py | 2 +- uvicorn/config.py | 4 +++- uvicorn/middleware/proxy_headers.py | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py index 435809b90..5af3a58fc 100644 --- a/tests/middleware/test_proxy_headers.py +++ b/tests/middleware/test_proxy_headers.py @@ -432,7 +432,7 @@ async def test_proxy_headers_multiple_proxies(trusted_hosts: str | list[str], ex (True, "1234", 1234), (False, "1234", 0), (False, "443", 0), - ] + ], ) async def test_proxy_headers_with_forwarded_port(forwarded_port, port, expected) -> None: async with make_httpx_client("*", forwarded_port=forwarded_port) as client: diff --git a/uvicorn/config.py b/uvicorn/config.py index d28f94e65..4a1d13d79 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -470,7 +470,9 @@ 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, forwarded_port=self.forwarded_port) + self.loaded_app = ProxyHeadersMiddleware( + self.loaded_app, trusted_hosts=self.forwarded_allow_ips, forwarded_port=self.forwarded_port + ) self.loaded = True diff --git a/uvicorn/middleware/proxy_headers.py b/uvicorn/middleware/proxy_headers.py index 6c4938635..84b2f56ae 100644 --- a/uvicorn/middleware/proxy_headers.py +++ b/uvicorn/middleware/proxy_headers.py @@ -20,8 +20,9 @@ class ProxyHeadersMiddleware: - """ - def __init__(self, app: ASGI3Application, trusted_hosts: list[str] | str = "127.0.0.1", - forwarded_port: bool = False) -> None: + def __init__( + self, app: ASGI3Application, trusted_hosts: list[str] | str = "127.0.0.1", forwarded_port: bool = False + ) -> None: self.app = app self.trusted_hosts = _TrustedHosts(trusted_hosts) self.forwarded_port = forwarded_port From 212d43f2d9206352725d107353debfd1b5984d98 Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 21:55:20 -0300 Subject: [PATCH 3/7] Fix docs formatting --- docs/deployment.md | 3 ++- docs/index.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 85a3f368f..51d001f5d 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -100,7 +100,8 @@ Options: variable if available, or '127.0.0.1'. The literal '*' means trust everything. --forwarded-port / --no-forwarded-port - Enable/Disable X-Forwarded-Port handling. + Enable/Disable X-Forwarded-Port to populate + remote address info. --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/index.md b/docs/index.md index 825954908..e4a383737 100644 --- a/docs/index.md +++ b/docs/index.md @@ -170,7 +170,8 @@ Options: variable if available, or '127.0.0.1'. The literal '*' means trust everything. --forwarded-port / --no-forwarded-port - Enable/Disable X-Forwarded-Port handling. + Enable/Disable X-Forwarded-Port to populate + remote address info. --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 ec18d10c743035f29cb6186c10f00f61df4a0ca7 Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 21:59:34 -0300 Subject: [PATCH 4/7] Add missing parameter --- uvicorn/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/uvicorn/main.py b/uvicorn/main.py index 0af2b903a..034788722 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -497,6 +497,7 @@ def run( server_header: bool = True, date_header: bool = True, forwarded_allow_ips: list[str] | str | None = None, + forwarded_port: bool = False, root_path: str = "", limit_concurrency: int | None = None, backlog: int = 2048, From 0bdbeef2f3087b349eeb59dbdce388d4f7820f2e Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 22:11:46 -0300 Subject: [PATCH 5/7] Add missing parameter --- uvicorn/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/uvicorn/main.py b/uvicorn/main.py index 034788722..3aaeadd67 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -396,6 +396,7 @@ def main( server_header: bool, date_header: bool, forwarded_allow_ips: str, + forwarded_port: bool, root_path: str, limit_concurrency: int, backlog: int, @@ -445,6 +446,7 @@ def main( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, + forwarded_port=forwarded_port, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, @@ -550,6 +552,7 @@ def run( server_header=server_header, date_header=date_header, forwarded_allow_ips=forwarded_allow_ips, + forwarded_port=forwarded_port, root_path=root_path, limit_concurrency=limit_concurrency, backlog=backlog, From c27c2a138096e89ac4b8694b954370580c12a87c Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Sun, 29 Sep 2024 22:20:38 -0300 Subject: [PATCH 6/7] Refactor --- uvicorn/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uvicorn/config.py b/uvicorn/config.py index 4a1d13d79..4de6abbcf 100644 --- a/uvicorn/config.py +++ b/uvicorn/config.py @@ -332,10 +332,10 @@ def __init__( self.forwarded_allow_ips: list[str] | str if forwarded_allow_ips is None: self.forwarded_allow_ips = os.environ.get("FORWARDED_ALLOW_IPS", "127.0.0.1") - self.forwarded_port = False else: self.forwarded_allow_ips = forwarded_allow_ips # pragma: full coverage - self.forwarded_port = forwarded_port + + self.forwarded_port = forwarded_port if self.reload and self.workers > 1: logger.warning('"workers" flag is ignored when reloading is enabled.') From 2164cc54f953a6be799ea711ca443cd2639cb5e7 Mon Sep 17 00:00:00 2001 From: Victor Torres Date: Mon, 30 Sep 2024 10:14:40 -0300 Subject: [PATCH 7/7] Fix command argument default --- uvicorn/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/uvicorn/main.py b/uvicorn/main.py index 3aaeadd67..d134f7288 100644 --- a/uvicorn/main.py +++ b/uvicorn/main.py @@ -222,7 +222,7 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No "--proxy-headers/--no-proxy-headers", is_flag=True, default=True, - help="Enable/Disable X-Forwarded-Proto, X-Forwarded-For, X-Forwarded-Port to " "populate remote address info.", + help="Enable/Disable X-Forwarded-Proto, X-Forwarded-For, X-Forwarded-Port to populate remote address info.", ) @click.option( "--server-header/--no-server-header", @@ -248,8 +248,8 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No @click.option( "--forwarded-port/--no-forwarded-port", is_flag=True, - default=True, - help="Enable/Disable X-Forwarded-Port to " "populate remote address info.", + default=False, + help="Enable/Disable X-Forwarded-Port to populate remote address info.", ) @click.option( "--root-path",