Cloud link/login security review and fixes

[mariusandra]

Security review of the merged cloud work (#263 linking, #264 login), plus fixes for what it found. Four parallel audits covered device-flow linking, the login handoff, offline/grant-sync behaviour, and device-side DoS. Every finding — fixed and still open — is recorded in docs/cloud-security-review.md.

Companion cloud-service fixes: https://github.com/FrameOS/frameos-cloud/pull/1.

Fixed here

The login handoff was not bound to the browser (HIGH)

The state token lived only server-side (Redis on the backend, login_states on the frame), so it proved a handoff was in flight, never who started it:

• Anyone able to call the open /api/cloud/login/start could complete a handoff for their own cloud account, not follow the final redirect, and feed the callback URL to the owner's browser — silently issuing them a 7-day session for the attacker's account.
• Any leaked code (proxy logs, history) became directly exchangeable: mint a state via the open /start, replay code+state, and the backend redeems it with its link token and issues a session as the code's owner.

The state is now also an HttpOnly, SameSite=Lax, path-scoped cookie that must match at the callback — checked before the state is consumed, so a forged callback cannot burn the real one. Implemented on both the Python backend and the Nim frame. (RFC 6749 §10.12.)

The frame skipped its owner check when it didn't know the owner

Which is exactly the case when the grants call failed during connect — so a network hiccup while linking silently downgraded the check to "trust the provider's word" before minting an admin session. Now it refuses and re-syncs the owner.

Device DoS

The frame's HTTP server runs 1–4 worker threads, so a blocking handler is an outage:

• /api/cloud/poll held the global cloud lock across two outbound cloud requests (70 s+ against a slow provider), freezing every cloud route. Sync moved outside the lock, results merged after.
• login_states grew without bound from the open /login/start, so every later cloud request re-parsed and rewrote a growing state file under that lock — O(n²). Capped at 16, oldest evicted.
• The open /login/callback rewrote the state file on every bogus request (SD-card wear + lock contention). Now writes only when pruning actually changed something.

SECRET_KEY could not be rotated without killing the cloud link

The stored link token is encrypted with a key derived from SECRET_KEY, so rotating it made the token permanently undecryptable — and the sync worker treated "cannot decrypt" the same as "no link", leaving the UI reporting a healthy connected link while every cloud request silently stopped. Two opt-in escape hatches, defaults unchanged:

Variable	Meaning
CLOUD_SECRET_KEY	Encrypt cloud secrets with their own key, so SECRET_KEY is freely rotatable from then on.
PREVIOUS_SECRET_KEYS	Comma-separated old keys, tried on decrypt only — recovery for an install that already rotated.

Self-healing: a secret that decrypts with an old key is re-encrypted with the current one on the next sync pass, so PREVIOUS_SECRET_KEYS can be dropped after one cycle. When nothing works, the worker logs a red error naming the cause and sets poll_error = "secret_key_changed" instead of looping silently. Procedure documented in docs/cloud-link.md.

Passwordless lockout

Cloud signup leaves password NULL and the change-password route demanded the current one — so those users could never set a local password, and a later broken link (revoked, provider gone, FRAMEOS_CLOUD_URL=disabled) locked them out permanently with no recovery short of editing the database. Setting a first password no longer requires the current one (they are already authenticated), and check_password no longer raises on a NULL hash. Empty passwords still never authenticate.

Verified good — no change needed

• No fall-open anywhere. Every broken-link path fails closed: cloud down, 500, timeout, revoked, expired, disabled. not user.password is checked before the hash comparison, so a NULL hash cannot be matched by an empty password.
• An offline server never loses its link. linked_clients has no expiry, last_seen_at is never read by any query, cleanup never touches links, and neither side auto-disconnects or requires rotation. Only an explicit 401 resets it — and that deliberately re-enables local password login.
• Identity mapping is keyed on issuer+subject, never email; a random cloud account cannot gain admin on someone's install.
• Worker-pool exhaustion does not starve the systemd watchdog — the runner thread heartbeats independently, so these are HTTP-only outages, not restart loops.

Tests

New regression coverage for the state-cookie binding (missing and mismatched cookie — neither may consume the real state), passwordless recovery, and all three key-rotation paths. 771 backend tests, all Nim server tests, and the frame build pass.

Still open

Tracked in docs/cloud-security-review.md with attack scenarios. The notable ones: unbounded DNS in the frame's outbound calls (one request per worker thread wedges the server for minutes), no rate limiting on the device at all, pre-auth 50 MB body buffering (the one path that does become an OOM restart loop), unauthenticated setup-mode linking, and unrevocable local sessions.

🤖 Generated with Claude Code