diff --git a/README.md b/README.md index f5b0b48..17d5da8 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ If ConnectHub sits behind a reverse proxy or tunnel (Cloudflare Tunnel, Traefik, TRUSTED_PROXIES=172.18.0.0/16 ``` -Without it, failed logins from anyone count against everyone: ten wrong passwords lock all visitors out for 15 minutes, and logs show the proxy's address. Only list addresses that are proxies: a listed address can claim to forward for any visitor. +Without it, failed logins from anyone count against everyone: ten wrong passwords lock all visitors out for 15 minutes, and logs show the proxy's address. It also lets `COOKIE_SECURE=auto` see that the proxy serves HTTPS when it talks plain HTTP to ConnectHub, so the session cookie is marked `Secure`. Only list addresses that are proxies: a listed address can claim to forward for any visitor. ### Which pages may open the stream diff --git a/root/app/backend/auth.py b/root/app/backend/auth.py index c45c0a4..292a85a 100644 --- a/root/app/backend/auth.py +++ b/root/app/backend/auth.py @@ -345,6 +345,11 @@ def _cookie_secure(request: Optional[Request]) -> bool: if request is None: return False proto = request.headers.get("X-Forwarded-Proto", request.url.scheme) + # nginx sets X-Forwarded-Proto to its own scheme, which is http when a TLS-terminating + # proxy or tunnel talks plain HTTP to it. From a trusted proxy, use what that proxy received. + upstream = request.headers.get("X-Proxy-Forwarded-Proto") + if upstream and _in_networks(peer_ip(request), PROXY_NETS): + proto = upstream return proto.split(",")[0].strip().lower() == "https" def create_session_cookie(response: Response, user_data: dict, request: Optional[Request] = None): diff --git a/root/defaults/default.conf b/root/defaults/default.conf index 404b127..bb37818 100644 --- a/root/defaults/default.conf +++ b/root/defaults/default.conf @@ -65,6 +65,8 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + # What a proxy in front of us received; the backend only believes it from TRUSTED_PROXIES + proxy_set_header X-Proxy-Forwarded-Proto $http_x_forwarded_proto; proxy_read_timeout 300s; } @@ -75,6 +77,7 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Proxy-Forwarded-Proto $http_x_forwarded_proto; } location /stream/ { diff --git a/tests/test_auth.py b/tests/test_auth.py index 04514dd..874586c 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -299,7 +299,8 @@ def proxied(headers): """A request as nginx hands it on: X-Real-IP is the address that connected to nginx.""" from starlette.requests import Request raw = [(k.lower().encode(), v.encode()) for k, v in headers.items()] - return Request({"type": "http", "headers": raw, "client": ("127.0.0.1", 0)}) + return Request({"type": "http", "headers": raw, "client": ("127.0.0.1", 0), "scheme": "http", + "server": ("127.0.0.1", 8000), "path": "/auth/login", "query_string": b""}) @pytest.mark.parametrize("trusted, headers, expected", [ @@ -333,3 +334,28 @@ def test_forward_auth_trusts_the_peer_not_a_forwarded_address(auth, monkeypatch) with pytest.raises(auth.HTTPException) as exc: auth.get_current_user(request) assert exc.value.status_code == 401 + + +# ----------------- Secure cookie behind a TLS-terminating proxy ----------------- + +@pytest.mark.parametrize("trusted, headers, secure", [ + # direct over HTTPS (port 3001) + ("", {"X-Real-IP": "203.0.113.9", "X-Forwarded-Proto": "https"}, True), + # tunnel terminates TLS and talks HTTP to port 3000: nginx says http, the tunnel said https + ("172.21.0.0/16", {"X-Real-IP": "172.21.0.4", "X-Forwarded-Proto": "http", + "X-Proxy-Forwarded-Proto": "https"}, True), + # the same without TRUSTED_PROXIES: the forwarded scheme isn't believed + ("", {"X-Real-IP": "172.21.0.4", "X-Forwarded-Proto": "http", "X-Proxy-Forwarded-Proto": "https"}, False), + # a direct visitor can't claim https + ("172.21.0.0/16", {"X-Real-IP": "203.0.113.9", "X-Forwarded-Proto": "http", + "X-Proxy-Forwarded-Proto": "https"}, False), + # trusted proxy that received plain http + ("172.21.0.0/16", {"X-Real-IP": "172.21.0.4", "X-Forwarded-Proto": "http", + "X-Proxy-Forwarded-Proto": "http"}, False), + # trusted proxy that sent no scheme: nginx's + ("172.21.0.0/16", {"X-Real-IP": "172.21.0.4", "X-Forwarded-Proto": "https"}, True), +]) +def test_cookie_secure_auto_behind_a_proxy(auth, monkeypatch, trusted, headers, secure): + monkeypatch.setattr(auth, "COOKIE_SECURE", "auto") + monkeypatch.setattr(auth, "PROXY_NETS", auth._parse_networks(trusted)) + assert auth._cookie_secure(proxied(headers)) is secure