mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-14 18:22:46 -06:00
fix(api): harden device pairing against brute-force (#87)
The 6-digit pairing code is generated client-side, so the server can't raise its entropy without a player change. Instead, harden server-side (no client change): - lib/pair-lockout.js: lock an IP out of POST /api/provision/pair after 5 failed claims (15-min lockout), and expire stale provisioning codes after 15 min so a code is not claimable indefinitely. A successful claim resets the IP. - /pair enforces both. Only an UNKNOWN code (404) counts toward the lockout (a real guess); an EXPIRED code (410) is a legitimate-but-stale code and does NOT count, so a slow bulk rollout from one shared-NAT IP can't lock itself out. getClientIp is Cloudflare-aware (CF-Connecting-IP validated against a trusted edge peer), so the lockout keys on the real per-client IP, never a shared edge. Unit-tested deterministically with injected time, incl. the bulk-rollout-never-locks case. Closes #87 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
3305e79e61
commit
f06a87f4be
39
server/lib/pair-lockout.js
Normal file
39
server/lib/pair-lockout.js
Normal file
|
|
@ -0,0 +1,39 @@
|
|||
'use strict';
|
||||
|
||||
// #87: brute-force hardening for device pairing. The 6-digit pairing code is generated
|
||||
// client-side, so the server can't raise its entropy without a player change - but it can
|
||||
// (a) lock out an IP after repeated failed claims and (b) expire stale provisioning codes
|
||||
// so a code is not claimable indefinitely. Together with the 5/min rate-limit on
|
||||
// /api/provision (#88), guessing the ~1M code space becomes infeasible (a locked-out IP
|
||||
// gets ~5 tries per 15 min, and each code only lives 15 min).
|
||||
|
||||
const MAX_FAILS = 5; // consecutive failed claims from an IP before lockout
|
||||
const LOCKOUT_MS = 15 * 60 * 1000; // how long the IP is then blocked from /pair
|
||||
const PAIRING_TTL_SEC = 15 * 60; // how long a provisioning code stays claimable
|
||||
|
||||
const failures = new Map(); // ip -> { count, lockedUntil }
|
||||
|
||||
function isLocked(ip, now = Date.now()) {
|
||||
const rec = failures.get(ip);
|
||||
return !!(rec && rec.lockedUntil > now);
|
||||
}
|
||||
|
||||
// Record one failed claim from an IP; trip the lockout once MAX_FAILS is reached.
|
||||
function recordFailure(ip, now = Date.now()) {
|
||||
const rec = failures.get(ip) || { count: 0, lockedUntil: 0 };
|
||||
rec.count += 1;
|
||||
if (rec.count >= MAX_FAILS) { rec.lockedUntil = now + LOCKOUT_MS; rec.count = 0; }
|
||||
failures.set(ip, rec);
|
||||
return rec;
|
||||
}
|
||||
|
||||
// A successful pair (or any reason to forgive an IP) clears its failure record.
|
||||
function reset(ip) { failures.delete(ip); }
|
||||
|
||||
// A provisioning code is stale once it is older than the TTL (devices.created_at is the
|
||||
// register time for a provisioning device).
|
||||
function isCodeExpired(createdAtSec, now = Date.now()) {
|
||||
return Math.floor(now / 1000) - createdAtSec > PAIRING_TTL_SEC;
|
||||
}
|
||||
|
||||
module.exports = { isLocked, recordFailure, reset, isCodeExpired, MAX_FAILS, LOCKOUT_MS, PAIRING_TTL_SEC };
|
||||
|
|
@ -575,7 +575,14 @@ const originalProvisionRoute = require('./routes/provisioning');
|
|||
|
||||
// Override provision to also notify device via WS
|
||||
const { checkDeviceLimit } = require('./middleware/subscription');
|
||||
const pairLockout = require('./lib/pair-lockout');
|
||||
app.post('/api/provision/pair', requireAuth, resolveTenancy, checkDeviceLimit, (req, res) => {
|
||||
// #87: lock out an IP after repeated failed pairing-code guesses (brute-force defense
|
||||
// beyond the 5/min rate-limit on /api/provision).
|
||||
const ip = getClientIp(req);
|
||||
if (pairLockout.isLocked(ip)) {
|
||||
return res.status(429).json({ error: 'Too many failed pairing attempts. Try again in a few minutes.' });
|
||||
}
|
||||
const { pairing_code, name } = req.body;
|
||||
if (!pairing_code) return res.status(400).json({ error: 'pairing_code required' });
|
||||
// Phase 2.2a: pair into the caller's current workspace. Refusing on no
|
||||
|
|
@ -584,7 +591,18 @@ app.post('/api/provision/pair', requireAuth, resolveTenancy, checkDeviceLimit, (
|
|||
if (!req.workspaceId) return res.status(403).json({ error: 'No workspace context. Switch to a workspace before pairing.' });
|
||||
|
||||
const device = db.prepare('SELECT * FROM devices WHERE pairing_code = ?').get(pairing_code);
|
||||
if (!device) return res.status(404).json({ error: 'No device found with that pairing code' });
|
||||
// #87: an UNKNOWN code is a brute-force guess - count it toward the per-IP lockout.
|
||||
if (!device) {
|
||||
pairLockout.recordFailure(ip);
|
||||
return res.status(404).json({ error: 'No device found with that pairing code' });
|
||||
}
|
||||
// An EXPIRED code is a legitimate-but-stale code (a slow rollout, not an attack), so it
|
||||
// does NOT count toward the lockout - it just asks the display to regenerate. This keeps
|
||||
// a bulk rollout from one office/NAT IP from locking itself out on expired codes.
|
||||
if (pairLockout.isCodeExpired(device.created_at)) {
|
||||
return res.status(410).json({ error: 'Pairing code expired - restart the display to get a new code' });
|
||||
}
|
||||
pairLockout.reset(ip); // a valid claim forgives prior failed attempts from this IP
|
||||
|
||||
const deviceName = name || 'Display ' + (db.prepare('SELECT COUNT(*) as count FROM devices WHERE user_id = ?').get(req.user.id).count + 1);
|
||||
db.prepare("UPDATE devices SET pairing_code = NULL, name = ?, user_id = ?, workspace_id = ?, status = 'online', updated_at = strftime('%s','now') WHERE id = ?")
|
||||
|
|
|
|||
57
server/test/pair-lockout.test.js
Normal file
57
server/test/pair-lockout.test.js
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
'use strict';
|
||||
|
||||
// #87: unit tests for the pairing brute-force hardening (lockout + code expiry). Pure
|
||||
// logic with injected time - deterministic and free of the /api/provision rate-limit's
|
||||
// 5/min interference, which is the right level to assert this security behaviour.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const crypto = require('node:crypto');
|
||||
const lk = require('../lib/pair-lockout');
|
||||
|
||||
const ip = () => 'test-' + crypto.randomUUID(); // unique IP per test (the map is module-level)
|
||||
|
||||
test('lockout: an IP is not locked until MAX_FAILS failed attempts', () => {
|
||||
const a = ip();
|
||||
assert.equal(lk.isLocked(a, 1000), false);
|
||||
for (let i = 0; i < lk.MAX_FAILS - 1; i++) lk.recordFailure(a, 1000);
|
||||
assert.equal(lk.isLocked(a, 1000), false, `still open after ${lk.MAX_FAILS - 1} fails`);
|
||||
lk.recordFailure(a, 1000); // the MAX_FAILS-th
|
||||
assert.equal(lk.isLocked(a, 1000), true, 'locked at MAX_FAILS');
|
||||
});
|
||||
|
||||
test('lockout: the block lifts after LOCKOUT_MS', () => {
|
||||
const a = ip();
|
||||
for (let i = 0; i < lk.MAX_FAILS; i++) lk.recordFailure(a, 1000);
|
||||
assert.equal(lk.isLocked(a, 1000 + lk.LOCKOUT_MS - 1), true, 'still locked just before the window ends');
|
||||
assert.equal(lk.isLocked(a, 1000 + lk.LOCKOUT_MS + 1), false, 'unlocked after the window');
|
||||
});
|
||||
|
||||
test('lockout: reset() (a successful pair) forgives prior failures', () => {
|
||||
const a = ip();
|
||||
for (let i = 0; i < lk.MAX_FAILS - 1; i++) lk.recordFailure(a, 1000);
|
||||
lk.reset(a);
|
||||
for (let i = 0; i < lk.MAX_FAILS - 1; i++) lk.recordFailure(a, 1000);
|
||||
assert.equal(lk.isLocked(a, 1000), false, 'reset cleared the earlier fails, so no lockout');
|
||||
});
|
||||
|
||||
test('expiry: a code is claimable inside the TTL and expired after it', () => {
|
||||
const now = 1_000_000_000_000; // fixed ms
|
||||
const nowSec = Math.floor(now / 1000);
|
||||
assert.equal(lk.isCodeExpired(nowSec, now), false, 'a fresh code is valid');
|
||||
assert.equal(lk.isCodeExpired(nowSec - (lk.PAIRING_TTL_SEC - 5), now), false, 'still valid just inside the TTL');
|
||||
assert.equal(lk.isCodeExpired(nowSec - (lk.PAIRING_TTL_SEC + 5), now), true, 'expired just past the TTL');
|
||||
});
|
||||
|
||||
test('lockout: a bulk rollout from one IP never locks (each successful pair resets)', () => {
|
||||
// The roofing-office scenario: many displays paired from one shared-NAT IP, even with the
|
||||
// odd fat-fingered code, never trips the 5-fail lockout because each success resets the
|
||||
// counter. (Expired codes don't count toward the lockout at all - see the /pair handler.)
|
||||
const office = ip();
|
||||
for (let i = 0; i < 20; i++) {
|
||||
lk.recordFailure(office, 1000); lk.recordFailure(office, 1000); // two mistypes before this display
|
||||
assert.equal(lk.isLocked(office, 1000), false, `display ${i}: still open after a couple of mistypes`);
|
||||
lk.reset(office); // the correct code pairs -> reset
|
||||
}
|
||||
assert.equal(lk.isLocked(office, 1000), false, 'all 20 displays paired, the IP never locked');
|
||||
});
|
||||
Loading…
Reference in a new issue