diff --git a/server/player/index.html b/server/player/index.html index 1463fac..6ed46a4 100644 --- a/server/player/index.html +++ b/server/player/index.html @@ -2612,7 +2612,14 @@ // tear down a live socket on the hidden->visible gap. if (document.visibilityState === 'visible') { requestWakeLock(); verifyLivenessSoon(); } }); - window.addEventListener('pageshow', verifyLivenessSoon); // sleep/resume via bfcache restore + // pairing-race fix: pageshow fires on EVERY load (persisted=false), not only bfcache restores. + // Unfiltered it ran verifyLivenessSoon() on the initial cold load, which opened the socket EARLY + // (if !socket -> connect) and registered a pairing code — then the intended boot path (tap-overlay + // / connectBtnFunc) called connect() again, tearing that socket down and recreating it (the + // connect->register->disconnect->reconnect flap that collided with the server's offline grace and + // produced the UNIQUE pairing_code collision). Only act on a REAL bfcache restore (persisted=true), + // mirroring the pagehide guard above; the cold-boot connect is owned solely by the boot path. + window.addEventListener('pageshow', (ev) => { if (ev && ev.persisted) verifyLivenessSoon(); }); // sleep/resume via bfcache restore window.addEventListener('online', verifyLivenessSoon); // network switch (wifi<->cellular) // feat/offline-cause-log: display sleep / backgrounding proxy — screen off/on on a TV. diff --git a/server/test/pairing-race.test.js b/server/test/pairing-race.test.js new file mode 100644 index 0000000..3e57b64 --- /dev/null +++ b/server/test/pairing-race.test.js @@ -0,0 +1,214 @@ +'use strict'; + +// Pairing-race (fix/pairing-race-1.9.9) — PROVES the deferred-offline zombie race is closed. +// +// THE BUG (now fixed in ws/deviceSocket.js): on socket disconnect the server defers +// heartbeat.removeConnection() by OFFLINE_DEBOUNCE_MS (~5s) via `pendingOfflines`, so +// heartbeat.getConnection() keeps returning a "live" ZOMBIE for the grace window. A +// same-fingerprint reconnect INSIDE that window used to hit the fingerprint-reclaim +// guard and false-reject with device:auth-error, and for an UNCLAIMED row the retry then +// INSERTed the on-screen pairing_code the zombie still held -> UNIQUE constraint failed: +// devices.pairing_code -> wedged, unclaimed player. +// +// WHAT WE VALIDATE: +// CASE 1 — a same-fingerprint + same-code reconnect INSIDE the deferred-offline window is +// NOT rejected, does NOT collide on UNIQUE(pairing_code), and settles to exactly ONE +// still-claimable row (Fix A gate `!inDeferredOffline` + Fix B same-code adopt). +// CASE 2 — a genuinely-live display (never disconnected, so NO pending-offline armed) STILL +// rejects a cloned-fingerprint takeover with device:auth-error (Fix A did not open a hole). +// +// Both cases run for a web-style fingerprint AND an android-style (SHA-256 hex) fingerprint. +// +// HARNESS: spawn the real server.js against a temp DATA_DIR + socket.io-client, mirroring +// fingerprint-reclaim.test.js. DETERMINISM: to guarantee socket B reconnects AFTER socket A's +// server-side disconnect has ARMED the pending-offline timer (and BEFORE the 5s timer fires), +// we poll the server log for the exact line the disconnect handler prints +// ("Device disconnected: (offline transition deferred ...)"). That line is emitted on the +// same synchronous tick, immediately before pendingOfflines.set(...), so observing it proves the +// zombie window is open — no wall-clock sleep, no reliance on a fixed reconnect budget. + +const { test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawn } = require('node:child_process'); +const path = require('node:path'); +const os = require('node:os'); +const fs = require('node:fs'); +const crypto = require('node:crypto'); +const ioClient = require('socket.io-client'); +const Database = require('better-sqlite3'); + +const { freePort } = require('./helpers/free-port'); + +let PORT, BASE; +const DATA_DIR = path.join(os.tmpdir(), 'st-prace-' + crypto.randomBytes(4).toString('hex')); +const LOG = path.join(os.tmpdir(), 'st-prace-' + crypto.randomBytes(4).toString('hex') + '.log'); +const DB_PATH = path.join(DATA_DIR, 'db', 'remote_display.db'); +let proc, tdb; + +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +before(async () => { + PORT = await freePort(); + BASE = `http://127.0.0.1:${PORT}`; + const logFd = fs.openSync(LOG, 'w'); + proc = spawn('node', ['server.js'], { + cwd: path.join(__dirname, '..'), + env: { + ...process.env, + DATA_DIR, + SELF_HOSTED: 'true', + PORT: String(PORT), + NODE_ENV: 'test', + // Raise the flap/connect-rate ceiling so rapid connect/disconnect/reconnect of the same + // fingerprint in these tests can never be soft-refused as flapping (would mask the result). + CONNECT_RATE_MAX: '1000', + CONNECT_RATE_ANON_MAX: '1000', + CONNECT_RATE_QUARANTINE_TRIPS: '0', + }, + stdio: ['ignore', logFd, logFd], + }); + let up = false; + for (let i = 0; i < 80; i++) { + try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* */ } + await sleep(250); + } + if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000)); + tdb = new Database(DB_PATH); tdb.pragma('busy_timeout = 3000'); tdb.pragma('foreign_keys = OFF'); +}); + +after(() => { + try { tdb && tdb.close(); } catch { /* */ } + try { proc.kill('SIGKILL'); } catch { /* */ } +}); + +// ---- helpers ------------------------------------------------------------- + +const rndCode = () => String(crypto.randomInt(100000, 1000000)); +const readLog = () => { try { return fs.readFileSync(LOG, 'utf8'); } catch { return ''; } }; + +// Poll the server log until `substr` appears (event-driven proof a server-side step ran). +async function waitForLog(substr, timeoutMs = 4000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (readLog().includes(substr)) return true; + await sleep(25); + } + throw new Error(`timed out waiting for log line: ${substr}`); +} + +function openSocket() { + return ioClient(`${BASE}/device`, { transports: ['websocket'], reconnection: false, forceNew: true }); +} + +// Emit device:register on `sock` and resolve on the FIRST terminal server verdict. +// Also records whether device:paired arrived (settle-checked by callers) on sock._paired. +function register(sock, payload) { + return new Promise((resolve) => { + let done = false; + const fin = (o) => { if (!done) { done = true; resolve(o); } }; + sock._paired = false; + sock.on('device:paired', () => { sock._paired = true; }); + sock.once('device:registered', (d) => fin({ outcome: 'registered', device_id: d.device_id, token: d.device_token, status: d.status })); + sock.once('device:auth-error', (e) => fin({ outcome: 'auth-error', error: (e && e.error) || '' })); + sock.once('device:throttled', (e) => fin({ outcome: 'throttled', reason: (e && e.reason) || '' })); + if (sock.connected) sock.emit('device:register', payload); + else sock.once('connect', () => sock.emit('device:register', payload)); + setTimeout(() => fin({ outcome: 'timeout' }), 4000); + }); +} + +// Fingerprint shapes: a short web-player id and an android SHA-256 hardware hash. Fresh each call. +const SHAPES = [ + { label: 'web-style id', fp: () => `web-${crypto.randomBytes(3).toString('hex')}-${crypto.randomBytes(1).toString('hex')}` }, + { label: 'android-style hash', fp: () => crypto.createHash('sha256').update('android|' + crypto.randomUUID()).digest('hex') }, +]; + +// ---- CASE 1 — the race is closed ---------------------------------------- + +for (const shape of SHAPES) { + test(`CASE 1 [${shape.label}]: same-fingerprint+same-code reconnect INSIDE the deferred-offline window is adopted (no false reject, no UNIQUE collision, exactly one claimable row)`, async () => { + const F = shape.fp(); + const C = rndCode(); + + // 1. Socket A: fresh unclaimed web-player-style registration (fingerprint + pairing_code, NO device_id). + const sockA = openSocket(); + const rA = await register(sockA, { fingerprint: F, pairing_code: C, device_info: {} }); + assert.equal(rA.outcome, 'registered', 'socket A provisions cleanly'); + const deviceId = rA.device_id; + assert.ok(deviceId, 'A got a device_id'); + + // Exactly one row for fingerprint F, holding code C, unclaimed. + const fpRow = tdb.prepare('SELECT device_id FROM device_fingerprints WHERE fingerprint = ?').get(F); + assert.equal(fpRow && fpRow.device_id, deviceId, 'fingerprint F is linked to A\'s new device row'); + let rowsForCode = tdb.prepare('SELECT id, user_id, pairing_code FROM devices WHERE pairing_code = ?').all(C); + assert.equal(rowsForCode.length, 1, 'exactly one device row holds pairing_code C after A'); + assert.equal(rowsForCode[0].id, deviceId, 'that row IS A\'s row'); + assert.equal(rowsForCode[0].user_id, null, 'row is unclaimed (user_id NULL)'); + + // 2. Disconnect A. Wait ONLY until the server has ARMED the deferred-offline timer (log proof), + // which is far inside the 5s window — the zombie connection is now live. + sockA.close(); + await waitForLog(`Device disconnected: ${deviceId} (offline transition deferred`); + + // Snapshot log length so the UNIQUE-collision check is scoped to the reconnect that follows. + const logBefore = readLog(); + assert.ok(!logBefore.includes('UNIQUE constraint failed: devices.pairing_code'), 'no UNIQUE collision before the reconnect'); + + // 3. Socket B: SAME fingerprint F, SAME pairing_code C, no device_id — reconnect inside the window. + const sockB = openSocket(); + const rB = await register(sockB, { fingerprint: F, pairing_code: C, device_info: {} }); + + // No false-positive reject / throttle — B is adopted. + assert.notEqual(rB.outcome, 'auth-error', `B must NOT be false-rejected (got ${rB.outcome}${rB.error ? ': ' + rB.error : ''})`); + assert.notEqual(rB.outcome, 'throttled', 'B must NOT be throttled'); + assert.equal(rB.outcome, 'registered', 'B resolves to device:registered (adopted the existing unclaimed row)'); + assert.equal(rB.device_id, deviceId, 'B adopted the SAME row (no new device_id)'); + + // Settle briefly to catch any late device:paired (there must be none — the row is unclaimed). + await sleep(200); + assert.equal(sockB._paired, false, 'no device:paired for an UNCLAIMED adopt (still on the pairing screen)'); + + // No UNIQUE(pairing_code) collision was hit by the reconnect. + assert.ok(!readLog().includes('UNIQUE constraint failed: devices.pairing_code'), 'no UNIQUE constraint failed on devices.pairing_code'); + + // After settling: EXACTLY ONE row for fingerprint F, still code C, still claimable. + const fpRow2 = tdb.prepare('SELECT device_id FROM device_fingerprints WHERE fingerprint = ?').get(F); + assert.equal(fpRow2 && fpRow2.device_id, deviceId, 'fingerprint F still maps to the same single row'); + rowsForCode = tdb.prepare('SELECT id, user_id, pairing_code FROM devices WHERE pairing_code = ?').all(C); + assert.equal(rowsForCode.length, 1, 'still EXACTLY ONE device row holds pairing_code C (no duplicate insert)'); + assert.equal(rowsForCode[0].id, deviceId, 'it is the same original row'); + assert.equal(rowsForCode[0].user_id, null, 'still claimable — user_id NULL'); + assert.equal(rowsForCode[0].pairing_code, C, 'still holds the on-screen code C'); + + sockB.close(); + }); +} + +// ---- CASE 2 — hijack boundary intact ------------------------------------ + +for (const shape of SHAPES) { + test(`CASE 2 [${shape.label}]: a genuinely-live display (no pending-offline armed) STILL rejects a cloned-fingerprint takeover`, async () => { + const F = shape.fp(); + const C = rndCode(); + + // Socket A registers and STAYS CONNECTED — it never disconnects, so NO pending-offline timer is armed. + const sockA = openSocket(); + const rA = await register(sockA, { fingerprint: F, pairing_code: C, device_info: {} }); + assert.equal(rA.outcome, 'registered', 'live socket A registers'); + const deviceId = rA.device_id; + + // A second socket presents the SAME fingerprint F (a clone) while A is genuinely live. + const sockB = openSocket(); + const rB = await register(sockB, { fingerprint: F, pairing_code: rndCode(), device_info: {} }); + + assert.equal(rB.outcome, 'auth-error', `cloned-fingerprint takeover of a LIVE display must be rejected (got ${rB.outcome})`); + assert.match(rB.error, /active on another connection/i, 'rejected with the "active on another connection" auth-error'); + + // No duplicate row was minted for the clone's code, and the fingerprint still points at A's row. + const fpRow = tdb.prepare('SELECT device_id FROM device_fingerprints WHERE fingerprint = ?').get(F); + assert.equal(fpRow && fpRow.device_id, deviceId, 'fingerprint still linked to the live device (takeover gained nothing)'); + + sockA.close(); + sockB.close(); + }); +} diff --git a/server/ws/deviceSocket.js b/server/ws/deviceSocket.js index 80e272d..f2354b2 100644 --- a/server/ws/deviceSocket.js +++ b/server/ws/deviceSocket.js @@ -482,7 +482,27 @@ module.exports = function setupDeviceSocket(io) { // was cleared by hand. The real security boundary is the operator claiming the // code in the dashboard; a cloned fingerprint that falls through only gets an // unclaimed, tokenless, content-less row — harmless. - if (liveConn) { + // Pairing-race (1.9.9): a device mid-DEFERRED-OFFLINE is NOT genuinely live. + // On disconnect we defer heartbeat.removeConnection() by OFFLINE_DEBOUNCE_MS + // (anti-flap), so heartbeat.getConnection() keeps returning a ZOMBIE entry for the + // just-closed socket for the whole grace window. A same-fingerprint reconnect inside + // that window (this branch is only reached via a device_fingerprints match, so it + // inherently IS the same physical display) would otherwise hit a FALSE-POSITIVE + // "active on another connection" reject, and for an unclaimed row the retry then + // collided on UNIQUE(devices.pairing_code) and wedged the player. Gate the reject on + // there being NO pending-offline timer for this device. + // + // ANTI-HIJACK BOUNDARY (unchanged): a DIFFERENT physical device presenting a CLONED + // fingerprint while the REAL device is actively connected never disconnected -> no + // pending-offline timer is armed -> inDeferredOffline is false -> the liveConn reject + // STILL fires. We ONLY relax the reject when the matched row's OWN connection is in + // its deferred-offline grace (i.e. it just dropped) — a legitimate reconnect of that + // same display. Even then, a CLAIMED row is reclaimed only because liveConn (the real + // anti-hijack check) already passed, and an UNCLAIMED fall-through only ever yields a + // tokenless, content-less row: the operator claiming the on-screen code stays the + // real trust boundary, so relaxing this reject grants an attacker nothing. + const inDeferredOffline = pendingOfflines.has(existing.device_id); + if (liveConn && !inDeferredOffline) { // Log at most once per device per window so a retrying/stuck device can't flood stdout. const nowMs = Date.now(); if (nowMs - (lastReclaimRejectLogAt.get(existing.device_id) || 0) >= config.reclaimRejectLogWindowMs) { @@ -552,6 +572,50 @@ module.exports = function setupDeviceSocket(io) { // Fall through to the pairing_code path below, which provisions a fresh row with the // on-screen code and (#150) relinks the fingerprint to it. reclaimSettleSeconds no // longer gates this path — a genuinely live socket (above) is the only rejection. + // + // IDEMPOTENCY (pairing-race 1.9.9): EXCEPT when the reconnecting player presents the + // SAME pairing_code this unclaimed row already holds. That is exactly the zombie case + // Fix A now lets through: the row is mid-deferred-offline and STILL holds the on-screen + // code, so the fall-through INSERT below collides on UNIQUE(devices.pairing_code) and + // wedges the player unclaimed with no content. The stale-code hazard in the comment + // above applies ONLY when the codes DIFFER; when they MATCH there is no stale code to + // strand, so ADOPT the existing row (mirror the claimed-reclaim refresh above) instead + // of re-INSERTing. Deliberately NO device:paired — the row is unclaimed and the + // operator must still claim the on-screen code (the real trust boundary is untouched). + if (pairing_code === oldDevice.pairing_code) { + const newToken = generateDeviceToken(); + db.prepare('UPDATE devices SET device_token = ? WHERE id = ?').run(newToken, existing.device_id); + console.log(`Fingerprint match: adopting UNCLAIMED same-code row ${existing.device_id} (code ${pairing_code}) instead of re-provisioning`); + authenticated = true; + // Cancel any pending offline timer - device is back in the grace window + if (pendingOfflines.has(existing.device_id)) { + clearTimeout(pendingOfflines.get(existing.device_id)); + pendingOfflines.delete(existing.device_id); + } + evictPriorSocket(existing.device_id, socket.id); + db.prepare("UPDATE devices SET status = 'online', last_heartbeat = strftime('%s','now'), ip_address = ?, updated_at = strftime('%s','now'), offline_reason = NULL, offline_reason_at = NULL, offline_detail = NULL WHERE id = ?") + .run(getClientIp(socket), existing.device_id); + socket.emit('device:registered', { device_id: existing.device_id, device_token: newToken, status: 'online' }); + // No device:paired — the row is unclaimed; the player stays on the pairing screen + // showing its (still-valid) code for the operator to claim. + currentDeviceId = existing.device_id; + heartbeat.registerConnection(existing.device_id, socket.id); + heartbeat.recordReconnect(existing.device_id); + persistIdentity(existing.device_id, data); + socket.join(existing.device_id); + logDeviceStatus(existing.device_id, 'online'); + emitToDeviceWorkspace(dashboardNs, existing.device_id, 'dashboard:device-status', { device_id: existing.device_id, status: 'online', liveness: heartbeat.livenessFor(existing.device_id) }); + // Flush any commands/playlist-updates queued while this device was offline. + commandQueue.flushQueue(deviceNs, existing.device_id, buildPlaylistPayload); + // Send playlist + const access = checkDeviceAccess(existing.device_id); + if (!access.allowed) { + socket.emit('device:playlist-update', { assignments: [], suspended: true, message: access.message, detail: access.detail }); + } else { + socket.emit('device:playlist-update', buildPlaylistPayload(existing.device_id)); + } + return; + } } } } else if (device_id || pairing_code) {