From 8809007d9e213a4f15e2544ad426022cb0f68c9c Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Thu, 2 Jul 2026 14:59:25 -0500 Subject: [PATCH] fix(#148) Item 1: exempt paired+authenticated devices from the flap-limiter quarantine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flap-limiter could 30-min quarantine a PAIRED, legitimate device on reconnect churn. Behind Bold's single SNAT IP a repeated edge flush -> every device reconnects -> trips flap -> quarantined -> a recoverable blip becomes a SUSTAINED FLEET-WIDE LOCKOUT we caused. check(key, now, {paired}) now skips (and clears) the quarantine escalation for a paired device — it still gets the brief soft cooldown if it truly hammers, but never the long lockout. The register gate computes paired = device_id && validateDeviceToken(...) (a matching STORED token, false for missing/mismatch) so a spoofed device_id can't claim the exemption; unpaired/anon flapping (attacker / unprovisioned hammering) still quarantines. Tests: unpaired flapper still quarantined; paired never quarantined (soft cooldown only); paired creds RELEASE an in-flight quarantine; N paired devices from one SNAT IP all admitted on reconnect and never quarantined across repeated flush cycles. --- server/lib/flap-limiter.js | 18 +++++-- server/test/flap-paired-exemption.test.js | 66 +++++++++++++++++++++++ server/ws/deviceSocket.js | 7 ++- 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 server/test/flap-paired-exemption.test.js diff --git a/server/lib/flap-limiter.js b/server/lib/flap-limiter.js index ed2f5ad..5de3787 100644 --- a/server/lib/flap-limiter.js +++ b/server/lib/flap-limiter.js @@ -39,8 +39,15 @@ function maxFor(key) { return key === ANON_KEY ? config.connectRateAnonMax : con // { allow: false, retryAfterMs, reason, tripped?, trips?, quarantined? } // reason: 'quarantined' (in-memory time-limited), 'flap-cooldown' (post-trip), 'flap-rate' // (the trip edge). `quarantined:true` marks the START of a quarantine (log once). -function check(key, now = Date.now()) { +function check(key, now = Date.now(), opts = {}) { if (!config.flapLimiterEnabled) return { allow: true }; // #146 P1.3 kill switch + // #148: a PAIRED + AUTHENTICATED device reconnecting is a legitimate client recovering + // (e.g. from an edge idle-reap / half-open TCP), NOT an attacker. Exempt it from the long + // 30-min QUARANTINE escalation — behind ONE SNAT IP a repeated edge flush would otherwise + // accumulate the whole paired fleet into quarantine at once, a self-inflicted fleet-wide + // lockout. It still gets the brief soft cooldown if it truly hammers; only the LONG lockout + // is waived. Unpaired/anon flapping (attacker / unprovisioned hammering) still quarantines. + const exemptQuarantine = !!opts.paired; let s = state.get(key); if (!s) { s = { hits: [], blockedUntil: 0, lastSeen: now, trips: 0, tripWinStart: now, quarantinedUntil: 0 }; state.set(key, s); } s.lastSeen = now; @@ -50,7 +57,10 @@ function check(key, now = Date.now()) { // is safe in-memory now that Item A ended the prune-induced restart loop; a // self-healing auto-action must NOT survive as a devices.blocked row. if (now < s.quarantinedUntil) { - return refuse(now, { allow: false, retryAfterMs: s.quarantinedUntil - now, reason: 'quarantined' }); + // #148: a device that presents valid paired creds is authenticated-legit — release any + // in-flight quarantine (e.g. tripped before it re-authed, or by a spoofer of its id). + if (exemptQuarantine) { s.quarantinedUntil = 0; s.trips = 0; } + else return refuse(now, { allow: false, retryAfterMs: s.quarantinedUntil - now, reason: 'quarantined' }); } // Inside an enforced cooldown -> refuse cheaply. @@ -68,7 +78,9 @@ function check(key, now = Date.now()) { if (now - s.tripWinStart > config.connectRateWindowMs) { s.tripWinStart = now; s.trips = 0; } s.trips += 1; // Escalate to a time-limited quarantine after N trips in the window (0 = off). - if (config.connectRateQuarantineTrips > 0 && s.trips >= config.connectRateQuarantineTrips) { + // #148: NEVER escalate a paired+authenticated device to the long lockout — the soft + // cooldown below is the most a legitimate reconnecting device ever gets. + if (!exemptQuarantine && config.connectRateQuarantineTrips > 0 && s.trips >= config.connectRateQuarantineTrips) { s.quarantinedUntil = now + config.connectRateQuarantineMs; bump(quarantineStartsCtr, now); // a quarantine event is visible even though the gauge decays return refuse(now, { allow: false, retryAfterMs: config.connectRateQuarantineMs, reason: 'flap-rate', tripped: true, trips: s.trips, quarantined: true }); diff --git a/server/test/flap-paired-exemption.test.js b/server/test/flap-paired-exemption.test.js new file mode 100644 index 0000000..6318977 --- /dev/null +++ b/server/test/flap-paired-exemption.test.js @@ -0,0 +1,66 @@ +'use strict'; + +// #148 Item 1 — paired + authenticated devices are exempt from the flap-limiter's 30-min +// QUARANTINE (a self-inflicted fleet-wide lockout behind one SNAT IP), while unpaired/anon +// flapping is still quarantined. Fast, deterministic: env shrinks the thresholds and we +// drive `now` explicitly instead of waiting. + +const os = require('node:os'); const path = require('node:path'); const crypto = require('node:crypto'); +process.env.DATA_DIR = path.join(os.tmpdir(), 'st-flapx-' + crypto.randomBytes(4).toString('hex')); +process.env.CONNECT_RATE_MAX = '2'; +process.env.CONNECT_RATE_COOLDOWN_MS = '10'; +process.env.CONNECT_RATE_QUARANTINE_TRIPS = '2'; +process.env.CONNECT_RATE_WINDOW_MS = '100000'; + +const { test, beforeEach } = require('node:test'); +const assert = require('node:assert/strict'); +const flap = require('../lib/flap-limiter'); + +beforeEach(() => flap.reset()); + +// Drive one "trip": exceed max within the window at time `now` (max=2 → 3 checks trips). +function tripAt(key, now, paired) { let v; for (let i = 0; i < 3; i++) v = flap.check(key, now, { paired }); return v; } + +test('unpaired flapper IS quarantined after the trip threshold', () => { + const k = 'attacker'; + assert.equal(tripAt(k, 0, false).tripped, true); // trip 1 + const v = tripAt(k, 11, false); // trip 2 (past cooldown) → quarantine + assert.equal(v.quarantined, true, 'unpaired escalates to quarantine'); + const q = flap.check(k, 12, { paired: false }); + assert.equal(q.allow, false); assert.equal(q.reason, 'quarantined'); +}); + +test('PAIRED device is NEVER quarantined — soft cooldown at most', () => { + const k = 'paired-device'; + tripAt(k, 0, true); + const v = tripAt(k, 11, true); // would quarantine an unpaired + assert.notEqual(v.reason, 'quarantined'); + assert.notEqual(v.quarantined, true, 'paired never escalates to the long lockout'); + // hammer for a long time — still never quarantined + for (let t = 22; t < 5000; t += 11) { + const r = tripAt(k, t, true); + assert.notEqual(r.reason, 'quarantined', `t=${t} paired must not be quarantined`); + } +}); + +test('presenting paired creds RELEASES an in-flight quarantine', () => { + const k = 'dev-x'; + tripAt(k, 0, false); tripAt(k, 11, false); // quarantine it as unpaired + assert.equal(flap.check(k, 12, { paired: false }).reason, 'quarantined'); + const released = flap.check(k, 13, { paired: true }); // now authenticated/paired + assert.notEqual(released.reason, 'quarantined', 'quarantine released for a now-authenticated device (soft cooldown at most)'); + assert.equal(flap.check(k, 100, { paired: true }).allow, true, 'admitted once the brief soft cooldown passes'); +}); + +test('SNAT: N paired devices from one IP all admitted on reconnect; repeated cycles never quarantine', () => { + const N = 50; + // A single flush → each device reconnects once (its own device_id key) → all admitted. + for (let d = 0; d < N; d++) assert.equal(flap.check('device-' + d, 1000, { paired: true }).allow, true); + // Repeated flush cycles → a paired device may hit the soft cooldown but is NEVER quarantined. + for (let cycle = 0; cycle < 10; cycle++) { + for (let d = 0; d < N; d++) { + const v = flap.check('device-' + d, 2000 + cycle * 5, { paired: true }); + assert.notEqual(v.reason, 'quarantined', `device ${d} cycle ${cycle} must not be quarantined`); + } + } +}); diff --git a/server/ws/deviceSocket.js b/server/ws/deviceSocket.js index bc6f0e9..5097b83 100644 --- a/server/ws/deviceSocket.js +++ b/server/ws/deviceSocket.js @@ -322,7 +322,12 @@ module.exports = function setupDeviceSocket(io) { // Keyed via the same SNAT-safe identity, NEVER IP. const isRefreshConnect = device_id && currentDeviceId === device_id; if (!isRefreshConnect) { - const fv = flapLimiter.check(ident.key); + // #148: a paired + AUTHENTICATED device reconnecting is exempt from the flap + // QUARANTINE (not from the soft cooldown). validateDeviceToken confirms device_id + + // a matching STORED token (false for missing/mismatch), so a spoofed device_id can't + // claim the exemption — an attacker without the real token is still quarantinable. + const paired = !!device_id && validateDeviceToken(device_id, device_token); + const fv = flapLimiter.check(ident.key, Date.now(), { paired }); if (!fv.allow) { // #146 P0: auto-quarantine is IN-MEMORY + TIME-LIMITED (lib/flap-limiter), // never a DB block — a stuck-then-recovered device self-heals. The