From 36118b4a4fe30c1717831145a5df553cf4a0bd7c Mon Sep 17 00:00:00 2001 From: Tony James Date: Mon, 1 Jun 2026 10:16:30 +0100 Subject: [PATCH] fix(security): harden owner pair-code against brute-force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Set express trust proxy=1 so req.ip is the real client (one Dokku nginx hop). Drop the X-Forwarded-For fallback — it was client-spoofable. - Add a global redeem rate limit (30/min across all IPs) so rotating proxies can't outpace the per-IP limiter. - Every failed redeem now decrements life from all active pair codes via a shared failures counter. After 5 wrong guesses during a code's 5-minute TTL, the owner's real code gets burned and they must re-mint. This makes the 20-bit entropy of a 6-digit code defensible: an attacker doesn't know which code is real and can't probe long enough to find it before triggering the burn. --- server/index.ts | 5 ++++ server/routes/owner.ts | 63 +++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/server/index.ts b/server/index.ts index d09a9f5..f19161b 100644 --- a/server/index.ts +++ b/server/index.ts @@ -17,6 +17,11 @@ import { createOptionalAuth } from './lib/auth.js'; const app = express(); +// We sit behind one Nginx hop (Dokku's per-app vhost). Trust exactly one +// proxy so req.ip reflects the real client and the rate limiters below +// can't be bypassed via a spoofed X-Forwarded-For header. +app.set('trust proxy', 1); + app.use(helmet({ contentSecurityPolicy: false })); app.use(cors({ origin: env.appUrl, credentials: true })); app.use(express.json({ limit: '2mb' })); diff --git a/server/routes/owner.ts b/server/routes/owner.ts index 8f158ab..9e40e48 100644 --- a/server/routes/owner.ts +++ b/server/routes/owner.ts @@ -15,24 +15,40 @@ const router = Router(); // ─── Device pair-code store (in-memory) ───────────────────────────────────── // 6-digit short codes minted by an already-logged-in owner session so a // touchscreen device (the Tesla in-car browser) can log in without retyping -// the long OWNER_SECRET. 5-minute TTL, single use, rate-limited. +// the long OWNER_SECRET. +// +// Defence in depth — 6 digits is only ~20 bits of entropy on its own: +// - single-use (delete on success) +// - 5-minute TTL +// - per-code failure cap: a code self-destructs after 5 wrong guesses +// - per-IP rate limit (req.ip via 'trust proxy' = 1, not spoofable) +// - global rate limit across all IPs (defeats parallel-IP brute force) const PAIR_CODE_TTL_MS = 5 * 60 * 1000; -const pairCodes = new Map(); -// IP → recent redeem attempt timestamps (oldest first). -const redeemAttempts = new Map(); -const MAX_REDEEM_PER_MIN = 8; +const MAX_FAILED_PER_CODE = 5; +const MAX_REDEEM_PER_IP_PER_MIN = 8; +const MAX_REDEEM_GLOBAL_PER_MIN = 30; + +interface PairCodeEntry { expiresAt: number; failures: number } +const pairCodes = new Map(); +const redeemAttemptsByIp = new Map(); +let redeemAttemptsGlobal: number[] = []; function cleanPairCodes(): void { const now = Date.now(); for (const [k, v] of pairCodes) if (v.expiresAt < now) pairCodes.delete(k); } -function rateLimited(ip: string): boolean { +function recordAttempt(ip: string): { ipLimited: boolean; globalLimited: boolean } { const now = Date.now(); - const arr = (redeemAttempts.get(ip) || []).filter(t => now - t < 60_000); - arr.push(now); - redeemAttempts.set(ip, arr); - return arr.length > MAX_REDEEM_PER_MIN; + const ipArr = (redeemAttemptsByIp.get(ip) || []).filter(t => now - t < 60_000); + ipArr.push(now); + redeemAttemptsByIp.set(ip, ipArr); + redeemAttemptsGlobal = redeemAttemptsGlobal.filter(t => now - t < 60_000); + redeemAttemptsGlobal.push(now); + return { + ipLimited: ipArr.length > MAX_REDEEM_PER_IP_PER_MIN, + globalLimited: redeemAttemptsGlobal.length > MAX_REDEEM_GLOBAL_PER_MIN, + }; } router.get('/api/auth/owner/status', (req, res) => { @@ -77,28 +93,49 @@ router.post('/api/auth/owner/pair-code', (req, res) => { do { code = String(crypto.randomInt(100_000, 1_000_000)); } while (pairCodes.has(code)); - pairCodes.set(code, { expiresAt: Date.now() + PAIR_CODE_TTL_MS }); + pairCodes.set(code, { expiresAt: Date.now() + PAIR_CODE_TTL_MS, failures: 0 }); res.json({ ok: true, code, expiresInS: Math.floor(PAIR_CODE_TTL_MS / 1000) }); }); +// Mark a failed redeem against every currently-active code. With 6-digit +// entropy, an attacker brute-forcing the keyspace doesn't know which code +// the owner minted — but every wrong guess they make burns life from the +// real code. Five misses during a code's TTL is enough to invalidate it +// and force the owner to re-mint, which defeats parallel-IP brute force. +function markFailureAgainstActiveCodes(): void { + for (const [code, entry] of pairCodes) { + entry.failures += 1; + if (entry.failures > MAX_FAILED_PER_CODE) { + pairCodes.delete(code); + } + } +} + // ─── Pair code: redeem on a new device ────────────────────────────────────── router.post('/api/auth/owner/pair-redeem', (req, res) => { - const ip = (req.ip || (req.headers['x-forwarded-for'] as string) || 'unknown').toString(); - if (rateLimited(ip)) { + // req.ip is trustworthy because we set 'trust proxy' = 1 in server/index.ts. + // Do NOT fall back to the X-Forwarded-For header — it's client-controlled. + const ip = req.ip || 'unknown'; + const { ipLimited, globalLimited } = recordAttempt(ip); + if (ipLimited || globalLimited) { + log.warn({ ip, ipLimited, globalLimited }, 'pair-redeem rate limited'); res.status(429).json({ ok: false, reason: 'rate_limited' }); return; } const { code } = (req.body || {}) as { code?: unknown }; if (typeof code !== 'string' || !/^\d{6}$/.test(code)) { + markFailureAgainstActiveCodes(); res.status(400).json({ ok: false, reason: 'bad_code' }); return; } cleanPairCodes(); const entry = pairCodes.get(code); if (!entry) { + markFailureAgainstActiveCodes(); res.status(401).json({ ok: false, reason: 'invalid_or_expired' }); return; } + // Success: consume the code, set the cookie. pairCodes.delete(code); setOwnerCookie(res); log.info({ ip }, 'Device paired via owner pair code');