From b50ecfa50c043f586b2f09286cd55ad31abd6efa Mon Sep 17 00:00:00 2001 From: gadgethd <111318106+gadgethd@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:47:20 +0000 Subject: [PATCH] security: revalidate MQTT auth and revoke owner sessions --- backend/package.json | 1 + backend/src/api/routes.ts | 5 +- backend/src/api/routes/owner.test.ts | 84 +++++++++++++++++++ .../src/owner/mqttCredentialVerifier.test.ts | 17 ++++ backend/src/owner/mqttCredentialVerifier.ts | 6 ++ backend/src/owner/ownerAccess.ts | 83 ++++++------------ backend/src/tools/revokeOwnerSessions.ts | 36 ++++++++ docs/runbook-owner-auth.md | 17 ++++ 8 files changed, 190 insertions(+), 59 deletions(-) create mode 100644 backend/src/api/routes/owner.test.ts create mode 100644 backend/src/owner/mqttCredentialVerifier.test.ts create mode 100644 backend/src/owner/mqttCredentialVerifier.ts create mode 100644 backend/src/tools/revokeOwnerSessions.ts create mode 100644 docs/runbook-owner-auth.md diff --git a/backend/package.json b/backend/package.json index 501d043..df5d3ad 100644 --- a/backend/package.json +++ b/backend/package.json @@ -14,6 +14,7 @@ "stats:seed-chart-snapshot": "node dist/tools/seedStatsChartSnapshot.js", "db:lifecycle": "node dist/tools/dataLifecycle.js", "owner-auth:inventory": "node dist/tools/inventoryOwnerAuthorization.js", + "owner-auth:revoke": "node dist/tools/revokeOwnerSessions.js", "analysis:cleanup-orphans": "node dist/tools/cleanupOrphanedAnalysisRuns.js", "bench:decode": "tsx src/tools/benchmarkPacketDecode.ts", "load:realtime": "tsx src/tools/loadRealtime.ts", diff --git a/backend/src/api/routes.ts b/backend/src/api/routes.ts index f59be5b..e9fd13b 100644 --- a/backend/src/api/routes.ts +++ b/backend/src/api/routes.ts @@ -138,9 +138,8 @@ async function requireOwnerSession(req: Request, res: Response): Promise next(); +const TEST_COOKIE_SECRET = 'test-owner-cookie-secret-at-least-32-bytes'; + +async function startOwnerRoutes(options: { + currentGeneration?: () => Promise; + verifyCredentials?: (username: string, password: string) => Promise; +} = {}): Promise<{ baseUrl: string; close: () => Promise; restore: () => void }> { + const previousSecret = process.env['OWNER_COOKIE_SECRET']; + process.env['OWNER_COOKIE_SECRET'] = TEST_COOKIE_SECRET; + const router = Router(); + registerOwnerRoutes(router, { + ownerCookieName: 'meshcore_owner_session', + ownerLiveCacheTtlMs: 1_000, + ownerLiveCache: new Map(), + ownerDashboardCacheTtlMs: 1_000, + ownerLastHopCacheTtlMs: 1_000, + ownerSessionTtlMs: 60_000, + mqttUsernameMaxLen: 128, + mqttPasswordMaxLen: 128, + ownerLoginLimiter: passThroughLimiter as Parameters[1]['ownerLoginLimiter'], + hasControlChars: (value) => /[\u0000-\u001F\u007F]/.test(value), + verifyMqttCredentials: options.verifyCredentials ?? (async () => true), + resolveOwnerNodeIds: async () => [NODE_ID], + autoLinkOwnerNodeIds: async () => [NODE_ID], + buildOwnerDashboard: async (nodeIds) => ({ nodes: nodeIds.map((nodeId) => ({ node_id: nodeId })) }), + encryptOwnerSession, + isSecureRequest: () => false, + getOwnerSession: (req) => getOwnerSession(req, 'meshcore_owner_session'), + requireOwnerSession: async () => [NODE_ID], + getOwnerCredentialGeneration: options.currentGeneration ?? (async () => 2), + invalidateOwnerNodeIdCache: () => undefined, + query: async () => ({ rows: [] as T[] }), + }); + const app = express(); + app.use(express.json()); + app.use(router); + const server = app.listen(0, '127.0.0.1'); + await once(server, 'listening'); + const { port } = server.address() as AddressInfo; + + return { + baseUrl: `http://127.0.0.1:${port}`, + close: async () => { + server.close(); + await once(server, 'close'); + }, + restore: () => { + if (previousSecret === undefined) delete process.env['OWNER_COOKIE_SECRET']; + else process.env['OWNER_COOKIE_SECRET'] = previousSecret; + }, + }; +} + +test('owner session route rejects cookies from an older credential generation', async () => { + const ownerServer = await startOwnerRoutes(); + const token = encryptOwnerSession({ + v: 3, + mqttUsername: 'owner', + exp: Date.now() + 60_000, + gen: 1, + }); + + try { + const response = await fetch(`${ownerServer.baseUrl}/owner/session`, { + headers: { cookie: `meshcore_owner_session=${token}` }, + }); + + assert.equal(response.status, 401); + assert.equal((await response.json() as { error: string }).error, 'Credentials have been rotated — please log in again'); + } finally { + await ownerServer.close(); + ownerServer.restore(); + } +}); diff --git a/backend/src/owner/mqttCredentialVerifier.test.ts b/backend/src/owner/mqttCredentialVerifier.test.ts new file mode 100644 index 0000000..f62d8a0 --- /dev/null +++ b/backend/src/owner/mqttCredentialVerifier.test.ts @@ -0,0 +1,17 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createMqttCredentialVerifier } from './mqttCredentialVerifier.js'; + +test('rechecks unchanged MQTT credentials against the broker on every login', async () => { + let checks = 0; + const verify = createMqttCredentialVerifier(async (username, password) => { + checks++; + assert.equal(username, 'owner'); + assert.equal(password, 'unchanged-password'); + return checks === 1; + }); + + assert.equal(await verify('owner', 'unchanged-password'), true); + assert.equal(await verify('owner', 'unchanged-password'), false); + assert.equal(checks, 2); +}); diff --git a/backend/src/owner/mqttCredentialVerifier.ts b/backend/src/owner/mqttCredentialVerifier.ts new file mode 100644 index 0000000..71624c0 --- /dev/null +++ b/backend/src/owner/mqttCredentialVerifier.ts @@ -0,0 +1,6 @@ +export type BrokerCredentialCheck = (mqttUsername: string, mqttPassword: string) => Promise; + +/** Verify each login against the broker so cached success cannot outlive a password reset. */ +export function createMqttCredentialVerifier(check: BrokerCredentialCheck): BrokerCredentialCheck { + return (mqttUsername, mqttPassword) => check(mqttUsername, mqttPassword); +} diff --git a/backend/src/owner/ownerAccess.ts b/backend/src/owner/ownerAccess.ts index 9597851..4d47d26 100644 --- a/backend/src/owner/ownerAccess.ts +++ b/backend/src/owner/ownerAccess.ts @@ -1,5 +1,5 @@ import mqtt from 'mqtt'; -import { createHash, randomBytes } from 'node:crypto'; +import { randomBytes } from 'node:crypto'; import { Redis } from 'ioredis'; import { BoundedTtlMap } from '../cache/boundedTtlMap.js'; import { getOwnerNodeIdsForUsername } from '../db/ownerAuth.js'; @@ -9,6 +9,7 @@ import { getRedisConnectionOptions, getRedisUrl } from '../platform/config/redis import { reconcileOwnerAuthorization } from './ownerAclReconciler.js'; import { groupOwnerNodes, type OwnerDashboardRow } from './ownerDashboard.js'; import { parseOwnerGrantConfig } from './ownerGrantConfig.js'; +import { createMqttCredentialVerifier } from './mqttCredentialVerifier.js'; function normalizeNodeIds(nodeIds: string[]): string[] { return Array.from(new Set( @@ -93,50 +94,12 @@ export async function autoLinkOwnerNodeIds(mqttUsername: string): Promise({ - name: 'owner_auth', - maxEntries: Number(process.env['OWNER_AUTH_CACHE_MAX_ENTRIES'] ?? 512), - maxWeight: 2 * 1024 * 1024, - ttlMs: AUTH_CACHE_TTL_MS, -}); - -function credentialHash(mqttUsername: string, mqttPassword: string): string { - return createHash('sha256').update(`${mqttUsername}\u0000${mqttPassword}`).digest('hex'); -} - -export function verifyMqttCredentials(mqttUsername: string, mqttPassword: string): Promise { - const hash = credentialHash(mqttUsername, mqttPassword); - const cached = authCache.get(mqttUsername); - if (cached && cached.credentialHash === hash && Date.now() - cached.ts < AUTH_CACHE_TTL_MS) { - return Promise.resolve(true); - } - return verifyMqttCredentialsViaBroker(mqttUsername, mqttPassword).then((ok) => { - if (ok) { - authCache.set(mqttUsername, { credentialHash: hash, ts: Date.now() }); - } else if (cached && cached.credentialHash === hash) { - // Same credential that previously worked now rejected (revoked) — drop it - // AND bump the credential generation so every live owner session minted - // under the old password is invalidated immediately. - authCache.delete(mqttUsername); - void bumpOwnerCredentialGeneration(mqttUsername); - } - return ok; - }); -} +export const verifyMqttCredentials = createMqttCredentialVerifier(verifyMqttCredentialsViaBroker); // ---- Credential generation (BUG-010) ---- -// Owner sessions are stateless signed cookies with a 30-day TTL. To make -// password revocation effective immediately, every session records the -// credential generation that minted it. When the broker rejects a previously -// valid password (the only signal the backend has that a password changed), -// the generation is bumped in Redis and all older sessions stop validating. +// Owner sessions record the credential generation that minted them. The +// operator revocation command bumps this Redis value after a password reset, +// and every authenticated owner request compares it with the cookie generation. const OWNER_CRED_GEN_KEY = (username: string) => `owner:credential-gen:${username}`; const OWNER_CRED_GEN_TTL_MS = 90 * 24 * 60 * 60 * 1000; @@ -154,27 +117,35 @@ function getCredGenRedis(): Redis { export async function getOwnerCredentialGeneration(mqttUsername: string): Promise { try { const value = await getCredGenRedis().get(OWNER_CRED_GEN_KEY(mqttUsername)); - const parsed = value === null ? NaN : Number(value); - return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; + if (value === null) return 0; + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed < 0) { + throw new Error('INVALID_OWNER_CREDENTIAL_GENERATION'); + } + return parsed; } catch (error) { - console.error('[owner-credgen] read failed, treating as 0', error instanceof Error ? error.message : error); - return 0; + console.error('[owner-credgen] read failed', error instanceof Error ? error.message : error); + throw new Error('OWNER_CREDENTIAL_GENERATION_UNAVAILABLE', { cause: error }); } } /** * Bump the credential generation, invalidating every session minted under an - * older password. Failures are logged but non-fatal: the next login still - * succeeds and re-records the current generation. + * older password. Errors propagate so an operator reset cannot report success + * unless revocation was recorded. */ export async function bumpOwnerCredentialGeneration(mqttUsername: string): Promise { - try { - const redis = getCredGenRedis(); - await redis.incr(OWNER_CRED_GEN_KEY(mqttUsername)); - await redis.pexpire(OWNER_CRED_GEN_KEY(mqttUsername), OWNER_CRED_GEN_TTL_MS); - } catch (error) { - console.error('[owner-credgen] bump failed', error instanceof Error ? error.message : error); - } + const redis = getCredGenRedis(); + const key = OWNER_CRED_GEN_KEY(mqttUsername); + await redis.incr(key); + await redis.pexpire(key, OWNER_CRED_GEN_TTL_MS); +} + +export async function closeOwnerCredentialGenerationClient(): Promise { + if (!credGenRedis) return; + const client = credGenRedis; + credGenRedis = null; + await client.quit(); } function verifyMqttCredentialsViaBroker(mqttUsername: string, mqttPassword: string): Promise { diff --git a/backend/src/tools/revokeOwnerSessions.ts b/backend/src/tools/revokeOwnerSessions.ts new file mode 100644 index 0000000..29f446b --- /dev/null +++ b/backend/src/tools/revokeOwnerSessions.ts @@ -0,0 +1,36 @@ +import { + bumpOwnerCredentialGeneration, + closeOwnerCredentialGenerationClient, +} from '../owner/ownerAccess.js'; + +async function main(): Promise { + const mqttUsername = process.argv[2]?.trim() ?? ''; + if (process.argv.length !== 3 || !/^[A-Za-z0-9_.@-]{1,128}$/.test(mqttUsername)) { + console.error('Usage: node dist/tools/revokeOwnerSessions.js '); + process.exitCode = 2; + return; + } + + try { + await bumpOwnerCredentialGeneration(mqttUsername); + console.log(`Revoked owner sessions for ${mqttUsername}`); + } catch (error) { + console.error( + 'Could not revoke owner sessions:', + error instanceof Error ? error.message : String(error), + ); + process.exitCode = 1; + } finally { + try { + await closeOwnerCredentialGenerationClient(); + } catch (error) { + console.error( + 'Could not close owner credential generation connection:', + error instanceof Error ? error.message : String(error), + ); + process.exitCode = 1; + } + } +} + +void main(); diff --git a/docs/runbook-owner-auth.md b/docs/runbook-owner-auth.md new file mode 100644 index 0000000..5c6f3d7 --- /dev/null +++ b/docs/runbook-owner-auth.md @@ -0,0 +1,17 @@ +# Owner credential rotation + +Owner cookies include the Redis credential generation that was current at login. +The owner session route and protected owner routes compare that generation on +each request. Cookies expire after seven days. + +After changing a Mosquitto password, revoke existing owner sessions immediately +by running the backend tool with the affected MQTT username: + +```sh +docker exec meshcore-analytics-backend-1 \ + node dist/tools/revokeOwnerSessions.js +``` + +The command fails if Redis cannot record the new generation. Confirm success +before treating password rotation as complete. Do not pass the password to this +command or record it in logs.