Skip to content

Commit

Permalink
Remove "better UDS handling"
Browse files Browse the repository at this point in the history
  • Loading branch information
nhairs committed Feb 10, 2024
1 parent 8d25920 commit 3f345c6
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 118 deletions.
4 changes: 0 additions & 4 deletions docs/deployment.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ Note that WSGI mode always disables WebSocket support, as it is not supported by
## HTTP

* `--root-path <str>` - 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> 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.

Expand Down
65 changes: 1 addition & 64 deletions tests/middleware/test_proxy_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions uvicorn/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,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
Expand Down
7 changes: 0 additions & 7 deletions uvicorn/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
40 changes: 5 additions & 35 deletions uvicorn/middleware/proxy_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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=}"
")"
)


Expand All @@ -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:
- <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Proxies>
Expand All @@ -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__(
Expand All @@ -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.
Expand Down Expand Up @@ -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)

0 comments on commit 3f345c6

Please sign in to comment.