mirror of
https://github.com/AletheiaVox/signal_bridge_remote.git
synced 2026-10-07 03:18:17 +08:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
24
CHANGELOG.md
24
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
|
||||
|
||||
14
README.md
14
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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
# ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user