diff --git a/CLAUDE.md b/CLAUDE.md index 23613879..bbcd67b0 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). 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 b3fe7e7e..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) +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. 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 e70bd7a2..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 分钟衰减） +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 速率限制** —— 失败 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 0a7cbe00..3007e24c 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1167,7 +1167,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 auth/IP → 429 (15min decay). 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/multi-user-plan.md b/docs/multi-user-plan.md index c8ae1cdc..f44902ff 100644 --- a/docs/multi-user-plan.md +++ b/docs/multi-user-plan.md @@ -105,7 +105,7 @@ Keep the existing single-user branch untouched. Add a parallel multi-user branch 1. **Credential check**: Basic header parsed into `username:password`, verified against the user store (scrypt + `timingSafeEqual`). Disabled users fail closed. 2. **Cookie sessions**: same `codeman_session` cookie and `StaleExpirationMap`, but `AuthSessionRecord` gains `username` and `role`. All existing TTL/sliding/eviction logic reused. Eviction cap becomes per-user aware (evict oldest _of that user_ first) so one user cannot flush everyone's sessions by logging in 100 times. 3. **Request identity**: decorate `req.authUser = { username, role }` (Fastify decorateRequest). In single-user mode `req.authUser` is `{ username: 'admin', role: 'admin' }` when auth is on, and a synthetic admin when auth is off, so downstream code has ONE code path. -4. **Rate limiting**: keep the per-IP bucket; add a per-username failure bucket (same `StaleExpirationMap` pattern) so a botnet cannot brute-force one account across IPs, and one flaky user behind a NAT cannot lock out the rest. +4. **Rate limiting**: keep the per-IP bucket; add a per-username failure bucket (same `StaleExpirationMap` pattern) so one flaky user behind a NAT cannot lock out the rest. As built, the per-username bucket is checked after the verify: it answers 429 to failed guesses once full and never denies a correct password, so it does not cap verified guesses. What caps them is a per-(username, IP) bucket checked before the verify (10 per pair), beside a per-IP pre-gate of 50 (#684). 5. **`mustChangePassword` gate**: when set, every API request except `GET /api/me`, `POST /api/me/password`, and static assets returns 403 with `errorCode: 'PASSWORD_CHANGE_REQUIRED'`; the frontend intercepts that code and shows the change-password modal. 6. **Password change vs Basic-auth caching**: browsers cache Basic credentials. After a password change we revoke all of that user's cookie sessions; the next request falls to Basic with stale creds, gets 401, and the browser re-prompts. Acceptable for v1; a proper login form is Phase 6 (see 15). 7. **Unchanged**: hook-secret loopback bypass (hooks authenticate the _instance_, not a user; the event maps to a session which has an owner), host guard, Origin/CSRF guard, security headers. diff --git a/docs/security-architecture.md b/docs/security-architecture.md index bf349a55..21f0b9e5 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 auth is tracked **per IP**: 10 failures → `429`, with a 15‑minute decay. +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,6 +158,22 @@ 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 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. +Both pre-verify gates count a request the moment it passes them, before its +verify starts, so a burst of concurrent attempts cannot all slip through on one +stale count; a correct password then clears its slot with the rest, and a +failure is not counted a second time. A verify still in flight therefore holds a +slot, even one carrying the right password, but identical credentials from the +same IP that arrive during it share that verify, its slot and its outcome, and +skip both gates: they add no guess and no scrypt work. So a browser replaying its +cached login on many parallel requests after a restart (cookie sessions live in +memory) is not refused by the slots its own password filled. --- 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 71b45c2a..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. | +| **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 3edf4f70..988f84aa 100644 --- a/plugins/codeman/skills/codeman/reference/endpoints.md +++ b/plugins/codeman/skills/codeman/reference/endpoints.md @@ -35,8 +35,13 @@ only and can never drive a session. 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). **Never retry a -failing credential in a loop**: you will lock the address out of the login path for +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 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). @@ -801,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 | 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 3edf4f70..988f84aa 100644 --- a/skills/codeman/reference/endpoints.md +++ b/skills/codeman/reference/endpoints.md @@ -35,8 +35,13 @@ only and can never drive a session. 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). **Never retry a -failing credential in a loop**: you will lock the address out of the login path for +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 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). @@ -801,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 | 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 9aa60482..8318d5c7 100644 --- a/src/config/auth-config.ts +++ b/src/config/auth-config.ts @@ -21,9 +21,25 @@ export const MAX_AUTH_SESSIONS = 100; // Rate Limiting // ============================================================================ -/** Max failed auth attempts per IP before 429 rejection */ +/** + * Max failed auth attempts per bucket before 429 rejection. Applies to the + * 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. 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; + /** Failed auth attempt tracking window (ms) */ export const AUTH_FAILURE_WINDOW_MS = 15 * 60 * 1000; diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 8773d669..ea7c7767 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -9,7 +9,7 @@ */ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; -import { randomBytes, timingSafeEqual } from 'node:crypto'; +import { createHash, randomBytes, timingSafeEqual } from 'node:crypto'; import { StaleExpirationMap } from '../../utils/index.js'; import type { AuthSessionRecord } from '../ports/auth-port.js'; import { isAllowedRequestHost, isAllowedRequestOrigin, type HostPolicy } from '../network-auth-policy.js'; @@ -17,6 +17,7 @@ import { AUTH_SESSION_TTL_MS, MAX_AUTH_SESSIONS, AUTH_FAILURE_MAX, + AUTH_IP_FAILURE_MAX, AUTH_FAILURE_WINDOW_MS, } from '../../config/auth-config.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../../config/hook-secret.js'; @@ -32,7 +33,7 @@ import { lostWebviewFramePage, LOST_FRAME_PAGE_CSP, } from '../webview-proxy.js'; -import { ApiErrorCode, createErrorResponse, type AuthUser } from '../../types.js'; +import { ApiErrorCode, createErrorResponse, type AuthUser, type UserRecord } from '../../types.js'; // Request-scoped identity (multi-user). Single-user leaves it undefined and the // ownership helpers default to a synthetic admin (see route-helpers). @@ -53,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. */ @@ -76,6 +79,22 @@ function parseBasicAuth(header?: string): { username: string; password: string } } } +/** + * Key of the multi-user in-flight verify map: a SHA-256 digest of (client IP, + * normalized username, password), so the plaintext password is never held as a key. + * The JSON array keeps the three parts unambiguous for any strings (a NUL-joined + * string is not, since a username may carry a NUL). Never logged or persisted, and + * gone from the map once its verify settles. + */ +function verifyFlightKey(clientIp: string, normUser: string, password: string): string { + return createHash('sha256') + .update(JSON.stringify([clientIp, normUser, password])) + .digest('hex'); +} + +/** What one Basic verify decided, shared by the request that ran it and every joiner. */ +type BasicVerifyOutcome = { user: UserRecord } | { user: null; limited: boolean }; + /** * The `/api/hook-event` + `/api/status-telemetry` localhost bypass, shared by the * single-user and multi-user auth hooks so the security-critical logic has ONE @@ -303,6 +322,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 @@ -350,13 +370,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: it holds one key + // per (IP, username) pair whose verify failed or is still in flight, swept by + // the same TTL, so like them it grows only with scrypt verifies. + state.userIpFailures = new StaleExpirationMap({ + ttlMs: AUTH_FAILURE_WINDOW_MS, + refreshOnGet: false, + }); + registerMultiUserAuthHook( + app, + https, + authSessions, + authFailures, + hookSecretFailures, + state.userFailures, + state.userIpFailures, + basePath + ); return state; } @@ -465,8 +505,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. Identical logins in flight at once + * share one verify. The single-user hook above is left untouched. */ function registerMultiUserAuthHook( app: FastifyInstance, @@ -475,6 +516,7 @@ function registerMultiUserAuthHook( authFailures: StaleExpirationMap, hookSecretFailures: StaleExpirationMap, userFailures: StaleExpirationMap, + userIpFailures: StaleExpirationMap, basePath = '' ): void { const setSessionCookie = (reply: FastifyReply, token: string) => @@ -515,6 +557,75 @@ function registerMultiUserAuthHook( return false; }; + // Basic verifies in flight, by verifyFlightKey(). A request whose identical + // credentials from the same IP are already being verified awaits that verify + // instead of starting its own. Each entry is removed when its verify settles, so the + // map never holds more than the distinct verifies in flight. + const verifiesInFlight = new Map>(); + + // The verify plus its once-only side effects, run by the first request for a key. + // A success clears the three buckets and records the login; a failure is counted + // here once (the per-IP and pair buckets already hold it: the caller reserved them). + const verifyOnce = async ( + username: string, + password: string, + normUser: string, + clientIp: string, + pairKey: string + ): Promise => { + const result = await verifyPassword(username, password); + if (result) { + const { user, needsRehash: rehash } = result; + if (rehash) void setPassword(user.username, password).catch(() => {}); + void touchLastLogin(user.username).catch(() => {}); + authFailures.delete(clientIp); + userFailures.delete(normUser); + userIpFailures.delete(pairKey); + return { user }; + } + // Failed guess: only the per-username bucket counts it here. Once that 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. Its count + // is re-read after the await, so concurrent failures against one account each add one. + const uFail = (userFailures.get(normUser) ?? 0) + 1; + userFailures.set(normUser, uFail); + return { user: null, limited: uFail >= AUTH_FAILURE_MAX }; + }; + + // Answer one request from a shared outcome. Every successful request, joiners + // included, gets its own cookie session under the usual capacity rules. + const sendBasicOutcome = ( + req: FastifyRequest, + reply: FastifyReply, + outcome: BasicVerifyOutcome, + normUser: string + ): void => { + if (outcome.user) { + const { user } = outcome; + const token = randomBytes(32).toString('hex'); + if (authSessions.size >= MAX_AUTH_SESSIONS) evictForCapacity(user.username); + authSessions.set(token, { + ip: req.ip, + ua: req.headers['user-agent'] ?? '', + createdAt: Date.now(), + method: 'basic', + username: user.username, + role: user.role, + mustChangePassword: !!user.mustChangePassword, + }); + req.authUser = { username: user.username, role: user.role }; + setSessionCookie(reply, token); + enforcePasswordChange(req, reply, !!user.mustChangePassword); + return; + } + if (outcome.limited) { + sendAuthRateLimit(reply, userFailures, normUser); + return; + } + reply.header('WWW-Authenticate', 'Basic realm="Codeman"'); + reply.code(401).send('Unauthorized'); + }; + app.addHook('onRequest', async (req, reply) => { const bypass = checkHookSecretBypass(req, reply, hookSecretFailures); if (bypass === 'bypass' || bypass === 'rejected') return; @@ -569,58 +680,69 @@ 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). + // 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). + // A request that passes both gates RESERVES its slot in both buckets before the + // scrypt await, as if it had already failed. Counting only after the verify let a + // concurrent burst pass the gates on the same stale read: 29 wrong guesses plus the + // right password, all in flight at once from one IP, logged in. A success deletes + // the reservation with the rest of the buckets; a failure is not counted again. + // A request whose IDENTICAL credentials from the same IP are already being verified + // joins that verify instead: it skips both gates, takes no slot, runs no scrypt and + // gets the same outcome, since it adds no guess and no CPU. Reserving for each of + // them refused a correct password with the slots that password itself had filled, + // which is every reload after a restart: cookie sessions live in memory, so the + // browser replays its cached Basic header on many parallel requests at once. A + // request with no verify in flight for its credentials is gated as normal. + const creds = parseBasicAuth(req.headers.authorization); + const normUser = creds ? creds.username.trim().toLowerCase() : ''; + const flightKey = creds ? verifyFlightKey(clientIp, normUser, creds.password) : ''; + const shared = creds ? verifiesInFlight.get(flightKey) : undefined; + if (shared) { + sendBasicOutcome(req, reply, await shared, normUser); + return; + } const ipFail = authFailures.get(clientIp) ?? 0; - if (ipFail >= AUTH_FAILURE_MAX) { + if (ipFail >= AUTH_IP_FAILURE_MAX) { sendAuthRateLimit(reply, authFailures, clientIp); return; } - 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. - const result = await verifyPassword(creds.username, creds.password); - if (result) { - const { user, needsRehash: rehash } = result; - if (rehash) void setPassword(user.username, creds.password).catch(() => {}); - void touchLastLogin(user.username).catch(() => {}); - authFailures.delete(clientIp); - userFailures.delete(normUser); - - const token = randomBytes(32).toString('hex'); - if (authSessions.size >= MAX_AUTH_SESSIONS) evictForCapacity(user.username); - authSessions.set(token, { - ip: clientIp, - ua: req.headers['user-agent'] ?? '', - createdAt: Date.now(), - method: 'basic', - username: user.username, - role: user.role, - mustChangePassword: !!user.mustChangePassword, - }); - req.authUser = { username: user.username, role: user.role }; - setSessionCookie(reply, token); - enforcePasswordChange(req, reply, !!user.mustChangePassword); + // 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; } - // 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. - const uFail = (userFailures.get(normUser) ?? 0) + 1; - userFailures.set(normUser, uFail); + // Reserve both slots and publish this verify for joiners, from the values the + // gates just read. Correct only while nothing between those reads and these + // writes awaits: keep it that way. authFailures.set(clientIp, ipFail + 1); - if (uFail >= AUTH_FAILURE_MAX) { - sendAuthRateLimit(reply, userFailures, normUser); - return; + userIpFailures.set(pairKey, pairFail + 1); + // 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 verify = verifyOnce(creds.username, creds.password, normUser, clientIp, pairKey); + verifiesInFlight.set(flightKey, verify); + let outcome: BasicVerifyOutcome; + try { + outcome = await verify; + } finally { + verifiesInFlight.delete(flightKey); } - reply.header('WWW-Authenticate', 'Basic realm="Codeman"'); - reply.code(401).send('Unauthorized'); + sendBasicOutcome(req, reply, outcome, normUser); return; } 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/auth-security.test.ts b/test/auth-security.test.ts index 2f6e633d..712bd5f5 100644 --- a/test/auth-security.test.ts +++ b/test/auth-security.test.ts @@ -15,6 +15,7 @@ import { WebServer } from '../src/web/server.js'; import { TmuxManager } from '../src/tmux-manager.js'; import { SettingsUpdateSchema } from '../src/web/schemas.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.js'; +import { AUTH_FAILURE_MAX } from '../src/config/auth-config.js'; const TEST_USER = 'admin'; const TEST_PASS = 'test-password-12345'; @@ -46,7 +47,7 @@ async function getSessionCookie(baseUrl: string): Promise { } async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise { - for (let i = 0; i < 10; i++) { + for (let i = 0; i < AUTH_FAILURE_MAX; i++) { const res = await fetch(`${baseUrl}/api/status`, { headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) }, }); diff --git a/test/multiuser-auth.test.ts b/test/multiuser-auth.test.ts index fd9c7204..196e779e 100644 --- a/test/multiuser-auth.test.ts +++ b/test/multiuser-auth.test.ts @@ -16,11 +16,21 @@ import path from 'node:path'; 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 { createUser, invalidateUsersCache, verifyPassword } from '../src/user-store.js'; +import type { FastifyInstance } from 'fastify'; +import { AUTH_FAILURE_MAX, AUTH_IP_FAILURE_MAX } from '../src/config/auth-config.js'; +import type { StaleExpirationMap } from '../src/utils/stale-expiration-map.js'; vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); +// Count scrypt verifies: the real verifyPassword, wrapped, so the middleware still checks +// the real store and a test can tell how many requests reached it. +vi.mock('../src/user-store.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, verifyPassword: vi.fn(actual.verifyPassword) }; +}); +const verifyCalls = vi.mocked(verifyPassword); + function basic(user: string, pass: string): string { return 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64'); } @@ -33,6 +43,8 @@ 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 = {}; @@ -72,6 +84,8 @@ beforeAll(async () => { 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; @@ -83,6 +97,24 @@ afterAll(async () => { const url = (p: string) => `http://localhost:${server.boundPort}${p}`; +/** The server's private failure buckets, read to count how many guesses reached the verify. */ +type FailureBuckets = Record<'authFailures' | 'userFailures' | 'userIpFailures', StaleExpirationMap>; + +/** + * 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; + const login = (ip: string, user: string, pass: string) => + app.inject({ method: 'GET', url: '/api/status', headers: { authorization: basic(user, pass) }, remoteAddress: ip }); + return { login, buckets: s as unknown as FailureBuckets }; +} + describe('multi-user auth', () => { it('rejects unauthenticated requests', async () => { const res = await fetch(url('/api/status')); @@ -157,7 +189,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}`; @@ -167,9 +199,10 @@ describe('multi-user auth', () => { const res = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', `bad-${i}`) } }); expect(res.status).toBe(401); } - // Finding #17: the CORRECT password must ALWAYS win (verified BEFORE the per-username - // throttle) — the accumulated failures can never lock the account out — and success - // clears the failure buckets. Previously this returned 429 (the DoS being fixed). + // Finding #17: the per-username bucket is checked AFTER the verify, so it never denies + // a correct password, and success clears the failure buckets. Previously this returned + // 429 (the DoS being fixed). Nine failures also stay under the per-(username, IP) gate, + // which is checked before the verify; that is why the correct password is 200 here. const good = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', 'bobpass123') } }); expect(good.status).toBe(200); // Self-heal: a fresh wrong attempt is 401 again (the counter was reset by the success). @@ -184,6 +217,212 @@ describe('multi-user auth', () => { } expect(limited).toBe(true); }); + + 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(); + const surl = (p: string) => `http://localhost:${sprayServer.boundPort}${p}`; + + // 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 < 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); + } + + // 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+$/); + + // The pre-gate runs before the password check, so even a correct password is refused. + 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-IP pre-gate: a concurrent burst from one IP reaches the verify at most AUTH_IP_FAILURE_MAX times', async () => { + const { login, buckets } = await startIpServer(); + const ip = '203.0.113.40'; + + // More than AUTH_IP_FAILURE_MAX wrong passwords in flight at once, spread so neither + // the pair nor the per-username bucket can refuse any of them. All the gate reads + // happen before the first verify ends, so only a slot reserved before the scrypt + // await stops the burst passing the pre-gate on one stale count. + const extra = 10; + const perUser = AUTH_FAILURE_MAX - 1; + const burst = []; + for (let i = 0; i < AUTH_IP_FAILURE_MAX + extra; i++) { + burst.push(login(ip, `burst${Math.floor(i / perUser)}`, `bad-${i}`)); + } + const codes = (await Promise.all(burst)).map((r) => r.statusCode); + + // No account fills a bucket of its own here, so every verified guess is a 401 and + // every 429 is the pre-gate. + expect(codes.filter((c) => c === 401).length).toBe(AUTH_IP_FAILURE_MAX); + expect(codes.filter((c) => c === 429).length).toBe(extra); + // Each admitted guess counted once: reserved before the verify, not again after it. + expect(buckets.authFailures.get(ip)).toBe(AUTH_IP_FAILURE_MAX); + + // The bucket is full, so the next request from that IP is refused before scrypt, + // a correct password included. + expect((await login(ip, 'burst-next', 'bad')).statusCode).toBe(429); + expect((await login(ip, 'alice', 'alicepass1')).statusCode).toBe(429); + }); + + it('per-(username, IP) gate: a concurrent burst cannot carry the right password past the cap', async () => { + const { login, buckets } = await startIpServer(); + const ip = '203.0.113.50'; + + // AUTH_FAILURE_MAX + 5 logins for bob from one IP, all in flight at once, the LAST + // carrying the correct password. Counting only after the verify let every one of + // them past the gate on the same stale read, and the correct one logged in. + const burst = []; + for (let i = 0; i < AUTH_FAILURE_MAX + 4; i++) burst.push(login(ip, 'bob', `bad-${i}`)); + burst.push(login(ip, 'bob', 'bobpass123')); + const codes = (await Promise.all(burst)).map((r) => r.statusCode); + + expect(codes.at(-1)).toBe(429); + expect(codes.filter((c) => c !== 401 && c !== 429)).toEqual([]); + // Every failed verify adds one to bob's per-username bucket, so it counts the guesses + // that reached scrypt: exactly the AUTH_FAILURE_MAX the pair gate admits. (The last + // of those is a 429 from that bucket, which is why the 401s are fewer.) + expect(buckets.userFailures.get('bob')).toBe(AUTH_FAILURE_MAX); + expect(codes.filter((c) => c === 401).length).toBeLessThanOrEqual(AUTH_FAILURE_MAX); + // Reserved once each, not counted again on failure. + expect(buckets.userIpFailures.get(`${ip}\nbob`)).toBe(AUTH_FAILURE_MAX); + expect(buckets.authFailures.get(ip)).toBe(AUTH_FAILURE_MAX); + + // The lockout is that pair's only: bob still logs in from another IP. + expect((await login('203.0.113.51', 'bob', 'bobpass123')).statusCode).toBe(200); + }); + + it('identical in-flight logins share one verify: a burst of correct ones all log in', async () => { + const { login, buckets } = await startIpServer(); + const ip = '203.0.113.60'; + verifyCalls.mockClear(); + + // A browser whose cookie died with a server restart replays its cached Basic header + // on many parallel requests. Had each reserved its own slot, every one past + // AUTH_FAILURE_MAX would be refused although all carry the right password. + const burst = []; + for (let i = 0; i < AUTH_FAILURE_MAX + 5; i++) burst.push(login(ip, 'bob', 'bobpass123')); + const responses = await Promise.all(burst); + + expect(responses.map((r) => r.statusCode)).toEqual(burst.map(() => 200)); + expect(verifyCalls).toHaveBeenCalledTimes(1); + // Each request still gets a cookie session of its own. + const tokens = responses.map((r) => /codeman_session=([^;]+)/.exec(String(r.headers['set-cookie']))?.[1]); + expect(new Set(tokens).size).toBe(burst.length); + expect(tokens).not.toContain(undefined); + // The one success cleared the buckets, the reservation included. + expect(buckets.userIpFailures.get(`${ip}\nbob`)).toBeUndefined(); + expect(buckets.authFailures.get(ip)).toBeUndefined(); + + // The entry left the map when its verify settled: the next login verifies afresh. + expect((await login(ip, 'bob', 'bobpass123')).statusCode).toBe(200); + expect(verifyCalls).toHaveBeenCalledTimes(2); + }); + + it('identical in-flight logins share one verify: a burst of one wrong password counts once', async () => { + const { login, buckets } = await startIpServer(); + const ip = '203.0.113.70'; + verifyCalls.mockClear(); + + const burst = []; + for (let i = 0; i < AUTH_FAILURE_MAX + 5; i++) burst.push(login(ip, 'bob', 'same-wrong')); + const codes = (await Promise.all(burst)).map((r) => r.statusCode); + + // One guess, verified once, counted once in each bucket, and the same 401 for all. + expect(codes).toEqual(burst.map(() => 401)); + expect(verifyCalls).toHaveBeenCalledTimes(1); + expect(buckets.userFailures.get('bob')).toBe(1); + expect(buckets.userIpFailures.get(`${ip}\nbob`)).toBe(1); + expect(buckets.authFailures.get(ip)).toBe(1); + }); + + it('identical in-flight logins skip the gates: the slot their own verify took does not refuse them', async () => { + const { login } = await startIpServer(); + const ip = '203.0.113.80'; + for (let i = 0; i < AUTH_FAILURE_MAX - 1; i++) { + expect((await login(ip, 'bob', `bad-${i}`)).statusCode, `attempt ${i + 1}`).toBe(401); + } + verifyCalls.mockClear(); + + // The first of these takes the pair's last slot; the rest join its verify, so a gate + // check of their own would refuse them with a full bucket. + const burst = []; + for (let i = 0; i < 5; i++) burst.push(login(ip, 'bob', 'bobpass123')); + const codes = (await Promise.all(burst)).map((r) => r.statusCode); + + expect(codes).toEqual(burst.map(() => 200)); + expect(verifyCalls).toHaveBeenCalledTimes(1); + }); + + it('a verify that throws leaves the in-flight map, and its joiners share the failure', async () => { + const { login } = await startIpServer(); + const ip = '203.0.113.90'; + verifyCalls.mockClear(); + verifyCalls.mockRejectedValueOnce(new Error('user store unreadable')); + + const burst = []; + for (let i = 0; i < 3; i++) burst.push(login(ip, 'bob', 'bobpass123')); + const codes = (await Promise.all(burst)).map((r) => r.statusCode); + + expect(verifyCalls).toHaveBeenCalledTimes(1); + expect(new Set(codes).size).toBe(1); + expect(codes[0]).toBeGreaterThanOrEqual(500); + + // Removed in a finally, so the next identical login runs a verify of its own. + expect((await login(ip, 'bob', 'bobpass123')).statusCode).toBe(200); + expect(verifyCalls).toHaveBeenCalledTimes(2); + }); + + 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)', () => {