From f69652d43b011adb85dd42106204dd6d7cc7e7b7 Mon Sep 17 00:00:00 2001 From: yiekheng Date: Sun, 10 May 2026 20:41:49 +0800 Subject: [PATCH] feat(web): AES-GCM cookies + per-username/global rate limit + origin check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three layers of login hardening pulled together — addresses the "don't let middleman / robot easily log in by mimicking headers" follow-up. 1. AES-256-GCM session cookie (apps/web/src/lib/auth-cookie.ts) The old format was base64-encoded JSON + HMAC-SHA256 signature, so anyone with the cookie could read userId/role straight off the bytes. Switched to AES-GCM authenticated encryption: the payload is encrypted with a 256-bit key derived from AUTH_SECRET via SHA-256, a fresh 12-byte nonce is drawn per encryption (never reused — locked in by test), and tampering with either the IV or ciphertext fails the GCM auth tag → decrypt throws → null. Cookie format: . Existing cookies become invalid on deploy because the IV portion doesn't decode to 12 bytes — middleware bounces them to /login. No env bump needed; users just sign in once with the new secret. 2. Three-layer rate limit on loginAction Old: per-IP only. An attacker with a residential-proxy pool or spoofed X-Forwarded-For could hop IPs and brute one account. New: Promise.all of three checkRateLimit calls - per-IP login: 10 / 5 min - per-username login-user: 5 / 15 min - global login-global 100 / min (backstop) First-hit wins; logger captures which limit tripped (ip / username / global) without telling the attacker which one. 3. Action-level Origin/Host check serverActions.allowedOrigins already does this at the framework layer; running it inside loginAction lets us log the mismatch and reject before bcrypt + DB. Missing Origin treated as same-origin (RFC: same-origin POSTs may omit it). Malformed Origin → reject. Tests: - auth-cookie.test.ts updated to AES-GCM (15 tests, +4 vs HMAC): fresh IV per encryption, ciphertext doesn't leak userId/role, IV-swap rejected, ciphertext-tamper rejected, wrong-length IV rejected, malformed b64 doesn't throw. - auth.test.ts adds 7 new cases: three-layer key shape, per-username limit alone trips, global limit alone trips, cross-origin rejected, same-origin accepted, missing-Origin treated as same-origin, malformed-Origin rejected. Web suite 453 → 463 tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/actions/auth.test.ts | 114 +++++++++++++++++++++++---- apps/web/src/actions/auth.ts | 61 +++++++++++++- apps/web/src/lib/auth-cookie.test.ts | 60 +++++++++++--- apps/web/src/lib/auth-cookie.ts | 109 +++++++++++++++---------- 4 files changed, 275 insertions(+), 69 deletions(-) diff --git a/apps/web/src/actions/auth.test.ts b/apps/web/src/actions/auth.test.ts index 1ea5785..abbb94e 100644 --- a/apps/web/src/actions/auth.test.ts +++ b/apps/web/src/actions/auth.test.ts @@ -6,6 +6,7 @@ const { cookiesDeleteMock, findUserMock, headersGetMock, + headerStore, checkRateLimitMock, redirectMock, loggerMock, @@ -14,6 +15,7 @@ const { cookiesDeleteMock: vi.fn(), findUserMock: vi.fn(), headersGetMock: vi.fn(() => "127.0.0.1"), + headerStore: new Map(), checkRateLimitMock: vi.fn(), redirectMock: vi.fn((_path: string) => { throw new Error("redirect"); @@ -24,7 +26,14 @@ const { vi.mock("next/headers", () => ({ cookies: async () => ({ set: cookiesSetMock, delete: cookiesDeleteMock }), headers: async () => ({ - get: (k: string) => (k.toLowerCase() === "x-forwarded-for" ? headersGetMock() : null), + get: (k: string) => { + const key = k.toLowerCase(); + if (key === "x-forwarded-for") return headersGetMock(); + // Tests opt-in to setting origin/host/etc. via headerStore; + // unset = null which lets hasSameOriginRequest treat the + // request as same-origin (Origin omitted = same-origin per RFC). + return headerStore.get(key) ?? null; + }, }), })); vi.mock("next/navigation", () => ({ @@ -56,6 +65,7 @@ beforeEach(() => { throw new Error("redirect"); }); loggerMock.warn.mockReset(); + headerStore.clear(); }); import { loginAction, logoutAction } from "./auth"; @@ -221,22 +231,21 @@ describe("logoutAction", () => { }); describe("loginAction — additional cases", () => { - it("issues a cookie with role='user' encoded in the payload for a non-admin user", async () => { + it("issues a cookie that decrypts to role='user' for a non-admin user", async () => { findUserMock.mockResolvedValue({ ...ADMIN_ROW, role: "user" }); await loginAction(fd({ username: "alice", password: "correct-horse" })).catch(() => {}); expect(cookiesSetMock).toHaveBeenCalledTimes(1); const [, cookieValue] = cookiesSetMock.mock.calls[0]!; - // Cookie shape: .. Decode payload and - // assert the role round-trips, so the middleware/AppShell role gate gets - // accurate data without us having to import auth-cookie. - const [payloadEnc] = (cookieValue as string).split("."); - const json = Buffer.from( - payloadEnc!.replace(/-/g, "+").replace(/_/g, "/") + - "=".repeat((4 - (payloadEnc!.length % 4)) % 4), - "base64", - ).toString("utf8"); - const decoded = JSON.parse(json); - expect(decoded.role).toBe("user"); + // The cookie is now AES-GCM encrypted, so we can't peel the payload + // off raw — decrypt with the same secret loginAction used. This + // also doubles as a confidentiality smoke test: 'user'/'alice' + // must NOT appear verbatim in the cookie bytes. + expect(cookieValue as string).not.toContain("alice"); + expect(cookieValue as string).not.toContain("user"); + const { verifySession } = await import("@/lib/auth-cookie"); + const decoded = await verifySession(cookieValue as string, SECRET); + expect(decoded?.role).toBe("user"); + expect(decoded?.userId).toBe(ADMIN_ROW.id); }); it("rejects when the user row has an unrecognised role string", async () => { @@ -265,12 +274,85 @@ describe("loginAction — additional cases", () => { expect(findUserMock).not.toHaveBeenCalled(); }); - it("rate limit key includes the client IP so a hostile IP can't lock everyone out", async () => { + it("uses three rate-limit layers: per-IP, per-username, global", async () => { findUserMock.mockResolvedValue(ADMIN_ROW); headersGetMock.mockReturnValue("198.51.100.42"); + await loginAction(fd({ username: "Admin", password: "correct-horse" })).catch(() => {}); + // Three checkRateLimit calls fired in parallel via Promise.all, + // in this order: ip / user / global. + expect(checkRateLimitMock).toHaveBeenCalledTimes(3); + const keys = checkRateLimitMock.mock.calls.map((c) => c[0] as string); + expect(keys[0]).toBe("login:198.51.100.42"); + // Username key is normalised to lowercase so "Admin" and "admin" + // share the same bucket — otherwise an attacker rotating case + // would dodge per-username throttling. + expect(keys[1]).toBe("login-user:admin"); + expect(keys[2]).toBe("login-global"); + }); + + it("rejects login when the per-username limit alone is hit (rotating-IPs attacker)", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + // First call (ip) passes, second (user) is over, third (global) passes. + checkRateLimitMock + .mockResolvedValueOnce({ limited: false, count: 1 }) + .mockResolvedValueOnce({ limited: true, count: 6 }) + .mockResolvedValueOnce({ limited: false, count: 5 }); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Too many attempts. Try again later." }); + expect(findUserMock).not.toHaveBeenCalled(); + // Logger captures which limit tripped so we can tune thresholds + // without leaking the answer to the attacker. + const meta = loggerMock.warn.mock.calls.find((c) => c[1] === "login rate-limited")?.[0]; + expect(meta).toMatchObject({ limit: "username" }); + }); + + it("rejects login when the global limit alone is hit (everyone-pile-on backstop)", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + checkRateLimitMock + .mockResolvedValueOnce({ limited: false, count: 1 }) + .mockResolvedValueOnce({ limited: false, count: 1 }) + .mockResolvedValueOnce({ limited: true, count: 101 }); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Too many attempts. Try again later." }); + const meta = loggerMock.warn.mock.calls.find((c) => c[1] === "login rate-limited")?.[0]; + expect(meta).toMatchObject({ limit: "global" }); + }); + + it("rejects a cross-origin POST before checking credentials", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + headerStore.set("origin", "https://attacker.example"); + headerStore.set("host", "wabot.04080616.xyz"); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Cross-origin request blocked." }); + expect(checkRateLimitMock).not.toHaveBeenCalled(); + expect(findUserMock).not.toHaveBeenCalled(); + }); + + it("accepts a same-origin POST (Origin host matches Host header)", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + headerStore.set("origin", "https://wabot.04080616.xyz"); + headerStore.set("host", "wabot.04080616.xyz"); await loginAction(fd({ username: "admin", password: "correct-horse" })).catch(() => {}); - const [key] = checkRateLimitMock.mock.calls[0]!; - expect(key).toContain("198.51.100.42"); + // Got past the origin check → DB lookup ran. + expect(findUserMock).toHaveBeenCalled(); + }); + + it("treats a missing Origin header as same-origin (legitimate native form POST)", async () => { + // Browsers don't always send Origin (e.g. plain top-level form + // submissions). Refusing those would brick login on some clients. + findUserMock.mockResolvedValue(ADMIN_ROW); + headerStore.delete("origin"); + headerStore.set("host", "wabot.04080616.xyz"); + await loginAction(fd({ username: "admin", password: "correct-horse" })).catch(() => {}); + expect(findUserMock).toHaveBeenCalled(); + }); + + it("rejects when Origin is malformed (non-URL string)", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + headerStore.set("origin", "not a url"); + headerStore.set("host", "wabot.04080616.xyz"); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Cross-origin request blocked." }); }); it("still hits the DB and bcrypt on the unknown-user path so timing equivalence holds", async () => { diff --git a/apps/web/src/actions/auth.ts b/apps/web/src/actions/auth.ts index fbf0773..730f945 100644 --- a/apps/web/src/actions/auth.ts +++ b/apps/web/src/actions/auth.ts @@ -32,6 +32,31 @@ async function clientIp(): Promise { return h.get("x-real-ip") ?? "unknown"; } +/** + * Compare the inbound Origin to the request's Host. Server Actions + * already get an Origin check via Next 16's + * `serverActions.allowedOrigins`, but that's a global config — running + * the same comparison here is cheap belt-and-braces and lets us log + * mismatches with action-level context. Returns true when: + * - no Origin header is present (same-origin POSTs from the same + * server), OR + * - Origin's host matches the Host header (same-origin) + * Anything else (cross-origin POST, malformed Origin, etc.) → false. + */ +async function hasSameOriginRequest(): Promise { + const h = await headers(); + const origin = h.get("origin"); + if (!origin) return true; // RFC: same-origin requests may omit Origin + const host = h.get("host"); + if (!host) return false; + try { + const u = new URL(origin); + return u.host === host; + } catch { + return false; + } +} + export async function loginAction(formData: FormData): Promise { const username = (formData.get("username") ?? "").toString(); const password = (formData.get("password") ?? "").toString(); @@ -44,9 +69,41 @@ export async function loginAction(formData: FormData): Promise { return { ok: false, error: "Input too long." }; } + // Action-level Origin check. Next 16's serverActions.allowedOrigins + // already gates this at the framework boundary, but doing it here + // with action context lets us log the mismatch and surface a clean + // error instead of relying on the global config alone. + if (!(await hasSameOriginRequest())) { + logger.warn({}, "login rejected: cross-origin request"); + return { ok: false, error: "Cross-origin request blocked." }; + } + const ip = await clientIp(); - const rl = await checkRateLimit(`login:${ip}`, { max: 10, windowSec: 300 }); - if (rl.limited) { + // Three-layer rate limit: + // per-IP — typical brute-forcer + // per-username — attacker who rotates IPs (X-Forwarded-For + // spoofing, residential proxy pool) but pounds + // a single account + // global — backstop. If the attacker controls enough + // IP+username combos to slip past the first two, + // this caps the total login attempts per minute + // across the install. Lock occurs at the FIRST + // limit hit; we don't reveal which one. + const usernameKey = username.trim().toLowerCase(); + const [rlIp, rlUser, rlGlobal] = await Promise.all([ + checkRateLimit(`login:${ip}`, { max: 10, windowSec: 300 }), + checkRateLimit(`login-user:${usernameKey}`, { max: 5, windowSec: 900 }), + checkRateLimit(`login-global`, { max: 100, windowSec: 60 }), + ]); + if (rlIp.limited || rlUser.limited || rlGlobal.limited) { + logger.warn( + { + ip, + username: usernameKey, + limit: rlIp.limited ? "ip" : rlUser.limited ? "username" : "global", + }, + "login rate-limited", + ); return { ok: false, error: "Too many attempts. Try again later." }; } diff --git a/apps/web/src/lib/auth-cookie.test.ts b/apps/web/src/lib/auth-cookie.test.ts index 71d7be0..8293bb5 100644 --- a/apps/web/src/lib/auth-cookie.test.ts +++ b/apps/web/src/lib/auth-cookie.test.ts @@ -23,28 +23,51 @@ const validPayload = (): SessionPayload => ({ v: 1, }); -describe("auth-cookie", () => { +describe("auth-cookie (AES-256-GCM)", () => { it("signSession + verifySession round-trips a valid payload", async () => { const cookie = await signSession(validPayload(), SECRET); const verified = await verifySession(cookie, SECRET, NOW); expect(verified).toEqual(validPayload()); }); - it("rejects when the payload portion has been tampered with", async () => { + it("uses a fresh IV per call so two encryptions of the same payload differ", async () => { + // Repeating the IV under AES-GCM is catastrophic (it leaks the XOR + // of plaintexts and the auth key). Lock in that signSession draws + // a new nonce every time — the byte-for-byte cookies must not match + // even when the inputs are identical. + const a = await signSession(validPayload(), SECRET); + const b = await signSession(validPayload(), SECRET); + expect(a).not.toBe(b); + // Both still decrypt correctly with the same secret. + expect(await verifySession(a, SECRET, NOW)).toEqual(validPayload()); + expect(await verifySession(b, SECRET, NOW)).toEqual(validPayload()); + }); + + it("ciphertext does NOT leak the userId in plaintext (confidentiality)", async () => { const cookie = await signSession(validPayload(), SECRET); - // Flip the role to admin → user in the payload, keep the same signature. - const [, sig] = cookie.split("."); - const tampered = btoa(JSON.stringify({ ...validPayload(), role: "user" })) - .replace(/\+/g, "-") - .replace(/\//g, "_") - .replace(/=+$/, "") + "." + sig; + // The whole point of the GCM upgrade: someone with only the cookie + // value should not be able to read the userId / role straight off + // it the way they could with the old base64-encoded JSON. + expect(cookie).not.toContain(validPayload().userId); + expect(cookie).not.toContain("admin"); + }); + + it("rejects when the ciphertext has been tampered with (auth tag mismatch)", async () => { + const cookie = await signSession(validPayload(), SECRET); + const [iv, ct] = cookie.split("."); + // Flip the last character of the ciphertext (still valid base64url). + const lastCh = ct!.slice(-1); + const replacement = lastCh === "A" ? "B" : "A"; + const tampered = `${iv}.${ct!.slice(0, -1)}${replacement}`; expect(await verifySession(tampered, SECRET, NOW)).toBeNull(); }); - it("rejects when the signature has been tampered with", async () => { + it("rejects when the IV has been swapped for another (auth tag mismatch)", async () => { const cookie = await signSession(validPayload(), SECRET); - const [payload] = cookie.split("."); - const tampered = payload + ".AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + const otherIv = await signSession(validPayload(), SECRET); + const [, ct] = cookie.split("."); + const [otherIvB64] = otherIv.split("."); + const tampered = `${otherIvB64}.${ct}`; expect(await verifySession(tampered, SECRET, NOW)).toBeNull(); }); @@ -91,6 +114,21 @@ describe("auth-cookie", () => { expect(await verifySession("", SECRET, NOW)).toBeNull(); }); + it("rejects a cookie whose IV decodes to the wrong byte length", async () => { + // GCM requires a 12-byte nonce. Swap the IV portion for something + // that decodes to a different length and confirm we bounce it + // before handing weird input to crypto.subtle.decrypt. + const cookie = await signSession(validPayload(), SECRET); + const [, ct] = cookie.split("."); + // 8 bytes encoded — too short. + const shortIv = "AAAAAAAAAAA"; + expect(await verifySession(`${shortIv}.${ct}`, SECRET, NOW)).toBeNull(); + }); + + it("rejects malformed (non-base64url) input gracefully — no throw, just null", async () => { + expect(await verifySession("$$$.$$$", SECRET, NOW)).toBeNull(); + }); + it("exposes COOKIE_NAME as 'session'", () => { expect(COOKIE_NAME).toBe("session"); }); diff --git a/apps/web/src/lib/auth-cookie.ts b/apps/web/src/lib/auth-cookie.ts index c3928f9..96f5771 100644 --- a/apps/web/src/lib/auth-cookie.ts +++ b/apps/web/src/lib/auth-cookie.ts @@ -1,7 +1,18 @@ /** - * Edge-runtime-safe HMAC-signed session cookie. Runs in middleware + * Edge-runtime-safe AES-256-GCM session cookie. Runs in middleware * and Server Actions. NO database, NO bcrypt, NO Node-only APIs — * pure Web Crypto so it survives Edge runtime. + * + * Why GCM instead of HMAC-then-plaintext: GCM is authenticated + * encryption, so a leaked cookie no longer hands the userId/role to + * an attacker who only sees the bytes. Tampering with either the IV + * or the ciphertext invalidates the auth tag → decrypt throws → we + * return null. Replay protection comes from the per-payload `exp` + * field plus the global `OPERATOR_TOKEN_VERSION` kill switch. + * + * Cookie format: `.` + * - iv: 12 random bytes (GCM nonce) + * - ciphertext+tag: AES-GCM output (plaintext + 16-byte auth tag) */ export const COOKIE_NAME = "session"; @@ -44,35 +55,40 @@ function b64urlDecode(str: string): Uint8Array { return out; } -async function importKey(secret: string): Promise { +/** + * Derive a 256-bit AES key from the operator-supplied AUTH_SECRET. + * SHA-256 hashes the secret to a fixed-length key so the secret can + * be any printable string in env (no min/max length policing here). + */ +async function deriveKey(secret: string): Promise { + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(secret), + ); return crypto.subtle.importKey( "raw", - new TextEncoder().encode(secret), - { name: "HMAC", hash: "SHA-256" }, + digest, + { name: "AES-GCM" }, false, - ["sign", "verify"], + ["encrypt", "decrypt"], ); } -/** Constant-time compare on byte arrays. Returns true iff equal. */ -function timingSafeEqual(a: Uint8Array, b: Uint8Array): boolean { - if (a.length !== b.length) return false; - let diff = 0; - for (let i = 0; i < a.length; i++) diff |= a[i]! ^ b[i]!; - return diff === 0; -} - export async function signSession( payload: SessionPayload, secret: string, ): Promise { - const json = JSON.stringify(payload); - const payloadEnc = b64urlEncode(new TextEncoder().encode(json)); - const key = await importKey(secret); - const sigBytes = new Uint8Array( - await crypto.subtle.sign("HMAC", key, new TextEncoder().encode(payloadEnc)), + const iv = crypto.getRandomValues(new Uint8Array(12)); + const key = await deriveKey(secret); + const plaintext = new TextEncoder().encode(JSON.stringify(payload)); + const ct = new Uint8Array( + await crypto.subtle.encrypt( + { name: "AES-GCM", iv: iv as BufferSource }, + key, + plaintext as BufferSource, + ), ); - return `${payloadEnc}.${b64urlEncode(sigBytes)}`; + return `${b64urlEncode(iv)}.${b64urlEncode(ct)}`; } export async function verifySession( @@ -83,37 +99,50 @@ export async function verifySession( if (!cookie || typeof cookie !== "string") return null; const dot = cookie.indexOf("."); if (dot <= 0 || dot === cookie.length - 1) return null; - const payloadEnc = cookie.slice(0, dot); - const sigEnc = cookie.slice(dot + 1); - - let sigBytes: Uint8Array; + let iv: Uint8Array; + let ct: Uint8Array; try { - sigBytes = b64urlDecode(sigEnc); + iv = b64urlDecode(cookie.slice(0, dot)); + ct = b64urlDecode(cookie.slice(dot + 1)); } catch { return null; } - - const key = await importKey(secret); - const expected = new Uint8Array( - await crypto.subtle.sign("HMAC", key, new TextEncoder().encode(payloadEnc)), - ); - if (!timingSafeEqual(sigBytes, expected)) return null; - - let json: string; - let payload: unknown; + // GCM nonces are exactly 12 bytes. A wrong-sized IV would still + // sometimes succeed at the WebCrypto layer on some platforms; + // guard explicitly so callers can't slip a non-standard nonce past us. + if (iv.length !== 12) return null; + let plain: string; try { - json = new TextDecoder().decode(b64urlDecode(payloadEnc)); - payload = JSON.parse(json); + const key = await deriveKey(secret); + // The IV in `AesGcmParams` must be backed by a non-shared + // ArrayBuffer; TS 5.7+ tightened Uint8Array's generic to reject + // `SharedArrayBuffer`-backed views. b64urlDecode produces a + // regular ArrayBuffer, but we cast to BufferSource explicitly so + // future allocator changes don't regress this site. + const buf = await crypto.subtle.decrypt( + { name: "AES-GCM", iv: iv as BufferSource }, + key, + ct as BufferSource, + ); + plain = new TextDecoder().decode(buf); + } catch { + // Auth-tag mismatch (tampered cookie or wrong key) lands here. + return null; + } + + let parsed: unknown; + try { + parsed = JSON.parse(plain); } catch { return null; } - if (!isValidPayload(payload)) return null; + if (!isValidPayload(parsed)) return null; - if (payload.exp <= now) return null; - if (payload.iat > now + CLOCK_SKEW_SECONDS) return null; + if (parsed.exp <= now) return null; + if (parsed.iat > now + CLOCK_SKEW_SECONDS) return null; const expectedV = Number(process.env.OPERATOR_TOKEN_VERSION ?? "1"); - if (payload.v !== expectedV) return null; + if (parsed.v !== expectedV) return null; - return payload; + return parsed; }