diff --git a/hermes_cli/dashboard_auth/native_flow.py b/hermes_cli/dashboard_auth/native_flow.py index 2f464a38667..f7c471b9f02 100644 --- a/hermes_cli/dashboard_auth/native_flow.py +++ b/hermes_cli/dashboard_auth/native_flow.py @@ -84,6 +84,13 @@ _CODE_TTL_SECONDS = 120 # 2 minutes — generous for a slow local hop. # concurrent-login count for a single desktop user. _MAX_ENTRIES = 256 +# Per-IP cap on concurrent PENDING authorizations. /auth/native/authorize is a +# public (pre-auth) route, so without this a single unauthenticated spammer +# could fill the global store (600s TTL each) and lock out legitimate native +# logins for the pending window. A real desktop runs at most a couple of +# concurrent sign-ins from one address; 8 is generous. +_MAX_PENDING_PER_IP = 8 + _lock = threading.Lock() @@ -98,6 +105,7 @@ class _Pending: code_challenge: str # the DESKTOP's S256 challenge (cc_d), base64url no-pad redirect_uri: str # the desktop's loopback redirect (127.0.0.1:/...) client_state: str # the desktop's own ``state`` (echoed back on redirect) + client_ip: str # requester IP at authorize time (per-IP pending cap) expires_at: int @@ -157,6 +165,7 @@ def register_pending( code_challenge: str, redirect_uri: str, client_state: str, + client_ip: str = "", now: Optional[int] = None, ) -> str: """Stash a pending native authorization; return an opaque ``broker_state``. @@ -165,12 +174,17 @@ def register_pending( S256 challenge (``cc_d``) — we never see the verifier until redemption. ``redirect_uri`` is the desktop's loopback callback and ``client_state`` is the desktop's own CSRF ``state`` (echoed verbatim on the final redirect). + ``client_ip`` is the requester's address, used only for the per-IP pending + cap below. The returned ``broker_state`` is what the gateway threads through its OWN upstream PKCE round trip (inside the ``hermes_session_pkce`` cookie), so the callback can find this entry again via :func:`complete_pending`. - Raises ``NativeFlowError`` if the store is at capacity (fail closed). + Raises ``NativeFlowError`` if the store is at capacity or the caller's IP + already holds ``_MAX_PENDING_PER_IP`` live pending entries (fail closed — + this is a public pre-auth route, so one spammer must not be able to fill + the global store and deny sign-in to everyone else). """ now = int(time.time()) if now is None else now broker_state = secrets.token_urlsafe(32) @@ -178,10 +192,18 @@ def register_pending( _gc_locked(now) if not _capacity_ok_locked(): raise NativeFlowError("native-flow authorization store at capacity") + if client_ip and ( + sum(1 for v in _pending.values() if v.client_ip == client_ip) + >= _MAX_PENDING_PER_IP + ): + raise NativeFlowError( + "too many pending native authorizations from this address" + ) _pending[broker_state] = _Pending( code_challenge=code_challenge, redirect_uri=redirect_uri, client_state=client_state, + client_ip=client_ip, expires_at=now + _PENDING_TTL_SECONDS, ) return broker_state diff --git a/hermes_cli/dashboard_auth/routes.py b/hermes_cli/dashboard_auth/routes.py index ed8bcad9076..58290fddc98 100644 --- a/hermes_cli/dashboard_auth/routes.py +++ b/hermes_cli/dashboard_auth/routes.py @@ -255,7 +255,10 @@ def _validate_loopback_redirect_uri(raw: str) -> str: RFC 8252 §7.3 restricts native-app redirects to the loopback interface. We accept only ``http://127.0.0.1[:port]/...`` and ``http://[::1][:port]/...`` - (and the literal ``localhost`` host, which some OS browsers normalise). + — literal loopback IPs. ``localhost`` is deliberately NOT accepted + (RFC 8252 §8.3: the name can resolve to a non-loopback address via the + hosts file or a hostile resolver, so clients "SHOULD use loopback IP + literals"; the desktop always sends ``127.0.0.1``). A non-loopback host would let an attacker who can reach ``/auth/native/ authorize`` (a public route) turn the gateway's authenticated callback into an open redirect that leaks a live authorization code to an @@ -272,10 +275,13 @@ def _validate_loopback_redirect_uri(raw: str) -> str: detail="native redirect_uri must be http:// on the loopback interface", ) host = (parsed.hostname or "").lower() - if host not in ("127.0.0.1", "::1", "localhost"): + if host not in ("127.0.0.1", "::1"): raise HTTPException( status_code=400, - detail="native redirect_uri host must be loopback (127.0.0.1 / ::1)", + detail=( + "native redirect_uri host must be a loopback IP literal " + "(127.0.0.1 / ::1)" + ), ) return raw @@ -338,6 +344,7 @@ async def auth_native_authorize( code_challenge=code_challenge, redirect_uri=redirect_uri, client_state=state, + client_ip=_client_ip(request), ) except native_flow.NativeFlowError as e: raise HTTPException(status_code=503, detail=str(e)) diff --git a/tests/hermes_cli/test_dashboard_auth_native_flow.py b/tests/hermes_cli/test_dashboard_auth_native_flow.py index a13c997bd62..d0d7c8262a9 100644 --- a/tests/hermes_cli/test_dashboard_auth_native_flow.py +++ b/tests/hermes_cli/test_dashboard_auth_native_flow.py @@ -190,6 +190,44 @@ def test_broker_capacity_fails_closed(): ) +def test_broker_per_ip_pending_cap(): + """One address cannot hog the pending store (public pre-auth route).""" + _verifier, challenge = _make_pkce() + for _ in range(native_flow._MAX_PENDING_PER_IP): + native_flow.register_pending( + code_challenge=challenge, redirect_uri="http://127.0.0.1:1/cb", + client_state="s", client_ip="203.0.113.7", + ) + # The capped IP is refused... + with pytest.raises(native_flow.NativeFlowError): + native_flow.register_pending( + code_challenge=challenge, redirect_uri="http://127.0.0.1:1/cb", + client_state="s", client_ip="203.0.113.7", + ) + # ...while a different address still signs in fine. + assert native_flow.register_pending( + code_challenge=challenge, redirect_uri="http://127.0.0.1:1/cb", + client_state="s", client_ip="198.51.100.9", + ) + + +def test_broker_per_ip_cap_frees_on_expiry(): + """Expired pending entries stop counting against the per-IP cap.""" + _verifier, challenge = _make_pkce() + now = int(time.time()) + for _ in range(native_flow._MAX_PENDING_PER_IP): + native_flow.register_pending( + code_challenge=challenge, redirect_uri="http://127.0.0.1:1/cb", + client_state="s", client_ip="203.0.113.7", now=now, + ) + # Past the pending TTL the old entries are GC'd and the IP can retry. + assert native_flow.register_pending( + code_challenge=challenge, redirect_uri="http://127.0.0.1:1/cb", + client_state="s", client_ip="203.0.113.7", + now=now + native_flow._PENDING_TTL_SECONDS + 1, + ) + + # --------------------------------------------------------------------------- # Route-level E2E against StubAuthProvider # --------------------------------------------------------------------------- @@ -314,6 +352,24 @@ def test_native_authorize_rejects_non_loopback_redirect(gated_client): assert "loopback" in r.json()["detail"].lower() +def test_native_authorize_rejects_localhost_name(gated_client): + """RFC 8252 §8.3 — loopback IP literals only; `localhost` can be + re-pointed via the hosts file / a hostile resolver.""" + _verifier, challenge = _make_pkce() + r = gated_client.get( + "/auth/native/authorize", + params={ + "provider": "stub", + "code_challenge": challenge, + "code_challenge_method": "S256", + "redirect_uri": "http://localhost:53999/cb", + "state": "s", + }, + ) + assert r.status_code == 400 + assert "loopback" in r.json()["detail"].lower() + + def test_native_authorize_requires_s256(gated_client): _verifier, challenge = _make_pkce() r = gated_client.get(