Skip to content

Commit

Permalink
Add ProxyFix middleware
Browse files Browse the repository at this point in the history
This allows for Hypercorn to be used behind a proxy with the headers
being "fixed" such that the proxy is not present as far as the app is
concerned. This makes it easier to write applications that run behind
proxies.

Note I've defaulted to legacy mode as AWS's load balancers don't
support the modern Forwarded header and I assume that makes up a large
percentage of real world usage.
  • Loading branch information
pgjones committed Dec 28, 2023
1 parent 2d2c62b commit 4fc0372
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/how_to_guides/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ How to guides
dispatch_apps.rst
http_https_redirect.rst
logging.rst
proxy_fix.rst
server_names.rst
statsd.rst
wsgi_apps.rst
33 changes: 33 additions & 0 deletions docs/how_to_guides/proxy_fix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Fixing proxy headers
====================

If you are serving Hypercorn behind a proxy e.g. a load balancer the
client-address, scheme, and host-header will match that of the
connection between the proxy and Hypercorn rather than the user-agent
(client). However, most proxies provide headers with the original
user-agent (client) values which can be used to "fix" the headers to
these values.

Modern proxies should provide this information via a ``Forwarded``
header from `RFC 7239
<https://datatracker.ietf.org/doc/html/rfc7239>`_. However, this is
rare in practice with legacy proxies using a combination of
``X-Forwarded-For``, ``X-Forwarded-Proto`` and
``X-Forwarded-Host``. It is important that you chose the correct mode
(legacy, or modern) based on the proxy you use.

To use the proxy fix middleware behind a single legacy proxy simply
wrap your app and serve the wrapped app,

.. code-block:: python
from hypercorn.middleware import ProxyFixMiddleware
fixed_app = ProxyFixMiddleware(app, mode="legacy", trusted_hops=1)
.. warning::

The mode and number of trusted hops must match your setup or the
user-agent (client) may be trusted and hence able to set
alternative for, proto, and host values. This can, depending on
your usage in the app, lead to security vulnerabilities.
2 changes: 2 additions & 0 deletions src/hypercorn/middleware/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

from .dispatcher import DispatcherMiddleware
from .http_to_https import HTTPToHTTPSRedirectMiddleware
from .proxy_fix import ProxyFixMiddleware
from .wsgi import AsyncioWSGIMiddleware, TrioWSGIMiddleware

__all__ = (
"AsyncioWSGIMiddleware",
"DispatcherMiddleware",
"HTTPToHTTPSRedirectMiddleware",
"ProxyFixMiddleware",
"TrioWSGIMiddleware",
)
78 changes: 78 additions & 0 deletions src/hypercorn/middleware/proxy_fix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
from __future__ import annotations

from copy import deepcopy
from typing import Callable, Iterable, Literal, Optional, Tuple

from ..typing import ASGIFramework, Scope


class ProxyFixMiddleware:
def __init__(
self,
app: ASGIFramework,
mode: Literal["legacy", "modern"] = "legacy",
trusted_hops: int = 1,
) -> None:
self.app = app
self.mode = mode
self.trusted_hops = trusted_hops

async def __call__(self, scope: Scope, receive: Callable, send: Callable) -> None:
if scope["type"] in {"http", "websocket"}:
scope = deepcopy(scope)
headers = scope["headers"] # type: ignore
client: Optional[str] = None
scheme: Optional[str] = None
host: Optional[str] = None

if (
self.mode == "modern"
and (value := _get_trusted_value(b"forwarded", headers, self.trusted_hops))
is not None
):
for part in value.split(";"):
if part.startswith("for="):
client = part[4:].strip()
elif part.startswith("host="):
host = part[5:].strip()
elif part.startswith("proto="):
scheme = part[6:].strip()

else:
client = _get_trusted_value(b"x-forwarded-for", headers, self.trusted_hops)
scheme = _get_trusted_value(b"x-forwarded-proto", headers, self.trusted_hops)
host = _get_trusted_value(b"x-forwarded-host", headers, self.trusted_hops)

