From ddc24e4fc71fe4ff0ec5b7d31a9a0161fd49428d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:55:29 +0100 Subject: [PATCH] fix: let a browser an admin signed in from before past the shared admin login limit --- app/api/admin/auth/route.ts | 12 +- .../admin-auth-stalwart-access.test.ts | 3 + .../admin-login-device-limit.test.ts | 106 +++++++++++++++ lib/admin/rate-limit.ts | 35 ++++- lib/admin/session.ts | 127 +++++++++++++----- 5 files changed, 245 insertions(+), 38 deletions(-) create mode 100644 lib/__tests__/admin-login-device-limit.test.ts diff --git a/app/api/admin/auth/route.ts b/app/api/admin/auth/route.ts index baaf584b8..8dd405cc2 100644 --- a/app/api/admin/auth/route.ts +++ b/app/api/admin/auth/route.ts @@ -1,8 +1,8 @@ import { NextRequest, NextResponse } from 'next/server'; import { createHash } from 'crypto'; import { initAdminPassword, verifyAdminPassword, updateLastLogin, isAdminEnabled, getAdminMeta } from '@/lib/admin/password'; -import { setAdminSessionCookie, clearAdminSessionCookie, requireAdminAuth, getClientIP, isSameOriginRequest } from '@/lib/admin/session'; -import { checkRateLimit } from '@/lib/admin/rate-limit'; +import { setAdminSessionCookie, clearAdminSessionCookie, requireAdminAuth, getClientIP, isSameOriginRequest, getAdminDeviceId, setAdminDeviceCookie } from '@/lib/admin/session'; +import { checkRateLimit, checkDeviceRateLimit } from '@/lib/admin/rate-limit'; import { auditLog } from '@/lib/admin/audit'; import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; @@ -160,6 +160,7 @@ export async function POST(request: NextRequest) { } await setAdminSessionCookie(request); + await setAdminDeviceCookie(request); await auditLog('admin.login', { method: 'stalwart' }, ip); return NextResponse.json({ ok: true }); } @@ -170,8 +171,10 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'Admin dashboard is not configured' }, { status: 404 }); } - // Rate limit check - const limit = checkRateLimit(ip); + // Rate limit check. A browser an admin signed in from before is limited + // on its own, so others using up the shared budget can't lock it out. + const deviceId = await getAdminDeviceId(); + const limit = deviceId ? checkDeviceRateLimit(deviceId) : checkRateLimit(ip); if (!limit.allowed) { const retryAfter = Math.ceil(limit.retryAfterMs / 1000); await auditLog('admin.login_blocked', { reason: 'rate_limit' }, ip); @@ -195,6 +198,7 @@ export async function POST(request: NextRequest) { } await setAdminSessionCookie(request); + await setAdminDeviceCookie(request); await updateLastLogin(); await auditLog('admin.login', {}, ip); diff --git a/lib/__tests__/admin-auth-stalwart-access.test.ts b/lib/__tests__/admin-auth-stalwart-access.test.ts index 3958c544f..cf56d0ab4 100644 --- a/lib/__tests__/admin-auth-stalwart-access.test.ts +++ b/lib/__tests__/admin-auth-stalwart-access.test.ts @@ -48,10 +48,13 @@ vi.mock('@/lib/admin/session', () => ({ requireAdminAuth: async () => (state.hasAdminSession ? { role: 'admin' } : { error: 'unauthorized' }), getClientIP: () => '127.0.0.1', isSameOriginRequest: () => true, + getAdminDeviceId: async () => null, + setAdminDeviceCookie: vi.fn(), })); vi.mock('@/lib/admin/rate-limit', () => ({ checkRateLimit: () => ({ allowed: true, retryAfterMs: 0 }), + checkDeviceRateLimit: () => ({ allowed: true, retryAfterMs: 0 }), })); const auditLog = vi.fn(); diff --git a/lib/__tests__/admin-login-device-limit.test.ts b/lib/__tests__/admin-login-device-limit.test.ts new file mode 100644 index 000000000..dd29b2eb2 --- /dev/null +++ b/lib/__tests__/admin-login-device-limit.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Admin password login allows 50 attempts across all clients per 15 minutes, +// because a client without a proxy in front can make up its X-Forwarded-For +// address. Anyone could spend that budget and lock every admin out. A browser +// an admin signed in from before carries a device cookie and is limited on +// its own (OWASP device cookies), so it still gets in. + +vi.mock('next/server', () => ({ + NextResponse: { + json: (data: unknown, init?: { status?: number }) => ({ json: async () => data, status: init?.status ?? 200 }), + }, +})); + +// One cookie jar per browser; `current` is the browser making the request. +const browsers = { current: new Map() }; +vi.mock('next/headers', () => ({ + cookies: async () => ({ + get: (name: string) => (browsers.current.has(name) ? { name, value: browsers.current.get(name)! } : undefined), + set: (name: string, value: string) => { browsers.current.set(name, value); }, + delete: (name: string) => { browsers.current.delete(name); }, + }), +})); + +vi.mock('@/lib/logger', () => ({ logger: { warn: vi.fn(), error: vi.fn(), debug: vi.fn(), info: vi.fn() } })); +vi.mock('@/lib/admin/audit', () => ({ auditLog: vi.fn() })); +vi.mock('@/lib/security/same-origin', () => ({ isSameOriginRequest: () => true })); +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { ensureLoaded: vi.fn(), get: (_key: string, fallback?: unknown) => fallback }, +})); +vi.mock('@/lib/stalwart/credentials', () => ({ getStalwartCredentials: async () => null })); +vi.mock('@/lib/admin/stalwart-admin-access', () => ({ getStalwartAdminAccessMode: async () => 'password' })); +vi.mock('@/lib/admin/password', () => ({ + initAdminPassword: vi.fn().mockResolvedValue(true), + isAdminEnabled: () => true, + verifyAdminPassword: async (password: string) => password === 'right-password', + updateLastLogin: vi.fn(), + getAdminMeta: () => null, + getAdminSessionGeneration: async () => 0, + isAdminSessionRevoked: async () => false, + revokeAdminSession: vi.fn(), +})); + +process.env.SESSION_SECRET = 'a'.repeat(48); + +type Route = typeof import('@/app/api/admin/auth/route'); +let POST: Route['POST']; + +beforeEach(async () => { + // Fresh rate limiter state for every test. + vi.resetModules(); + ({ POST } = await import('@/app/api/admin/auth/route')); +}); + +async function login(browser: Map, password: string, ip: string): Promise { + browsers.current = browser; + const request = new Request('https://bulwark.example/api/admin/auth', { + method: 'POST', + headers: { 'content-type': 'application/json', 'x-forwarded-for': ip }, + body: JSON.stringify({ password }), + }); + const response = await POST(request as never); + return response.status; +} + +/** Wrong passwords from 50 made-up addresses: the whole shared budget. */ +async function spendSharedBudget() { + const attacker = new Map(); + for (let i = 0; i < 50; i++) await login(attacker, 'guess', `198.51.100.${i}`); + expect(await login(attacker, 'guess', '198.51.100.200')).toBe(429); +} + +describe('admin login device cookie', () => { + it('lets a browser an admin signed in from before in after others spent the shared budget', async () => { + const admin = new Map(); + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + expect(admin.has('admin_device')).toBe(true); + + await spendSharedBudget(); + + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + }); + + it('still blocks a new browser once the shared budget is spent', async () => { + await spendSharedBudget(); + expect(await login(new Map(), 'right-password', '203.0.113.8')).toBe(429); + }); + + it('limits a device on its own to 5 attempts', async () => { + const admin = new Map(); + expect(await login(admin, 'right-password', '203.0.113.7')).toBe(200); + for (let i = 0; i < 5; i++) expect(await login(admin, 'wrong', `192.0.2.${i}`)).toBe(401); + expect(await login(admin, 'right-password', '192.0.2.99')).toBe(429); + }); + + it('does not take an admin session token as a device cookie', async () => { + const { createAdminSession, createAdminDeviceToken, verifyAdminDeviceToken, verifyAdminSession } = await import('@/lib/admin/session'); + expect(verifyAdminDeviceToken(createAdminSession())).toBeNull(); + expect(verifyAdminSession(createAdminDeviceToken())).toBeNull(); + expect(verifyAdminDeviceToken(createAdminDeviceToken())).toEqual(expect.any(String)); + + await spendSharedBudget(); + const forged = new Map([['admin_device', createAdminSession()]]); + expect(await login(forged, 'right-password', '203.0.113.9')).toBe(429); + }); +}); diff --git a/lib/admin/rate-limit.ts b/lib/admin/rate-limit.ts index c4b13930a..57dcb5065 100644 --- a/lib/admin/rate-limit.ts +++ b/lib/admin/rate-limit.ts @@ -1,6 +1,8 @@ /** * In-memory rate limiter for admin login. - * Max 5 attempts per IP per 15 minutes, and 50 across all clients. + * Max 5 attempts per IP per 15 minutes, and 50 across all clients. A browser + * with the admin device cookie (lib/admin/session.ts) gets 5 per device + * instead and stays out of the shared ceiling, which anyone can use up. */ const MAX_ATTEMPTS = 5; @@ -22,14 +24,17 @@ interface RateLimitEntry { } const attempts = new Map(); +const deviceAttempts = new Map(); let globalAttempts: RateLimitEntry | null = null; // Clean up expired entries periodically setInterval(() => { const now = Date.now(); - for (const [key, entry] of attempts) { - if (entry.resetAt <= now) { - attempts.delete(key); + for (const map of [attempts, deviceAttempts]) { + for (const [key, entry] of map) { + if (entry.resetAt <= now) { + map.delete(key); + } } } }, 60_000).unref(); @@ -64,3 +69,25 @@ export function checkRateLimit(ip: string): { allowed: boolean; remaining: numbe globalAttempts.count++; return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 }; } + +/** + * Check a browser with a valid admin device cookie, by its device id: 5 + * attempts per 15 minutes, not counted against or stopped by the shared + * ceiling. + */ +export function checkDeviceRateLimit(deviceId: string): { allowed: boolean; remaining: number; retryAfterMs: number } { + const now = Date.now(); + const entry = deviceAttempts.get(deviceId); + + if (!entry || entry.resetAt <= now) { + deviceAttempts.set(deviceId, { count: 1, resetAt: now + WINDOW_MS }); + return { allowed: true, remaining: MAX_ATTEMPTS - 1, retryAfterMs: 0 }; + } + + if (entry.count >= MAX_ATTEMPTS) { + return { allowed: false, remaining: 0, retryAfterMs: entry.resetAt - now }; + } + + entry.count++; + return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 }; +} diff --git a/lib/admin/session.ts b/lib/admin/session.ts index 29f9c64c7..ff690bef3 100644 --- a/lib/admin/session.ts +++ b/lib/admin/session.ts @@ -40,24 +40,12 @@ function getSessionTTL(): number { return isNaN(ttl) || ttl <= 0 ? DEFAULT_ADMIN_SESSION_TTL : ttl; } -/** - * Create an encrypted admin session token, in session generation - * `generation` (see getAdminSessionGeneration). - */ -export function createAdminSession(generation = 0): string { +/** Encrypt `payload` under the session key, bound to `aad`. */ +function sealToken(payload: object, aad: Buffer): string { const key = getKey(); const iv = randomBytes(IV_LENGTH); const cipher = createCipheriv(ALGORITHM, key, iv); - cipher.setAAD(ADMIN_SESSION_AAD); - - const now = Math.floor(Date.now() / 1000); - const payload: AdminSessionPayload = { - role: 'admin', - iat: now, - exp: now + getSessionTTL(), - sid: randomBytes(16).toString('base64url'), - gen: generation, - }; + cipher.setAAD(aad); const json = JSON.stringify(payload); const encrypted = Buffer.concat([cipher.update(json, 'utf8'), cipher.final()]); @@ -66,27 +54,48 @@ export function createAdminSession(generation = 0): string { return Buffer.concat([iv, tag, encrypted]).toString('base64'); } +/** Decrypt a token sealToken made with the same `aad`. Throws when it doesn't open. */ +function openToken(token: string, aad: Buffer): unknown { + const key = getKey(); + const data = Buffer.from(token, 'base64'); + if (data.length < IV_LENGTH + TAG_LENGTH) throw new Error('Token too short'); + + const iv = data.subarray(0, IV_LENGTH); + const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); + const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); + + const decipher = createDecipheriv(ALGORITHM, key, iv); + decipher.setAAD(aad); + decipher.setAuthTag(tag); + + const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); + return JSON.parse(decrypted.toString('utf8')); +} + +/** + * Create an encrypted admin session token, in session generation + * `generation` (see getAdminSessionGeneration). + */ +export function createAdminSession(generation = 0): string { + const now = Math.floor(Date.now() / 1000); + const payload: AdminSessionPayload = { + role: 'admin', + iat: now, + exp: now + getSessionTTL(), + sid: randomBytes(16).toString('base64url'), + gen: generation, + }; + return sealToken(payload, ADMIN_SESSION_AAD); +} + /** * Verify and decode an admin session token. Returns null if invalid or expired. */ export function verifyAdminSession(token: string): AdminSessionPayload | null { try { - const key = getKey(); - const data = Buffer.from(token, 'base64'); - if (data.length < IV_LENGTH + TAG_LENGTH) return null; + const payload = openToken(token, ADMIN_SESSION_AAD) as AdminSessionPayload; - const iv = data.subarray(0, IV_LENGTH); - const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); - const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); - - const decipher = createDecipheriv(ALGORITHM, key, iv); - decipher.setAAD(ADMIN_SESSION_AAD); - decipher.setAuthTag(tag); - - const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); - const payload = JSON.parse(decrypted.toString('utf8')) as AdminSessionPayload; - - if (payload.role !== 'admin') return null; + if (!payload || payload.role !== 'admin') return null; // Every token this version issues has both; ADMIN_SESSION_AAD already // signed the ones from before out. if (typeof payload.sid !== 'string' || typeof payload.gen !== 'number') return null; @@ -167,6 +176,64 @@ export async function setAdminSessionCookie(request?: Request): Promise { }); } +/** + * A browser that has signed in to the admin dashboard before (OWASP's + * "device cookie"). Password attempts from it are limited per device instead + * of by the budget every other client shares (lib/admin/rate-limit.ts), so + * wrong passwords sent from anywhere else can't lock an admin's own browser + * out. It grants nothing else. Sealed like the session token, under its own + * associated data, so neither passes for the other. + */ +export const ADMIN_DEVICE_COOKIE = 'admin_device'; +const ADMIN_DEVICE_AAD = Buffer.from('bulwark-admin-device', 'utf8'); +const ADMIN_DEVICE_TTL = 180 * 24 * 60 * 60; + +interface AdminDevicePayload { + typ: 'admin-device'; + id: string; + exp: number; +} + +export function createAdminDeviceToken(): string { + const payload: AdminDevicePayload = { + typ: 'admin-device', + id: randomBytes(16).toString('base64url'), + exp: Math.floor(Date.now() / 1000) + ADMIN_DEVICE_TTL, + }; + return sealToken(payload, ADMIN_DEVICE_AAD); +} + +/** The device id in an admin device token, or null if it's invalid or expired. */ +export function verifyAdminDeviceToken(token: string): string | null { + try { + const payload = openToken(token, ADMIN_DEVICE_AAD) as AdminDevicePayload; + if (!payload || payload.typ !== 'admin-device' || typeof payload.id !== 'string' || !payload.id) return null; + if (typeof payload.exp !== 'number' || payload.exp < Math.floor(Date.now() / 1000)) return null; + return payload.id; + } catch { + return null; + } +} + +/** The id of this browser's admin device cookie, if it has a valid one. */ +export async function getAdminDeviceId(): Promise { + const cookieStore = await cookies(); + const token = cookieStore.get(ADMIN_DEVICE_COOKIE)?.value; + return token ? verifyAdminDeviceToken(token) : null; +} + +/** Mark this browser as one an admin signed in from. Kept across sign-outs. */ +export async function setAdminDeviceCookie(request?: Request): Promise { + const cookieStore = await cookies(); + cookieStore.set(ADMIN_DEVICE_COOKIE, createAdminDeviceToken(), { + httpOnly: true, + secure: secureCookieFlag(request), + sameSite: 'strict', + path: '/', + maxAge: ADMIN_DEVICE_TTL, + }); +} + /** * Clear the admin session cookie and sign its token out, so a copy of the * cookie stops working too.