diff --git a/server/lib/numeric-code.js b/server/lib/numeric-code.js new file mode 100644 index 0000000..dce4285 --- /dev/null +++ b/server/lib/numeric-code.js @@ -0,0 +1,26 @@ +'use strict'; + +// Six-digit codes that gate access: the on-device settings PIN (devices.settings_pin) and +// the provisioning pairing code (devices.pairing_code). +// +// Both were generated with `Math.floor(100000 + Math.random() * 900000)`. Math.random is +// not a CSPRNG — V8 implements it as xorshift128+, whose internal state is recoverable +// from a handful of consecutive outputs, and every call in a process draws from that one +// shared stream. These two values are also OBSERVABLE by ordinary users (the PIN is +// returned in device API responses today), so an attacker who can collect a few can +// predict the ones minted around them for other tenants. +// +// crypto.randomInt is CSPRNG-backed and rejection-samples, so the distribution stays +// uniform across the range rather than skewing the way a modulo would. +// +// Range is 100000..999999 inclusive — identical to what the old expression produced, so +// the code is always exactly six digits with no leading zero, which is what the on-device +// keypad and the pairing UI expect. + +const crypto = require('crypto'); + +function sixDigitCode() { + return String(crypto.randomInt(100000, 1000000)); +} + +module.exports = { sixDigitCode }; diff --git a/server/routes/status.js b/server/routes/status.js index 9449f65..284a501 100644 --- a/server/routes/status.js +++ b/server/routes/status.js @@ -5,6 +5,7 @@ const os = require('os'); const path = require('path'); const fs = require('fs'); const config = require('../config'); +const { sixDigitCode } = require('../lib/numeric-code'); const VERSION = require('../version'); const { PLATFORM_ROLES, resolveSessionUser } = require('../middleware/auth'); const loopLag = require('../services/loop-lag'); @@ -332,7 +333,7 @@ router.post('/import', importUpload.single('file'), async (req, res) => { for (const d of (data.devices || [])) { const newId = uuid.v4(); idMap.devices[d.id] = newId; - const pairingCode = String(Math.floor(100000 + Math.random() * 900000)); + const pairingCode = sixDigitCode(); // CSPRNG (lib/numeric-code): this code claims a device db.prepare(`INSERT INTO devices (id, user_id, workspace_id, name, pairing_code, status, screen_width, screen_height, created_at) VALUES (?, ?, ?, ?, ?, 'provisioning', ?, ?, ?)`).run(newId, userId, workspaceId, d.name, pairingCode, d.screen_width || null, d.screen_height || null, d.created_at || Math.floor(Date.now() / 1000)); stats.devices++; } diff --git a/server/server.js b/server/server.js index 6410459..f9a82f7 100644 --- a/server/server.js +++ b/server/server.js @@ -555,6 +555,7 @@ app.get('/api/content/:id/thumbnail', (req, res) => { // req.isPlatformAdmin, req.actingAs. Route handlers in 2.1 don't read these // yet (they still filter by user_id); 2.2 will migrate them one route at a time. const { requireAuth } = require('./middleware/auth'); +const { sixDigitCode } = require('./lib/numeric-code'); const { resolveTenancy } = require('./lib/tenancy'); // Public API token front door (Phase 1). Attached ONLY to the public routers below. const { bearerAuth, tokenScopeGate, agencyGate } = require('./middleware/apiToken'); @@ -917,8 +918,11 @@ app.post('/api/provision/pair', requireAuth, resolveTenancy, checkDeviceLimit, ( const deviceName = name || 'Display ' + (db.prepare('SELECT COUNT(*) as count FROM devices WHERE user_id = ?').get(req.user.id).count + 1); // Generate a random 6-digit PIN for the hidden settings menu — each device gets a - // unique PIN provisioned by the server (never a hardcoded default). - const settingsPin = String(Math.floor(100000 + Math.random() * 900000)); + // unique PIN provisioned by the server (never a hardcoded default). CSPRNG-backed + // (lib/numeric-code): this PIN gates the on-device settings menu and is observable in + // device API responses, so Math.random's recoverable state would let one tenant predict + // another's. + const settingsPin = sixDigitCode(); db.prepare("UPDATE devices SET pairing_code = NULL, name = ?, user_id = ?, workspace_id = ?, status = 'online', settings_pin = ?, updated_at = strftime('%s','now') WHERE id = ?") .run(deviceName, req.user.id, req.workspaceId, settingsPin, device.id); diff --git a/server/test/numeric-code.test.js b/server/test/numeric-code.test.js new file mode 100644 index 0000000..77d9770 --- /dev/null +++ b/server/test/numeric-code.test.js @@ -0,0 +1,66 @@ +'use strict'; + +// The settings PIN and the provisioning pairing code both gate access — the PIN to the +// on-device settings menu, the pairing code to claiming a device into a workspace. Both +// must come from a CSPRNG. +// +// Math.random is not one: V8 implements it as xorshift128+, whose state is recoverable +// from a few consecutive outputs, and every call in a process shares that stream. Since +// the PIN is observable in device API responses, a user who collects a few could predict +// values minted around them. +// +// The source assertion at the bottom is the one that fails if a future change reintroduces +// Math.random for either value — the statistical tests below cannot distinguish a CSPRNG +// from a good PRNG, so they alone would not catch a regression. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { sixDigitCode } = require('../lib/numeric-code'); + +test('a code is always exactly six digits, 100000-999999', () => { + for (let i = 0; i < 2000; i++) { + const c = sixDigitCode(); + assert.match(c, /^[1-9]\d{5}$/, `got ${c}`); + const n = Number(c); + assert.ok(n >= 100000 && n <= 999999, `out of range: ${n}`); + } +}); + +test('codes are not obviously degenerate (spread across the range, few repeats)', () => { + const seen = new Set(); + const buckets = new Array(9).fill(0); // by leading digit 1..9 + for (let i = 0; i < 5000; i++) { + const c = sixDigitCode(); + seen.add(c); + buckets[Number(c[0]) - 1]++; + } + assert.ok(seen.size > 4900, `too many collisions in 5000 draws: ${seen.size} unique`); + // Every leading digit should appear; a badly-scaled range would starve 1 or 9. + for (let d = 0; d < 9; d++) assert.ok(buckets[d] > 0, `leading digit ${d + 1} never appeared`); +}); + +test('the generator is CSPRNG-backed, not Math.random', () => { + // Inspect the FUNCTION body, not the file: the file's comment legitimately mentions + // Math.random while explaining why it is not used. + const body = sixDigitCode.toString(); + assert.ok(/crypto\.randomInt|randomInt/.test(body), `must use crypto.randomInt, got: ${body}`); + assert.ok(!/Math\.random/.test(body), 'must not use Math.random'); +}); + +test('no access-gating code is generated with Math.random anywhere on the server', () => { + // Scans the two call sites that mint access-gating values. Deliberately narrow: other + // Math.random uses in the tree are non-security (an image-generation seed in + // lib/image-gen.js, and anti-burn-in pixel jitter inside generated widget HTML), and + // sweeping those in would make this test a nuisance rather than a guard. + const root = path.join(__dirname, '..'); + for (const rel of ['server.js', 'routes/status.js']) { + const src = fs.readFileSync(path.join(root, rel), 'utf8'); + for (const line of src.split('\n')) { + if (!/Math\.random/.test(line)) continue; + assert.ok(!/(pin|pairing|code|secret|token)/i.test(line), + `${rel} mints an access-gating value with Math.random: ${line.trim()}`); + } + } +});