if client is not None:
scope["client"] = (client, 0) # type: ignore

if scheme is not None:
scope["scheme"] = scheme # type: ignore

if host is not None:
headers = [
(name, header_value)
for name, header_value in headers
if name.lower() != b"host"
]
headers.append((b"host", host))
scope["headers"] = headers # type: ignore

await self.app(scope, receive, send)


def _get_trusted_value(
name: bytes, headers: Iterable[Tuple[bytes, bytes]], trusted_hops: int
) -> Optional[str]:
if trusted_hops == 0:
return None

values = []
for header_name, header_value in headers:
if header_name.lower() == name:
values.extend([value.decode("latin1").strip() for value in header_value.split(b",")])

if len(values) >= trusted_hops:
return values[-trusted_hops]

return None
64 changes: 64 additions & 0 deletions tests/middleware/test_proxy_fix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
from __future__ import annotations

from unittest.mock import AsyncMock

import pytest

from hypercorn.middleware import ProxyFixMiddleware
from hypercorn.typing import HTTPScope


@pytest.mark.asyncio
async def test_proxy_fix_legacy() -> None:
mock = AsyncMock()
app = ProxyFixMiddleware(mock)
scope: HTTPScope = {
"type": "http",
"asgi": {},
"http_version": "2",
"method": "GET",
"scheme": "http",
"path": "/",
"raw_path": b"/",
"query_string": b"",
"root_path": "",
"headers": [
(b"x-forwarded-for", b"127.0.0.1"),
(b"x-forwarded-for", b"127.0.0.2"),

This comment has been minimized.

Copy link
@apollo13

apollo13 Dec 31, 2023

Contributor

Hi @pgjones, I am writing here because I don't think this is a security issue yet since it is not in a released version. I don't think that there is any guarantee that proxies add x-forwarded-for in order. This means that you cannot rely on the order for hop detection. From a security point I would error out / ignore the header if it exists multiple times and only support it when sent as a single header.

It would also be great to be able to configure this via configuration options instead of wrapping the app.

Another option would be to support an ip range to trust those headers from, this can be useful when an app is reached via a proxy as well as directly.

This comment has been minimized.

Copy link
@pgjones

pgjones Jan 1, 2024

Author Owner

I think it is guaranteed, given my understanding of https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For (especially the parsing section). This is also what Werkzeug has been doing for a number of years.

This comment has been minimized.

Copy link
@apollo13

apollo13 Jan 1, 2024

Contributor

Yes, your understanding of the mdn page is correct. I guess I am just a little bit conservative when it comes to security and given that X-Forwarded-* has no spec, who knows what servers implemented :) Anyways I'll try to wipe up a PR for supporting trusted_cids as alternative to trust_hops, shouldn't be to hard and would be a nice improvement.

(b"x-forwarded-proto", b"http,https"),
],
"client": ("127.0.0.3", 80),
"server": None,
"extensions": {},
}
await app(scope, None, None)
mock.assert_called()
assert mock.call_args[0][0]["client"] == ("127.0.0.2", 0)
assert mock.call_args[0][0]["scheme"] == "https"


@pytest.mark.asyncio
async def test_proxy_fix_modern() -> None:
mock = AsyncMock()
app = ProxyFixMiddleware(mock, mode="modern")
scope: HTTPScope = {
"type": "http",
"asgi": {},
"http_version": "2",
"method": "GET",
"scheme": "http",
"path": "/",
"raw_path": b"/",
"query_string": b"",
"root_path": "",
"headers": [
(b"forwarded", b"for=127.0.0.1;proto=http,for=127.0.0.2;proto=https"),
],
"client": ("127.0.0.3", 80),
"server": None,
"extensions": {},
}
await app(scope, None, None)
mock.assert_called()
assert mock.call_args[0][0]["client"] == ("127.0.0.2", 0)
assert mock.call_args[0][0]["scheme"] == "https"

0 comments on commit 4fc0372

Please sign in to comment.