From 7bb9d8473b330af4267a56d1ba594f0bab6deddc Mon Sep 17 00:00:00 2001 From: AletheiaVox <264355139+AletheiaVox@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:55:16 +0200 Subject: [PATCH] fix(security): require auth on every MCP request; stop stale refreshes banning clients - Remove the authless "sole connected phone" fallback and SB_REQUIRE_MCP_AUTH: an unauthenticated request no longer reaches whichever phone is online alone. - Mcp-Session-Id is no longer a credential; the Bearer token is checked on every request (MCP auth spec). - Refresh tokens live 90 days (was 30, equal to the access token, so they were always dead when first needed). - A rejected refresh no longer counts toward the IP ban: a client with an expired token was retrying into a self-renewing ban on its own IP. - 401s carry the RFC 9728 WWW-Authenticate discovery header. --- .env.example | 4 ---- CHANGELOG.md | 24 +++++++++++++++++++++ README.md | 14 ++---------- server/app.py | 43 ++++++++++++++++--------------------- server/config.py | 3 ++- server/oauth.py | 7 +++++- server/oauth_routes.py | 30 ++++++++++++++++++++++++-- server/session_registry.py | 8 ------- tests/verify_server.py | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 124 insertions(+), 53 deletions(-) diff --git a/.env.example b/.env.example index 2bd66d7..e34a3f4 100644 --- a/.env.example +++ b/.env.example @@ -14,10 +14,6 @@ SB_PORT=8420 SB_REGISTRATION_OPEN=true # How long login tokens last (hours) SB_TOKEN_EXPIRY_HOURS=168 -# Set to "true" to require OAuth/token auth on every MCP request (multi-user -# servers). Default "false" keeps the single-user convenience fallback: an -# unauthenticated MCP request is routed to the sole connected phone session. -SB_REQUIRE_MCP_AUTH=false # ═══ Rate Limiting (anti-harassment) ════════════════════════════════════ # Auth endpoint: strict to prevent credential stuffing diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a88a8c..df85a7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,32 @@ ## Unreleased +### Security + +- **Removed the authless "sole connected phone" fallback.** With + `SB_REQUIRE_MCP_AUTH=false` (the old default), any unauthenticated MCP + request was routed to whichever phone was connected, as long as it was the + only one. On a shared server that means: a stranger connects while you're + the only one online and gets your hardware. Every MCP request now needs a + valid Bearer token (OAuth or login JWT). `SB_REQUIRE_MCP_AUTH` is gone; an + old `.env` that still sets it is harmless. +- **`Mcp-Session-Id` is no longer a credential.** A session ID used to stand + in for the token on follow-up requests, so it kept working after the + token expired or was revoked. The Bearer token is now checked on every + request, as the MCP auth spec requires. + ### Fixed +- **Clients no longer ban their own IP when their token expires.** Refresh + tokens lasted 30 days, the same as a 720-hour access token, and clients + only refresh once the access token is dead, so the refresh token was + always dead too. Every rejected refresh counted as a failed login, and a + client retrying on a timer reached `SB_BAN_THRESHOLD` in about half an + hour, then got banned again every time the ban lapsed. Refresh tokens now + last 90 days, and rotation issues a new one on every refresh. A rejected + refresh (the client has already proven its `client_secret`) no longer + counts toward the ban. Wrong passwords and wrong client secrets still do. + - **Disabling the governor now disables the governor.** `enabled` gated only `Governor.check()` — the enforcement call. The heat model kept integrating on every heartbeat, the state kept riding along on heartbeat pings, and diff --git a/README.md b/README.md index 0d5d06a..b78c939 100644 --- a/README.md +++ b/README.md @@ -163,10 +163,6 @@ SB_HOST=0.0.0.0 SB_PORT=8420 SB_REGISTRATION_OPEN=true SB_TOKEN_EXPIRY_HOURS=720 -# true = every MCP request must be authenticated (multi-user servers). -# false = single-user convenience: unauthenticated MCP requests go to the -# sole connected phone. -SB_REQUIRE_MCP_AUTH=false SB_HEARTBEAT_INTERVAL=2.0 SB_HEARTBEAT_TIMEOUT=6.0 SB_BAN_THRESHOLD=20 @@ -313,12 +309,6 @@ The server implements the full OAuth 2.0 flow that claude.ai custom connectors e 4. Save — claude.ai will open your server's login page 5. Sign in with the username and password you registered in step 1.4 -### Option C: claude.ai without login (single-user fallback) - -If you skip the OAuth login, the server falls back to routing unauthenticated MCP requests to the sole connected phone — convenient for a private single-user server. - -**Important**: The authless fallback only works when exactly one phone/relay client is connected to the server (and is disabled entirely when `SB_REQUIRE_MCP_AUTH=true`). If no phones are connected, claude.ai will show a connection error. Start your relay client first, then connect from claude.ai. - --- ## Part 4: Relay Client — Windows PC @@ -524,8 +514,8 @@ Too many rapid reconnection attempts. Restart the Docker container to clear in-m **Relay connects but finds 0 devices** Intiface Central can't see your Bluetooth devices. Make sure devices are turned on and in range. On Android, verify Location and Bluetooth permissions are granted to Intiface. -**claude.ai stuck on "checking connection"** -The authless fallback requires at least one relay client connected. Start your relay first, then add the connector in claude.ai. +**Connector keeps asking to reconnect / sign-in popup hangs** +Check whether your IP is banned: `curl -X POST https://your-server/mcp` answering `{"error":"Temporarily banned"}` means yes. Bans are in memory; restart the container to clear one. Since v1.2, expired refresh tokens no longer count toward the ban, so a client with a stale token can't ban its own IP any more. **Heartbeat timeout / disconnects after a few commands** Use the Termux v3 relay (`termux_relay_v3.py`), which processes commands in background tasks so heartbeat responses are never blocked. diff --git a/server/app.py b/server/app.py index 4e935a9..bbd549e 100644 --- a/server/app.py +++ b/server/app.py @@ -29,7 +29,7 @@ from .auth import ( ) from .mcp_tools import TOOLS, HANDLERS, current_user_id from .oauth import init_oauth_db -from .oauth_routes import router as oauth_router +from .oauth_routes import router as oauth_router, _base_url from .relay_hub import check_ws_ip_limit, release_ws_ip_slot, get_ip_from_headers from .session_registry import registry from .governor import governor @@ -55,7 +55,6 @@ async def lifespan(app: FastAPI): await dead_man_switch.start() log.info(f"Signal Bridge Remote started on {config.HOST}:{config.PORT}") log.info(f"Registration {'OPEN' if config.REGISTRATION_OPEN else 'CLOSED'}") - log.info(f"MCP auth {'REQUIRED' if config.REQUIRE_MCP_AUTH else 'optional (sole-phone fallback enabled)'}") yield await dead_man_switch.stop() log.info("Signal Bridge Remote shutting down") @@ -166,7 +165,7 @@ async def login(request: Request): # - POST: JSON-RPC requests from client # - GET: SSE stream for server-to-client notifications (kept open) # - Mcp-Session-Id header for session tracking -# - Authless mode for claude.ai connector, Bearer token for Claude Desktop +# - Bearer token (OAuth or login JWT) required on every request # ════════════════════════════════════════════════════════════════════════ # In-memory MCP session tracking (maps session_id → user_id) @@ -175,28 +174,16 @@ _mcp_sessions: dict[str, str] = {} async def _resolve_mcp_user(request: Request) -> dict | None: """ - Resolve the user for an MCP request. - Priority: Bearer token > Mcp-Session-Id lookup > sole active phone session. + Resolve the user for an MCP request: a valid Bearer token, on every + request, or nobody. + + There used to be two more paths here — an Mcp-Session-Id lookup and a + "sole connected phone" fallback for unauthenticated requests. Both are + gone: the fallback handed an anonymous caller whichever single phone was + online, and a session ID must not outlive or replace the token that + opened it (MCP auth spec: authorization on every HTTP request). """ - # 1. Try Bearer token auth (Claude Desktop) - user = await _require_auth(request) - if user: - return user - - # 2. Try Mcp-Session-Id (subsequent requests from claude.ai) - session_id = request.headers.get("mcp-session-id", "") - if session_id and session_id in _mcp_sessions: - return {"user_id": _mcp_sessions[session_id]} - - # 3. Fall back to sole active phone session (authless / claude.ai init) - # Disabled when SB_REQUIRE_MCP_AUTH=true (multi-user mode). - if not config.REQUIRE_MCP_AUTH: - fallback_user_id = await registry.get_sole_user_id() - if fallback_user_id: - log.info(f"MCP request without auth — using active session: {fallback_user_id}") - return {"user_id": fallback_user_id} - - return None + return await _require_auth(request) @app.post("/mcp") @@ -231,9 +218,15 @@ async def mcp_endpoint(request: Request): # Resolve user user = await _resolve_mcp_user(request) if not user: + # RFC 9728: the challenge header is how MCP clients discover where + # to start the OAuth flow — a bare 401 leaves them stranded. + base = _base_url(request) return JSONResponse( - {"jsonrpc": "2.0", "error": {"code": -32000, "message": "No auth token and no active phone session"}}, + {"jsonrpc": "2.0", "error": {"code": -32000, "message": "Authentication required: send a Bearer token (OAuth or login JWT)"}}, status_code=401, + headers={ + "WWW-Authenticate": f'Bearer resource_metadata="{base}/.well-known/oauth-protected-resource"' + }, ) # Rate limit per user for commands diff --git a/server/config.py b/server/config.py index 9cc0b03..18295c3 100644 --- a/server/config.py +++ b/server/config.py @@ -19,7 +19,8 @@ CORS_ORIGINS = os.getenv("SB_CORS_ORIGINS", "*").split(",") # ── Auth ──────────────────────────────────────────────────────────────── TOKEN_EXPIRY_HOURS = int(os.getenv("SB_TOKEN_EXPIRY_HOURS", "168")) # 1 week REGISTRATION_OPEN = os.getenv("SB_REGISTRATION_OPEN", "true").lower() == "true" -REQUIRE_MCP_AUTH = os.getenv("SB_REQUIRE_MCP_AUTH", "false").lower() == "true" +# SB_REQUIRE_MCP_AUTH is gone: MCP auth is always required. An old .env +# that still sets it is harmless. # ── Rate Limiting ─────────────────────────────────────────────────────── # Format: "count/period" — e.g. "5/minute", "100/hour" diff --git a/server/oauth.py b/server/oauth.py index 63c0978..3d666f1 100644 --- a/server/oauth.py +++ b/server/oauth.py @@ -40,7 +40,12 @@ log = logging.getLogger("signal_bridge.oauth") # ════════════════════════════════════════════════════════════════════════ AUTH_CODE_EXPIRY_S = 300 # 5 minutes — per OAuth spec recommendation -REFRESH_TOKEN_EXPIRY_S = 86400 * 30 # 30 days +# Must comfortably outlive the access token (SB_TOKEN_EXPIRY_HOURS, often +# 30 days): clients only refresh once the access token has expired, so a +# refresh token with the same lifetime is already dead when it's needed. +# Rotation issues a fresh one on every refresh, so active clients never +# reach this limit. +REFRESH_TOKEN_EXPIRY_S = 86400 * 90 # 90 days # ════════════════════════════════════════════════════════════════════════ diff --git a/server/oauth_routes.py b/server/oauth_routes.py index db87aa8..1801606 100644 --- a/server/oauth_routes.py +++ b/server/oauth_routes.py @@ -70,12 +70,32 @@ async def oauth_metadata(request: Request): "registration_endpoint": f"{base}/oauth/register", "response_types_supported": ["code"], "grant_types_supported": ["authorization_code", "refresh_token"], - "code_challenge_methods_supported": ["S256", "plain"], + "code_challenge_methods_supported": ["S256"], "token_endpoint_auth_methods_supported": ["client_secret_post"], "scopes_supported": ["signal_bridge"], }) +# ════════════════════════════════════════════════════════════════════════ +# RFC 9728 — OAuth Protected Resource Metadata +# ════════════════════════════════════════════════════════════════════════ + +@router.get("/.well-known/oauth-protected-resource") +async def protected_resource_metadata(request: Request): + """ + Discovery endpoint for the MCP auth spec: clients that get a 401 from + /mcp follow the WWW-Authenticate header here, then on to the + authorization server metadata above. + """ + base = _base_url(request) + return JSONResponse({ + "resource": base, + "authorization_servers": [base], + "scopes_supported": ["signal_bridge"], + "bearer_methods_supported": ["header"], + }) + + # ════════════════════════════════════════════════════════════════════════ # RFC 7591 — Dynamic Client Registration # ════════════════════════════════════════════════════════════════════════ @@ -374,7 +394,13 @@ async def _handle_refresh_token(body: dict, client_id: str, ip: str): result = await asyncio.to_thread(consume_refresh_token, token, client_id) if not result: - await ip_tracker.record_failure(ip) + # Deliberately NOT counted toward the IP ban. The client has already + # proven its client_secret above, and refresh tokens are 512 bits of + # randomness — nothing to brute-force. A dead refresh token only + # ever comes from a legitimate client whose token expired, and such + # clients retry on a timer: counting them banned the owner's own + # home IP overnight. + log.info(f"OAuth refresh rejected (expired/revoked) for client={client_id[:12]}...") return JSONResponse({"error": "invalid_grant"}, status_code=400) # Look up user diff --git a/server/session_registry.py b/server/session_registry.py index 91267dd..89d48f7 100644 --- a/server/session_registry.py +++ b/server/session_registry.py @@ -154,14 +154,6 @@ class SessionRegistry: async with self._lock: return dict(self._sessions) - async def get_sole_user_id(self) -> Optional[str]: - """If exactly one phone session is active, return its user_id. - Used for authless MCP access (e.g. claude.ai connector).""" - async with self._lock: - if len(self._sessions) == 1: - return next(iter(self._sessions)) - return None - @property def active_count(self) -> int: return len(self._sessions) diff --git a/tests/verify_server.py b/tests/verify_server.py index 1aa47fa..74ff1e2 100644 --- a/tests/verify_server.py +++ b/tests/verify_server.py @@ -163,5 +163,49 @@ with TestClient(app) as client: j = r.json() if r.status_code == 200 else {} check("governor re-enables (one-way ratchet fixed)", j.get("governor_enabled") is True, r.text[:200]) + # ── No authless fallback: a lone connected phone is NOT a credential ── + import asyncio + from server.session_registry import registry + from server.auth import verify_token as _vt + + class _FakeWS: + async def send(self, data): pass + async def close(self, code=1000, reason=""): pass + + uid = _vt(access)["user_id"] + asyncio.run(registry.register(uid, _FakeWS())) + r = client.post("/mcp", json={"jsonrpc": "2.0", "id": 20, "method": "initialize", "params": {}}) + check("sole phone online: unauthenticated initialize still 401", r.status_code == 401, f"got {r.status_code}") + r = client.post("/mcp", headers={"mcp-session-id": sess}, json={ + "jsonrpc": "2.0", "id": 21, "method": "tools/call", + "params": {"name": "list_devices", "arguments": {}}, + }) + check("session id without token -> 401", r.status_code == 401, f"got {r.status_code}") + check("401 carries WWW-Authenticate discovery header", + "resource_metadata=" in r.headers.get("www-authenticate", ""), str(dict(r.headers))[:200]) + asyncio.run(registry.unregister(uid)) + + # ── Refresh: rotation works, dead refresh tokens don't ban the IP ── + from server import oauth as _oauth + from server import config as _cfg + check("refresh token outlives access token", + _oauth.REFRESH_TOKEN_EXPIRY_S > _cfg.TOKEN_EXPIRY_HOURS * 3600 * 2, + f"refresh={_oauth.REFRESH_TOKEN_EXPIRY_S}s access={_cfg.TOKEN_EXPIRY_HOURS}h") + rt = tok.get("refresh_token", "") + r = client.post("/oauth/token", json={ + "grant_type": "refresh_token", "refresh_token": rt, + "client_id": creds.get("client_id", ""), "client_secret": creds.get("client_secret", ""), + }) + check("refresh grant rotates", r.status_code == 200 and r.json().get("refresh_token") not in ("", rt), r.text[:150]) + for _ in range(_cfg.BAN_THRESHOLD + 5): + r = client.post("/oauth/token", json={ + "grant_type": "refresh_token", "refresh_token": rt, # now revoked + "client_id": creds.get("client_id", ""), "client_secret": creds.get("client_secret", ""), + }) + check("stale refresh -> invalid_grant, not a ban", + r.status_code == 400 and r.json().get("error") == "invalid_grant", f"{r.status_code} {r.text[:100]}") + r = client.post("/mcp", headers=hdrs, json={"jsonrpc": "2.0", "id": 22, "method": "tools/list"}) + check("same IP still served after repeated stale refreshes", r.status_code == 200, f"got {r.status_code}") + print(f"\n{len(PASS)} passed, {len(FAIL)} failed") sys.exit(1 if FAIL else 0)