diff --git a/CLAUDE.md b/CLAUDE.md index 23e22eda..14ba025e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -274,7 +274,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | **CSRF / Origin** | Always-on cross-site Origin guard on state-changing requests. **A missing Origin is allowed** so curl/CLI and hooks keep working. ⚠️ The body parser keeps `text/plain` RAW; auto-JSON-parsing it enabled simple-request CSRF | | **QR Auth** | Single-use 6-char tokens (60s TTL) for tunnel login. See `docs/qr-auth-plan.md` | | **Sessions** | 24h cookie (`codeman_session`), auto-extend, device context audit | -| **Rate limit** | 10 failed auth/IP → 429 (15min decay). Multi-user mode instead allows 50/IP before the password check (`AUTH_IP_FAILURE_MAX`) plus 10 per username. QR and hook-secret have separate buckets, so neither can lock out login | +| **Rate limit** | 10 failed auth/IP → 429 (15min decay). Multi-user mode instead checks 50/IP (`AUTH_IP_FAILURE_MAX`) and 10 per (username, IP) BEFORE the password, so past either even a correct password from that IP waits (the account still logs in from another IP), plus 10 per username AFTER it (429 to wrong guesses from any IP, never to a correct password). QR and hook-secret have separate buckets, so neither can lock out login | | **Hook bypass** | `/api/hook-event` + `/api/status-telemetry` skip Basic auth (localhost-only, schema-validated), but when auth is active the loopback bypass requires `X-Codeman-Hook-Secret` **unconditionally** (Codeman cannot detect a user's own loopback reverse proxy) | | **Lost-frame page** | The THIRD unauthenticated 200, beside the two hook routes, and the only one decided by request headers alone: a `GET`/`HEAD` carrying `Sec-Fetch-Dest: iframe\|frame`, `Accept: text/html` and mode `navigate` (or none), for a path that is NOT a registered route (never `/api/`, `/ws/`, `/q/`), is answered BEFORE the credential checks with the static web-tab recovery page (`lostWebviewFramePage`: no reflected input, `default-src 'none'` plus its own script hash, `no-store`). `/` is the one registered route also admitted, only when the request carries neither `codeman_session` nor `Authorization` (nothing in Codeman frames its own root; a sandboxed frame has neither), since the landing page masks to exactly `/` and its reload otherwise rendered Codeman inside the web tab. ⚠️ A non-browser client can set those headers, so an unauthenticated caller can tell a registered route (401) from a non-route (200) and enumerate the route table; accepted, the routes are public in `docs/api-reference.md`. Pinned by `test/webview-auth-exemption.test.ts` + `test/webview-lost-root-frame.test.ts` | | **Tunnel** | Enabling a tunnel **refuses** without `CODEMAN_PASSWORD` unless exposure is acknowledged via `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` or the per-request `acknowledgeUnauthTunnel:true` action field (never persisted) | diff --git a/README.md b/README.md index 3cbb3e77..1b4506c3 100644 --- a/README.md +++ b/README.md @@ -580,7 +580,7 @@ loginctl enable-linger $USER 1. First request → browser shows Basic Auth prompt (username: `admin` or `CODEMAN_USERNAME`) 2. On success → server issues a `codeman_session` cookie (24h TTL, auto-extends on activity) 3. Subsequent requests authenticate silently via cookie -4. 10 failed attempts per IP → 429 rate limit (15-minute decay). In multi-user mode the per-IP cap is 50, and each username is capped at 10 failed guesses +4. 10 failed attempts per IP → 429 rate limit (15-minute decay). In multi-user mode the per-IP cap is 50, and each account allows 10 failed guesses per IP; past either, even a correct password from that IP waits for the decay, while the account still logs in from another IP **Always set `CODEMAN_PASSWORD`** before exposing via tunnel — without it, anyone with the URL has full access to your sessions. @@ -670,7 +670,7 @@ By default Codeman launches sessions with `--dangerously-skip-permissions`, so t - **Loopback by default** — the server binary binds `127.0.0.1`, reachable only from the same machine, so the no-password default is safe out of the box (the guided installer asks about network access and configures the binding + password for you). Binding a non-loopback host without `CODEMAN_PASSWORD` _starts but prints a loud warning_ with three concrete fixes (set a password, loopback + an authenticated tunnel, or explicitly acknowledge with `--allow-unauthenticated-network`) - **Optional auth, real sessions** — HTTP Basic via `CODEMAN_USERNAME` (default `admin`) / `CODEMAN_PASSWORD`. Success issues an opaque 256-bit `codeman_session` cookie (`randomBytes(32)`) — validated server-side, not client-signed, so it can't be forged offline (24h TTL, auto-extend, device-context audit log) -- **Per-IP rate limiting** — 10 failed attempts → `429` with `Retry-After` (15-min decay). A valid cookie or correct password recovers _immediately_ even while an attacker hammers the same IP — important because all tunnel traffic shares one loopback IP. Multi-user mode allows 50 failures per IP instead (checked before the password, so past it a correct password also waits for the decay), plus 10 failed guesses per username. QR auth has its own separate limiter +- **Per-IP rate limiting** — 10 failed attempts → `429` with `Retry-After` (15-min decay). A valid cookie or correct password recovers _immediately_ even while an attacker hammers the same IP — important because all tunnel traffic shares one loopback IP. Multi-user mode allows 50 failures per IP instead, plus 10 failed guesses per account from one IP; both are checked before the password, so past either a correct password from that IP also waits for the decay, while the account still logs in from another IP. QR auth has its own separate limiter - **Configurable permission mode** - `--dangerously-skip-permissions` is only the default. **App Settings → Agents & CLIs → Claude → Startup Mode** can switch new sessions to Anthropic's classifier-guarded `auto` mode (low-prompt, needs Claude Code 2.1.207+), `normal` prompting, or an explicit allowed-tools list. In multi-user mode, non-granted users are forced to `auto`, and shell sessions / skip-permissions require an explicit per-user grant ### Always-on browser hardening (v0.9.5) diff --git a/README.zh-CN.md b/README.zh-CN.md index ae298d08..39f858eb 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -581,7 +581,7 @@ loginctl enable-linger $USER 1. 首次请求 → 浏览器弹出 Basic Auth 提示（用户名：`admin` 或 `CODEMAN_USERNAME`） 2. 成功后 → 服务端签发 `codeman_session` cookie（24 小时 TTL，活动时自动延长） 3. 后续请求通过 cookie 静默认证 -4. 同一 IP 失败 10 次 → 429 速率限制（15 分钟衰减）。多用户模式下每个 IP 的上限为 50 次，每个用户名另有 10 次失败上限 +4. 同一 IP 失败 10 次 → 429 速率限制（15 分钟衰减）。多用户模式下每个 IP 的上限为 50 次，且同一 IP 对每个账户最多 10 次失败；超过任一上限后，来自该 IP 的正确密码也须等待衰减，但该账户仍可从其他 IP 登录 通过隧道暴露前**务必设置 `CODEMAN_PASSWORD`** —— 否则任何拿到 URL 的人都能完全访问你的会话。 @@ -671,7 +671,7 @@ Codeman 默认用 `--dangerously-skip-permissions` 启动会话，因此 Web UI - **默认仅环回** —— 服务器程序本身绑定 `127.0.0.1`，仅可从本机访问，因此「无密码」默认配置开箱即安全（引导式安装器会询问网络访问方式，并替你配置绑定和密码）。在未设置 `CODEMAN_PASSWORD` 的情况下绑定非环回主机会*启动但打印一条醒目警告*，并给出三个具体修复方案（设置密码、环回 + 一个带认证的隧道，或用 `--allow-unauthenticated-network` 显式确认） - **可选认证，真实会话** —— 通过 `CODEMAN_USERNAME`（默认 `admin`）/ `CODEMAN_PASSWORD` 的 HTTP Basic 认证。成功后签发一个不透明的 256 位 `codeman_session` cookie（`randomBytes(32)`）—— 服务端校验，而非客户端签名，因此无法离线伪造（24h TTL、自动延长、设备上下文审计日志） -- **按 IP 速率限制** —— 失败 10 次 → `429` 并带 `Retry-After`（15 分钟衰减）。即便攻击者在同一 IP 上猛攻，有效 cookie 或正确密码也能*立即*恢复 —— 这很重要，因为所有隧道流量共享同一个环回 IP。多用户模式下每个 IP 改为 50 次（在校验密码之前检查，超过后正确密码也须等待衰减），每个用户名另有 10 次失败上限。二维码认证有自己独立的限制器 +- **按 IP 速率限制** —— 失败 10 次 → `429` 并带 `Retry-After`（15 分钟衰减）。即便攻击者在同一 IP 上猛攻，有效 cookie 或正确密码也能*立即*恢复 —— 这很重要，因为所有隧道流量共享同一个环回 IP。多用户模式下每个 IP 改为 50 次，同一 IP 对每个账户另有 10 次失败上限；两者都在校验密码之前检查，超过任一上限后，来自该 IP 的正确密码也须等待衰减，但该账户仍可从其他 IP 登录。二维码认证有自己独立的限制器 - **可配置的权限模式**：`--dangerously-skip-permissions` 只是默认值。**App Settings → Agents & CLIs → Claude → Startup Mode** 可以把新会话切换为 Anthropic 的分类器护栏 `auto` 模式（低打扰，需要 Claude Code 2.1.207+）、`normal` 提示模式，或一份显式的允许工具列表。多用户模式下，未获授权的用户会被强制为 `auto`，shell 会话与跳过权限需要按用户显式授权 ### 始终开启的浏览器加固（v0.9.5） diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 96c339a2..9f9f7514 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1163,7 +1163,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u | **CSRF / Origin** | Always-on cross-site Origin guard rejects state-changing requests from foreign origins (covers self-update, session create/input, settings/tunnel toggles). **A missing Origin is allowed** so curl/CLI and Claude Code hooks keep working. The global body parser keeps `text/plain` RAW (no auto-JSON-parse, which had enabled simple-request CSRF); `/api/crash-diag` self-parses. WebSocket upgrade validates Origin+Host (anti-CSWSH) in `ws-routes.ts`. Added in `c669518` (closes 2026-06-09 review CRITICALs) | | **QR Auth** | Single-use 6-char tokens (60s TTL) for tunnel login. See `docs/qr-auth-plan.md` | | **Sessions** | 24h cookie (`codeman_session`), auto-extend, device context audit | -| **Rate limit** | 10 failed logins/IP → 429 (15min decay). Multi-user mode instead has a 50/IP pre-gate (`AUTH_IP_FAILURE_MAX`, checked before scrypt) plus a 10-per-username bucket (429 from the 10th failed guess). QR has separate limiter | +| **Rate limit** | 10 failed logins/IP → 429 (15min decay). Multi-user mode instead has two gates checked before scrypt, so past either even a correct password from that IP gets 429: 50/IP (`AUTH_IP_FAILURE_MAX`) and 10 per (username, IP) (`AUTH_FAILURE_MAX`; the same account still logs in from another IP). A 10-per-username bucket (multi-user only) is checked after the verify: 429 to failed guesses from the 10th on, from any IP, never to a correct password. QR has separate limiter | | **Hook bypass** | `/api/hook-event` (and `/api/status-telemetry`, the statusLine exporter) skip Basic auth (localhost-only, schema-validated). When auth is active (`CODEMAN_PASSWORD` set), the loopback bypass requires the per-instance `X-Codeman-Hook-Secret` header **unconditionally** — COD-54 introduced it tunnel-gated; COD-91 (PR #127) made it always-on because Codeman can't detect a user's own loopback reverse proxy (own cloudflared/`tailscale serve`/nginx → 127.0.0.1), closing that residual plain-bypass gap. Hook curls cat the secret file at exec time via `$CODEMAN_HOOK_SECRET_FILE` (session env, `config/hook-secret.ts`); a missing/wrong secret gets 401 and rate-limits in a dedicated bucket (never locks out login). Tunnel enable **refuses** without `CODEMAN_PASSWORD` unless exposure is acknowledged — via `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` (env, COD-55) **or** the per-request `acknowledgeUnauthTunnel:true` action field (1.1.9): the welcome/settings tunnel toggle pops a security confirm dialog and, on confirm, resends with that flag (server logs a loud warning on every passwordless tunnel start; curl/API stay refused without password/env/flag). The flag is an action field, never persisted | | **Env vars** | `CODEMAN_MUX` (managed session), `CODEMAN_API_URL` (auto-set for hooks), `CODEMAN_ALLOWED_HOSTS` (extra Host/Origin allowlist entries for reverse proxies, comma-separated; bare `.suffix` matches subdomains), `CODEMAN_DOCKER_BRIDGE_HOOKS`=1 (opt-in hooks-only listener on the docker bridge gateway so in-container hooks reach a loopback-bound server; bind IP from `CODEMAN_DOCKER_BRIDGE_HOST` or auto-detect) | | **Validation** | Zod schemas, Unicode-aware path allowlist regex, env prefix allowlist (`CLAUDE_CODE_*`/`OPENCODE_*`/`CODEX_*`/`GEMINI_*`/`GOOGLE_*`/`ANTIGRAVITY_*`/`PI_*`) | diff --git a/docs/security-architecture.md b/docs/security-architecture.md index 21aefee7..cc9bb362 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -149,7 +149,7 @@ by presence in a server‑side map, so they cannot be forged offline. ### Rate limiting / lockout recovery -Failed logins are tracked **per IP**: 10 failures (`AUTH_FAILURE_MAX`) → `429`, with a 15‑minute decay. Multi-user mode replaces that with a wider per-IP **pre-gate** of 50 failures (`AUTH_IP_FAILURE_MAX`), checked before the scrypt verify, plus a **per-username** bucket that exists in multi-user mode only and answers `429` from the 10th failed guess (`AUTH_FAILURE_MAX`; the count is incremented before the check), so the wider per-IP cap (for users sharing one egress IP) does not widen guessing against any one account. +Failed logins are tracked **per IP**: 10 failures (`AUTH_FAILURE_MAX`) → `429`, with a 15‑minute decay. Multi-user mode replaces that with three buckets, all with the same decay: a wider per-IP **pre-gate** of 50 failures (`AUTH_IP_FAILURE_MAX`), checked before the scrypt verify, which bounds scrypt cost from one source and tolerates many users behind one egress IP; a **per-(username, IP)** gate of 10 failures (`AUTH_FAILURE_MAX`), also checked before the verify, which is what caps verified guesses against one account from one source; and a **per-username** bucket, checked after the verify, that answers `429` to failed guesses from the 10th on, from any IP (the count is incremented before the check). The last two exist in multi-user mode only. So the wider per-IP cap does not widen guessing against any one account from one IP, and other IPs cannot lock an account out. The QR path has its own separate limiter. The lockout check sits **after** the cookie/credential checks (step 4, not first). @@ -158,9 +158,13 @@ immediately** even while an attacker is hammering the same IP — important beca all traffic through a tunnel shares one source IP (loopback). Wrong credentials are still counted and still hit the `429` at the threshold, so brute‑force protection is unchanged. -The multi-user per-IP pre-gate is the exception: it runs before the password is -verified, so once it trips even a correct password gets `429` until the decay, -which is why its cap is wider than the per-username one. +The two multi-user pre-verify gates are the exception: once the per-IP pre-gate +or an account's (username, IP) gate trips, even a correct password from that IP +gets `429` until the decay (for the pair gate, only for that account; the same +account from another IP still logs in), which is why the per-IP cap is the +widest. The per-username bucket stays after the verify on purpose: checked +first, a username-wide lockout let throwaway IPs keep a known account (admin +included) locked out even with the right password. --- diff --git a/docs/wiki/Remote-Access.md b/docs/wiki/Remote-Access.md index 7627d7b4..4cba2a6c 100644 --- a/docs/wiki/Remote-Access.md +++ b/docs/wiki/Remote-Access.md @@ -239,6 +239,12 @@ minute decay. A valid cookie or a correct password recovers immediately even while an attacker is hammering the same IP, which matters because all tunnel traffic arrives from one loopback address. +Multi-user mode is different: there the per-IP cap is 50, each account allows ten failed +guesses from one IP, and both are checked before the password. Behind a tunnel every client +shares the loopback address, so past 50 failures in total a correct password waits for the +decay for everyone, and past ten failures for one account it waits for that account (the +account still logs in from another IP). A valid cookie keeps working throughout. + ## Terminal alternatives You do not have to use a browser. `codeman tui` is a full-screen session dashboard that diff --git a/docs/wiki/Security.md b/docs/wiki/Security.md index 77dff864..c3686143 100644 --- a/docs/wiki/Security.md +++ b/docs/wiki/Security.md @@ -53,7 +53,7 @@ When authentication is enabled: | ------------------- | ------------------------------------------------------------------------------------------------ | | **HTTP Basic** | `CODEMAN_USERNAME` (default `admin`) and `CODEMAN_PASSWORD`. | | **Session cookie** | A 256-bit opaque token validated server side, so it cannot be forged offline. 24 hours, extended on activity, with a device-context audit trail. | -| **Rate limiting** | Ten failed attempts per IP produce a `429` with a 15 minute decay. A correct password or valid cookie recovers immediately even under attack, which matters because all tunnel traffic shares one loopback address. In multi-user mode the per-IP cap is 50 and is checked before the password, so past it a correct password also waits for the decay; each username is also capped at ten failed guesses. | +| **Rate limiting** | Ten failed attempts per IP produce a `429` with a 15 minute decay. A correct password or valid cookie recovers immediately even under attack, which matters because all tunnel traffic shares one loopback address. In multi-user mode the per-IP cap is 50, and each account allows ten failed guesses from any one IP. Both are checked before the password, so past either a correct password from that IP also waits for the decay (the account still logs in from another IP, and a valid cookie keeps working). From the tenth failed guess on, an account's wrong guesses get a `429` from every IP, which never refuses a correct password. | | **QR auth** | Single-use 60-second tokens with their own separate rate limiter, so a mistyped password cannot lock out QR login. | | **Hook endpoints** | The hook and telemetry endpoints skip Basic auth because they are called from localhost by the CLI, but when auth is on, that bypass additionally requires a per-instance hook secret. | diff --git a/plugins/codeman/skills/codeman/reference/endpoints.md b/plugins/codeman/skills/codeman/reference/endpoints.md index ca12ff97..988f84aa 100644 --- a/plugins/codeman/skills/codeman/reference/endpoints.md +++ b/plugins/codeman/skills/codeman/reference/endpoints.md @@ -36,10 +36,12 @@ with a parse error and `.errorCode` is simply absent (see [symptom 6](#6-jq-parse-error-instead-of-an-errorcode)). Ten failed attempts from one IP then get a plain-text `429 Too Many Requests` with `Retry-After`, decaying over 15 minutes (`AUTH_FAILURE_MAX` = 10, `AUTH_FAILURE_WINDOW_MS` = 15 min). In multi-user -mode the per-IP cap is 50 (`AUTH_IP_FAILURE_MAX`) and is checked before the password, -so past it even a correct password gets the 429; each username is also capped at 10 -failed guesses. **Never retry a -failing credential in a loop**: you will lock the address out of the login path for +mode the per-IP cap is 50 (`AUTH_IP_FAILURE_MAX`) and each account allows 10 failed +guesses per IP; both are checked before the password, so past either even a correct +password from that address gets the 429 (for the per-account one, only for that +account). From the 10th failed guess on, an account's wrong guesses also get the 429 +from every address, but a correct password still logs in. **Never retry a failing +credential in a loop**: you will lock the address out of the login path for everything, including the user's browser through a tunnel (tunneled traffic arrives as 127.0.0.1, so one bucket covers it all). @@ -804,7 +806,7 @@ response echoes. | sessions, per user | half the global cap (25 by default) in multi-user mode | the same 409, with a different message | | SSE clients, process-wide | 100 (`MAX_SSE_CLIENTS`) | plain-text `503 Too many SSE connections`; shared with every browser tab | | active bash tools tracked | 20 per session | oldest entries drop off `active-tools` | -| auth failures per IP | 10, decaying over 15 min (multi-user: 50 per IP, plus 10 per username) | plain-text 429 with `Retry-After`; locks out the login path, so never loop a bad credential | +| auth failures per IP | 10, decaying over 15 min (multi-user: 50 per IP, plus 10 per account per IP) | plain-text 429 with `Retry-After`; locks out the login path, so never loop a bad credential | Case creation is **uncapped**, which is the one place restraint has to come from you: every `quick-start` with a new `caseName` creates a real directory on the user's disk. diff --git a/skills/codeman/reference/endpoints.md b/skills/codeman/reference/endpoints.md index ca12ff97..988f84aa 100644 --- a/skills/codeman/reference/endpoints.md +++ b/skills/codeman/reference/endpoints.md @@ -36,10 +36,12 @@ with a parse error and `.errorCode` is simply absent (see [symptom 6](#6-jq-parse-error-instead-of-an-errorcode)). Ten failed attempts from one IP then get a plain-text `429 Too Many Requests` with `Retry-After`, decaying over 15 minutes (`AUTH_FAILURE_MAX` = 10, `AUTH_FAILURE_WINDOW_MS` = 15 min). In multi-user -mode the per-IP cap is 50 (`AUTH_IP_FAILURE_MAX`) and is checked before the password, -so past it even a correct password gets the 429; each username is also capped at 10 -failed guesses. **Never retry a -failing credential in a loop**: you will lock the address out of the login path for +mode the per-IP cap is 50 (`AUTH_IP_FAILURE_MAX`) and each account allows 10 failed +guesses per IP; both are checked before the password, so past either even a correct +password from that address gets the 429 (for the per-account one, only for that +account). From the 10th failed guess on, an account's wrong guesses also get the 429 +from every address, but a correct password still logs in. **Never retry a failing +credential in a loop**: you will lock the address out of the login path for everything, including the user's browser through a tunnel (tunneled traffic arrives as 127.0.0.1, so one bucket covers it all). @@ -804,7 +806,7 @@ response echoes. | sessions, per user | half the global cap (25 by default) in multi-user mode | the same 409, with a different message | | SSE clients, process-wide | 100 (`MAX_SSE_CLIENTS`) | plain-text `503 Too many SSE connections`; shared with every browser tab | | active bash tools tracked | 20 per session | oldest entries drop off `active-tools` | -| auth failures per IP | 10, decaying over 15 min (multi-user: 50 per IP, plus 10 per username) | plain-text 429 with `Retry-After`; locks out the login path, so never loop a bad credential | +| auth failures per IP | 10, decaying over 15 min (multi-user: 50 per IP, plus 10 per account per IP) | plain-text 429 with `Retry-After`; locks out the login path, so never loop a bad credential | Case creation is **uncapped**, which is the one place restraint has to come from you: every `quick-start` with a new `caseName` creates a real directory on the user's disk. diff --git a/src/config/auth-config.ts b/src/config/auth-config.ts index e61dfc7c..8318d5c7 100644 --- a/src/config/auth-config.ts +++ b/src/config/auth-config.ts @@ -23,17 +23,20 @@ export const MAX_AUTH_SESSIONS = 100; /** * Max failed auth attempts per bucket before 429 rejection. Applies to the - * single-user per-IP bucket, the multi-user per-username lockout and the - * hook-secret bucket. + * single-user per-IP bucket, the hook-secret bucket and two multi-user buckets: + * the (username, IP) gate, checked before the scrypt verify, and the + * per-username bucket, checked after it. */ export const AUTH_FAILURE_MAX = 10; /** * Max failed login attempts per client IP for the multi-user pre-gate, which * runs before the scrypt verify and so refuses even a correct password once - * reached. Wider than AUTH_FAILURE_MAX because one egress IP (NAT, a tunnel) can - * carry many users' failures; the per-username bucket still caps guesses against - * each account at AUTH_FAILURE_MAX. Single-user mode does not use this. + * reached. It bounds scrypt cost from one source and is wider than + * AUTH_FAILURE_MAX because one egress IP (NAT, a tunnel) can carry many users' + * failures. Verified guesses against any one account are capped by the + * (username, IP) gate at AUTH_FAILURE_MAX, not by this. Single-user mode does + * not use this. */ export const AUTH_IP_FAILURE_MAX = 50; diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 767da950..0d63dac1 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -54,6 +54,8 @@ interface AuthState { hookSecretFailures: StaleExpirationMap | null; /** Per-username Basic-auth failure bucket (multi-user only). */ userFailures: StaleExpirationMap | null; + /** Per-(username, client IP) Basic-auth failure bucket (multi-user only). */ + userIpFailures: StaleExpirationMap | null; } /** Rate-limit response for a client that exceeded the failure cap. */ @@ -304,6 +306,7 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean, bas qrAuthFailures: null, hookSecretFailures: null, userFailures: null, + userIpFailures: null, }; // Always declare req.authUser so downstream reads are safe (single-user leaves it @@ -351,13 +354,33 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean, bas const hookSecretFailures = state.hookSecretFailures; if (multiUser) { - // Per-username failure bucket: a botnet can't brute-force one account across - // many IPs, and one user behind a NAT can't lock out everyone else. + // Per-username failure bucket: failed guesses against one account answer 429 + // from any IP once it fills, and one user behind a NAT can't lock out everyone + // else. Checked after the verify, so it never denies a correct password. state.userFailures = new StaleExpirationMap({ ttlMs: AUTH_FAILURE_WINDOW_MS, refreshOnGet: false, }); - registerMultiUserAuthHook(app, https, authSessions, authFailures, hookSecretFailures, state.userFailures, basePath); + // Per-(username, IP) failure bucket: checked BEFORE the verify, so it is what + // caps verified guesses against one account from one source. Keyed on the pair, + // so other IPs can't lock the account out and two users behind one shared IP + // don't lock each other out. Same decay as the other buckets; a key is only + // created by a failed verify that the per-IP pre-gate also counts, so it grows + // no faster than userFailures. + state.userIpFailures = new StaleExpirationMap({ + ttlMs: AUTH_FAILURE_WINDOW_MS, + refreshOnGet: false, + }); + registerMultiUserAuthHook( + app, + https, + authSessions, + authFailures, + hookSecretFailures, + state.userFailures, + state.userIpFailures, + basePath + ); return state; } @@ -466,8 +489,9 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean, bas /** * Multi-user auth hook (async, because password verification runs scrypt). Verifies * `username:password` against the user store, mints an identity-carrying cookie, - * decorates `req.authUser`, enforces the per-IP + per-username rate limits, and the - * `mustChangePassword` lockbox. The single-user hook above is left untouched. + * decorates `req.authUser`, enforces the per-IP, per-(username, IP) and per-username + * rate limits, and the `mustChangePassword` lockbox. The single-user hook above is + * left untouched. */ function registerMultiUserAuthHook( app: FastifyInstance, @@ -476,6 +500,7 @@ function registerMultiUserAuthHook( authFailures: StaleExpirationMap, hookSecretFailures: StaleExpirationMap, userFailures: StaleExpirationMap, + userIpFailures: StaleExpirationMap, basePath = '' ): void { const setSessionCookie = (reply: FastifyReply, token: string) => @@ -570,11 +595,17 @@ function registerMultiUserAuthHook( return; } - // 2. Basic Auth against the user store (scrypt verify). - // Per-IP pre-gate bounds scrypt CPU cost from one source (does NOT gate on the - // per-username bucket here; see below). Wider than AUTH_FAILURE_MAX: it refuses - // even a correct password, so behind a shared egress IP (NAT, a tunnel) a narrow - // cap would let a few wrong guesses lock everyone out. + // 2. Basic Auth against the user store (scrypt verify). Three failure buckets: + // - per IP (AUTH_IP_FAILURE_MAX), checked first: bounds scrypt CPU cost from one + // source. It refuses even a correct password, so it is wider than + // AUTH_FAILURE_MAX: behind a shared egress IP (NAT, a tunnel) a narrow cap + // would let a few wrong guesses lock everyone out. + // - per (username, IP) (AUTH_FAILURE_MAX), checked BEFORE the verify: caps + // verified guesses against one account from one source. Only that IP is + // refused for that account; the same account from another IP still logs in. + // - per username (AUTH_FAILURE_MAX), checked AFTER the verify; see below. + // Neither pre-verify refusal is counted, since nothing was verified (and counting + // a locked pair's retries would fill the shared per-IP bucket for everyone). const ipFail = authFailures.get(clientIp) ?? 0; if (ipFail >= AUTH_IP_FAILURE_MAX) { sendAuthRateLimit(reply, authFailures, clientIp); @@ -583,10 +614,18 @@ function registerMultiUserAuthHook( const creds = parseBasicAuth(req.headers.authorization); if (creds) { const normUser = creds.username.trim().toLowerCase(); - // Security: VERIFY FIRST, then throttle only FAILED attempts. Consulting the - // per-username bucket before verifying let throwaway IPs lock out a known account - // (incl. admin) even with the correct password. A correct password must always - // win and self-heal both buckets, regardless of the username-failure count. + // IPs never contain a newline, so this key is unambiguous for any username. + const pairKey = `${clientIp}\n${normUser}`; + const pairFail = userIpFailures.get(pairKey) ?? 0; + if (pairFail >= AUTH_FAILURE_MAX) { + sendAuthRateLimit(reply, userIpFailures, pairKey); + return; + } + // Security: the per-username bucket is consulted only AFTER the verify, and only + // FAILED attempts are throttled by it. Consulting it before verifying let + // throwaway IPs lock out a known account (incl. admin) even with the correct + // password. A correct password that passes the two gates above always wins and + // self-heals the per-IP, per-username and this pair's buckets. const result = await verifyPassword(creds.username, creds.password); if (result) { const { user, needsRehash: rehash } = result; @@ -594,6 +633,7 @@ function registerMultiUserAuthHook( void touchLastLogin(user.username).catch(() => {}); authFailures.delete(clientIp); userFailures.delete(normUser); + userIpFailures.delete(pairKey); const token = randomBytes(32).toString('hex'); if (authSessions.size >= MAX_AUTH_SESSIONS) evictForCapacity(user.username); @@ -611,12 +651,12 @@ function registerMultiUserAuthHook( enforcePasswordChange(req, reply, !!user.mustChangePassword); return; } - // Failed guess: count it against BOTH buckets. Once the per-username bucket - // reaches the cap, further FAILED attempts get 429 (throttles distributed - // brute-force), but this path is only reached on a wrong password, so it can - // never deny a correct one. + // Failed guess: count it against all three buckets. Once the per-username bucket + // reaches the cap, further FAILED attempts get 429 from any IP, but this path is + // only reached on a wrong password, so it can never deny a correct one. const uFail = (userFailures.get(normUser) ?? 0) + 1; userFailures.set(normUser, uFail); + userIpFailures.set(pairKey, (userIpFailures.get(pairKey) ?? 0) + 1); authFailures.set(clientIp, ipFail + 1); if (uFail >= AUTH_FAILURE_MAX) { sendAuthRateLimit(reply, userFailures, normUser); diff --git a/src/web/server.ts b/src/web/server.ts index b3f537e9..755617d5 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -457,6 +457,7 @@ export class WebServer extends EventEmitter { private qrAuthFailures: StaleExpirationMap | null = null; private hookSecretFailures: StaleExpirationMap | null = null; private userFailures: StaleExpirationMap | null = null; + private userIpFailures: StaleExpirationMap | null = null; private pushStore: PushSubscriptionStore = new PushSubscriptionStore(); /** ntfy / Slack / Discord / generic webhook for the push events; config in webhook.json (0600). */ private webhookNotifier = new WebhookNotifier(() => readWebhookConfig(getDataDir()), webviewFetch); @@ -959,6 +960,7 @@ export class WebServer extends EventEmitter { this.qrAuthFailures = authState.qrAuthFailures; this.hookSecretFailures = authState.hookSecretFailures; this.userFailures = authState.userFailures; + this.userIpFailures = authState.userIpFailures; } // WebSocket support (terminal I/O — low-latency bidirectional channel) @@ -4324,6 +4326,10 @@ export class WebServer extends EventEmitter { this.userFailures.dispose(); this.userFailures = null; } + if (this.userIpFailures) { + this.userIpFailures.dispose(); + this.userIpFailures = null; + } this.activePlanOrchestrators.clear(); this.cleaningUp.clear(); this.killingSessions.clear(); diff --git a/test/multiuser-auth.test.ts b/test/multiuser-auth.test.ts index fc9e765e..12f6e4fa 100644 --- a/test/multiuser-auth.test.ts +++ b/test/multiuser-auth.test.ts @@ -17,7 +17,8 @@ import { WebServer } from '../src/web/server.js'; import { TmuxManager } from '../src/tmux-manager.js'; import { TunnelManager } from '../src/tunnel-manager.js'; import { createUser, invalidateUsersCache } from '../src/user-store.js'; -import { AUTH_FAILURE_MAX } from '../src/config/auth-config.js'; +import type { FastifyInstance } from 'fastify'; +import { AUTH_FAILURE_MAX, AUTH_IP_FAILURE_MAX } from '../src/config/auth-config.js'; vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); @@ -34,6 +35,7 @@ function cookieFrom(res: Response): string | null { let server: WebServer; let rateServer: WebServer; let sprayServer: WebServer; +const ipServers: WebServer[] = []; let dataDir: string; let spacesDir: string; const saved: Record = {}; @@ -74,6 +76,7 @@ afterAll(async () => { await server?.stop(); await rateServer?.stop().catch(() => {}); await sprayServer?.stop().catch(() => {}); + for (const s of ipServers) await s.stop().catch(() => {}); for (const [k, v] of Object.entries(saved)) { if (v === undefined) delete process.env[k]; else process.env[k] = v; @@ -85,6 +88,20 @@ afterAll(async () => { const url = (p: string) => `http://localhost:${server.boundPort}${p}`; +/** + * A fresh multi-user server (empty failure buckets) and a login helper that sets the + * client IP through inject's `remoteAddress`: a real fetch always arrives from + * loopback, so it cannot show two sources. + */ +async function startIpServer() { + const s = new WebServer(0, false, true); + await s.start(); + ipServers.push(s); + const app = (s as unknown as { app: FastifyInstance }).app; + return (ip: string, user: string, pass: string) => + app.inject({ method: 'GET', url: '/api/status', headers: { authorization: basic(user, pass) }, remoteAddress: ip }); +} + describe('multi-user auth', () => { it('rejects unauthenticated requests', async () => { const res = await fetch(url('/api/status')); @@ -159,7 +176,7 @@ describe('multi-user auth', () => { expect(after.status).toBe(200); }); - it('verify-first: a correct password is never rate-limited and self-heals failures (#17)', async () => { + it('verify-first: the per-username bucket never denies a correct password and self-heals failures (#17)', async () => { rateServer = new WebServer(0, false, true); await rateServer.start(); const rurl = (p: string) => `http://localhost:${rateServer.boundPort}${p}`; @@ -187,7 +204,7 @@ describe('multi-user auth', () => { expect(limited).toBe(true); }); - it('per-IP pre-gate: wrong passwords spread across accounts are throttled at 50, before scrypt', async () => { + it('per-IP pre-gate: wrong passwords spread across accounts are throttled at AUTH_IP_FAILURE_MAX, before scrypt', async () => { // Fresh server so no earlier test's failures sit in the per-IP bucket. sprayServer = new WebServer(0, false, true); await sprayServer.start(); @@ -196,13 +213,13 @@ describe('multi-user auth', () => { // Spread the guesses so no single username reaches its own AUTH_FAILURE_MAX bucket: // only the per-IP pre-gate can produce a 429 here. const perUser = AUTH_FAILURE_MAX - 1; - for (let i = 0; i < 50; i++) { + for (let i = 0; i < AUTH_IP_FAILURE_MAX; i++) { const user = `spray${Math.floor(i / perUser)}`; const res = await fetch(surl('/api/status'), { headers: { Authorization: basic(user, `bad-${i}`) } }); expect(res.status, `attempt ${i + 1}`).toBe(401); } - // The 51st failure from the same IP is refused by the pre-gate. + // One more failure from the same IP is refused by the pre-gate. const over = await fetch(surl('/api/status'), { headers: { Authorization: basic('spray-extra', 'bad') } }); expect(over.status).toBe(429); expect(over.headers.get('retry-after')).toMatch(/^\d+$/); @@ -211,6 +228,50 @@ describe('multi-user auth', () => { const good = await fetch(surl('/api/status'), { headers: { Authorization: basic('alice', 'alicepass1') } }); expect(good.status).toBe(429); }); + + it('per-(username, IP) gate: failures lock that account on that IP only, before scrypt', async () => { + const login = await startIpServer(); + const ipA = '203.0.113.10'; + const ipB = '203.0.113.20'; + + // AUTH_FAILURE_MAX wrong passwords for bob from one IP. The last one is already a + // 429 from the per-username bucket, which counts before it checks. + for (let i = 0; i < AUTH_FAILURE_MAX; i++) { + const res = await login(ipA, 'bob', `bad-${i}`); + expect(res.statusCode, `attempt ${i + 1}`).toBe(i < AUTH_FAILURE_MAX - 1 ? 401 : 429); + } + + // The (bob, ipA) bucket is checked before the password, so even the correct one is + // refused from that IP, far below the per-IP pre-gate. + expect(AUTH_FAILURE_MAX).toBeLessThan(AUTH_IP_FAILURE_MAX); + const lockedOut = await login(ipA, 'bob', 'bobpass123'); + expect(lockedOut.statusCode).toBe(429); + expect(String(lockedOut.headers['retry-after'])).toMatch(/^\d+$/); + + // Another IP cannot lock the account out: bob logs in from ipB. + expect((await login(ipB, 'bob', 'bobpass123')).statusCode).toBe(200); + // Another account behind the same IP is not locked out either. + expect((await login(ipA, 'alice', 'alicepass1')).statusCode).toBe(200); + // Neither success cleared bob's bucket on ipA: that one is per pair. + expect((await login(ipA, 'bob', 'bobpass123')).statusCode).toBe(429); + }); + + it('per-(username, IP) gate: a correct password clears the pair bucket', async () => { + const login = await startIpServer(); + const ip = '203.0.113.30'; + + for (let i = 0; i < AUTH_FAILURE_MAX - 1; i++) { + expect((await login(ip, 'bob', `bad-${i}`)).statusCode, `attempt ${i + 1}`).toBe(401); + } + expect((await login(ip, 'bob', 'bobpass123')).statusCode).toBe(200); + + // Had the success left the pair at AUTH_FAILURE_MAX - 1, the second of these would + // already be refused before the password check. + for (let i = 0; i < AUTH_FAILURE_MAX - 1; i++) { + expect((await login(ip, 'bob', `again-${i}`)).statusCode, `retry ${i + 1}`).toBe(401); + } + expect((await login(ip, 'bob', 'bobpass123')).statusCode).toBe(200); + }); }); describe('QR token identity (tunnel-manager)', () => {