From ae7af4b833750a3e3955484594cc58ff203daf3a Mon Sep 17 00:00:00 2001 From: aaldersondev Date: Sat, 8 Aug 2026 14:15:27 +0200 Subject: [PATCH] fix(security): the console channel enforced less than it claimed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console JWT is what lets a browser talk straight to a daemon that runs as root and drives Docker. It had no tests at all — 274 lines of issuing and 422 of verifying, resting on manual discipline. Writing the attacks found that several of its guarantees were not there. **Live console output was ungated.** The listeners were attached the moment a session authenticated; only the *replay* of the buffer checked `control.console`. A subuser holding nothing but the implicit `websocket.connect` read the console live — including whatever an operator typed into it, which is where RCON passwords, `op` and plugin keys go. The gate is now on emission rather than on subscription, so a renewal that narrows a token stops the stream mid-session; state, stats and install lifecycle still flow, because *that* something happened is not the same as *what was said*. **A read-scoped API key could open a console.** The endpoint is a GET, scope is decided from the verb, and the permission resolver has never heard of API keys — so an owner's key came back with every permission and they were frozen into the token. `docs/api.md` already said this was impossible. It is now. **Expiry closed the socket without removing authority.** A close is a handshake, not an instant, and nothing obliges a peer to answer: commands arriving in that window were executed with a token the daemon had already declared dead. Authority is dropped first, the frame second. **The quotas were arithmetic rather than limits.** The command counter lived on the session, so the ceiling was sixty times however many sockets one token cared to open. Quotas now hang off the identity the panel signed — the user, on one server — which cannot be multiplied by reconnecting. Power actions had no quota at all, though each is container work on a root-privileged daemon; they have their own, tighter one. Console replays have theirs, charged on the connect path too, since leaving it free would have priced the allowance at nothing. The window is genuinely sliding now, where the comment had claimed sliding and the code reset wholesale — sixty commands either side of a boundary went through in a fifth of a second. Two documented capabilities did not exist. Signed download URLs are described as single-use and are not — and in fact nothing in the product issues one, so the docs now say that instead. The `jti` claim was documented as allowing a targeted revocation that is implemented nowhere. A sentence an incident responder would act on, describing a mechanism that is not there, is worse than no sentence. What did hold, and is now pinned: `alg: none` and algorithm confusion, a token for another node by audience and by key independently, a foreign issuer, every direction of cross-family replay, payload tampering of every field, and a node operator forging a panel administrator token with the node secret they own. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QL3QL3ReEa9Sk68mxJW6Fu --- .../src/websocket/console-gateway.spec.ts | 1646 +++++++++++++++++ apps/daemon/src/websocket/console-gateway.ts | 387 +++- .../src/modules/auth/guards/jwt-auth.guard.ts | 5 + apps/panel/src/modules/auth/request-user.ts | 14 + .../src/modules/auth/token.service.spec.ts | 1292 +++++++++++++ apps/panel/src/modules/auth/token.service.ts | 118 +- .../src/modules/servers/console.controller.ts | 54 +- docs/api.md | 10 +- docs/security.md | 47 +- packages/shared/src/contract/jwt.ts | 42 +- 10 files changed, 3549 insertions(+), 66 deletions(-) create mode 100644 apps/daemon/src/websocket/console-gateway.spec.ts create mode 100644 apps/panel/src/modules/auth/token.service.spec.ts diff --git a/apps/daemon/src/websocket/console-gateway.spec.ts b/apps/daemon/src/websocket/console-gateway.spec.ts new file mode 100644 index 0000000..b27d262 --- /dev/null +++ b/apps/daemon/src/websocket/console-gateway.spec.ts @@ -0,0 +1,1646 @@ +import { EventEmitter } from 'node:events'; +import { randomBytes } from 'node:crypto'; +import { connect, type Socket } from 'node:net'; +import websocket from '@fastify/websocket'; +import { + CONSOLE_BUFFER_LINES, + PERMISSIONS, + WS_ERROR_CODES, + type Permission, + type PowerAction, + type ResourceUsage, + type ServerState, +} from '@hopper/shared'; +import Fastify, { type FastifyInstance } from 'fastify'; +import { SignJWT, UnsecuredJWT, generateKeyPair, jwtVerify } from 'jose'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { daemonConfigSchema, type DaemonConfig } from '../config/schema.js'; +import type { Logger } from '../logger.js'; +import type { ServerInstance } from '../server/server-instance.js'; +import type { ServerManager } from '../server/server-manager.js'; +import { registerConsoleGateway } from './console-gateway.js'; + +/** + * The console gateway, attacked rather than covered. + * + * This is the one place in Hopper where a **browser talks straight to the + * daemon**: no panel in the path, no database lookup, no callback. Everything + * the daemon is willing to believe about the person on the other end comes out + * of a JWT it verifies on its own, and the daemon is the process with something + * to lose — it runs as root and drives Docker. A signature check that can be + * talked round here is not a bug in a web page, it is arbitrary command + * execution on somebody's game server. + * + * So these tests are written from the outside, as an attacker has to work: a + * real Fastify server with the real `@fastify/websocket` plugin, the real + * gateway registered on it, and a hand-rolled WebSocket client speaking down a + * raw TCP socket. Nothing calls a private method. The hand-rolled client is + * deliberate on two counts — it can set (or omit) the `Origin` header the way a + * non-browser attacker would, and it can decline to answer a close frame, which + * is how the expiry window below is held open long enough to be measured. + */ + +// --------------------------------------------------------------------------- +// The world the gateway believes in +// --------------------------------------------------------------------------- + +const NODE_UUID = '11111111-1111-4111-8111-111111111111'; +const OTHER_NODE_UUID = '99999999-9999-4999-8999-999999999999'; +const SERVER_UUID = '22222222-2222-4222-8222-222222222222'; +const OTHER_SERVER_UUID = '33333333-3333-4333-8333-333333333333'; +const UNHOSTED_SERVER_UUID = '44444444-4444-4444-8444-444444444444'; +const USER_UUID = '55555555-5555-4555-8555-555555555555'; + +const ISSUER = 'https://panel.example.com'; +const OTHER_ISSUER = 'https://panel.attacker.example'; + +const JWT_SECRET = 'j'.repeat(64); +const WRONG_SECRET = 'k'.repeat(64); +const NODE_TOKEN_SECRET = 'z'.repeat(64); + +const SIGNING_KEY = Buffer.from(JWT_SECRET, 'utf8'); + +const PANEL_ORIGIN = 'https://panel.example.com'; +const EVIL_ORIGIN = 'https://minecraft-free-diamonds.example'; + +/** The claim set the panel actually mints, before any tampering. */ +const HONEST_PERMISSIONS: Permission[] = [ + PERMISSIONS.WEBSOCKET_CONNECT, + PERMISSIONS.CONTROL_CONSOLE, + PERMISSIONS.CONTROL_START, + PERMISSIONS.CONTROL_STOP, +]; + +// --------------------------------------------------------------------------- +// Minting tokens, honest and otherwise +// --------------------------------------------------------------------------- + +type SigningKey = Parameters['sign']>[0]; + +interface MintOptions { + /** Overrides merged over the honest claim set. */ + claims?: Record; + /** Claims to remove entirely, to probe what the verifier insists on. */ + without?: string[]; + /** Seconds until expiry; `null` mints a token with no `exp` at all. */ + ttlSeconds?: number | null; + alg?: string; + key?: SigningKey; +} + +async function mintToken(options: MintOptions = {}): Promise { + const now = Math.floor(Date.now() / 1000); + + const claims: Record = { + iss: ISSUER, + aud: NODE_UUID, + sub: USER_UUID, + jti: 'console-token-1', + iat: now, + serverUuid: SERVER_UUID, + permissions: HONEST_PERMISSIONS, + ...options.claims, + }; + + if (options.ttlSeconds !== null) { + claims.exp = now + (options.ttlSeconds ?? 600); + } + + for (const claim of options.without ?? []) { + delete claims[claim]; + } + + return new SignJWT(claims) + .setProtectedHeader({ alg: options.alg ?? 'HS256' }) + .sign(options.key ?? SIGNING_KEY); +} + +function encodeSegment(value: unknown): string { + return Buffer.from(JSON.stringify(value), 'utf8').toString('base64url'); +} + +function decodeSegment(segment: string): Record { + return JSON.parse(Buffer.from(segment, 'base64url').toString('utf8')) as Record; +} + +function splitToken(token: string): [string, string, string] { + const [header, payload, signature] = token.split('.'); + return [header!, payload!, signature!]; +} + +/** Swaps the header while keeping the original signature bytes. */ +function withHeader(token: string, header: Record): string { + const [, payload, signature] = splitToken(token); + return `${encodeSegment(header)}.${payload}.${signature}`; +} + +/** Rewrites the claims while keeping the signature the panel produced. */ +function withTamperedPayload( + token: string, + mutate: (claims: Record) => void, +): string { + const [header, payload, signature] = splitToken(token); + const claims = decodeSegment(payload); + mutate(claims); + return `${header}.${encodeSegment(claims)}.${signature}`; +} + +function withFlippedSignatureBit(token: string): string { + const [header, payload, signature] = splitToken(token); + const bytes = Buffer.from(signature, 'base64url'); + bytes[0] = bytes[0]! ^ 0x01; + return `${header}.${payload}.${bytes.toString('base64url')}`; +} + +function withTruncatedSignature(token: string): string { + const [header, payload, signature] = splitToken(token); + const bytes = Buffer.from(signature, 'base64url'); + return `${header}.${payload}.${bytes.subarray(0, bytes.length - 1).toString('base64url')}`; +} + +// --------------------------------------------------------------------------- +// A WebSocket client that does only what it is told +// --------------------------------------------------------------------------- + +interface Frame { + event?: unknown; + [key: string]: unknown; +} + +interface CloseFrame { + code: number; + reason: string; +} + +/** + * A WebSocket client written by hand over a TCP socket. + * + * The `ws` library would be shorter, but it is a well-behaved peer: it always + * sets the headers a browser sets and always answers a close frame. Two of the + * questions here are precisely about a peer that does neither. + */ +class AttackerSocket { + private buffer = Buffer.alloc(0); + private readonly received: Frame[] = []; + private cursor = 0; + private notify: (() => void) | null = null; + private closeFrame: CloseFrame | null = null; + + /** + * Whether to answer the server's close frame. A browser does, and the + * connection then goes down at once. Nothing forces an attacker to. + */ + politeClose = true; + + private constructor(private readonly socket: Socket) { + socket.on('data', (chunk: Buffer) => this.consume(chunk)); + socket.on('error', () => this.wake()); + socket.on('close', () => this.wake()); + } + + static open( + port: number, + path: string, + options: { origin?: string } = {}, + ): Promise { + return new Promise((resolve, reject) => { + const socket = connect({ port, host: '127.0.0.1' }); + let head = Buffer.alloc(0); + + const onData = (chunk: Buffer): void => { + head = Buffer.concat([head, chunk]); + const bodyAt = head.indexOf('\r\n\r\n'); + + if (bodyAt === -1) { + return; + } + + socket.off('data', onData); + const status = head.subarray(0, head.indexOf('\r\n')).toString('ascii'); + + if (!status.includes('101')) { + socket.destroy(); + reject(new Error(`Upgrade refused: ${status}`)); + return; + } + + const client = new AttackerSocket(socket); + const leftover = head.subarray(bodyAt + 4); + + if (leftover.length > 0) { + client.consume(leftover); + } + + resolve(client); + }; + + socket.on('data', onData); + socket.on('error', reject); + socket.on('connect', () => { + const lines = [ + `GET ${path} HTTP/1.1`, + `Host: 127.0.0.1:${port}`, + 'Upgrade: websocket', + 'Connection: Upgrade', + `Sec-WebSocket-Key: ${randomBytes(16).toString('base64')}`, + 'Sec-WebSocket-Version: 13', + ]; + + if (options.origin !== undefined) { + lines.push(`Origin: ${options.origin}`); + } + + socket.write(`${lines.join('\r\n')}\r\n\r\n`); + }); + }); + } + + send(message: unknown): void { + this.sendText(JSON.stringify(message)); + } + + sendText(text: string): void { + this.socket.write(frameForServer(Buffer.from(text, 'utf8'), 0x1)); + } + + /** Every frame received so far, for assertions about what was *not* sent. */ + all(): Frame[] { + return [...this.received]; + } + + get closedWith(): CloseFrame | null { + return this.closeFrame; + } + + async next(timeoutMs = 3_000): Promise { + const frame = await this.until(() => this.received[this.cursor], 'a message', timeoutMs); + this.cursor += 1; + return frame; + } + + /** Waits for a frame matching `predicate`, discarding what comes before it. */ + async waitFor( + predicate: (frame: Frame) => boolean, + description: string, + timeoutMs = 3_000, + ): Promise { + return this.until( + () => { + while (this.cursor < this.received.length) { + const frame = this.received[this.cursor++]!; + + if (predicate(frame)) { + return frame; + } + } + + return undefined; + }, + description, + timeoutMs, + ); + } + + async waitForEvent(event: string, timeoutMs = 3_000): Promise { + return this.waitFor((frame) => frame.event === event, `event ${event}`, timeoutMs); + } + + async waitForClose(timeoutMs = 3_000): Promise { + return this.until(() => this.closeFrame ?? undefined, 'the close frame', timeoutMs); + } + + /** Lets the server finish whatever it is doing, then stops listening. */ + async settle(milliseconds = 120): Promise { + await new Promise((resolve) => setTimeout(resolve, milliseconds)); + } + + /** + * Stops taking anything off the wire, without closing anything. + * + * The receive window fills, TCP stops the daemon writing, and whatever it + * still wants to say accumulates in its own memory — which is the whole of + * the attack the send-buffer ceiling exists to stop. + */ + pause(): void { + this.socket.pause(); + } + + resume(): void { + this.socket.resume(); + } + + destroy(): void { + this.socket.destroy(); + } + + private async until( + read: () => T | undefined, + description: string, + timeoutMs: number, + ): Promise { + const deadline = Date.now() + timeoutMs; + + for (;;) { + const value = read(); + + if (value !== undefined) { + return value; + } + + const remaining = deadline - Date.now(); + + if (remaining <= 0) { + throw new Error( + `Timed out waiting for ${description}. Received: ${JSON.stringify(this.received)}`, + ); + } + + await new Promise((resolve) => { + const timer = setTimeout(resolve, Math.min(remaining, 20)); + this.notify = (): void => { + clearTimeout(timer); + resolve(); + }; + }); + } + } + + private wake(): void { + const notify = this.notify; + this.notify = null; + notify?.(); + } + + private consume(chunk: Buffer): void { + this.buffer = Buffer.concat([this.buffer, chunk]); + + for (;;) { + if (this.buffer.length < 2) { + return; + } + + const opcode = this.buffer[0]! & 0x0f; + let length = this.buffer[1]! & 0x7f; + let offset = 2; + + if (length === 126) { + if (this.buffer.length < 4) return; + length = this.buffer.readUInt16BE(2); + offset = 4; + } else if (length === 127) { + if (this.buffer.length < 10) return; + length = Number(this.buffer.readBigUInt64BE(2)); + offset = 10; + } + + if (this.buffer.length < offset + length) { + return; + } + + const payload = this.buffer.subarray(offset, offset + length); + this.buffer = this.buffer.subarray(offset + length); + this.handleFrame(opcode, payload); + } + } + + private handleFrame(opcode: number, payload: Buffer): void { + if (opcode === 0x1) { + this.received.push(JSON.parse(payload.toString('utf8')) as Frame); + } else if (opcode === 0x8) { + this.closeFrame = { + code: payload.length >= 2 ? payload.readUInt16BE(0) : 1005, + reason: payload.subarray(2).toString('utf8'), + }; + + if (this.politeClose) { + this.socket.write(frameForServer(payload.subarray(0, 2), 0x8)); + this.socket.end(); + } + } else if (opcode === 0x9) { + this.socket.write(frameForServer(payload, 0xa)); + } + + this.wake(); + } +} + +/** Builds a masked frame; every client-to-server frame has to be masked. */ +function frameForServer(payload: Buffer, opcode: number): Buffer { + const mask = randomBytes(4); + let header: Buffer; + + if (payload.length < 126) { + header = Buffer.from([0x80 | opcode, 0x80 | payload.length]); + } else if (payload.length < 65_536) { + header = Buffer.alloc(4); + header[0] = 0x80 | opcode; + header[1] = 0x80 | 126; + header.writeUInt16BE(payload.length, 2); + } else { + header = Buffer.alloc(10); + header[0] = 0x80 | opcode; + header[1] = 0x80 | 127; + header.writeBigUInt64BE(BigInt(payload.length), 2); + } + + const masked = Buffer.from(payload); + + for (let index = 0; index < masked.length; index += 1) { + masked[index] = masked[index]! ^ mask[index % 4]!; + } + + return Buffer.concat([header, mask, masked]); +} + +// --------------------------------------------------------------------------- +// The daemon side, real gateway over a real server +// --------------------------------------------------------------------------- + +const SNAPSHOT_LINES = ['[Server] Done (3.412s)!', '[Server] whitelist add nobody']; + +/** A sample shaped the way the stats stream produces one for a live server. */ +const RUNNING_SAMPLE: ResourceUsage = { + state: 'running', + uptime: 12_000, + memoryBytes: 512 * 1024 * 1024, + memoryLimitBytes: 1024 * 1024 * 1024, + cpuPercent: 37.5, + diskBytes: 4_096, + networkRxBytes: 10, + networkTxBytes: 20, +}; + +class FakeServerInstance extends EventEmitter { + currentState: ServerState = 'offline'; + + readonly idleUsage: ResourceUsage = { + state: 'offline', + uptime: 0, + memoryBytes: 0, + memoryLimitBytes: 1024, + cpuPercent: 0, + diskBytes: 42, + networkRxBytes: 0, + networkTxBytes: 0, + }; + + readonly sendCommand = vi.fn((_command: string) => Promise.resolve()); + readonly power = vi.fn((_action: PowerAction) => Promise.resolve()); + readonly consoleSnapshot = vi.fn(() => [...SNAPSHOT_LINES]); +} + +interface Harness { + port: number; + instance: FakeServerInstance; + app: FastifyInstance; +} + +function buildConfig(overrides: { allowedOrigins?: string[] } = {}): DaemonConfig { + return daemonConfigSchema.parse({ + uuid: NODE_UUID, + tokenId: 'abcdefghijklmnop', + tokenSecret: NODE_TOKEN_SECRET, + api: { allowedOrigins: overrides.allowedOrigins ?? [PANEL_ORIGIN] }, + panel: { url: ISSUER, jwtSecret: JWT_SECRET }, + }); +} + +async function startHarness(configOverrides: { allowedOrigins?: string[] } = {}): Promise { + const instance = new FakeServerInstance(); + + const manager = { + get: (uuid: string): ServerInstance | undefined => + uuid === SERVER_UUID ? (instance as unknown as ServerInstance) : undefined, + } as unknown as ServerManager; + + const logger = { + debug: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + error: vi.fn(), + } as unknown as Logger; + + const app = Fastify({ logger: false }); + + // The same payload ceiling the daemon's HTTP server applies, so a test that + // sends something large fails here for the same reason it would in + // production rather than for a reason peculiar to the harness. + await app.register(websocket, { options: { maxPayload: 64 * 1024 } }); + registerConsoleGateway(app, manager, buildConfig(configOverrides), logger); + + await app.listen({ port: 0, host: '127.0.0.1' }); + const address = app.server.address(); + + if (address === null || typeof address === 'string') { + throw new Error('Expected a TCP address.'); + } + + return { port: address.port, instance, app }; +} + +// --------------------------------------------------------------------------- + +describe('registerConsoleGateway', () => { + let harness: Harness; + const opened: AttackerSocket[] = []; + + async function openSocket( + options: { path?: string; origin?: string } = {}, + ): Promise { + const client = await AttackerSocket.open( + harness.port, + options.path ?? `/api/servers/${SERVER_UUID}/ws`, + { origin: options.origin ?? PANEL_ORIGIN }, + ); + + opened.push(client); + return client; + } + + /** Opens a connection and drives it to an authenticated state. */ + async function authenticated(token: string): Promise { + const client = await openSocket(); + client.send({ event: 'auth', token }); + await client.waitForEvent('auth_success'); + return client; + } + + /** + * Waits until the daemon has handled everything sent so far. + * + * A message the gateway is obliged to answer, used as a fence: frames are + * handled in arrival order, so its reply means the ones before it are done. + * It also moves the read cursor past every frame received up to that point, + * which is what lets a test that follows assert on the *next* thing said. + */ + async function fence(client: AttackerSocket): Promise { + client.send({ event: 'nonsense' }); + await client.waitFor( + (frame) => frame.code === WS_ERROR_CODES.INVALID_MESSAGE, + 'the fence reply', + ); + } + + /** + * Presents `token` and asserts the daemon threw it out. + * + * The three assertions are one claim: the token bought nothing. A refusal + * that still attached the session to the server would leak every console + * line to the holder of a forged token even though `auth_success` never + * arrived. + */ + async function expectRefused(token: string, path?: string): Promise { + const client = await openSocket(path === undefined ? {} : { path }); + client.send({ event: 'auth', token }); + + const error = await client.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.INVALID_TOKEN); + + const closed = await client.waitForClose(); + expect(closed.code).toBe(1008); + + expect(client.all().some((frame) => frame.event === 'auth_success')).toBe(false); + expect(harness.instance.listenerCount('console')).toBe(0); + } + + beforeEach(async () => { + harness = await startHarness(); + }); + + afterEach(async () => { + for (const client of opened.splice(0)) { + client.destroy(); + } + + await harness.app.close(); + vi.useRealTimers(); + }); + + // ------------------------------------------------------------------------- + + describe('forging a token', () => { + it('accepts the token the panel would really have signed', async () => { + const client = await authenticated(await mintToken()); + const success = client.all().find((frame) => frame.event === 'auth_success')!; + + expect(success.permissions).toEqual(HONEST_PERMISSIONS); + }); + + it('refuses a token signed with a different secret', async () => { + // The whole architecture is this line. If a signature made with anything + // other than this node's secret opened a console, the panel's + // authorisation model would be decoration. + await expectRefused(await mintToken({ key: Buffer.from(WRONG_SECRET, 'utf8') })); + }); + + it('refuses a token signed with the node token secret instead of the JWT secret', async () => { + // The daemon holds two secrets and they authenticate opposite + // directions: `tokenSecret` proves the *panel* to the daemon, + // `panel.jwtSecret` proves a *browser*. Verifying a console token + // against the wrong one would mean a leaked node token — a value that + // travels on every panel-to-daemon call — could mint console sessions. + await expectRefused(await mintToken({ key: Buffer.from(NODE_TOKEN_SECRET, 'utf8') })); + }); + + it('refuses an unsecured token (alg: none)', async () => { + const unsecured = new UnsecuredJWT({ + serverUuid: SERVER_UUID, + permissions: HONEST_PERMISSIONS, + sub: USER_UUID, + jti: 'none-1', + }) + .setIssuer(ISSUER) + .setAudience(NODE_UUID) + .setIssuedAt() + .setExpirationTime('600s') + .encode(); + + await expectRefused(unsecured); + }); + + it('refuses an algorithm the verifier did not pin, even with the right secret', async () => { + // HS512 with the correct key is a valid signature by any general + // definition. Pinning `algorithms` is what stops the verifier being + // steered by a header the attacker writes. + await expectRefused(await mintToken({ alg: 'HS512' })); + }); + + it('refuses an RS256 signature wearing an HS256 header', async () => { + // The classic confusion: the attacker signs asymmetrically and relabels + // the header so the verifier reaches for its symmetric path. It cannot + // work here — the key is a shared secret, not a published public key — + // but the day someone swaps `jwtSecret` for a key pair, this is the test + // that has to keep failing the forgery. + const { privateKey } = await generateKeyPair('RS256'); + const rs256 = await mintToken({ alg: 'RS256', key: privateKey }); + + await expectRefused(withHeader(rs256, { alg: 'HS256' })); + await expectRefused(rs256); + }); + + it('refuses a signature with a single bit flipped', async () => { + await expectRefused(withFlippedSignatureBit(await mintToken())); + }); + + it('refuses a signature one byte short', async () => { + // A truncated MAC must not be compared prefix-wise. If it were, an + // attacker could search the remaining bytes one at a time. + await expectRefused(withTruncatedSignature(await mintToken())); + }); + + it('refuses a token with no signature at all', async () => { + const [header, payload] = splitToken(await mintToken()); + await expectRefused(`${header}.${payload}.`); + }); + + it('refuses permissions escalated after signing', async () => { + // The forgery a subuser with console-only access would actually try: + // keep the panel's own signature, edit the claim list on the way past. + const token = withTamperedPayload( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + (claims) => { + claims.permissions = [ + PERMISSIONS.WEBSOCKET_CONNECT, + PERMISSIONS.CONTROL_CONSOLE, + PERMISSIONS.CONTROL_STOP, + ]; + }, + ); + + await expectRefused(token); + }); + + it('refuses a payload swapped between two honestly signed tokens', async () => { + // Cut-and-paste: two tokens the panel really issued, recombined. The + // signature covers the payload, so neither half survives the other. + const mine = await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }); + const powerful = await mintToken({ claims: { jti: 'console-token-2' } }); + + const [header, , signature] = splitToken(mine); + const [, powerfulPayload] = splitToken(powerful); + + await expectRefused(`${header}.${powerfulPayload}.${signature}`); + }); + + it.each([ + ['empty-ish string', ' '], + ['not a JWT', 'give-me-a-console'], + ['two segments only', 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhIn0'], + ['five segments', 'a.b.c.d.e'], + ['header that is not JSON', `${Buffer.from('nope').toString('base64url')}.e30.sig`], + ])('refuses a malformed token: %s', async (_label, token) => { + await expectRefused(token); + }); + }); + + // ------------------------------------------------------------------------- + + describe('pointing a valid token at the wrong place', () => { + it("refuses a token minted for another node's audience", async () => { + // `aud` is this node's UUID. Without that check, an operator of any + // compromised node could take the tokens their own daemon receives and + // replay them against every other node sharing the secret — which is + // exactly what a shared-secret design has to rule out explicitly. + await expectRefused(await mintToken({ claims: { aud: OTHER_NODE_UUID } })); + }); + + it('refuses a token issued by another panel', async () => { + await expectRefused(await mintToken({ claims: { iss: OTHER_ISSUER } })); + }); + + it('refuses a token naming a different server than the URL', async () => { + // The reason a subuser on one server cannot read the console of + // another: the URL is attacker-chosen, `serverUuid` is signed, and the + // two have to agree. + await expectRefused(await mintToken({ claims: { serverUuid: OTHER_SERVER_UUID } })); + }); + + it('refuses a valid token presented on another server’s URL', async () => { + await expectRefused(await mintToken(), `/api/servers/${OTHER_SERVER_UUID}/ws`); + }); + + it('does not confuse an unknown server with a bad token', async () => { + // A token perfectly valid for a server this node does not host: the + // daemon has to say so with its own code rather than call the token + // invalid, or an operator debugging a migrated server chases a + // signature problem that does not exist. + const client = await openSocket({ path: `/api/servers/${UNHOSTED_SERVER_UUID}/ws` }); + client.send({ + event: 'auth', + token: await mintToken({ claims: { serverUuid: UNHOSTED_SERVER_UUID } }), + }); + + const error = await client.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.INTERNAL); + expect(await client.waitForClose()).toMatchObject({ code: 1011 }); + }); + }); + + // ------------------------------------------------------------------------- + + describe('time', () => { + it('refuses a token that expired a second ago', async () => { + await expectRefused(await mintToken({ ttlSeconds: -1 })); + }); + + it('refuses a token whose nbf has not arrived', async () => { + // A token minted for later must not work now. `jose` enforces this with + // no clock tolerance configured, and nothing in the contract schema + // would catch it — so if the `nbf` default ever changed, this is the + // only thing that would notice. + await expectRefused( + await mintToken({ claims: { nbf: Math.floor(Date.now() / 1000) + 300 } }), + ); + }); + + it('refuses a token with no exp, which jose alone would have accepted', async () => { + const eternal = await mintToken({ ttlSeconds: null }); + + // Proof that the signature layer is *not* what saves us here: with the + // daemon's own verification options, `jwtVerify` resolves happily on a + // token that never expires. An `exp`-less console token would be a + // permanent, unrevocable key to a server's console. + await expect( + jwtVerify(eternal, SIGNING_KEY, { + issuer: ISSUER, + audience: NODE_UUID, + algorithms: ['HS256'], + }), + ).resolves.toBeDefined(); + + await expectRefused(eternal); + }); + + it('accepts a token inside its normal lifetime', async () => { + const client = await authenticated(await mintToken({ ttlSeconds: 600 })); + const success = client.all().find((frame) => frame.event === 'auth_success')!; + + expect(success.expiresAt).toBeGreaterThan(Date.now()); + }); + + it('closes the console when the token expires under it', async () => { + const client = await authenticated(await mintToken({ ttlSeconds: 1 })); + + expect((await client.waitForEvent('token_expired', 4_000)).event).toBe('token_expired'); + expect(await client.waitForClose()).toMatchObject({ code: 1008 }); + }); + + it('warns before expiry so the client can renew', async () => { + // The renewal margin is 60 seconds, so a 61-second token asks for a new + // one after about a second. Without this warning the console would drop + // mid-session, which is the failure the whole renewal dance exists to + // avoid. + const client = await authenticated(await mintToken({ ttlSeconds: 61 })); + + expect((await client.waitForEvent('token_expiring', 4_000)).event).toBe('token_expiring'); + }); + + it('puts no ceiling on how far ahead exp may be', async () => { + // Nothing in the daemon compares `exp - iat` against the short + // lifetime the design promises: `maxTokenAge` is not set and the schema + // only requires `exp` to be an integer. A token good for a fortnight is + // authenticated in full, and short lifetimes are the *only* revocation + // this design has. It is the panel, and nothing here, that keeps them + // short. + const client = await authenticated(await mintToken({ ttlSeconds: 14 * 24 * 60 * 60 })); + const success = client.all().find((frame) => frame.event === 'auth_success')!; + + expect(success.expiresAt).toBeGreaterThan(Date.now() + 13 * 24 * 60 * 60 * 1000); + }); + + it('cannot hold a token valid beyond a 32-bit timer and closes at once', async () => { + // Past about 24.8 days the expiry delay overflows `setTimeout`, which + // clamps to 1ms — so an absurdly long token authenticates and is then + // torn down immediately. Fail-closed, and worth pinning: an accidental + // change to fail-open here would turn the previous test's observation + // into an eternal session. + const client = await authenticated(await mintToken({ ttlSeconds: 10 * 365 * 24 * 60 * 60 })); + + expect((await client.waitForEvent('token_expired')).event).toBe('token_expired'); + expect(await client.waitForClose()).toMatchObject({ code: 1008 }); + }); + }); + + // ------------------------------------------------------------------------- + + describe('the contract as a second gate', () => { + /** + * Payloads that `jwtVerify` is perfectly happy with. + * + * Each case asserts twice: that the signature layer lets it through, and + * that the gateway does not. Delete the `consoleTokenPayloadSchema` check + * from `authenticate` and every one of these turns into a live session — + * which is the point of having the second gate at all. + */ + const acceptedByJose: [string, MintOptions][] = [ + ['no exp', { ttlSeconds: null }], + ['no jti', { without: ['jti'] }], + ['no sub', { without: ['sub'] }], + ['no serverUuid', { without: ['serverUuid'] }], + ['no permissions', { without: ['permissions'] }], + ['aud as an array containing this node', { claims: { aud: [NODE_UUID, OTHER_NODE_UUID] } }], + ['permissions holding a value outside the enum', { claims: { permissions: ['control.*'] } }], + ['permissions holding an object', { claims: { permissions: [{ all: true }] } }], + ['permissions as a bare string', { claims: { permissions: PERMISSIONS.CONTROL_CONSOLE } }], + [ + 'exp that is not a whole number', + { ttlSeconds: null, claims: { exp: Math.floor(Date.now() / 1000) + 600.5 } }, + ], + ]; + + it.each(acceptedByJose)('refuses a payload jose accepts: %s', async (_label, options) => { + const token = await mintToken(options); + + await expect( + jwtVerify(token, SIGNING_KEY, { + issuer: ISSUER, + audience: NODE_UUID, + algorithms: ['HS256'], + }), + ).resolves.toBeDefined(); + + await expectRefused(token); + }); + + it('refuses a serverUuid that is not a UUID, presented at its own URL', async () => { + // Deliberately opened at the URL the claim names. The obvious version of + // this test sends `serverUuid: '../../etc/passwd'` to the normal URL, + // where the `claims.serverUuid !== this.serverUuid` comparison refuses it + // before the schema is ever consulted — mutate `z.uuid()` to `z.string()` + // and it stays green, which makes it a test of the comparison wearing the + // schema's name. + // + // Matching the two makes the schema the only thing left that can object. + // Weaken it and this token reaches `manager.get`, which refuses an + // unknown server with `internal` and code 1011 — a different answer, + // caught below. + const serverUuid = 'not-a-uuid'; + const token = await mintToken({ claims: { serverUuid } }); + + await expect( + jwtVerify(token, SIGNING_KEY, { + issuer: ISSUER, + audience: NODE_UUID, + algorithms: ['HS256'], + }), + ).resolves.toBeDefined(); + + await expectRefused(token, `/api/servers/${serverUuid}/ws`); + }); + + it('ignores claims the contract does not know about', async () => { + // Extra claims are stripped, not honoured and not fatal. A forged + // `role: ADMIN` must buy nothing, and a claim the panel adds in a later + // version must not break older daemons. + const client = await authenticated( + await mintToken({ + claims: { + role: 'ADMIN', + admin: true, + scope: '*', + permissions: [PERMISSIONS.WEBSOCKET_CONNECT], + }, + }), + ); + + const success = client.all().find((frame) => frame.event === 'auth_success')!; + expect(success.permissions).toEqual([PERMISSIONS.WEBSOCKET_CONNECT]); + + client.send({ event: 'send_command', command: 'op attacker' }); + const error = await client.waitForEvent('error'); + + expect(error.code).toBe(WS_ERROR_CODES.FORBIDDEN); + expect(harness.instance.sendCommand).not.toHaveBeenCalled(); + }); + }); + + // ------------------------------------------------------------------------- + + describe('what the token is allowed to do', () => { + it('refuses a command from a session without control.console', async () => { + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + + client.send({ event: 'send_command', command: 'op attacker' }); + const error = await client.waitForEvent('error'); + + expect(error.code).toBe(WS_ERROR_CODES.FORBIDDEN); + expect(error.message).toContain(PERMISSIONS.CONTROL_CONSOLE); + expect(harness.instance.sendCommand).not.toHaveBeenCalled(); + }); + + it('delivers a command from a session that holds control.console', async () => { + const client = await authenticated(await mintToken()); + client.send({ event: 'send_command', command: 'say hello' }); + await client.settle(); + + expect(harness.instance.sendCommand).toHaveBeenCalledWith('say hello'); + }); + + it.each([ + ['start', PERMISSIONS.CONTROL_START], + ['stop', PERMISSIONS.CONTROL_STOP], + ['restart', PERMISSIONS.CONTROL_RESTART], + // Killing is destructive but shares the stop permission, by design. + ['kill', PERMISSIONS.CONTROL_STOP], + ] satisfies [PowerAction, Permission][])( + 'refuses %s without %s and allows it with', + async (action, required) => { + const others = [ + PERMISSIONS.CONTROL_START, + PERMISSIONS.CONTROL_STOP, + PERMISSIONS.CONTROL_RESTART, + ].filter((permission) => permission !== required); + + const denied = await authenticated(await mintToken({ claims: { permissions: others } })); + denied.send({ event: 'set_state', action }); + + const error = await denied.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.FORBIDDEN); + expect(harness.instance.power).not.toHaveBeenCalled(); + + const allowed = await authenticated( + await mintToken({ claims: { permissions: [required] } }), + ); + allowed.send({ event: 'set_state', action }); + await allowed.settle(); + + expect(harness.instance.power).toHaveBeenCalledWith(action); + }, + ); + + it('never replays the console buffer to a session without control.console', async () => { + // The buffer is 500 lines of whatever the server printed — chat, + // command output, sometimes a token an operator pasted. `websocket. + // connect` is implicit for every subuser, so this is the line between + // "can see the server exists" and "can read everything said on it". + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + + client.send({ event: 'request_logs' }); + + // Refused out loud, not ignored: a client left waiting on a buffer that + // is never coming cannot tell that from a server with nothing to say. + const error = await client.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.FORBIDDEN); + expect(error.message).toContain(PERMISSIONS.CONTROL_CONSOLE); + + expect(client.all().some((frame) => frame.event === 'console_output')).toBe(false); + expect(harness.instance.consoleSnapshot).not.toHaveBeenCalled(); + }); + + it('replays the console buffer to a session that holds control.console', async () => { + const client = await authenticated(await mintToken()); + await client.waitFor( + (frame) => frame.event === 'console_output' && frame.line === SNAPSHOT_LINES[1], + 'the end of the console snapshot', + ); + + expect(harness.instance.consoleSnapshot).toHaveBeenCalled(); + }); + + it('never streams live output to a session without control.console either', async () => { + // The replay gate was worth very little on its own: it held back the + // buffer and then handed over everything printed from that second + // onwards, which is the same secrets a minute later — the RCON password + // an operator pastes, the key a plugin prints on load, the `op` that + // names the next administrator. `websocket.connect` is implicit for + // every subuser, so this was every subuser. + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + await fence(client); + + harness.instance.emit('console', 'rcon.password=hunter2'); + harness.instance.emit('install_output', 'curl https://cdn.example/egg?key=s3cret'); + await fence(client); + + const events = client.all().map((frame) => frame.event); + expect(events).not.toContain('console_output'); + expect(events).not.toContain('install_output'); + }); + + it('still tells a session without control.console what its server is doing', async () => { + // The other half of the same decision, and the reason the gate is on the + // output rather than on the subscription: a subuser who may not read the + // console is still meant to watch the server go up and down, see what it + // is consuming, and know an installation is running. Those say *that* + // something happened; only the output says *what was said*. + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + await fence(client); + + harness.instance.emit('state', 'running'); + harness.instance.emit('stats', RUNNING_SAMPLE); + harness.instance.emit('install_started'); + harness.instance.emit('install_completed', true); + await fence(client); + + const frames = client.all(); + expect(frames).toContainEqual({ event: 'status', state: 'running' }); + expect(frames).toContainEqual({ event: 'stats', usage: RUNNING_SAMPLE }); + expect(frames).toContainEqual({ event: 'install_started' }); + expect(frames).toContainEqual({ event: 'install_completed', successful: true }); + }); + + it('says so rather than showing an empty console for no stated reason', async () => { + // A withheld console and a silent server look identical from the browser. + // Whoever is handed a subuser account with no console permission should + // read why their terminal is empty, not open a ticket about it. + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + const notice = await client.waitForEvent('daemon_message'); + + expect(notice.message).toContain(PERMISSIONS.CONTROL_CONSOLE); + }); + + it('streams live output to a session that holds control.console', async () => { + const client = await authenticated(await mintToken()); + await fence(client); + + harness.instance.emit('console', 'a line worth reading'); + harness.instance.emit('install_output', 'unpacking'); + + expect((await client.waitForEvent('console_output')).line).toBe('a line worth reading'); + expect((await client.waitForEvent('install_output')).line).toBe('unpacking'); + }); + + it('stops streaming the moment a renewal drops the permission, and resumes when it returns', async () => { + // The permission is consulted when the line arrives, not when the + // listener is attached — which is the only reading that makes the + // short token lifetime mean anything. Attaching conditionally would have + // frozen the session at the permissions it opened with, in both + // directions. + const client = await authenticated(await mintToken()); + await fence(client); + + client.send({ + event: 'auth', + token: await mintToken({ + claims: { jti: 'narrowed-1', permissions: [PERMISSIONS.WEBSOCKET_CONNECT] }, + }), + }); + await client.waitForEvent('auth_success'); + await fence(client); + + harness.instance.emit('console', 'said after the narrowing'); + await fence(client); + + expect(client.all().some((frame) => frame.line === 'said after the narrowing')).toBe(false); + + client.send({ event: 'auth', token: await mintToken({ claims: { jti: 'restored-1' } }) }); + await client.waitForEvent('auth_success'); + await fence(client); + + harness.instance.emit('console', 'said after the restoration'); + await fence(client); + + expect(client.all().some((frame) => frame.line === 'said after the restoration')).toBe(true); + }); + + it('answers request_stats with a stats sample rather than a status', async () => { + // It replied with `status`: the client asked what the server is + // consuming and was told what state it is in. `ResourceUsage` carries + // the state too, so the right event says everything the wrong one did. + const client = await authenticated(await mintToken()); + await fence(client); + + client.send({ event: 'request_stats' }); + const stats = await client.waitForEvent('stats'); + + expect(stats.usage).toEqual(harness.instance.idleUsage); + }); + + it('answers request_stats with the latest sample, and forgets it when the state moves', async () => { + const client = await authenticated(await mintToken()); + await fence(client); + + harness.instance.emit('stats', RUNNING_SAMPLE); + await fence(client); + + client.send({ event: 'request_stats' }); + expect((await client.waitForEvent('stats')).usage).toEqual(RUNNING_SAMPLE); + + // A sample describes the state it was taken in. Once the server has left + // it, reporting half a gigabyte of memory for a stopped process would be + // worse than reporting nothing. + harness.instance.emit('state', 'offline'); + await fence(client); + + client.send({ event: 'request_stats' }); + expect((await client.waitForEvent('stats')).usage).toEqual(harness.instance.idleUsage); + }); + + it('reads permissions from the token presented, not from anything cached', async () => { + // Renewal happens over the same socket. If the session kept the + // permissions it started with, a narrowed token would be cosmetic and + // the short lifetime — the only revocation this design has — would + // buy nothing at all. + const client = await authenticated(await mintToken()); + client.send({ event: 'send_command', command: 'say still allowed' }); + await client.settle(); + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(1); + + client.send({ + event: 'auth', + token: await mintToken({ + claims: { jti: 'renewal-1', permissions: [PERMISSIONS.WEBSOCKET_CONNECT] }, + }), + }); + await client.waitForEvent('auth_success'); + + client.send({ event: 'send_command', command: 'op attacker' }); + const error = await client.waitForEvent('error'); + + expect(error.code).toBe(WS_ERROR_CODES.FORBIDDEN); + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(1); + }); + + it('kills the session when a renewal token is forged', async () => { + const client = await authenticated( + await mintToken({ claims: { permissions: [PERMISSIONS.WEBSOCKET_CONNECT] } }), + ); + + client.send({ + event: 'auth', + token: await mintToken({ + claims: { permissions: HONEST_PERMISSIONS }, + key: Buffer.from(WRONG_SECRET, 'utf8'), + }), + }); + + const error = await client.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.INVALID_TOKEN); + expect(await client.waitForClose()).toMatchObject({ code: 1008 }); + }); + + it('does not multiply its output when a session re-authenticates repeatedly', async () => { + // Renewal must not attach a second set of listeners. If it did, a client + // re-authenticating in a loop would make the daemon serialise every + // console line N times — a free amplifier pointed at its own memory. + const client = await authenticated(await mintToken()); + + for (let index = 0; index < 5; index += 1) { + client.send({ + event: 'auth', + token: await mintToken({ claims: { jti: `renew-${index}` } }), + }); + await client.waitForEvent('auth_success'); + } + + expect(harness.instance.listenerCount('console')).toBe(1); + + harness.instance.emit('console', 'a single line'); + await client.settle(); + + const lines = client.all().filter((frame) => frame.line === 'a single line'); + expect(lines).toHaveLength(1); + }); + + it('detaches from the server when the console closes', async () => { + const client = await authenticated(await mintToken()); + expect(harness.instance.listenerCount('console')).toBe(1); + + client.destroy(); + await new Promise((resolve) => setTimeout(resolve, 150)); + + expect(harness.instance.listenerCount('console')).toBe(0); + }); + }); + + // ------------------------------------------------------------------------- + + describe('the connection itself', () => { + it('refuses every message sent before authentication', async () => { + const client = await openSocket(); + + client.send({ event: 'send_command', command: 'stop' }); + client.send({ event: 'set_state', action: 'kill' }); + client.send({ event: 'request_logs' }); + + for (let index = 0; index < 3; index += 1) { + expect((await client.next()).code).toBe(WS_ERROR_CODES.UNAUTHENTICATED); + } + + expect(harness.instance.sendCommand).not.toHaveBeenCalled(); + expect(harness.instance.power).not.toHaveBeenCalled(); + expect(harness.instance.consoleSnapshot).not.toHaveBeenCalled(); + }); + + it('answers a malformed message without dropping the connection', async () => { + const client = await authenticated(await mintToken()); + + client.sendText('{not json'); + expect((await client.waitForEvent('error')).code).toBe(WS_ERROR_CODES.INVALID_MESSAGE); + + client.send({ event: 'send_command' }); + expect((await client.waitForEvent('error')).code).toBe(WS_ERROR_CODES.INVALID_MESSAGE); + + client.send({ event: 'become_admin' }); + expect((await client.waitForEvent('error')).code).toBe(WS_ERROR_CODES.INVALID_MESSAGE); + + // Still alive and still obeying: the parser must not be a way to knock + // other people's consoles over, nor a way to skip the switch. + client.send({ event: 'send_command', command: 'say alive' }); + await client.settle(); + expect(harness.instance.sendCommand).toHaveBeenCalledWith('say alive'); + expect(client.closedWith).toBeNull(); + }); + + it('costs a fresh connection per signature attempt', async () => { + // A refused token closes the socket, so guessing a 64-byte HMAC secret + // costs a TCP and WebSocket handshake per guess rather than a frame. + const client = await openSocket(); + client.send({ event: 'auth', token: await mintToken({ key: Buffer.from(WRONG_SECRET) }) }); + await client.waitForClose(); + + client.send({ event: 'auth', token: await mintToken() }); + await client.settle(200); + + expect(client.all().some((frame) => frame.event === 'auth_success')).toBe(false); + }); + + it('closes a connection that never authenticates, and only that one', async () => { + // Ten seconds of real time, on purpose: the timeout is the only thing + // bounding how many half-open sockets an unauthenticated stranger can + // park on a daemon, and a fake clock would not prove the timer is + // actually armed on the live socket. The second connection is here to + // show the timer is cleared on success rather than merely late. + const silent = await openSocket(); + const busy = await authenticated(await mintToken()); + + const error = await silent.waitForEvent('error', 13_000); + expect(error.code).toBe(WS_ERROR_CODES.UNAUTHENTICATED); + expect(await silent.waitForClose()).toMatchObject({ code: 1008 }); + + expect(busy.closedWith).toBeNull(); + busy.send({ event: 'send_command', command: 'say survived' }); + await busy.settle(); + expect(harness.instance.sendCommand).toHaveBeenCalledWith('say survived'); + }, 20_000); + + it('stops a console at sixty commands a minute', async () => { + const client = await authenticated(await mintToken()); + + for (let index = 0; index < 61; index += 1) { + client.send({ event: 'send_command', command: `say ${index}` }); + } + + const error = await client.waitForEvent('error'); + expect(error.code).toBe(WS_ERROR_CODES.RATE_LIMITED); + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(60); + }); + + it('counts that quota per user and per server, so a second socket adds nothing', async () => { + // The counter used to live on the session object, which made it + // arithmetic rather than a limit: one token, two sockets, 120 commands a + // minute, and nothing anywhere caps the sockets. The quota now hangs off + // the identity the panel signed — this user, on this server — which the + // holder of a token cannot multiply by opening connections. + const token = await mintToken(); + const first = await authenticated(token); + const second = await authenticated(token); + + for (const client of [first, second]) { + for (let index = 0; index < 61; index += 1) { + client.send({ event: 'send_command', command: `say ${index}` }); + } + } + + await first.waitFor( + (frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED, + 'the first socket to be cut off', + ); + await second.waitFor( + (frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED, + 'the second socket to be cut off', + ); + await fence(first); + await fence(second); + + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(60); + }); + + it('does not let one subuser spend the quota of another', async () => { + // The tempting key is the server, since the server is what the quota + // protects. It would also mean a subuser with console access could burn + // sixty commands and leave the owner unable to type `stop` — a denial of + // service handed out with the console permission. + const mine = await authenticated(await mintToken()); + const theirs = await authenticated( + await mintToken({ claims: { sub: '66666666-6666-4666-8666-666666666666' } }), + ); + + for (let index = 0; index < 61; index += 1) { + mine.send({ event: 'send_command', command: `say ${index}` }); + } + + await mine.waitFor( + (frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED, + 'my own quota running out', + ); + + theirs.send({ event: 'send_command', command: 'stop' }); + await fence(theirs); + + expect(harness.instance.sendCommand).toHaveBeenCalledWith('stop'); + }); + + it('frees a used slot a minute after that use, not a minute after the first', async () => { + // The window the comment called sliding was tumbling: it reset wholesale + // once a minute had elapsed since it began, so sixty commands timed just + // before that instant and sixty just after went through in a fifth of a + // second — twice the advertised limit, exactly when someone is trying. + // + // The arithmetic below is written to tell the two apart precisely. One + // command, then fifty-nine a moment before the minute is up, spends the + // whole allowance. Two hundred milliseconds later exactly **one** slot + // has come free — the one used at the start, and nothing else — so of + // five further commands exactly one may pass. Any window that starts + // over instead, whether it counts from the session or from the first + // command, lets all five through. + // + // Only `Date` is faked: the sockets, the daemon's timers and this test's + // own waits stay on real time, so what is measured is the quota's + // arithmetic and nothing else. + const client = await authenticated(await mintToken()); + await fence(client); + + vi.useFakeTimers({ toFake: ['Date'] }); + + client.send({ event: 'send_command', command: 'say first' }); + await fence(client); + + vi.setSystemTime(Date.now() + 59_900); + + for (let index = 0; index < 59; index += 1) { + client.send({ event: 'send_command', command: `say ${index}` }); + } + + await fence(client); + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(60); + + vi.setSystemTime(Date.now() + 200); + + for (let index = 0; index < 5; index += 1) { + client.send({ event: 'send_command', command: `say over the boundary ${index}` }); + } + + await fence(client); + + expect(harness.instance.sendCommand).toHaveBeenCalledTimes(61); + expect(client.all().some((frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED)).toBe(true); + }); + + it('stops power actions well before it stops commands', async () => { + // A start/kill loop is far more expensive than console chatter — every + // iteration is container work on a daemon running as root — and this + // path consulted no quota whatsoever: all two hundred reached + // `ServerInstance.power`. Its own allowance, and a much smaller one, + // rather than a share of the console's: an operator must never find + // themselves unable to stop a server because a console was busy. + const client = await authenticated(await mintToken()); + + for (let index = 0; index < 200; index += 1) { + client.send({ event: 'set_state', action: index % 2 === 0 ? 'start' : 'kill' }); + } + + await fence(client); + + expect(harness.instance.power).toHaveBeenCalledTimes(10); + expect(client.all().some((frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED)).toBe(true); + + // The console still works: the two allowances are separate. + client.send({ event: 'send_command', command: 'say still here' }); + await fence(client); + expect(harness.instance.sendCommand).toHaveBeenCalledWith('say still here'); + }); + + it('rations console replays, which are the one amplifying request', async () => { + // One thirty-byte frame asks the daemon to serialise the whole buffer — + // up to 500 separate messages. Asked in a loop by a client that never + // reads its socket, the replies pile up in the daemon's send buffer: a + // cheap amplifier aimed at the memory of the process that owns every + // container on the host. + const before = harness.instance.consoleSnapshot.mock.calls.length; + const client = await authenticated(await mintToken()); + + // Counted from before the connection, deliberately: authenticating spends + // one of the six. Leaving the connect path free would have priced the + // whole quota at nothing, since fifty replays would then cost fifty + // sockets rather than one. + for (let index = 0; index < 50; index += 1) { + client.send({ event: 'request_logs' }); + } + + await fence(client); + + expect(harness.instance.consoleSnapshot.mock.calls.length - before).toBe(6); + expect(client.all().some((frame) => frame.code === WS_ERROR_CODES.RATE_LIMITED)).toBe(true); + }); + + it('spends a replay on the snapshot it sends at authentication', async () => { + // The bypass this closes: the connect path called `sendConsoleSnapshot` + // directly, so the allowance added against `request_logs` was avoidable + // by opening another socket instead of asking again. + const before = harness.instance.consoleSnapshot.mock.calls.length; + + for (let index = 0; index < 8; index += 1) { + await authenticated(await mintToken()); + } + + expect(harness.instance.consoleSnapshot.mock.calls.length - before).toBe(6); + }); + + it('hangs up on a client that has stopped reading its socket', async () => { + // The other half of the amplification: the quota bounds how often a + // replay may be asked for, not what becomes of it. A client that asks + // and then stops reading leaves the answer queued inside the daemon, and + // a queue nobody drains is just a slower way of spending its memory. + // + // A buffer of maximum-length lines is the worst case the daemon can + // legitimately be asked to write, so the ceiling has to sit above one of + // them and below a pile. + harness.instance.consoleSnapshot.mockReturnValue( + Array.from({ length: CONSOLE_BUFFER_LINES }, (_, index) => `${index} ${'x'.repeat(8_000)}`), + ); + + const client = await authenticated(await mintToken()); + client.pause(); + + for (let index = 0; index < 4; index += 1) { + client.send({ event: 'request_logs' }); + } + + await client.settle(500); + client.resume(); + + expect(await client.waitForClose(10_000)).toMatchObject({ code: 1013 }); + }, 20_000); + + it('executes nothing more once it has told the client the token expired', async () => { + // Expiry used to be enforced by closing the socket and by nothing else: + // the session stayed marked authenticated, and `ws` keeps delivering + // frames until the peer answers the close frame or the 30-second close + // timer fires. A client that simply does not answer — this one — went on + // driving a root-privileged daemon with a token already declared dead, + // blind but perfectly effective, since a console command needs no reply + // to do its work. + const client = await authenticated(await mintToken({ ttlSeconds: 1 })); + client.politeClose = false; + + await client.waitForEvent('token_expired', 4_000); + harness.instance.sendCommand.mockClear(); + harness.instance.power.mockClear(); + + // Well clear of the tick the close frame went out on, so this is the + // handshake window and not a race with the timer. + await client.settle(400); + + client.send({ event: 'send_command', command: 'op attacker' }); + client.send({ event: 'set_state', action: 'kill' }); + await client.settle(200); + + expect(harness.instance.sendCommand).not.toHaveBeenCalled(); + expect(harness.instance.power).not.toHaveBeenCalled(); + }); + + it('executes nothing more once it has refused a renewal token', async () => { + // The same handshake window, reached by the other door: a session + // already authorised presents a forged renewal, and the socket takes as + // long to shut as the peer cares to take. The refusal has to be of the + // session, not merely of the token. + const client = await authenticated(await mintToken()); + client.politeClose = false; + + client.send({ + event: 'auth', + token: await mintToken({ key: Buffer.from(WRONG_SECRET, 'utf8') }), + }); + + await client.waitForEvent('error'); + harness.instance.sendCommand.mockClear(); + await client.settle(200); + + client.send({ event: 'send_command', command: 'op attacker' }); + await client.settle(200); + + expect(harness.instance.sendCommand).not.toHaveBeenCalled(); + }); + }); + + // ------------------------------------------------------------------------- + + describe('the origin check', () => { + it('lets the panel through', async () => { + const client = await openSocket({ origin: PANEL_ORIGIN }); + client.send({ event: 'auth', token: await mintToken() }); + + expect((await client.waitForEvent('auth_success')).event).toBe('auth_success'); + }); + + it('refuses another site before a token is even offered', async () => { + // Browsers do not apply the same-origin policy to WebSockets. Without + // this check any page a signed-in user visited could open a console on + // to their servers using the session they already have. + const client = await openSocket({ origin: EVIL_ORIGIN }); + const closed = await client.waitForClose(); + + expect(closed.code).toBe(1008); + expect(closed.reason).toBe('Origin not allowed.'); + }); + + it('refuses another site even holding a perfectly valid token', async () => { + const client = await openSocket({ origin: EVIL_ORIGIN }); + client.send({ event: 'auth', token: await mintToken() }); + await client.settle(200); + + expect(client.all().some((frame) => frame.event === 'auth_success')).toBe(false); + expect(harness.instance.listenerCount('console')).toBe(0); + }); + + it.each([ + ['a trailing slash', `${PANEL_ORIGIN}/`], + ['a different scheme', 'http://panel.example.com'], + ['a different case', 'https://PANEL.example.com'], + ['a subdomain', 'https://evil.panel.example.com'], + ['a prefix match', 'https://panel.example.com.evil.test'], + ['the null origin a sandboxed frame sends', 'null'], + ])('refuses %s', async (_label, origin) => { + const client = await openSocket({ origin }); + expect(await client.waitForClose()).toMatchObject({ reason: 'Origin not allowed.' }); + }); + + it('lets a connection with no Origin header through', async () => { + // Deliberate, and worth knowing: the check is `origin && !allowed`, so + // anything that omits the header skips it. Browsers always send one, so + // this is not a way back in from a web page — but it does mean the + // origin check protects only browsers, and a script with a stolen token + // is unaffected by it. + const client = await AttackerSocket.open(harness.port, `/api/servers/${SERVER_UUID}/ws`, {}); + opened.push(client); + + client.send({ event: 'auth', token: await mintToken() }); + expect((await client.waitForEvent('auth_success')).event).toBe('auth_success'); + }); + + it('blocks every browser when no origin is configured', async () => { + const empty = await startHarness({ allowedOrigins: [] }); + + try { + const client = await AttackerSocket.open(empty.port, `/api/servers/${SERVER_UUID}/ws`, { + origin: PANEL_ORIGIN, + }); + + expect(await client.waitForClose()).toMatchObject({ reason: 'Origin not allowed.' }); + client.destroy(); + } finally { + await empty.app.close(); + } + }); + }); +}); diff --git a/apps/daemon/src/websocket/console-gateway.ts b/apps/daemon/src/websocket/console-gateway.ts index 8c156dc..004aa1f 100644 --- a/apps/daemon/src/websocket/console-gateway.ts +++ b/apps/daemon/src/websocket/console-gateway.ts @@ -26,9 +26,70 @@ import type { ServerManager } from '../server/server-manager.js'; */ const AUTH_TIMEOUT_MS = 10_000; -/** Commands per minute, per connection. */ -const COMMAND_RATE_LIMIT = 60; -const COMMAND_RATE_WINDOW_MS = 60_000; +/** Window every quota below is measured over. */ +const QUOTA_WINDOW_MS = 60_000; + +/** + * What one user may ask of one server, per minute, whatever the shape of their + * client. + * + * Three separate allowances rather than one, because the three requests cost + * the daemon wildly different amounts and share nothing but the socket they + * arrive on. Keeping them apart also means a busy console can never leave an + * operator unable to stop their server. + */ +const QUOTAS = { + /** Console commands: one line handed to the server, over stdin or RCON. */ + command: 60, + + /** + * Power actions. Far below the command quota on purpose: each one creates, + * stops or destroys a container, which is the most expensive thing an + * authorised session can ask for and the cheapest way to hurt a host. A + * start/kill loop at console speed was, until this quota existed, free. + */ + power: 10, + + /** + * Console replays. A `request_logs` frame is a couple of dozen bytes and is + * answered with up to `CONSOLE_BUFFER_LINES` separate JSON messages: the one + * request in this protocol whose cost to the daemon bears no relation to its + * cost to the sender. + * + * The snapshot sent on authentication is charged here too, which is why six + * and not one: Hopper's own client spends one on every connection, and a + * browser reconnecting across a flaky link may spend several in a minute + * without anybody doing anything wrong. Charging the connect path is the + * point of the bucket rather than an afterthought — left free, ten replays + * cost ten sockets, which is the multiplication a per-user quota exists to + * stop. + */ + replay: 6, + + /** + * `request_stats` has no quota, and that is a decision rather than an + * omission. It is answered from a sample the session already holds — one + * `send` of a fixed, small shape, with no reach into the server, Docker or + * the buffer — so its cost to the daemon is the cost of the frame that asked + * for it. The send-buffer ceiling below is what bounds a client that asks in + * a loop and never reads the answers, and it bounds every message equally. + */ +} as const; + +type QuotaBucket = keyof typeof QUOTAS; + +/** + * Outbound bytes the daemon will hold for one client before hanging up. + * + * Above anything legitimate and below anything ruinous. The largest thing the + * daemon writes in one go is a buffer replay, and the buffer is bounded on both + * axes — `CONSOLE_BUFFER_LINES` lines of at most `MAX_LINE_LENGTH` each, so + * roughly four megabytes at its absolute worst and a few tens of kilobytes in + * life. Twice that leaves a slow connection room to receive one while the + * server is talking, and still refuses to grow a queue without end in the + * memory of the process that owns every container on this host. + */ +const MAX_BUFFERED_BYTES = 8 * 1024 * 1024; /** Permission required for each power action. */ const POWER_PERMISSIONS: Record = { @@ -40,6 +101,78 @@ const POWER_PERMISSIONS: Record = { kill: PERMISSIONS.CONTROL_STOP, }; +/** + * Sliding-window quotas, shared by every console session on this daemon. + * + * The counters this replaces lived on the session object, one per socket, which + * made them arithmetic rather than a limit: the same token opened as many + * sockets as it liked and every one of them arrived with a fresh allowance. A + * quota is worth exactly its key, so these hang off the identity the panel + * signed and the holder cannot multiply — the user (`sub`) on one server. + * + * Not the token: the panel mints a fresh one, with a fresh `jti`, whenever the + * browser asks, and renewal is an ordinary part of the protocol. Not the server + * on its own either, tempting as that is when the server is the thing being + * protected — a single allowance shared by everyone with access would let one + * subuser spend it and leave the owner unable to type `stop`. + */ +class ConsoleQuotas { + /** Timestamps of the uses still inside the window, per bucket and identity. */ + private readonly uses = new Map(); + private sweptAt = Date.now(); + + /** + * Records one use and says whether it fell within the bucket's allowance. + * + * Sliding, and the word matters: the counter this replaces reset itself + * wholesale once a window had elapsed since it started, so sixty commands + * timed just before that boundary and sixty just after went through in a + * fraction of a second — twice the advertised limit, precisely when someone + * is trying. + */ + consume(bucket: QuotaBucket, identity: string): boolean { + const now = Date.now(); + this.sweep(now); + + const key = `${bucket}:${identity}`; + const recent = (this.uses.get(key) ?? []).filter((at) => at > now - QUOTA_WINDOW_MS); + + // A refused use is not recorded. A client that keeps knocking while it is + // over the limit recovers as its earlier uses age out, instead of + // extending its own lockout by asking. + if (recent.length >= QUOTAS[bucket]) { + this.uses.set(key, recent); + return false; + } + + recent.push(now); + this.uses.set(key, recent); + return true; + } + + /** + * Drops keys with nothing left inside the window, at most once per window. + * + * Users and servers come and go, and their keys would otherwise accumulate + * for the life of the process. Sweeping on every call would mean walking the + * whole map sixty times a minute per console, for entries that cost a handful + * of numbers each. + */ + private sweep(now: number): void { + if (now - this.sweptAt < QUOTA_WINDOW_MS) { + return; + } + + this.sweptAt = now; + + for (const [key, uses] of this.uses) { + if (uses.every((at) => at <= now - QUOTA_WINDOW_MS)) { + this.uses.delete(key); + } + } + } +} + /** * WebSocket gateway for the console. * @@ -49,7 +182,7 @@ const POWER_PERMISSIONS: Record = { * network call. That is what lets fifty open consoles cost the panel nothing. * * The accepted consequence: a permission revoked in the panel only takes effect - * when the token is renewed, hence its ten-minute lifetime. + * when the token is renewed, hence its deliberately short lifetime. */ export function registerConsoleGateway( app: FastifyInstance, @@ -57,6 +190,10 @@ export function registerConsoleGateway( config: DaemonConfig, logger: Logger, ): void { + // One registry for the whole daemon, deliberately outside the session: a + // quota an attacker can reset by opening a second socket is not a quota. + const quotas = new ConsoleQuotas(); + app.get('/api/servers/:uuid/ws', { websocket: true }, (socket, request) => { const { uuid } = request.params as { uuid: string }; @@ -71,7 +208,7 @@ export function registerConsoleGateway( return; } - new ConsoleSession(socket, uuid, manager, config, logger).start(); + new ConsoleSession(socket, uuid, manager, config, logger, quotas).start(); }); } @@ -80,12 +217,24 @@ class ConsoleSession { private authenticated = false; private server: ServerInstance | null = null; + /** + * Identity the quotas are counted against: the user this token names, on this + * server. Null exactly when the session holds no authority. + */ + private quotaIdentity: string | null = null; + private authTimer: NodeJS.Timeout | null = null; private expiryTimer: NodeJS.Timeout | null = null; private renewTimer: NodeJS.Timeout | null = null; - private commandCount = 0; - private commandWindowStart = Date.now(); + /** + * Most recent resource sample seen on this connection, so `request_stats` can + * answer with figures rather than with nothing. + */ + private lastUsage: ResourceUsage | null = null; + + /** Whether the client has already been told its console output is withheld. */ + private hiddenConsoleAnnounced = false; private detachers: (() => void)[] = []; @@ -95,6 +244,7 @@ class ConsoleSession { private readonly manager: ServerManager, private readonly config: DaemonConfig, private readonly logger: Logger, + private readonly quotas: ConsoleQuotas, ) {} start(): void { @@ -104,7 +254,7 @@ class ConsoleSession { code: WS_ERROR_CODES.UNAUTHENTICATED, message: 'No authentication supplied.', }); - this.socket.close(1008, 'No authentication supplied.'); + this.hangUp(1008, 'No authentication supplied.'); }, AUTH_TIMEOUT_MS); this.socket.on('message', (raw: Buffer) => { @@ -116,9 +266,24 @@ class ConsoleSession { } private send(message: ServerMessage): void { - if (this.socket.readyState === this.socket.OPEN) { - this.socket.send(JSON.stringify(message)); + if (this.socket.readyState !== this.socket.OPEN) { + return; + } + + // A client that stops reading does not stop the daemon writing: `ws` queues + // whatever will not fit down the socket, in the memory of the process that + // owns every container on this host. A console this far behind is being + // read by nobody, so it is hung up rather than buffered indefinitely. + if (this.socket.bufferedAmount > MAX_BUFFERED_BYTES) { + this.logger.warn( + { server: this.serverUuid, bufferedBytes: this.socket.bufferedAmount }, + 'Console closed: the client is not reading its socket', + ); + this.hangUp(1013, 'Client not reading.'); + return; } + + this.socket.send(JSON.stringify(message)); } private async handleMessage(raw: Buffer): Promise { @@ -152,7 +317,9 @@ class ConsoleSession { } // Any message other than `auth` before authentication is ignored: without - // that, a client could send commands and authenticate afterwards. + // that, a client could send commands and authenticate afterwards. The same + // gate catches a session whose token has expired under it, which is why + // expiry clears the authority rather than merely closing the socket. if (!this.authenticated || !this.server) { this.send({ event: 'error', @@ -170,10 +337,10 @@ class ConsoleSession { await this.handlePower(message.data.action); break; case 'request_logs': - this.sendConsoleSnapshot(); + this.handleLogsRequest(); break; case 'request_stats': - this.send({ event: 'status', state: this.server.currentState }); + this.handleStatsRequest(); break; } } @@ -207,7 +374,7 @@ class ConsoleSession { code: WS_ERROR_CODES.INTERNAL, message: 'Server unknown to this node.', }); - this.socket.close(1011, 'Unknown server.'); + this.hangUp(1011, 'Unknown server.'); return; } @@ -224,6 +391,7 @@ class ConsoleSession { this.authenticated = true; this.permissions = claims.data.permissions; this.server = server; + this.quotaIdentity = `${this.serverUuid}:${claims.data.sub}`; this.scheduleTokenTimers(claims.data.exp); @@ -237,6 +405,8 @@ class ConsoleSession { expiresAt: claims.data.exp * 1000, }); + this.announceHiddenConsole(); + if (!renewal) { // Replaying the state and the console buffer only makes sense on the // first authentication: sending them again would show the client a @@ -250,7 +420,20 @@ class ConsoleSession { this.send({ event: 'stats', usage: server.idleUsage }); } - this.sendConsoleSnapshot(); + // Charged against the same allowance as an explicit `request_logs`, + // and for the reason the allowance exists at all: a replay is up to + // five hundred messages bought with one small frame. Leaving the + // connect path free would have priced the quota at nothing — a client + // that wants ten replays opens ten sockets, which is the multiplication + // the per-user bucket was introduced to stop. + // + // A refusal here is silent, unlike `handleLogsRequest`: nobody asked + // for this snapshot, and a client whose console opens without history + // has already been told why by the permission notice or, failing that, + // will get it on the next `request_logs`. + if (this.consumeQuota('replay')) { + this.sendConsoleSnapshot(); + } } } catch (error: unknown) { this.logger.debug({ server: this.serverUuid, err: error }, 'Console token refused'); @@ -259,10 +442,39 @@ class ConsoleSession { code: WS_ERROR_CODES.INVALID_TOKEN, message: 'Invalid or expired token.', }); - this.socket.close(1008, 'Invalid token.'); + // A refused **renewal** arrives on a session that is already authorised, + // and the socket takes a handshake to shut: without this the token just + // rejected would go on driving the server for as long as the peer + // declined to answer the close frame. + this.hangUp(1008, 'Invalid token.'); } } + /** + * Tells the client its console output is being withheld. + * + * Without it the withholding is indistinguishable from a silent server: the + * page shows an empty terminal and no reason for it, and the first person to + * debug that goes looking at the game server. Said once per session, and + * again if a renewal takes the permission away mid-console. + */ + private announceHiddenConsole(): void { + if (this.has(PERMISSIONS.CONTROL_CONSOLE)) { + this.hiddenConsoleAnnounced = false; + return; + } + + if (this.hiddenConsoleAnnounced) { + return; + } + + this.hiddenConsoleAnnounced = true; + this.send({ + event: 'daemon_message', + message: `Console output is hidden: this session does not hold ${PERMISSIONS.CONTROL_CONSOLE}.`, + }); + } + /** * The token expires while the console is open. The client is warned ahead of * time so it can ask the panel for a new one, with no visible break. @@ -280,18 +492,61 @@ class ConsoleSession { this.expiryTimer = setTimeout( () => { this.send({ event: 'token_expired' }); - this.socket.close(1008, 'Token expired.'); + this.logger.debug({ server: this.serverUuid }, 'Console session ended: token expired'); + this.hangUp(1008, 'Token expired.'); }, Math.max(0, remainingMs), ); } + /** + * Subscribes the session to its server's events. + * + * What arrives here divides in two, and the division is the whole point. + * State changes, resource samples and the start and finish of an installation + * say **that** something happened: a subuser holding only the implicit + * `websocket.connect` is meant to watch their server go up and down, and the + * panel's header is built from exactly these. Console and installation output + * say **what was said** — an operator pasting an RCON password, a plugin + * printing its API key, an `op` that names the next administrator — and that + * is the same thing the buffer replay has always held back. Streaming it live + * to anyone who managed to authenticate, as this did, meant the replay gate + * withheld five hundred lines of history and nothing at all of the present — + * and the present is where the operator is typing. + * + * The permission is read when each line arrives rather than when the listener + * is attached: renewal re-authenticates over the same connection, so a live + * session's permissions change underneath these handlers, and the point of + * the short token lifetime is that the change bites. + */ private attachToServer(server: ServerInstance): void { - const onState = (state: ServerState): void => this.send({ event: 'status', state }); - const onConsole = (line: string): void => this.send({ event: 'console_output', line }); - const onStats = (usage: ResourceUsage): void => this.send({ event: 'stats', usage }); + const onState = (state: ServerState): void => { + // The last sample described a state the server has now left. Answering + // `request_stats` from it would report the memory of a server that has + // since stopped. + this.lastUsage = null; + this.send({ event: 'status', state }); + }; + + const onConsole = (line: string): void => { + if (this.has(PERMISSIONS.CONTROL_CONSOLE)) { + this.send({ event: 'console_output', line }); + } + }; + + const onStats = (usage: ResourceUsage): void => { + this.lastUsage = usage; + this.send({ event: 'stats', usage }); + }; + const onInstallStarted = (): void => this.send({ event: 'install_started' }); - const onInstallOutput = (line: string): void => this.send({ event: 'install_output', line }); + + const onInstallOutput = (line: string): void => { + if (this.has(PERMISSIONS.CONTROL_CONSOLE)) { + this.send({ event: 'install_output', line }); + } + }; + const onInstallCompleted = (successful: boolean): void => this.send({ event: 'install_completed', successful }); @@ -312,6 +567,15 @@ class ConsoleSession { }); } + /** + * Replays the console buffer, silently doing nothing when the session may not + * read it. + * + * Silent because this also runs on the connect path, where a session that + * cannot see the console has already been told so by `announceHiddenConsole` + * and does not need the same news as an error. A client that *asked* for the + * replay is answered explicitly — see `handleLogsRequest`. + */ private sendConsoleSnapshot(): void { if (!this.server || !this.has(PERMISSIONS.CONTROL_CONSOLE)) { return; @@ -334,18 +598,29 @@ class ConsoleSession { }); } + private rateLimited(message: string): void { + this.send({ event: 'error', code: WS_ERROR_CODES.RATE_LIMITED, message }); + } + + /** + * Spends one unit of a quota. + * + * A session with no identity has no authority either — every caller sits + * behind the authentication gate — so refusing is the honest reading of a + * state that should not arise. + */ + private consumeQuota(bucket: QuotaBucket): boolean { + return this.quotaIdentity !== null && this.quotas.consume(bucket, this.quotaIdentity); + } + private async handleCommand(command: string): Promise { if (!this.has(PERMISSIONS.CONTROL_CONSOLE)) { this.deny(PERMISSIONS.CONTROL_CONSOLE); return; } - if (!this.consumeCommandQuota()) { - this.send({ - event: 'error', - code: WS_ERROR_CODES.RATE_LIMITED, - message: 'Too many commands sent. Wait a moment.', - }); + if (!this.consumeQuota('command')) { + this.rateLimited('Too many commands sent. Wait a moment.'); return; } @@ -368,6 +643,11 @@ class ConsoleSession { return; } + if (!this.consumeQuota('power')) { + this.rateLimited('Too many power actions. Wait a moment.'); + return; + } + try { await this.server!.power(action); } catch (error: unknown) { @@ -379,21 +659,50 @@ class ConsoleSession { } } - /** - * An open console must not be used to drown a server in commands. - * The sliding window is local to the connection: that is enough here, since - * the token itself is issued by the panel, which already limits its issuance. - */ - private consumeCommandQuota(): boolean { - const now = Date.now(); + private handleLogsRequest(): void { + if (!this.has(PERMISSIONS.CONTROL_CONSOLE)) { + this.deny(PERMISSIONS.CONTROL_CONSOLE); + return; + } - if (now - this.commandWindowStart > COMMAND_RATE_WINDOW_MS) { - this.commandWindowStart = now; - this.commandCount = 0; + if (!this.consumeQuota('replay')) { + this.rateLimited('Too many console replays requested. Wait a moment.'); + return; } - this.commandCount += 1; - return this.commandCount <= COMMAND_RATE_LIMIT; + this.sendConsoleSnapshot(); + } + + private handleStatsRequest(): void { + // Answering a request for statistics with a `status` event was simply the + // wrong reply: the client asked what the server is consuming and was told + // what state it is in. `ResourceUsage` carries the state as well, so the + // right event says everything the wrong one did. + // + // The last live sample, or the idle one when none has arrived — a stopped + // server never emits, and its disk is occupied all the same. + this.send({ event: 'stats', usage: this.lastUsage ?? this.server!.idleUsage }); + } + + /** + * Ends the session: authority first, socket second. + * + * Closing a WebSocket is a handshake, not a switch. `ws` goes on delivering + * frames until the peer answers the close frame or the thirty-second close + * timer fires, and nothing obliges a peer to answer — a client that simply + * stays quiet used to keep driving a root-privileged daemon on the strength + * of a token already declared dead. Every close this session decides on goes + * through here, so the authority is gone before the frame leaves and the + * frame is only a courtesy. (The origin check closes its socket directly: + * there is no session yet, and nothing to take away.) + */ + private hangUp(code: number, reason: string): void { + this.authenticated = false; + this.permissions = []; + this.server = null; + this.quotaIdentity = null; + this.clearTokenTimers(); + this.socket.close(code, reason); } private clearAuthTimer(): void { diff --git a/apps/panel/src/modules/auth/guards/jwt-auth.guard.ts b/apps/panel/src/modules/auth/guards/jwt-auth.guard.ts index 73157ca..85b0eec 100644 --- a/apps/panel/src/modules/auth/guards/jwt-auth.guard.ts +++ b/apps/panel/src/modules/auth/guards/jwt-auth.guard.ts @@ -95,6 +95,7 @@ export class JwtAuthGuard implements CanActivate { // reconnecte. role: session.user.role, sessionId: payload.sid, + authenticatedBy: 'session', }; const requiredRole = this.reflector.getAllAndOverride(REQUIRED_ROLE_KEY, [ @@ -146,6 +147,10 @@ export class JwtAuthGuard implements CanActivate { // A key opens no session: there is nothing to revoke on the session // side, and the identifier tells the origin apart in the logs. sessionId: `api-key:${key.id}`, + // Read by the console route, which refuses to mint a daemon credential + // for a key: a key's scope is decided from the HTTP verb, and the console + // is handed out by a GET. + authenticatedBy: 'api-key', }; const requiredRole = this.reflector.getAllAndOverride(REQUIRED_ROLE_KEY, [ diff --git a/apps/panel/src/modules/auth/request-user.ts b/apps/panel/src/modules/auth/request-user.ts index f8d0a60..d620bef 100644 --- a/apps/panel/src/modules/auth/request-user.ts +++ b/apps/panel/src/modules/auth/request-user.ts @@ -10,6 +10,20 @@ export interface RequestUser { email: string; role: 'ADMIN' | 'USER'; sessionId: string; + /** + * Which credential authenticated this request. + * + * Stated as a field of its own rather than read off the shape of + * `sessionId`, which happens to be prefixed `api-key:` for one of the two. + * A route that has to refuse API keys — the console does — must rest on + * something a reader can find and a compiler can check, not on a string + * whose format reads like a logging detail and could be changed by someone + * improving the logs. + * + * Being required rather than optional is the point: a third way of + * authenticating cannot be added without deciding what it is worth here. + */ + authenticatedBy: 'session' | 'api-key'; } /** diff --git a/apps/panel/src/modules/auth/token.service.spec.ts b/apps/panel/src/modules/auth/token.service.spec.ts new file mode 100644 index 0000000..a6ea650 --- /dev/null +++ b/apps/panel/src/modules/auth/token.service.spec.ts @@ -0,0 +1,1292 @@ +import { + ALL_PERMISSIONS, + CONSOLE_TOKEN_RENEW_MARGIN_SECONDS, + CONSOLE_TOKEN_TTL_SECONDS, + PERMISSIONS, + SIGNED_URL_TTL_SECONDS, + type Permission, +} from '@hopper/shared'; +import { ForbiddenException, Logger } from '@nestjs/common'; +import type { ConfigService } from '@nestjs/config'; +import { SignJWT, decodeJwt, decodeProtectedHeader } from 'jose'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { CryptoService } from '../../common/crypto/crypto.service.js'; +import type { Environment } from '../../config/environment.js'; +import type { PrismaService } from '../../prisma/prisma.service.js'; +import { scopeAllows } from '../api-keys/api-key.js'; +import type { AuditService } from '../audit/audit.service.js'; +import type { NodesService } from '../nodes/nodes.service.js'; +import { ConsoleController } from '../servers/console.controller.js'; +import { AuthService } from './auth.service.js'; +import type { PasswordService } from './password.service.js'; +import type { RequestServer, RequestUser } from './request-user.js'; +import { + ACCESS_TOKEN_TTL_SECONDS, + REFRESH_TOKEN_TTL_SECONDS, + TokenService, +} from './token.service.js'; +import type { TotpService } from './totp.service.js'; + +/** + * The console JWT is the one token in this codebase that is verified by a + * machine the panel does not control, on a connection the panel never sees. If + * it can be forged, widened or moved between servers, nothing downstream will + * notice: the daemon has no database to check it against. + * + * These tests are written from the attacker's side. Each one names the claim or + * the key that is doing the work, so that removing it breaks a test rather than + * a deployment. + */ + +const APP_URL = 'https://panel.example.test'; + +/** Two nodes, with the shape and length the panel actually generates (64 chars). */ +const NODE_A = { + uuid: 'a1111111-1111-4111-8111-111111111111', + secret: 'A'.repeat(32) + 'a'.repeat(32), +}; +const NODE_B = { + uuid: 'b2222222-2222-4222-8222-222222222222', + secret: 'B'.repeat(32) + 'b'.repeat(32), +}; + +/** Two servers on the *same* node: the escalation the daemon has to refuse. */ +const SERVER_A = '3f2504e0-4f89-41d3-9a0c-0305e82c3301'; +const SERVER_B = '5c9d1a02-1b7e-4c2f-9d8a-77bb1f0e4402'; + +const USER_UUID = '9e107d9d-3721-4a1c-8b3f-2f0a9c8d1e55'; + +function makeCrypto(secret = 'a-test-app-secret-long-enough-1234567890'): CryptoService { + return new CryptoService({ get: () => secret } as unknown as ConfigService); +} + +function makeTokens(crypto: CryptoService = makeCrypto()): TokenService { + const config = { get: () => APP_URL } as unknown as ConfigService; + return new TokenService(crypto, config); +} + +function claimsOf(token: string): Record { + return decodeJwt(token); +} + +/** + * Re-encodes a token's payload while keeping the original signature. This is + * the cheapest forgery there is — no key needed — and the one every JWT + * verifier has to defeat. + */ +function tamper(token: string, mutate: (claims: Record) => void): string { + const [header, body, signature] = token.split('.'); + const claims = JSON.parse(Buffer.from(body ?? '', 'base64url').toString('utf8')) as Record< + string, + unknown + >; + mutate(claims); + const forged = Buffer.from(JSON.stringify(claims), 'utf8').toString('base64url'); + return `${header}.${forged}.${signature}`; +} + +/** The same payload, re-headed as `alg: none` and stripped of its signature. */ +function unsigned(token: string): string { + const header = Buffer.from(JSON.stringify({ alg: 'none' }), 'utf8').toString('base64url'); + const body = Buffer.from(JSON.stringify(claimsOf(token)), 'utf8').toString('base64url'); + return `${header}.${body}.`; +} + +// --------------------------------------------------------------------------- +// Console token — the claims and the lifetime +// --------------------------------------------------------------------------- + +describe('console token: what it carries', () => { + const tokens = makeTokens(); + + async function mint(overrides: Partial[0]> = {}) { + return tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [PERMISSIONS.WEBSOCKET_CONNECT, PERMISSIONS.CONTROL_CONSOLE], + ...overrides, + }); + } + + /** + * The claim set is pinned exactly, not merely checked for what it contains. + * A console token and a signed download URL are signed with the *same* node + * secret for the *same* audience; the only thing keeping one from being read + * as the other is that their payload shapes do not overlap. Zod object + * schemas ignore surplus keys, so a single extra claim added here — a + * `resource` in particular — would silently make every console token a valid + * file-download URL. That is why an addition has to break this test. + */ + it('carries exactly eight claims, and no more', async () => { + expect(Object.keys(claimsOf(await mint())).sort()).toEqual([ + 'aud', + 'exp', + 'iat', + 'iss', + 'jti', + 'permissions', + 'serverUuid', + 'sub', + ]); + }); + + it('names the panel as issuer, the node as audience and the user as subject', async () => { + const claims = claimsOf(await mint()); + + expect(claims.iss).toBe(APP_URL); + expect(claims.aud).toBe(NODE_A.uuid); + expect(claims.sub).toBe(USER_UUID); + expect(claims.serverUuid).toBe(SERVER_A); + }); + + /** + * The figure is two minutes, down from ten, and it is pinned here because it + * is not a tuning knob: it is the entire revocation delay. Nothing the panel + * does reaches a console mid-session — see the session test below — so a + * sign-out or a password change costs the attacker this long and no longer. + * + * It cannot go much lower either, and the floor is the renewal margin: the + * daemon warns the client `CONSOLE_TOKEN_RENEW_MARGIN_SECONDS` before expiry + * and that warning is the only renewal trigger there is. A lifetime at or + * below the margin means no warning, and every console dropping at expiry + * and reconnecting from scratch instead of renewing in place. The assertion + * below is that relationship rather than the bare number. + */ + it('lives for two minutes, comfortably clear of the renewal margin', async () => { + const claims = claimsOf(await mint()); + + expect(CONSOLE_TOKEN_TTL_SECONDS).toBe(120); + expect((claims.exp as number) - (claims.iat as number)).toBe(CONSOLE_TOKEN_TTL_SECONDS); + expect(CONSOLE_TOKEN_TTL_SECONDS).toBeGreaterThanOrEqual( + CONSOLE_TOKEN_RENEW_MARGIN_SECONDS * 2, + ); + // Still far longer than a signed URL, which is a link in an address bar + // rather than a live session. + expect(CONSOLE_TOKEN_TTL_SECONDS).toBeGreaterThan(SIGNED_URL_TTL_SECONDS); + }); + + /** + * `alg` is pinned in the header at signing and in the allow-list at + * verification. The unsigned copy is refused by `jose` itself, which has no + * `none` implementation at all; the allow-list is what refuses everything + * *else*, and HS512 below is the case that proves it is doing work. A + * verifier that takes the algorithm from the header instead is the classic + * route to signature confusion. + */ + it('is signed with HS256, and no other algorithm is accepted', async () => { + const token = await mint(); + + expect(decodeProtectedHeader(token).alg).toBe('HS256'); + expect(await tokens.verifyConsoleToken(unsigned(token), NODE_A.uuid, NODE_A.secret)).toBeNull(); + + const hs512 = await new SignJWT({ serverUuid: SERVER_A, permissions: [...ALL_PERMISSIONS] }) + .setProtectedHeader({ alg: 'HS512' }) + .setSubject(USER_UUID) + .setIssuer(APP_URL) + .setAudience(NODE_A.uuid) + .setJti('hs512') + .setIssuedAt() + .setExpirationTime('600s') + .sign(Buffer.from(NODE_A.secret, 'utf8')); + + expect(await tokens.verifyConsoleToken(hs512, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); + + it('gives every token a distinct jti', async () => { + const [first, second] = await Promise.all([mint(), mint()]); + + expect(claimsOf(first).jti).not.toBe(claimsOf(second).jti); + expect(claimsOf(first).jti).toMatch(/^[0-9a-f-]{36}$/); + }); + + /** + * The `jti` is an identifier, not a revocation handle, and the contract now + * says so where it is declared. Nothing consumes it — no deny-list in the + * panel, no seen-set in the daemon — so the same token authenticates as many + * connections as its bearer likes, for its whole two minutes. This test is + * what keeps the comment and the behaviour together: implementing a deny-list + * would break it, which is the right moment to rewrite the claim. + */ + it('is replayable for its whole lifetime: nothing consumes the jti', async () => { + const token = await mint(); + + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + }); + + /** + * The console token carries no session identifier, unlike the access token, + * which carries `sid` for exactly this reason. A sign-out, a password change + * or a suspension revokes every session — and leaves any console token minted + * since fully live, with `control.console` on it. + * + * The absence is deliberate and cannot be fixed by adding the claim: the + * daemon has no session table to check it against and no way to ask. What + * bounds it is the lifetime above, which is why that one is two minutes, and + * what closes it immediately is re-keying the node. Both are written down in + * `docs/security.md`, under "a console already open", for whoever is reading + * during an incident rather than during a refactor. Pinning the absence here + * means restoring the binding shows up as a deliberate change — and that + * whoever restores it has to go and correct that section. + */ + it('carries nothing that ties it to a session, so no revocation can reach it', async () => { + const claims = claimsOf(await mint()); + + expect(claims.sid).toBeUndefined(); + expect(claims.family).toBeUndefined(); + }); + + describe('expiry', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('still verifies one second before expiry and not one second after', async () => { + const token = await mint(); + + vi.setSystemTime(new Date('2026-01-01T00:01:59Z')); + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + + vi.setSystemTime(new Date('2026-01-01T00:02:01Z')); + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); + + /** Extending `exp` by hand is the first thing anyone tries. */ + it('refuses a token whose exp was pushed out by re-encoding the payload', async () => { + const token = await mint(); + const forged = tamper(token, (claims) => { + claims.exp = (claims.exp as number) + 86_400; + }); + + vi.setSystemTime(new Date('2026-01-01T00:05:00Z')); + expect(await tokens.verifyConsoleToken(forged, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Console token — bound to one server +// --------------------------------------------------------------------------- + +describe('console token: binding to one server', () => { + const tokens = makeTokens(); + + function mintFor(serverUuid: string, permissions: Permission[] = [...ALL_PERMISSIONS]) { + return tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid, + permissions, + }); + } + + /** + * A user legitimately holding a token for server A points it at server B, + * which lives on the same node and is therefore reached with the same key and + * the same audience. Signature, issuer and audience all pass — cross-server + * isolation on a node rests on one string comparison, not on cryptography. + * + * What this file can assert is its own half: that the token names A and only + * A, so the daemon has something to compare. It deliberately does **not** + * reimplement the comparison — an earlier version of this test defined a + * local `opens()` that re-derived the daemon's `!==` and then asserted on + * that, which passes whatever the daemon does and would go on passing if the + * check there were deleted. The comparison itself belongs to + * `console-gateway.spec.ts`, which exercises the real gateway. + */ + it('names exactly one server, leaving the daemon a claim to compare', async () => { + const payload = await tokens.verifyConsoleToken( + await mintFor(SERVER_A), + NODE_A.uuid, + NODE_A.secret, + ); + + // Verified, on the node both servers live on: nothing cryptographic here + // separates A from B. + expect(payload).not.toBeNull(); + expect(payload?.serverUuid).toBe(SERVER_A); + expect(payload?.serverUuid).not.toBe(SERVER_B); + // And the claim is present, which is what the daemon's check depends on: + // an absent one would parse as `undefined` and match nothing — or, worse, + // be read as "any server" by a laxer schema. + expect(Object.keys(claimsOf(await mintFor(SERVER_A)))).toContain('serverUuid'); + }); + + /** + * `verifyConsoleToken` takes a node uuid and a node secret — and no server. + * It therefore cannot enforce the binding above, and any future caller that + * uses it to gate a per-server action without comparing `serverUuid` itself + * has an authorisation bypass with no visible mistake in the call site. + */ + it('verifies with no server in scope at all, leaving the comparison to the caller', async () => { + const token = await mintFor(SERVER_B); + + expect(tokens.verifyConsoleToken.length).toBe(3); + expect((await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret))?.serverUuid).toBe( + SERVER_B, + ); + }); + + /** Swapping the target server means re-signing, which the bearer cannot do. */ + it('refuses a token whose serverUuid was rewritten', async () => { + const forged = tamper(await mintFor(SERVER_A), (claims) => { + claims.serverUuid = SERVER_B; + }); + + expect(await tokens.verifyConsoleToken(forged, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); + + /** + * `z.uuid()` accepts either case — the check added at signing time is a + * format check, not a normalisation — while the daemon compares with `!==`, + * which does not. So the service will still sign a spelling the daemon + * cannot match. + * + * What stops that reaching anyone is one layer up: `ConsoleController` signs + * `server.uuid`, the value the guard read out of the database, and builds the + * socket URL from the same value rather than from the route parameter. Both + * halves therefore carry the stored spelling whatever the caller typed. This + * test pins the service's laxity so that a future second caller knows it is + * responsible for the same discipline. + */ + it('accepts an upper-case serverUuid that would no longer match the daemon by ===', async () => { + const upper = SERVER_A.toUpperCase(); + const payload = await tokens.verifyConsoleToken( + await mintFor(upper), + NODE_A.uuid, + NODE_A.secret, + ); + + expect(payload?.serverUuid).toBe(upper); + expect(payload?.serverUuid).not.toBe(SERVER_A); + }); +}); + +// --------------------------------------------------------------------------- +// Console token — bound to one node +// --------------------------------------------------------------------------- + +describe('console token: binding to one node', () => { + const tokens = makeTokens(); + + function mintOn(node: { uuid: string; secret: string }) { + return tokens.signConsoleToken({ + nodeUuid: node.uuid, + nodeJwtSecret: node.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [...ALL_PERMISSIONS], + }); + } + + it('is worthless on another node', async () => { + const token = await mintOn(NODE_A); + + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + expect(await tokens.verifyConsoleToken(token, NODE_B.uuid, NODE_B.secret)).toBeNull(); + }); + + /** + * The two barriers are separated deliberately, because "it fails" tells you + * nothing about which one is load-bearing. Here the key is right and only the + * audience differs — the case that would arise if two nodes were ever + * provisioned with the same `jwtSecret`, by a restored backup or a copied + * `daemon.yml`. The `aud` claim has to hold on its own. + */ + it('is refused by audience alone, even when the signing key is shared', async () => { + const token = await mintOn({ uuid: NODE_A.uuid, secret: NODE_A.secret }); + + expect(await tokens.verifyConsoleToken(token, NODE_B.uuid, NODE_A.secret)).toBeNull(); + }); + + /** And the mirror case: right audience, wrong key. The signature has to hold on its own. */ + it('is refused by signature alone, even when the audience matches', async () => { + const token = await mintOn({ uuid: NODE_B.uuid, secret: NODE_A.secret }); + + expect(await tokens.verifyConsoleToken(token, NODE_B.uuid, NODE_B.secret)).toBeNull(); + }); + + /** + * Proving the key is the node's own secret rather than the panel's, instead + * of trusting the parameter name. If the panel's signing key were used here, + * every node operator could mint tokens for every other node — the daemon + * config file is deliberately readable by whoever runs the machine. + */ + it('is not signed with the panel key: the node secret is the only key that opens it', async () => { + const crypto = makeCrypto(); + const panelKey = crypto.getSigningKey(); + const token = await makeTokens(crypto).signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [], + }); + + // The panel's own key is not the node's, and the reverse forgery fails too: + // a token signed with the panel key is not accepted for a node. + const forgedWithPanelKey = await new SignJWT({ serverUuid: SERVER_A, permissions: [] }) + .setProtectedHeader({ alg: 'HS256' }) + .setSubject(USER_UUID) + .setIssuer(APP_URL) + .setAudience(NODE_A.uuid) + .setJti('forged') + .setIssuedAt() + .setExpirationTime('600s') + .sign(panelKey); + + expect(panelKey.equals(Buffer.from(NODE_A.secret, 'utf8'))).toBe(false); + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + expect( + await tokens.verifyConsoleToken(forgedWithPanelKey, NODE_A.uuid, NODE_A.secret), + ).toBeNull(); + }); + + /** The issuer is pinned too: a second panel pointed at the same node is not this panel. */ + it('refuses a token minted by a different panel URL', async () => { + const other = new TokenService(makeCrypto(), { + get: () => 'https://evil.example.test', + } as unknown as ConfigService); + + const token = await other.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [...ALL_PERMISSIONS], + }); + + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Keeping the three token families apart +// --------------------------------------------------------------------------- + +describe('token families do not cross', () => { + const crypto = makeCrypto(); + const tokens = makeTokens(crypto); + + const access = () => + tokens.signAccessToken({ sub: USER_UUID, username: 'julien', role: 'USER', sid: '42' }); + + const console_ = () => + tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [...ALL_PERMISSIONS], + }); + + it('refuses a console token presented to the panel API', async () => { + expect(await tokens.verifyAccessToken(await console_())).toBeNull(); + }); + + it('refuses a panel access token presented to a node', async () => { + expect(await tokens.verifyConsoleToken(await access(), NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); + + /** + * The sharpest version of the previous test. A node's `jwtSecret` sits in + * `/etc/hopper/daemon.yml` on the node machine, so anyone who roots a single + * node holds it. If that secret could mint panel tokens, one compromised game + * host would become panel administrator over the whole install. It cannot: + * the panel verifies with its own HKDF-derived key. + */ + it('does not let a node operator mint a panel administrator token', async () => { + const forged = await new SignJWT({ username: 'julien', role: 'ADMIN', sid: '1' }) + .setProtectedHeader({ alg: 'HS256' }) + .setSubject(USER_UUID) + .setIssuer(APP_URL) + .setAudience('hopper:panel') + .setIssuedAt() + .setExpirationTime('900s') + .sign(Buffer.from(NODE_A.secret, 'utf8')); + + expect(await tokens.verifyAccessToken(forged)).toBeNull(); + }); + + it('refuses a password-setup token as an access token, and the reverse', async () => { + const setup = await tokens.signPasswordSetup({ + userUuid: USER_UUID, + passwordHash: '$argon2id$v=19$m=19456,t=2,p=1$c2FsdA$aGFzaA', + ttlSeconds: 3600, + }); + + expect(await tokens.verifyAccessToken(setup)).toBeNull(); + expect(await tokens.verifyPasswordSetup(await access())).toBeNull(); + }); + + /** + * The console token and the signed URL are the one pair the class comment's + * claim — "the audience separates the families" — does not cover: both are + * signed with the node secret, for the node's uuid, by the same issuer. Only + * the payload shape tells them apart, and Zod's object schemas strip surplus + * keys rather than rejecting them, so a payload carrying both `permissions` + * and `resource` would parse as *both*. Nothing mints such a payload today. + * This test is what keeps that true. + */ + it('separates a console token from a signed URL by shape alone, not by audience', async () => { + const url = await tokens.signResourceUrl({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + resource: { type: 'file-download', path: '/server.properties' }, + }); + const ws = await console_(); + + // Same issuer, same audience, same key: the cryptography does not separate them. + expect(claimsOf(url).aud).toBe(claimsOf(ws).aud); + expect(claimsOf(url).iss).toBe(claimsOf(ws).iss); + + // Only the missing claim does. + expect(await tokens.verifyConsoleToken(url, NODE_A.uuid, NODE_A.secret)).toBeNull(); + expect(await tokens.verifyResourceUrl(ws, NODE_A.uuid, NODE_A.secret)).toBeNull(); + expect(claimsOf(ws).resource).toBeUndefined(); + expect(claimsOf(url).permissions).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// Permissions: what the panel is willing to sign +// --------------------------------------------------------------------------- + +describe('console token: the permission list', () => { + const tokens = makeTokens(); + + function mintWith(permissions: unknown) { + return tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: permissions as Permission[], + }); + } + + /** + * Known permissions are passed through untouched — same order, duplicates + * included. The filter added below is a filter and not a rewrite: an owner's + * fifty permissions have to arrive as the fifty the resolver computed. + */ + it('signs the list verbatim, in order, without deduplicating', async () => { + const claims = claimsOf( + await mintWith([PERMISSIONS.CONTROL_STOP, PERMISSIONS.CONTROL_STOP, PERMISSIONS.FILE_READ]), + ); + + expect(claims.permissions).toEqual([ + PERMISSIONS.CONTROL_STOP, + PERMISSIONS.CONTROL_STOP, + PERMISSIONS.FILE_READ, + ]); + }); + + /** + * `Permission[]` is a compile-time claim and nothing more: the list reaches + * this method from a database column, through the resolver. An unknown value + * used to go into the signature untouched, and the array schema then failed + * on that one member rather than dropping it — so the *whole* token was + * refused and the console did not open at all. + * + * `sanitizePermissions` now runs on this path too, as it already did in + * `ServerPermissionResolver` for subusers; owners and administrators reach + * here without passing through that. The bearer keeps the permissions + * everyone still agrees on, and the console opens. + * + * The dropping is logged, not silent: a value that survived in the database + * past the version that defined it is either a leftover or a bug, and neither + * should be discovered from a permission quietly not applying. + */ + it('drops a permission it does not recognise, and says so, instead of killing the token', async () => { + const warn = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + + try { + const token = await mintWith([PERMISSIONS.CONTROL_CONSOLE, 'server.root']); + + expect(claimsOf(token).permissions).toEqual([PERMISSIONS.CONTROL_CONSOLE]); + + const payload = await tokens.verifyConsoleToken(token, NODE_A.uuid, NODE_A.secret); + expect(payload?.permissions).toEqual([PERMISSIONS.CONTROL_CONSOLE]); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('server.root')); + } finally { + warn.mockRestore(); + } + }); + + /** + * The half the filter does **not** fix, kept next to it so nobody reads it as + * fixed: `sanitizePermissions` knows the *panel's* enum. A permission new + * enough that the daemons have not been upgraded to it passes here and is + * refused there, taking the whole token with it and every console on those + * nodes. Only upgrading the nodes closes that. Signing every permission this + * panel holds is therefore still a bet on the fleet being current — which is + * why the assertion is that the list signed is exactly the panel's, and not + * something narrower that would pretend otherwise. + */ + it('signs every permission this panel knows, and bets the daemons know them too', async () => { + const payload = await tokens.verifyConsoleToken( + await mintWith([...ALL_PERMISSIONS]), + NODE_A.uuid, + NODE_A.secret, + ); + + expect(payload?.permissions).toEqual([...ALL_PERMISSIONS]); + }); + + /** + * An empty list is legitimate — a subuser can hold zero permissions — and the + * token stays valid, which is the part that matters here: an empty + * `permissions` must not be mistaken for a malformed token and refused. + * + * What a session holding one is then allowed to *see* is the daemon's + * decision, taken per message, and is pinned in `console-gateway.spec.ts`. + * This file deliberately says nothing about it: a claim made here about + * behaviour implemented there would be a comment nothing can keep honest. + */ + it('signs an empty permission list, and the token verifies', async () => { + const payload = await tokens.verifyConsoleToken(await mintWith([]), NODE_A.uuid, NODE_A.secret); + + expect(payload?.permissions).toEqual([]); + }); + + it('refuses a token whose permission array was widened after signing', async () => { + const forged = tamper(await mintWith([PERMISSIONS.WEBSOCKET_CONNECT]), (claims) => { + claims.permissions = [...ALL_PERMISSIONS]; + }); + + expect(await tokens.verifyConsoleToken(forged, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// The issuing path: the only caller of signConsoleToken +// --------------------------------------------------------------------------- + +describe('the issuing path: ConsoleController', () => { + const tokens = makeTokens(); + + const OWNER: RequestUser = { + id: 1, + uuid: USER_UUID, + username: 'julien', + email: 'julien@example.test', + role: 'USER', + sessionId: '7', + authenticatedBy: 'session', + }; + + /** The same account, reaching the same route with a personal API key. */ + const OWNER_WITH_KEY: RequestUser = { + ...OWNER, + sessionId: 'api-key:3', + authenticatedBy: 'api-key', + }; + + function makeController(secret = NODE_A.secret) { + const prisma = { + node: { + findUniqueOrThrow: vi.fn().mockResolvedValue({ + uuid: NODE_A.uuid, + scheme: 'https', + fqdn: 'node.example', + port: 8443, + }), + }, + } as unknown as PrismaService; + + const nodes = { getJwtSecret: vi.fn().mockResolvedValue(secret) } as unknown as NodesService; + + return new ConsoleController(prisma, nodes, tokens); + } + + function access(permissions: Permission[]): RequestServer { + return { id: 10, uuid: SERVER_A, nodeId: 2, permissions, isOwner: true }; + } + + /** + * The permissions in the token come from `request.serverAccess`, which only + * `ServerPermissionGuard` writes, from `ServerPermissionResolver`. There is + * no body, no query and no header on this route, and the `:serverId` in the + * path is not a parameter of the handler either — its whole input is two + * guard-populated objects, so a caller has nothing to influence the token + * with, list or server. + */ + it('signs the permissions the resolver computed, with nothing the caller can add', async () => { + const granted = [PERMISSIONS.WEBSOCKET_CONNECT, PERMISSIONS.FILE_READ]; + const credentials = await makeController().credentials(OWNER, access(granted)); + + const payload = await tokens.verifyConsoleToken(credentials.token, NODE_A.uuid, NODE_A.secret); + + expect(payload?.permissions).toEqual(granted); + expect(payload?.sub).toBe(USER_UUID); + // The handler takes the user and the resolved access — nothing else. + expect(ConsoleController.prototype.credentials.length).toBe(2); + }); + + /** + * The uuid signed, and the one in the socket URL, are both the value the + * guard read out of the database. They have to be the same value: the daemon + * compares the token's `serverUuid` against the uuid in the URL the socket + * was opened on, with `!==`, so two spellings of the same server would be a + * console that never opens. + */ + it('names the resolved server in both the token and the socket URL', async () => { + const credentials = await makeController().credentials(OWNER, access([])); + const payload = await tokens.verifyConsoleToken(credentials.token, NODE_A.uuid, NODE_A.secret); + + // The uuid the guard resolved, in the token… + expect(payload?.serverUuid).toBe(SERVER_A); + // …and the same one in the URL, which is the other half of the comparison. + expect(credentials.socketUrl).toBe(`wss://node.example:8443/api/servers/${SERVER_A}/ws`); + }); + + it("signs with the secret of the server's own node, fetched per request", async () => { + const controller = makeController(NODE_B.secret); + const credentials = await controller.credentials(OWNER, access([...ALL_PERMISSIONS])); + + // Signed with B's secret, so A's cannot open it even though `aud` says A. + expect( + await tokens.verifyConsoleToken(credentials.token, NODE_A.uuid, NODE_A.secret), + ).toBeNull(); + expect( + await tokens.verifyConsoleToken(credentials.token, NODE_A.uuid, NODE_B.secret), + ).not.toBeNull(); + }); + + it('announces the same lifetime it signed', async () => { + const credentials = await makeController().credentials(OWNER, access([])); + const claims = claimsOf(credentials.token); + + expect(credentials.expiresIn).toBe(CONSOLE_TOKEN_TTL_SECONDS); + expect((claims.exp as number) - (claims.iat as number)).toBe(credentials.expiresIn); + }); + + /** + * The escalation this route used to allow, now closed. + * + * Nothing in the layers *above* the handler stops an API key here, and the + * first two assertions are the real predicate saying so: the route is a + * `GET`, `scopeAllows` decides scope from the verb, so a key scoped `read` + * walks through — while the same key is correctly refused the ordinary way of + * stopping a server. The resolver, for its part, knows nothing about API keys + * and hands an owner every permission there is. + * + * So the refusal has to be the handler's own, and it has to come before + * anything is signed: what this route returns is not a read but a credential + * the daemon honours without ever learning a key was involved — on a + * Minecraft server, arbitrary command execution from a token whose whole + * promise was that it could not stop anything. + */ + it('refuses to mint anything at all for a request authenticated by an API key', async () => { + const path = `/api/servers/${SERVER_A}/console`; + + // The guard-level checks that do *not* save us, stated as facts. + expect(scopeAllows(['read'], 'GET', path)).toBe(true); + expect(scopeAllows(['read'], 'POST', `/api/servers/${SERVER_A}/power`)).toBe(false); + + const controller = makeController(); + + await expect( + controller.credentials(OWNER_WITH_KEY, access([...ALL_PERMISSIONS])), + ).rejects.toBeInstanceOf(ForbiddenException); + }); + + /** + * And refused *early*: before the node is read, before its signing secret is + * decrypted, and therefore before any token exists to leak. A refusal that + * happened after the signing would still be a token minted on the strength of + * a read key, sitting in a log or a stack trace. + */ + it('refuses the API key before reading the node or its secret', async () => { + const prisma = { + node: { findUniqueOrThrow: vi.fn() }, + } as unknown as PrismaService; + const nodes = { getJwtSecret: vi.fn() } as unknown as NodesService; + + const controller = new ConsoleController(prisma, nodes, tokens); + + await expect( + controller.credentials(OWNER_WITH_KEY, access([...ALL_PERMISSIONS])), + ).rejects.toThrow(/API key/i); + + expect(prisma.node.findUniqueOrThrow).not.toHaveBeenCalled(); + expect(nodes.getJwtSecret).not.toHaveBeenCalled(); + }); + + /** The same account, signed into the panel, is unaffected. */ + it('still issues a console to the same account when it is a browser session', async () => { + const credentials = await makeController().credentials(OWNER, access([...ALL_PERMISSIONS])); + const payload = await tokens.verifyConsoleToken(credentials.token, NODE_A.uuid, NODE_A.secret); + + expect(payload?.permissions).toContain(PERMISSIONS.CONTROL_CONSOLE); + }); +}); + +// --------------------------------------------------------------------------- +// Malformed input: anything the panel signs, the daemon has to refuse +// --------------------------------------------------------------------------- + +describe('console token: malformed input', () => { + const tokens = makeTokens(); + + /** + * The sign path and the verify path used to disagree: `signConsoleToken` + * imposed no format on `serverUuid` while `consoleTokenPayloadSchema` — the + * daemon's schema too — requires a UUID. The panel would mint, sign and hand + * back over HTTPS a credential its own verifier rejects. + * + * Nothing reached it — the only caller signs a uuid the guard has already + * looked up. It is fixed anyway, and by refusing rather than by correcting: a + * caller that has lost track of what a server uuid is has a bug, and the + * useful place to learn that is the stack trace of the request that caused it, + * not a WebSocket the daemon closes with a debug line on the node. + */ + it('refuses to sign a serverUuid that is not a UUID', async () => { + await expect( + tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: '../../etc/passwd', + permissions: [PERMISSIONS.WEBSOCKET_CONNECT], + }), + ).rejects.toThrow(/serverUuid/); + }); + + /** The signed URL is held to the same rule, by the same reasoning. */ + it('refuses to sign a resource URL for a serverUuid that is not a UUID', async () => { + await expect( + tokens.signResourceUrl({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: 'not-a-uuid', + resource: { type: 'file-download', path: '/server.properties' }, + }), + ).rejects.toThrow(/serverUuid/); + }); + + /** + * An empty `sub` passes the contract schema — it is only `z.string()` — and + * the panel is deliberately stricter than the contract here. `sub` is the one + * claim naming a person, and the only handle the daemon has on who is at the + * other end of a console; a token signed without one produces a session + * belonging to nobody. The place to catch that is the issuing side, since + * nothing downstream can recover an attribution that was never signed. + */ + it('refuses to sign a subject that is not a user uuid', async () => { + for (const subject of ['', 'julien', '7']) { + await expect( + tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: subject, + serverUuid: SERVER_A, + permissions: [], + }), + ).rejects.toThrow(/userUuid/); + } + }); + + /** + * A node row whose `jwtSecret` decrypted to an empty string would mean every + * console token on that node was signed with a key an attacker also has. + * Node's HMAC refuses a zero-length key, so this fails loudly at signing time + * — a 500 on the console route — instead of quietly issuing forgeable + * credentials. That is the right failure, and it is worth a test because it + * comes from Node rather than from any check written here. + */ + it('cannot sign with an empty node secret', async () => { + await expect( + tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: '', + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [], + }), + ).rejects.toThrow(); + }); + + /** + * There is no floor on key material here, though. The daemon's config schema + * insists on `min(32)` for the same secret; the panel, which is the side that + * *signs*, insists on nothing. The generator produces 64 characters, so this + * is latent rather than live — but the invariant is being enforced on the + * verifying side only, which is the side that cannot do anything about it. + */ + it('will sign with a one-character node secret', async () => { + const token = await tokens.signConsoleToken({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: 'x', + userUuid: USER_UUID, + serverUuid: SERVER_A, + permissions: [...ALL_PERMISSIONS], + }); + + expect(await tokens.verifyConsoleToken(token, NODE_A.uuid, 'x')).not.toBeNull(); + }); + + it('returns null rather than throwing on rubbish input', async () => { + for (const rubbish of ['', 'not.a.jwt', 'a.b', '....', 'Bearer ' + 'x'.repeat(40)]) { + expect(await tokens.verifyConsoleToken(rubbish, NODE_A.uuid, NODE_A.secret)).toBeNull(); + } + }); +}); + +// --------------------------------------------------------------------------- +// Signed URLs +// --------------------------------------------------------------------------- + +describe('signed URL', () => { + const tokens = makeTokens(); + + const mint = () => + tokens.signResourceUrl({ + nodeUuid: NODE_A.uuid, + nodeJwtSecret: NODE_A.secret, + userUuid: USER_UUID, + serverUuid: SERVER_A, + resource: { type: 'backup-download', backupUuid: SERVER_B }, + }); + + it('lives for one minute and names exactly what it authorises', async () => { + const claims = claimsOf(await mint()); + + expect(SIGNED_URL_TTL_SECONDS).toBe(60); + expect((claims.exp as number) - (claims.iat as number)).toBe(SIGNED_URL_TTL_SECONDS); + expect(claims.resource).toEqual({ type: 'backup-download', backupUuid: SERVER_B }); + }); + + it('is bound to its node like a console token is', async () => { + const url = await mint(); + + expect(await tokens.verifyResourceUrl(url, NODE_B.uuid, NODE_A.secret)).toBeNull(); + expect(await tokens.verifyResourceUrl(url, NODE_A.uuid, NODE_B.secret)).toBeNull(); + }); + + /** + * It is not single-use, and nothing calls it that any more — not the doc + * comment on `signResourceUrl`, not the contract schema, not + * `docs/security.md`. Nothing consumes the jti, so the same link works for + * its whole minute and for as many downloads as anyone holding it cares to + * start. A minute in a browser history is the real mitigation; this test is + * what keeps that the whole of the claim. + */ + it('is not in fact single-use: the same URL verifies twice', async () => { + const url = await mint(); + + expect(await tokens.verifyResourceUrl(url, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + expect(await tokens.verifyResourceUrl(url, NODE_A.uuid, NODE_A.secret)).not.toBeNull(); + }); + + it('refuses a rewritten download path', async () => { + const forged = tamper(await mint(), (claims) => { + claims.resource = { type: 'file-download', path: '/../../etc/shadow' }; + }); + + expect(await tokens.verifyResourceUrl(forged, NODE_A.uuid, NODE_A.secret)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Access token +// --------------------------------------------------------------------------- + +describe('access token', () => { + const tokens = makeTokens(); + + const mint = () => + tokens.signAccessToken({ sub: USER_UUID, username: 'julien', role: 'USER', sid: '42' }); + + it('lives for fifteen minutes and is addressed to the panel', async () => { + const claims = claimsOf(await mint()); + + expect(ACCESS_TOKEN_TTL_SECONDS).toBe(15 * 60); + expect((claims.exp as number) - (claims.iat as number)).toBe(ACCESS_TOKEN_TTL_SECONDS); + expect(claims.aud).toBe('hopper:panel'); + expect(claims.sid).toBe('42'); + }); + + /** + * The `sid` is what makes an access token revocable: the guard reads the + * session back on every request, so a sign-out or a suspension bites before + * the fifteen minutes are up. Losing this claim would turn the access token + * into a bearer token nobody can call back — exactly the property the console + * token has, and the reason that one is limited to two minutes. + */ + it('is refused if the session identifier is stripped', async () => { + const forged = tamper(await mint(), (claims) => { + delete claims.sid; + }); + + expect(await tokens.verifyAccessToken(forged)).toBeNull(); + }); + + /** + * A role outside the pair is refused rather than treated as a plain user. + * Failing closed on an unknown role matters more than it looks: a future + * third role added to the token and not to this check would otherwise be + * silently downgraded — or, with a laxer check, silently accepted. + */ + it('refuses an unknown role instead of falling back', async () => { + const forged = await new SignJWT({ username: 'julien', role: 'SUPERADMIN', sid: '1' }) + .setProtectedHeader({ alg: 'HS256' }) + .setSubject(USER_UUID) + .setIssuer(APP_URL) + .setAudience('hopper:panel') + .setIssuedAt() + .setExpirationTime('900s') + .sign(makeCrypto().getSigningKey()); + + expect(await tokens.verifyAccessToken(forged)).toBeNull(); + }); + + it('refuses a token signed with a different APP_SECRET', async () => { + const other = makeTokens(makeCrypto('a-completely-different-app-secret-xyz')); + + expect( + await tokens.verifyAccessToken( + await other.signAccessToken({ + sub: USER_UUID, + username: 'julien', + role: 'ADMIN', + sid: '1', + }), + ), + ).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Refresh token: the opaque half +// --------------------------------------------------------------------------- + +describe('refresh token', () => { + const tokens = makeTokens(); + + it('lasts thirty days', () => { + expect(REFRESH_TOKEN_TTL_SECONDS).toBe(30 * 24 * 60 * 60); + }); + + /** + * Deliberately not a JWT. A signed token cannot be withdrawn before it + * expires, and thirty days is far too long to be unable to withdraw + * something; an opaque value looked up in a table can be revoked in one + * update. + */ + it('is opaque, not a JWT', () => { + const { token } = tokens.generateRefreshToken(); + + expect(token).toHaveLength(64); + expect(token).toMatch(/^[A-Za-z0-9]{64}$/); + expect(() => decodeJwt(token)).toThrow(); + }); + + it('stores a digest that does not contain the token', () => { + const { token, hash } = tokens.generateRefreshToken(); + + expect(hash).not.toContain(token); + expect(hash).not.toBe(token); + // Deterministic, or `refresh` could never find the row by its digest. + expect(makeCrypto().hashToken(token)).toBe(hash); + expect(tokens.generateRefreshToken().hash).not.toBe(hash); + }); + + it('does not repeat itself', () => { + const drawn = new Set(Array.from({ length: 500 }, () => tokens.generateRefreshToken().token)); + + expect(drawn.size).toBe(500); + }); +}); + +/** + * Rotation and replay live in `AuthService`, not here — but they are what make + * the opaque token above worth anything, and the brief asks for them, so they + * are exercised against a fake session table rather than assumed. + */ +describe('refresh rotation and replay', () => { + interface FakeUser { + id: number; + uuid: string; + username: string; + email: string; + role: 'ADMIN' | 'USER'; + suspended: boolean; + totpConfirmed: boolean; + } + + interface FakeSession { + id: number; + userId: number; + tokenHash: string; + family: string; + revokedAt: Date | null; + expiresAt: Date; + } + + const CONTEXT = { ip: '203.0.113.7', userAgent: 'vitest' }; + + let user: FakeUser; + let sessions: FakeSession[]; + let nextId: number; + let auth: AuthService; + let crypto: CryptoService; + let record: ReturnType; + + beforeEach(() => { + // The reuse path logs a warning by design; it is noise in the test output. + vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + + user = { + id: 1, + uuid: USER_UUID, + username: 'julien', + email: 'julien@example.test', + role: 'USER', + suspended: false, + totpConfirmed: false, + }; + sessions = []; + nextId = 1; + crypto = makeCrypto(); + record = vi.fn().mockResolvedValue(undefined); + + const prisma = { + session: { + findUnique: vi.fn(({ where }: { where: { tokenHash: string } }) => { + const found = sessions.find((s) => s.tokenHash === where.tokenHash); + return Promise.resolve(found ? { ...found, user } : null); + }), + create: vi.fn(({ data }: { data: Omit }) => { + const created: FakeSession = { ...data, id: nextId++, revokedAt: null }; + sessions.push(created); + return Promise.resolve(created); + }), + update: vi.fn(({ where, data }: { where: { id: number }; data: { revokedAt: Date } }) => { + const found = sessions.find((s) => s.id === where.id); + if (found) found.revokedAt = data.revokedAt; + return Promise.resolve(found); + }), + updateMany: vi.fn( + ({ where, data }: { where: { family: string }; data: { revokedAt: Date } }) => { + let count = 0; + for (const s of sessions) { + if (s.family === where.family && s.revokedAt === null) { + s.revokedAt = data.revokedAt; + count += 1; + } + } + return Promise.resolve({ count }); + }, + ), + }, + } as unknown as PrismaService; + + auth = new AuthService( + prisma, + {} as unknown as PasswordService, + makeTokens(crypto), + {} as unknown as TotpService, + crypto, + // The refresh path does not touch the rate limiter. + {} as never, + { record } as unknown as AuditService, + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + /** Seeds a live session and returns its plaintext refresh token. */ + function seed(family = 'family-1'): string { + const tokens = makeTokens(crypto); + const { token, hash } = tokens.generateRefreshToken(); + sessions.push({ + id: nextId++, + userId: user.id, + tokenHash: hash, + family, + revokedAt: null, + expiresAt: new Date(Date.now() + REFRESH_TOKEN_TTL_SECONDS * 1000), + }); + return token; + } + + it('rotates the token and revokes the one presented', async () => { + const original = seed(); + const result = await auth.refresh(original, CONTEXT); + + expect(result.refreshToken).not.toBe(original); + expect( + sessions.find((s) => s.tokenHash === crypto.hashToken(original))?.revokedAt, + ).toBeInstanceOf(Date); + expect( + sessions.find((s) => s.tokenHash === crypto.hashToken(result.refreshToken))?.revokedAt, + ).toBeNull(); + }); + + it('keeps the rotated session in the same family', async () => { + const result = await auth.refresh(seed('the-family'), CONTEXT); + + expect( + sessions.find((s) => s.tokenHash === crypto.hashToken(result.refreshToken))?.family, + ).toBe('the-family'); + }); + + /** + * The attack this defends against: a refresh token stolen from a browser and + * used once by the thief. The legitimate client then presents the same token, + * finds it already revoked, and the whole family falls — thief included. Both + * parties are signed out, which is the intended trade: a re-login beats a + * session quietly plundered for thirty days. + */ + it('cannot be replayed, and burns the whole family when it is tried', async () => { + const stolen = seed('doomed'); + const rotated = await auth.refresh(stolen, CONTEXT); + + await expect(auth.refresh(stolen, CONTEXT)).rejects.toThrow(/revoked/i); + + // The token the honest client obtained is dead too. + expect( + sessions.find((s) => s.tokenHash === crypto.hashToken(rotated.refreshToken))?.revokedAt, + ).toBeInstanceOf(Date); + await expect(auth.refresh(rotated.refreshToken, CONTEXT)).rejects.toThrow(/revoked/i); + + expect(record).toHaveBeenCalledWith( + expect.objectContaining({ metadata: { family: 'doomed' } }), + ); + }); + + it('refuses an unknown token without saying whether it ever existed', async () => { + await expect(auth.refresh('A'.repeat(64), CONTEXT)).rejects.toThrow(/unknown or expired/i); + }); + + it('refuses a session past its thirty days', async () => { + const token = seed(); + const session = sessions.find((s) => s.tokenHash === crypto.hashToken(token))!; + session.expiresAt = new Date(Date.now() - 1000); + + await expect(auth.refresh(token, CONTEXT)).rejects.toThrow(/expired/i); + }); + + it('refuses to rotate for a suspended account', async () => { + const token = seed(); + user.suspended = true; + + await expect(auth.refresh(token, CONTEXT)).rejects.toThrow(/suspended/i); + }); +}); diff --git a/apps/panel/src/modules/auth/token.service.ts b/apps/panel/src/modules/auth/token.service.ts index 9ce2c0a..5397f15 100644 --- a/apps/panel/src/modules/auth/token.service.ts +++ b/apps/panel/src/modules/auth/token.service.ts @@ -3,15 +3,18 @@ import { CONSOLE_TOKEN_TTL_SECONDS, SIGNED_URL_TTL_SECONDS, consoleTokenPayloadSchema, + isPermission, + sanitizePermissions, signedUrlPayloadSchema, type ConsoleTokenPayload, type Permission, type SignedUrlPayload, } from '@hopper/shared'; -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { createHash } from 'node:crypto'; import { SignJWT, jwtVerify } from 'jose'; +import { z } from 'zod'; import { CryptoService } from '../../common/crypto/crypto.service.js'; import type { Environment } from '../../config/environment.js'; @@ -28,6 +31,47 @@ export interface AccessTokenPayload { sid: string; } +/** + * The rule `serverUuid` has to satisfy, borrowed from the contract schemas + * themselves rather than restated here. + * + * The two cannot drift that way: what a claim must satisfy to be *verified* is + * exactly what it must satisfy to be *signed*. Restating `z.uuid()` in this + * file would work today and stop working the day the contract tightens. + */ +const serverUuidSchema = consoleTokenPayloadSchema.shape.serverUuid; +const resourceServerUuidSchema = signedUrlPayloadSchema.shape.serverUuid; + +/** + * The subject, held to more than the contract asks of it. + * + * Both payload schemas type `sub` as a plain string, so an empty one verifies — + * and `sub` is the only thing in either token that names a person. It is what + * the daemon has to tell one console session from another's, having no other + * handle on who is connected. A token signed without one produces a session + * belonging to nobody, and nothing downstream can recover an attribution that + * was never signed. Every caller has a user's uuid to hand. + */ +const subjectSchema = z.uuid(); + +/** + * Validates a claim before it is signed, or refuses to sign at all. + * + * Throwing is deliberate. The alternative — signing anyway — hands the browser + * a credential the daemon will refuse, which surfaces as a console that will + * not open or a download that fails, with the reason a debug line on the node. + * A 500 on the issuing route names the fault where it happened. + */ +function parseOrThrow(schema: z.ZodType, value: unknown, field: string): T { + const parsed = schema.safeParse(value); + + if (!parsed.success) { + throw new Error(`Refusing to sign a token whose ${field} the daemon would reject.`); + } + + return parsed.data; +} + /** * Issuing and verifying the panel's tokens. * @@ -45,6 +89,7 @@ export interface AccessTokenPayload { */ @Injectable() export class TokenService { + private readonly logger = new Logger(TokenService.name); private readonly issuer: string; constructor( @@ -175,8 +220,10 @@ export class TokenService { * * Signed with the node's secret, not the panel's: the daemon verifies it on * its own, with no network call. That is what keeps the console fluid, and it - * is why the lifetime is short — a permission revoked in the panel only takes - * effect on renewal. + * is why the lifetime is short — a permission revoked in the panel, or the + * whole session behind it, only takes effect on renewal. See + * `CONSOLE_TOKEN_TTL_SECONDS` for the size of that window and why it is the + * only bound there is. */ async signConsoleToken(input: { nodeUuid: string; @@ -185,12 +232,20 @@ export class TokenService { serverUuid: string; permissions: Permission[]; }): Promise { + // Everything below is checked *before* being signed, against the very + // schema the daemon will apply on the way back in. The panel must not be + // able to hand a browser credentials it already knows are dead: the browser + // would open a WebSocket with them and be disconnected by the daemon, and + // the only trace of the reason lives on the node, in a debug line. + const serverUuid = parseOrThrow(serverUuidSchema, input.serverUuid, 'serverUuid'); + const userUuid = parseOrThrow(subjectSchema, input.userUuid, 'userUuid'); + return new SignJWT({ - serverUuid: input.serverUuid, - permissions: input.permissions, + serverUuid, + permissions: this.signablePermissions(input.permissions, serverUuid), }) .setProtectedHeader({ alg: 'HS256' }) - .setSubject(input.userUuid) + .setSubject(userUuid) .setIssuer(this.issuer) .setAudience(input.nodeUuid) .setJti(randomUUID()) @@ -199,6 +254,40 @@ export class TokenService { .sign(Buffer.from(input.nodeJwtSecret, 'utf8')); } + /** + * The permissions this panel is prepared to put its signature on. + * + * `Permission[]` is a compile-time claim only — the list reaches here from a + * database column through the resolver — and the token's schema validates the + * array as a whole, so a single unknown value does not degrade the token, it + * *destroys* it: the daemon refuses the lot and the console will not open at + * all. Dropping the value instead leaves the bearer with the permissions + * everyone still agrees on, which is the same reasoning that put + * `sanitizePermissions` in `ServerPermissionResolver` for subusers; owners and + * administrators reach this path without passing through it. + * + * This does **not** make a panel upgraded ahead of its daemons safe. A + * permission new enough that the *daemon* does not know it is known here, so + * it passes this filter and is refused there, taking the token with it. Only + * upgrading the nodes fixes that; what is fixed here is the panel signing + * something its own contract rejects. + */ + private signablePermissions(permissions: Permission[], serverUuid: string): Permission[] { + const unknown = permissions.filter((permission) => !isPermission(permission)); + + if (unknown.length > 0) { + // Never silently: a permission disappearing between the database and the + // token is either a leftover from an older version or a bug, and both are + // worth a line naming the values and the server. + this.logger.warn( + `Console token for server ${serverUuid}: dropping ${unknown.length} unknown ` + + `permission(s) — ${unknown.join(', ')}. Signing them would make the whole token unusable.`, + ); + } + + return sanitizePermissions(permissions); + } + /** Verifies a console token. Used by the tests and by the daemon. */ async verifyConsoleToken( token: string, @@ -220,9 +309,13 @@ export class TokenService { } /** - * Single-use signed URL for a file download or upload. + * Signed URL for a file download or upload. + * * Very short-lived: the URL travels in the clear through the address bar and - * the browser history. + * the browser history, and that brevity is the entire protection. It is not + * single-use — nothing consumes the `jti`, on either side — so a link that + * leaks works for whoever holds it until it expires, as many times as they + * like. Treat the lifetime as the whole of the guarantee. */ async signResourceUrl(input: { nodeUuid: string; @@ -231,9 +324,14 @@ export class TokenService { serverUuid: string; resource: SignedUrlPayload['resource']; }): Promise { - return new SignJWT({ serverUuid: input.serverUuid, resource: input.resource }) + // Same reasoning as the console token: a URL the daemon's schema refuses is + // a download that fails in the browser with nothing to point at. + const serverUuid = parseOrThrow(resourceServerUuidSchema, input.serverUuid, 'serverUuid'); + const userUuid = parseOrThrow(subjectSchema, input.userUuid, 'userUuid'); + + return new SignJWT({ serverUuid, resource: input.resource }) .setProtectedHeader({ alg: 'HS256' }) - .setSubject(input.userUuid) + .setSubject(userUuid) .setIssuer(this.issuer) .setAudience(input.nodeUuid) .setJti(randomUUID()) diff --git a/apps/panel/src/modules/servers/console.controller.ts b/apps/panel/src/modules/servers/console.controller.ts index 7f1ef4d..45e93f6 100644 --- a/apps/panel/src/modules/servers/console.controller.ts +++ b/apps/panel/src/modules/servers/console.controller.ts @@ -1,5 +1,5 @@ import { CONSOLE_TOKEN_TTL_SECONDS, PERMISSIONS } from '@hopper/shared'; -import { Controller, Get, Param } from '@nestjs/common'; +import { Controller, ForbiddenException, Get } from '@nestjs/common'; import { TokenService } from '../auth/token.service.js'; import { RequireServerPermission } from '../auth/decorators.js'; import { @@ -27,8 +27,12 @@ export interface ConsoleCredentials { * straight to the daemon. That is what lets fifty open consoles cost the panel * nothing, and what keeps the console fluid even when the panel is busy. * - * The price of that choice: a permission revoked in the panel only takes effect - * when the token is renewed, hence its deliberately short lifetime. + * The price of that choice: the daemon cannot ask the panel anything about the + * token it is holding. A revoked permission, a sign-out, a password change, a + * suspension — none of them reach a console that is already open. They only + * stop the *renewal*, which comes back through this route and is checked like + * any other request. `CONSOLE_TOKEN_TTL_SECONDS` is therefore the whole of the + * revocation delay, and the reason it is two minutes rather than ten. */ @Controller('api/servers') export class ConsoleController { @@ -38,13 +42,49 @@ export class ConsoleController { private readonly tokens: TokenService, ) {} + /** + * The `:serverId` in the path is deliberately **not** a parameter of this + * handler. + * + * `ServerPermissionGuard` has already looked that string up and put the row + * it found on the request, so `server.uuid` is the uuid the database holds + * rather than the spelling the caller used. Signing the caller's spelling is + * what would let a request for `3F2504E0-…` produce a token and a socket URL + * the daemon cannot match against a server it knows by `3f2504e0-…`: a + * credential issued dead. Taking nothing from the caller here also means the + * handler's whole input is guard-computed, which is the property the token's + * permissions rest on. + */ @Get(':serverId/console') @RequireServerPermission(PERMISSIONS.WEBSOCKET_CONNECT) async credentials( - @Param('serverId') serverId: string, @CurrentUser() user: RequestUser, @CurrentServer() server: RequestServer, ): Promise { + /** + * An API key does not open a console. Refused here, first thing, and not + * left to the scopes. + * + * A key's scope is decided from the HTTP verb — `scopeAllows` — and this + * route is a `GET`, so a key scoped `read` walks straight through it. What + * comes back is not a read: it is a token carrying whatever the resolver + * computed for the account, `control.console` and `control.stop` included + * for an owner, honoured by a daemon that has no idea a key was involved. + * On a Minecraft server that is arbitrary command execution from a + * credential whose whole promise was that it could not stop anything. + * + * Requiring `write` instead would only move the problem: a `write` key + * would then hold a console token off-session, outside every revocation the + * panel has, for as long as it kept renewing. So the answer is the one + * `docs/api.md` has always given its readers — the console is for a + * signed-in browser. + */ + if (user.authenticatedBy === 'api-key') { + throw new ForbiddenException( + 'The console cannot be opened with an API key: sign in to the panel.', + ); + } + const node = await this.prisma.node.findUniqueOrThrow({ where: { id: server.nodeId }, select: { uuid: true, scheme: true, fqdn: true, port: true }, @@ -56,7 +96,7 @@ export class ConsoleController { nodeUuid: node.uuid, nodeJwtSecret: jwtSecret, userUuid: user.uuid, - serverUuid: serverId, + serverUuid: server.uuid, // The permissions are frozen into the token: that is what lets the // daemon decide on its own, without calling the panel on every message. permissions: server.permissions, @@ -68,7 +108,9 @@ export class ConsoleController { const scheme = node.scheme === 'https' ? 'wss' : 'ws'; return { - socketUrl: `${scheme}://${node.fqdn}:${node.port}/api/servers/${serverId}/ws`, + // The same uuid the token names, for the same reason: the daemon refuses + // a token whose `serverUuid` is not the server the socket was opened on. + socketUrl: `${scheme}://${node.fqdn}:${node.port}/api/servers/${server.uuid}/ws`, token, expiresIn: CONSOLE_TOKEN_TTL_SECONDS, }; diff --git a/docs/api.md b/docs/api.md index 30c0420..903b70c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -35,8 +35,14 @@ POST /api/servers/:uuid/backups triggers a backup GET /api/servers/:uuid/webhooks outgoing notifications ``` -The console does not open with an API key: it uses a very short-lived token issued by -`GET /api/servers/:uuid/console`, which the browser presents directly to the daemon. +**The console does not open with an API key.** `GET /api/servers/:uuid/console` answers `403` to any +request carrying one, whatever its scopes — the route is a `GET`, so a `read` key would otherwise +reach it, and what it hands back is not a read: it is a two-minute token carrying the account's +permissions on that server, which the browser presents directly to the daemon and which the daemon +honours without asking the panel anything. Open a console from a signed-in browser. + +That two-minute lifetime is also the delay before a withdrawn access stops working; see +[Securing your instance](security.md) if you are handling an incident. ## Outgoing notifications diff --git a/docs/security.md b/docs/security.md index 01ff041..d9d3be7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -88,7 +88,9 @@ You have nothing to set for the following — it is the default behaviour: so a dump of the database on its own yields nothing usable. A dump that comes with `APP_SECRET` yields control of every node, which is why that file's permissions are the ones to guard. - **Short-lived console JWTs**, carrying the bearer's permissions, verified by the daemon — which - also checks the origin of the WebSocket connection. + also checks the origin of the WebSocket connection. They are never issued to an API key: that + route answers `403`. Read "short-lived" as the guarantee itself, and see + [what a revocation does not reach](#what-hopper-does-not-protect-a-console-already-open). - **Startup commands as templates**, never a concatenation handed to a shell. - **A bounded install container**: the server's own memory limit, at least a whole core of CPU (its own entitlement where that is more), a pids limit of its own — 512, rather than the server's, @@ -192,15 +194,56 @@ script that stages its download in `/tmp` writes to the container's own layer, u root, and so lands on the filesystem the split was meant to protect. It is also the filesystem the preflight above does **not** measure — that one reads the volume's, and says so when it refuses. +## What Hopper does not protect: a console already open + +**Revoking an access does not close a console that is already connected.** It closes the next one. + +The console is the one place a browser talks straight to the daemon, and the daemon verifies the +token on its own — no call back to the panel, which is what lets fifty consoles cost the panel +nothing. The consequence is exact: nothing the panel does reaches a console mid-session. Signing +out, changing a password, suspending an account, deleting a subuser, taking a permission away — all +of them take effect at the **next renewal**, which is an ordinary +authenticated request to the panel and is refused like any other. Until then the connection stays +up, with the permissions frozen into the token rather than the current ones. + +**That window is two minutes**, the token's whole lifetime. It is deliberately short because it is +the only bound there is: the token carries no session identifier, and there is no channel by which +the panel could tell a daemon to drop a session. The token does carry a unique identifier, but +nothing anywhere reads it — it is not a handle you can revoke one console by. + +To cut live consoles on a node **now** rather than within two minutes, re-key it — the procedure +below does exactly that, because it replaces the secret those tokens are signed with. Legitimate +users reconnect on their own within seconds; a revoked one cannot, having nothing left to obtain a +new token with. + +Three neighbours of this, worth knowing while you are here: + +- **An SFTP session already open is the same shape of hole, without the two minutes.** The daemon + asks the panel to authenticate an SFTP connection once, when it opens, and applies the permissions + it got back for as long as the session lasts. `systemctl restart hopperd` is what ends one; the + client's automatic reconnect then goes back through the panel and is refused. +- **There are no signed download URLs.** The panel can mint and verify them — the code is there, + and the contract calls them single-use, which they are not — but nothing in the product issues + one. Every download goes through an authenticated call instead. Said here because the machinery + is visible to anyone reading the source, and an unused mechanism described as a protection is a + protection somebody will count on. +- **The file manager is not affected.** Every one of its operations is an authenticated call to the + panel, so a revocation bites there at once. + ## After an incident If you suspect credentials were stolen: ```bash hopper user:password --username # also closes every session -hopper node:token --node # invalidates the daemon's token +hopper node:token --node # new daemon token *and* new console signing key +# put the printed daemon.yml on the node, then: systemctl restart hopperd ``` +The middle command is the one that reaches consoles already open: it re-keys the node, so every +console token issued before it becomes unverifiable the moment the daemon restarts. Without it, +count on the two minutes described above. + Then read back the activity log of each affected server — it carries the IP address and the author of every action — and change the passwords of the databases created from the panel. diff --git a/packages/shared/src/contract/jwt.ts b/packages/shared/src/contract/jwt.ts index 054afb6..bf7088d 100644 --- a/packages/shared/src/contract/jwt.ts +++ b/packages/shared/src/contract/jwt.ts @@ -21,7 +21,16 @@ export const consoleTokenPayloadSchema = z.object({ /** UUID of the server this token grants access to. */ serverUuid: z.uuid(), permissions: z.array(permissionSchema), - /** Unique token identifier, allowing a targeted revocation. */ + /** + * Unique token identifier. + * + * It makes every token distinct even when two are minted in the same second + * with otherwise identical claims — and that is the whole of it: nothing + * reads this claim. It is **not** a revocation handle. There is no deny-list + * in the panel and no seen-set in the daemon, so a token authenticates as + * many connections as its bearer likes until it expires, and withdrawing an + * access before then means the lifetime below and nothing else. + */ jti: z.string(), iat: z.number().int(), exp: z.number().int(), @@ -30,18 +39,37 @@ export const consoleTokenPayloadSchema = z.object({ export type ConsoleTokenPayload = z.infer; /** - * Lifetime of a console token. Deliberately short: the daemon cannot know a - * permission was withdrawn between two renewals. + * Lifetime of a console token — and, being the only bound there is on one, the + * window during which a withdrawn access still works. + * + * The daemon verifies this token alone and never calls the panel back, so it + * cannot know that the session behind it was signed out, that the password was + * changed or that the account was suspended. Revocation reaches a console only + * by refusing the *next* renewal, which is an ordinary authenticated call to + * the panel; until then the console stays open. That is what this figure buys, + * and why it was brought down from ten minutes to two. + * + * It cannot usefully go much lower. The daemon warns the client + * `CONSOLE_TOKEN_RENEW_MARGIN_SECONDS` before expiry and that warning is the + * only thing that triggers a renewal: a lifetime at or below the margin means + * no warning at all, and every console dies at expiry and reconnects from + * scratch instead of renewing in place. This is also the tolerance to a node + * clock running ahead of the panel's — beyond this many seconds of skew, tokens + * arrive already expired. */ -export const CONSOLE_TOKEN_TTL_SECONDS = 600; +export const CONSOLE_TOKEN_TTL_SECONDS = 120; /** Margin before expiry at which the daemon emits `token_expiring`. */ export const CONSOLE_TOKEN_RENEW_MARGIN_SECONDS = 60; /** - * Payload of a single-use signed URL (file or backup download). Much shorter - * than a console token: the URL travels in the clear through the address bar - * and the browser history. + * Payload of a signed URL (file or backup download). Much shorter-lived than a + * console token: the URL travels in the clear through the address bar and the + * browser history. + * + * That brevity is the whole protection. The URL is **not** single-use: its + * `jti` is consumed by nobody, so it serves as many downloads as whoever holds + * it cares to start, for as long as it is valid. */ export const signedUrlPayloadSchema = z.object({ iss: z.string(),