From bdabecbb63d46d363603704b8f343ae02a4d66d7 Mon Sep 17 00:00:00 2001 From: mptyl Date: Mon, 17 Aug 2026 07:03:19 +0200 Subject: [PATCH] fix(auth): bound OIDC initiation and transport --- backend/src/auth/config.ts | 18 +- backend/src/auth/oidc-client.ts | 46 ++-- backend/src/auth/routes.ts | 119 +++++--- backend/src/auth/session-store.ts | 253 ++++++++++++++++-- backend/src/auth/types.ts | 1 + backend/src/auth/url-policy.ts | 42 +++ backend/src/auth/windows-auth-storage.ts | 14 +- backend/test/auth-config.test.ts | 25 +- backend/test/auth-routes-oidc.test.ts | 55 +++- backend/test/auth-session-store.test.ts | 95 ++++++- backend/test/fixtures/oidc-state-creator.mts | 41 +++ backend/test/oidc-client.test.ts | 76 ++++++ backend/test/windows-auth-storage.test.ts | 36 +++ tools/tht/internal/authstorage/storage.go | 15 +- .../tht/internal/authstorage/storage_test.go | 22 ++ 15 files changed, 747 insertions(+), 111 deletions(-) create mode 100644 backend/src/auth/url-policy.ts create mode 100644 backend/test/fixtures/oidc-state-creator.mts diff --git a/backend/src/auth/config.ts b/backend/src/auth/config.ts index d3aa3074..2b60cbd2 100644 --- a/backend/src/auth/config.ts +++ b/backend/src/auth/config.ts @@ -10,6 +10,7 @@ import type { Permission, Role, } from "./types.js"; +import { parseConfiguredTransportUrl } from "./url-policy.js"; export type { AuthenticationConfig, @@ -81,25 +82,12 @@ function readBoundedConfig(path: string): { source: string; identity: FileIdenti } } -function loopbackHost(host: string): boolean { - return host === "::1" || /^127(?:\.\d{1,3}){3}$/.test(host); -} - function validOrigin(value: string, httpLoopbackAllowed: boolean): boolean { - try { - const url = new URL(value); - return (url.protocol === "https:" || (httpLoopbackAllowed && url.protocol === "http:" && loopbackHost(url.hostname))) - && url.username.length === 0 && url.password.length === 0 && url.pathname === "/" - && url.search.length === 0 && url.hash.length === 0; - } catch { return false; } + return parseConfiguredTransportUrl(value, { allowLoopbackHttp: httpLoopbackAllowed, originOnly: true }) !== undefined; } function validIssuer(value: string): boolean { - try { - const url = new URL(value); - return url.protocol === "https:" && url.username.length === 0 && url.password.length === 0 - && url.search.length === 0 && url.hash.length === 0; - } catch { return false; } + return parseConfiguredTransportUrl(value, { allowLoopbackHttp: false }) !== undefined; } function validUsersFile(value: string): boolean { diff --git a/backend/src/auth/oidc-client.ts b/backend/src/auth/oidc-client.ts index 5af6b7b8..8a12dadf 100644 --- a/backend/src/auth/oidc-client.ts +++ b/backend/src/auth/oidc-client.ts @@ -7,6 +7,7 @@ import { type Configuration, } from "openid-client"; import { constants, createPublicKey, verify as verifySignature } from "node:crypto"; +import { parseConfiguredTransportUrl } from "./url-policy.js"; export interface OidcIdentity { issuer: string; @@ -49,14 +50,15 @@ const MAX_JWKS_TIMEOUT_MS = 30_000; const text = (value: unknown, maximum = 2048): value is string => typeof value === "string" && value.length > 0 && value.length <= maximum && !/\p{Cc}/u.test(value); -function configuredUrl(value: string): URL { - let url: URL; - try { - url = new URL(value); - } catch { - throw new OidcProtocolError(); - } - if (url.protocol !== "https:" || url.username || url.password || url.search || url.hash) throw new OidcProtocolError(); +function configuredHttpsUrl(value: string): URL { + const url = parseConfiguredTransportUrl(value, { allowLoopbackHttp: false }); + if (!url) throw new OidcProtocolError(); + return url; +} + +function configuredCallbackUrl(value: string): URL { + const url = parseConfiguredTransportUrl(value, { allowLoopbackHttp: true }); + if (!url) throw new OidcProtocolError(); return url; } @@ -180,29 +182,30 @@ async function readWithAbort( } async function boundedJwksBody(response: Response, signal: AbortSignal): Promise { - const declaredLength = response.headers.get("content-length"); - if (declaredLength !== null) { - if (!/^\d+$/.test(declaredLength)) throw new OidcProtocolError(); - const length = Number(declaredLength); - if (!Number.isSafeInteger(length) || length > MAX_JWKS_BYTES) throw new OidcProtocolError(); - } if (!response.body) throw new OidcProtocolError(); const reader = response.body.getReader(); const chunks: Buffer[] = []; let total = 0; + let completed = false; try { + if (!response.ok) throw new OidcProtocolError(); + const declaredLength = response.headers.get("content-length"); + if (declaredLength !== null) { + if (!/^\d+$/.test(declaredLength)) throw new OidcProtocolError(); + const length = Number(declaredLength); + if (!Number.isSafeInteger(length) || length > MAX_JWKS_BYTES) throw new OidcProtocolError(); + } while (true) { const { done, value } = await readWithAbort(reader, signal); if (done) break; - if (value.byteLength > MAX_JWKS_BYTES - total) { - await reader.cancel().catch(() => undefined); - throw new OidcProtocolError(); - } + if (value.byteLength > MAX_JWKS_BYTES - total) throw new OidcProtocolError(); total += value.byteLength; chunks.push(Buffer.from(value)); } + completed = true; } finally { - reader.releaseLock(); + if (!completed) await reader.cancel().catch(() => undefined); + try { reader.releaseLock(); } catch { /* cancellation already made the response unusable */ } } signal.throwIfAborted(); try { @@ -235,7 +238,6 @@ async function verifyIdTokenSignature( response = await (options.fetch ?? globalThis.fetch)(jwksUrl, { headers: { accept: "application/json" }, redirect: "error", signal: controller.signal, }); - if (!response.ok) throw new OidcProtocolError(); const body = await boundedJwksBody(response, controller.signal); const parsed = JSON.parse(body) as { keys?: unknown }; if (!Array.isArray(parsed.keys) || parsed.keys.length === 0 || parsed.keys.length > 16) throw new OidcProtocolError(); @@ -268,8 +270,8 @@ async function verifyIdTokenSignature( } export function createOidcProtocol(options: OidcProtocolOptions): OidcProtocol { - const issuerUrl = configuredUrl(options.issuer); - const callbackUrl = configuredUrl(options.callbackUrl); + const issuerUrl = configuredHttpsUrl(options.issuer); + const callbackUrl = configuredCallbackUrl(options.callbackUrl); if (!text(options.clientId, 512) || !text(options.clientSecret, 4096) || !text(options.groupsClaim, 128) || options.scopes.length === 0 || options.scopes.length > 16 || options.scopes.some((scope) => !text(scope, 128)) diff --git a/backend/src/auth/routes.ts b/backend/src/auth/routes.ts index 9c414b61..81a62337 100644 --- a/backend/src/auth/routes.ts +++ b/backend/src/auth/routes.ts @@ -2,7 +2,7 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; import { createHash, randomBytes, timingSafeEqual } from "node:crypto"; import type { AuthenticationConfigProvider, LoadedAuthConfig, OidcAuthenticationConfig, Role } from "./types.js"; import type { LocalUserRecord, LocalUserRegistry } from "./local-registry.js"; -import type { AuthSessionStore } from "./session-store.js"; +import { OidcStateCapacityError, type AuthSessionStore } from "./session-store.js"; import { rolesToPermissions } from "./config.js"; import { captureAuthConfigSnapshot, getPrincipal, requireExactOrigin, sessionCookieName } from "./auth.js"; import { requirePermission, isPrincipalContext } from "./authorization.js"; @@ -15,6 +15,7 @@ const REMEMBER_COOKIE_SECONDS = 2_592_000; const MAX_USERNAME_LENGTH = 64; const MAX_PASSWORD_LENGTH = 1024; const MAX_LIMIT_ENTRIES = 10_000; +const MAX_OIDC_INITIATIONS_PER_ADDRESS = 20; const MAX_OIDC_CALLBACK_QUERY_LENGTH = 4096; const OIDC_CALLBACK_PATH = "/api/auth/oidc/callback"; const OIDC_TRANSACTION_COOKIE = "__Host-thothii_oidc_tx"; @@ -38,6 +39,44 @@ interface LoginPayload { remember: boolean; } +function countActiveAttempts( + bucket: ReadonlyMap, + key: string, + now: number, +): number { + const attempts = bucket.get(key); + if (!attempts) return 0; + const earliest = now - TEN_MINUTES_MS; + let count = 0; + for (const timestamp of attempts) { + if (timestamp > earliest) count += 1; + } + return count; +} + +function pruneExpiredAttempts(bucket: Map, now: number): void { + const earliest = now - TEN_MINUTES_MS; + for (const [key, attempts] of bucket) { + const active = attempts.filter((timestamp) => timestamp > earliest); + if (active.length === 0) bucket.delete(key); + else if (active.length !== attempts.length) bucket.set(key, active); + } +} + +function canRecordAttempt( + bucket: ReadonlyMap, + key: string, + limit: number, + maximumEntries: number, +): boolean { + return (bucket.get(key)?.length ?? 0) < limit + && (bucket.has(key) || bucket.size < maximumEntries); +} + +function appendAttempt(bucket: Map, key: string, now: number): void { + bucket.set(key, [...(bucket.get(key) ?? []), now]); +} + export class LoginFailureLimiter { private readonly usernames = new Map(); private readonly addresses = new Map(); @@ -48,42 +87,34 @@ export class LoginFailureLimiter { } isLimited(username: string, address: string, now = Date.now()): boolean { - return this.countActive(this.usernames, username, now) >= 10 - || this.countActive(this.addresses, address, now) >= 20; + return countActiveAttempts(this.usernames, username, now) >= 10 + || countActiveAttempts(this.addresses, address, now) >= 20; } recordFailure(username: string, address: string, now = Date.now()): boolean { - this.pruneExpired(this.usernames, now); - this.pruneExpired(this.addresses, now); - const usernameAttempts = this.usernames.get(username) ?? []; - const addressAttempts = this.addresses.get(address) ?? []; - if (usernameAttempts.length >= 10 || addressAttempts.length >= 20) return false; - if ((!this.usernames.has(username) && this.usernames.size >= this.maximumEntries) - || (!this.addresses.has(address) && this.addresses.size >= this.maximumEntries)) return false; - - this.usernames.set(username, [...usernameAttempts, now]); - this.addresses.set(address, [...addressAttempts, now]); + pruneExpiredAttempts(this.usernames, now); + pruneExpiredAttempts(this.addresses, now); + if (!canRecordAttempt(this.usernames, username, 10, this.maximumEntries) + || !canRecordAttempt(this.addresses, address, 20, this.maximumEntries)) return false; + appendAttempt(this.usernames, username, now); + appendAttempt(this.addresses, address, now); return true; } +} - private countActive(bucket: ReadonlyMap, key: string, now: number): number { - const attempts = bucket.get(key); - if (!attempts) return 0; - const earliest = now - TEN_MINUTES_MS; - let count = 0; - for (const timestamp of attempts) { - if (timestamp > earliest) count += 1; - } - return count; - } +class OidcInitiationLimiter { + private readonly addresses = new Map(); - private pruneExpired(bucket: Map, now: number): void { - const earliest = now - TEN_MINUTES_MS; - for (const [key, attempts] of bucket) { - const active = attempts.filter((timestamp) => timestamp > earliest); - if (active.length === 0) bucket.delete(key); - else if (active.length !== attempts.length) bucket.set(key, active); - } + consume(address: string, now = Date.now()): boolean { + pruneExpiredAttempts(this.addresses, now); + if (!canRecordAttempt( + this.addresses, + address, + MAX_OIDC_INITIATIONS_PER_ADDRESS, + MAX_LIMIT_ENTRIES, + )) return false; + appendAttempt(this.addresses, address, now); + return true; } } @@ -120,6 +151,7 @@ async function unavailableAfterDummy( export function registerAuthRoutes(app: FastifyInstance, deps: AuthRouteDependencies): void { const limiter = new LoginFailureLimiter(); + const oidcInitiationLimiter = new OidcInitiationLimiter(); const verificationGate = new VerificationGate(); app.get("/auth/config", async (request, reply) => { @@ -189,6 +221,7 @@ export function registerAuthRoutes(app: FastifyInstance, deps: AuthRouteDependen }); app.get("/auth/oidc/login", async (request, reply) => { + if (!oidcInitiationLimiter.consume(boundedAddress(request.ip))) return loginLimited(reply); const loaded = captureAuthConfigSnapshot(request, deps.authentication); const configured = currentOidcConfig(loaded, deps); if (!configured || !deps.sessionStore) { @@ -216,8 +249,9 @@ export function registerAuthRoutes(app: FastifyInstance, deps: AuthRouteDependen clearOidcTransactionCookie(reply); return unavailable(reply); } - } catch { + } catch (error) { clearOidcTransactionCookie(reply); + if (error instanceof OidcStateCapacityError) return loginLimited(reply); return unavailable(reply); } }); @@ -421,7 +455,9 @@ function oidcCallbackUrl( request: FastifyRequest, publicUrl: string | undefined, ): { currentUrl?: URL; state?: string } { - if (request.url.length > MAX_OIDC_CALLBACK_QUERY_LENGTH) return {}; + if (request.url.length > MAX_OIDC_CALLBACK_QUERY_LENGTH) { + return { state: oversizedOidcCallbackState(request.url) }; + } let supplied: URL; try { supplied = new URL(request.url, "http://callback.invalid"); @@ -452,6 +488,25 @@ function oidcCallbackUrl( return { currentUrl: target, state }; } +function oversizedOidcCallbackState(rawUrl: string): string | undefined { + const prefix = "/auth/oidc/callback?"; + if (!rawUrl.startsWith(prefix)) return undefined; + const boundedQuery = rawUrl.slice(prefix.length, MAX_OIDC_CALLBACK_QUERY_LENGTH); + let offset = 0; + while (offset < boundedQuery.length) { + const separator = boundedQuery.indexOf("&", offset); + const end = separator === -1 ? boundedQuery.length : separator; + const parameter = boundedQuery.slice(offset, end); + if (parameter.startsWith("state=")) { + const value = parameter.slice("state=".length); + if (OIDC_VALUE_PATTERN.test(value)) return value; + } + if (separator === -1) break; + offset = separator + 1; + } + return undefined; +} + function oidcCallbackFailed(reply: FastifyReply) { return reply.code(401).send({ code: "oidc_callback_failed", error: "OIDC sign-in could not be completed" }); } diff --git a/backend/src/auth/session-store.ts b/backend/src/auth/session-store.ts index af6ceae7..5d4a5e84 100644 --- a/backend/src/auth/session-store.ts +++ b/backend/src/auth/session-store.ts @@ -31,12 +31,15 @@ const TOKEN_BYTES = 32; const TOKEN_PATTERN = /^[A-Za-z0-9_-]{43}$/; const DIGEST_FILENAME_PATTERN = /^[a-f0-9]{64}\.json$/; const CLAIM_FILENAME_PATTERN = /^[a-f0-9]{64}\.claim$/; +const OIDC_SLOT_FILENAME_PATTERN = /^slot-(\d{2})\.json$/; const PRIVATE_DIRECTORY_MODE = 0o700; const PRIVATE_FILE_MODE = 0o600; const MAX_SESSION_RECORD_BYTES = 16 * 1024; const MAX_OIDC_STATE_RECORD_BYTES = 8 * 1024; +const MAX_OIDC_SLOT_RECORD_BYTES = 512; const MAX_TTL_MS = 365 * 24 * 60 * 60 * 1000; const OIDC_STATE_TTL_MS = 10 * 60 * 1000; +const OIDC_STATE_CAPACITY = 64; const TOUCH_INTERVAL_MS = 5 * 60 * 1000; const CSRF_CONTEXT = Buffer.from("thothii-csrf-v1", "utf8"); const EMPTY_HKDF_SALT = Buffer.alloc(0); @@ -96,6 +99,12 @@ export class AuthSessionOperationalError extends Error { } } +export class OidcStateCapacityError extends Error { + constructor() { + super("auth_oidc_state_capacity"); + } +} + /** * The route layer supplies the current installation revision and local-registry lookup. * Supplying this hook makes every resolve an authorization-generation check. @@ -119,6 +128,8 @@ export interface AuthSessionStore { /** Narrow test seam for the native Windows tht-backed storage adaptor. */ export interface FileAuthSessionStoreOptions { windowsStorageBridge?: WindowsAuthStorageBridge; + /** Test-only capacity seam; production always uses the fixed 64-state bound. */ + oidcStateCapacity?: number; } interface FileIdentity { @@ -194,6 +205,7 @@ const oidcStateRecordSchema = z.strictObject({ authConfigRevision: z.string().regex(/^[a-f0-9]{64}$/), issuer: z.string().min(1).max(2048).refine((value) => !/\p{Cc}/u.test(value)), browserTransactionDigest: z.string().regex(/^[a-f0-9]{64}$/), + capacitySlot: z.number().int().min(0).max(OIDC_STATE_CAPACITY - 1).optional(), createdAt: timestamp, expiresAt: timestamp, }).superRefine((record, context) => { @@ -202,6 +214,11 @@ const oidcStateRecordSchema = z.strictObject({ context.addIssue({ code: "custom", message: "invalid OIDC state lifetime" }); } }); +const oidcSlotRecordSchema = z.strictObject({ + version: z.literal(1), + stateFilename: z.string().regex(DIGEST_FILENAME_PATTERN), + expiresAt: timestamp, +}); const sessionInputSchema = z.strictObject({ principal: z.strictObject({ issuer: text, @@ -269,8 +286,21 @@ function claimFilename(filename: string): string { return filename.slice(0, -".json".length) + ".claim"; } +function oidcSlotFilename(index: number): string { + if (!Number.isInteger(index) || index < 0 || index >= OIDC_STATE_CAPACITY) throw invalid(); + return `slot-${String(index).padStart(2, "0")}.json`; +} + +function oidcSlotIndex(filename: string): number | undefined { + const match = OIDC_SLOT_FILENAME_PATTERN.exec(filename); + if (!match) return undefined; + const index = Number(match[1]); + return Number.isInteger(index) && index >= 0 && index < OIDC_STATE_CAPACITY ? index : undefined; +} + function assertFilename(filename: string): void { - if (!DIGEST_FILENAME_PATTERN.test(filename) && !CLAIM_FILENAME_PATTERN.test(filename)) throw invalid(); + if (!DIGEST_FILENAME_PATTERN.test(filename) && !CLAIM_FILENAME_PATTERN.test(filename) + && oidcSlotIndex(filename) === undefined) throw invalid(); } function filePath(directory: string, filename: string): string { @@ -555,6 +585,16 @@ function parseOidcStateRecord(source: string): OidcStateRecord { } } +type OidcSlotRecord = z.infer; + +function parseOidcSlotRecord(source: string): OidcSlotRecord { + try { + return oidcSlotRecordSchema.parse(JSON.parse(source)); + } catch { + throw invalid(); + } +} + function parseWindowsRecord(contents: Buffer, maximumBytes: number, parse: (source: string) => T): T { if (!Buffer.isBuffer(contents) || contents.length === 0 || contents.length > maximumBytes) throw invalid(); try { @@ -569,6 +609,13 @@ interface OidcStateClaim { claimIdentity: FileIdentity; } +interface StoredOidcSlot { + filename: string; + index: number; + record: OidcSlotRecord; + identity?: FileIdentity; +} + function inspectOidcStateClaim( directory: string, filename: string, @@ -660,7 +707,7 @@ function removeClaimedOidcState(directory: string, filename: string, claim: Oidc if (!removeTrusted(directory, claimFilename(filename), claim.claimIdentity)) throw invalid(); } -function serialize(record: AuthSessionRecord | OidcStateRecord, maximumBytes: number): Buffer { +function serialize(record: AuthSessionRecord | OidcStateRecord | OidcSlotRecord, maximumBytes: number): Buffer { const contents = Buffer.from(`${JSON.stringify(record)}\n`, "utf8"); if (contents.length > maximumBytes) throw invalid(); return contents; @@ -752,6 +799,10 @@ export function createFileAuthSessionStore( validity?: AuthSessionValidity, options: FileAuthSessionStoreOptions = {}, ): AuthSessionStore { + const oidcStateCapacity = options.oidcStateCapacity ?? OIDC_STATE_CAPACITY; + if (!Number.isInteger(oidcStateCapacity) || oidcStateCapacity < 1 || oidcStateCapacity > OIDC_STATE_CAPACITY) { + throw invalid(); + } const windowsStorage = process.platform === "win32" ? options.windowsStorageBridge ?? createWindowsAuthStorageBridge() : undefined; @@ -761,6 +812,110 @@ export function createFileAuthSessionStore( return windowsStorage; } + async function oidcStorageEntries(): Promise { + const entries = process.platform === "win32" + ? (await requiredWindowsStorage().list(root, "oidc")).map((entry) => entry.name) + : (() => { + try { + return readdirSync(storageDirectories(root).oidc); + } catch { + throw invalid(); + } + })(); + if (entries.some((entry) => !DIGEST_FILENAME_PATTERN.test(entry) + && !CLAIM_FILENAME_PATTERN.test(entry) && oidcSlotIndex(entry) === undefined)) throw invalid(); + return entries; + } + + async function storedOidcSlots(suppliedEntries?: string[]): Promise { + const entries = suppliedEntries ?? await oidcStorageEntries(); + const slots: StoredOidcSlot[] = []; + const stateFilenames = new Set(); + for (const filename of entries) { + const index = oidcSlotIndex(filename); + if (index === undefined) continue; + if (process.platform === "win32") { + const contents = await requiredWindowsStorage().read(root, "oidc", filename); + if (!contents) continue; + const record = parseWindowsRecord(contents, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord); + if (stateFilenames.has(record.stateFilename)) throw invalid(); + stateFilenames.add(record.stateFilename); + slots.push({ filename, index, record }); + continue; + } + let trusted: TrustedFile | undefined; + let lastError: unknown; + for (const retryDelayMs of [0, 1, 2, 4, 8, 16, 32]) { + if (retryDelayMs > 0) await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); + try { + trusted = readTrusted( + storageDirectories(root).oidc, + filename, + MAX_OIDC_SLOT_RECORD_BYTES, + parseOidcSlotRecord, + ); + lastError = undefined; + break; + } catch (error) { + lastError = error; + } + } + if (lastError !== undefined) throw invalid(); + if (!trusted) continue; + if (stateFilenames.has(trusted.value.stateFilename)) throw invalid(); + stateFilenames.add(trusted.value.stateFilename); + slots.push({ filename, index, record: trusted.value, identity: trusted.identity }); + } + return slots; + } + + async function removeOidcSlot(slot: StoredOidcSlot): Promise { + if (process.platform === "win32") { + const current = await requiredWindowsStorage().read(root, "oidc", slot.filename); + if (!current) return; + const record = parseWindowsRecord(current, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord); + if (record.stateFilename !== slot.record.stateFilename || record.expiresAt !== slot.record.expiresAt + || !await requiredWindowsStorage().remove(root, "oidc", slot.filename)) throw invalid(); + return; + } + if (!slot.identity || !removeTrusted(storageDirectories(root).oidc, slot.filename, slot.identity)) throw invalid(); + } + + async function releaseOidcSlot(index: number | undefined, stateFilename: string): Promise { + if (index === undefined) return; + const filename = oidcSlotFilename(index); + const slot = (await storedOidcSlots([filename]))[0]; + if (!slot || slot.record.stateFilename !== stateFilename) throw invalid(); + await removeOidcSlot(slot); + } + + async function reserveOidcSlot(stateFilename: string, expiresAt: string): Promise { + const entries = await oidcStorageEntries(); + const slots = await storedOidcSlots(entries); + const representedStates = new Set(slots.map((slot) => slot.record.stateFilename)); + const legacyStates = new Set(); + for (const entry of entries) { + const filename = CLAIM_FILENAME_PATTERN.test(entry) + ? `${entry.slice(0, -".claim".length)}.json` + : entry; + if (DIGEST_FILENAME_PATTERN.test(filename) && !representedStates.has(filename)) legacyStates.add(filename); + } + const availableSlotCount = oidcStateCapacity - legacyStates.size; + if (availableSlotCount <= 0) throw new OidcStateCapacityError(); + const occupied = new Set(slots.map((slot) => slot.index)); + const record: OidcSlotRecord = { version: 1, stateFilename, expiresAt }; + const contents = serialize(record, MAX_OIDC_SLOT_RECORD_BYTES); + for (let index = 0; index < availableSlotCount; index += 1) { + if (occupied.has(index)) continue; + const filename = oidcSlotFilename(index); + const created = process.platform === "win32" + ? await requiredWindowsStorage().create(root, "oidc", filename, contents) + : writeExclusive(storageDirectories(root).oidc, filename, contents); + if (created) return index; + } + throw new OidcStateCapacityError(); + } + async function createSession(input: SessionCreateInput, now = new Date()): Promise { const nowMs = dateMilliseconds(now); let validated: z.infer; @@ -931,32 +1086,33 @@ export function createFileAuthSessionStore( } const expiresMs = nowMs + OIDC_STATE_TTL_MS; if (!Number.isSafeInteger(expiresMs)) throw invalid(); - const record: OidcStateRecord = { - version: 1, - nonce: validated.nonce, - codeVerifier: validated.codeVerifier, - returnTo: validated.returnTo, - authConfigRevision: validated.authConfigRevision, - issuer: validated.issuer, - browserTransactionDigest: validated.browserTransactionDigest, - createdAt: isoAt(nowMs), - expiresAt: isoAt(expiresMs), - }; - const contents = serialize(record, MAX_OIDC_STATE_RECORD_BYTES); - if (process.platform === "win32") { - const bridge = requiredWindowsStorage(); + return withLock(lockKey(root, "oidc", "capacity"), async () => { + await prune(now); for (let attempt = 0; attempt < 8; attempt += 1) { const state = randomBytes(TOKEN_BYTES).toString("base64url"); - if (await bridge.create(root, "oidc", digestFilename(state), contents)) return { state, record }; + const filename = digestFilename(state); + const capacitySlot = await reserveOidcSlot(filename, isoAt(expiresMs)); + const record: OidcStateRecord = { + version: 1, + nonce: validated.nonce, + codeVerifier: validated.codeVerifier, + returnTo: validated.returnTo, + authConfigRevision: validated.authConfigRevision, + issuer: validated.issuer, + browserTransactionDigest: validated.browserTransactionDigest, + capacitySlot, + createdAt: isoAt(nowMs), + expiresAt: isoAt(expiresMs), + }; + const contents = serialize(record, MAX_OIDC_STATE_RECORD_BYTES); + const created = process.platform === "win32" + ? await requiredWindowsStorage().create(root, "oidc", filename, contents) + : writeExclusive(storageDirectories(root).oidc, filename, contents); + if (created) return { state, record }; + await releaseOidcSlot(capacitySlot, filename); } throw invalid(); - } - const directories = storageDirectories(root); - for (let attempt = 0; attempt < 8; attempt += 1) { - const state = randomBytes(TOKEN_BYTES).toString("base64url"); - if (writeExclusive(directories.oidc, digestFilename(state), contents)) return { state, record }; - } - throw invalid(); + }); } async function consumeOidcState(state: string, now = new Date()): Promise { @@ -968,6 +1124,7 @@ export function createFileAuthSessionStore( const contents = await requiredWindowsStorage().claimConsume(root, filename); if (!contents) return undefined; const record = parseWindowsRecord(contents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); + await releaseOidcSlot(record.capacitySlot, filename); return oidcStateExpired(record, nowMs) ? undefined : record; } const directories = storageDirectories(root); @@ -977,9 +1134,11 @@ export function createFileAuthSessionStore( if (!claim) return undefined; if (oidcStateExpired(claim.state.value, nowMs)) { removeClaimedOidcState(directories.oidc, filename, claim); + await releaseOidcSlot(claim.state.value.capacitySlot, filename); return undefined; } removeClaimedOidcState(directories.oidc, filename, claim); + await releaseOidcSlot(claim.state.value.capacitySlot, filename); return claim.state.value; }); } @@ -1002,7 +1161,8 @@ export function createFileAuthSessionStore( const claimEntries = new Map(oidcEntries .filter((entry) => CLAIM_FILENAME_PATTERN.test(entry.name)) .map((entry) => [entry.name, entry])); - if (stateNames.size + claimEntries.size !== oidcEntries.length) throw invalid(); + const slotEntries = oidcEntries.filter((entry) => oidcSlotIndex(entry.name) !== undefined); + if (stateNames.size + claimEntries.size + slotEntries.length !== oidcEntries.length) throw invalid(); for (const filename of stateNames) { const claim = claimFilename(filename); const contents = claimEntries.has(claim) @@ -1023,6 +1183,25 @@ export function createFileAuthSessionStore( if (nowMs >= entry.modifiedUnixMs + OIDC_STATE_TTL_MS && await bridge.remove(root, "oidc", claim)) removed += 1; } + for (const entry of slotEntries) { + const contents = await bridge.read(root, "oidc", entry.name); + if (!contents) continue; + const slot = parseWindowsRecord(contents, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord); + if (nowMs < Date.parse(slot.expiresAt)) continue; + const claim = claimFilename(slot.stateFilename); + const stateContents = claimEntries.has(claim) + ? await bridge.readClaim(root, slot.stateFilename) + : await bridge.read(root, "oidc", slot.stateFilename); + if (stateContents) { + const state = parseWindowsRecord(stateContents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); + if (!oidcStateExpired(state, nowMs)) throw invalid(); + const didRemove = claimEntries.has(claim) + ? await bridge.removeClaim(root, slot.stateFilename) + : await bridge.remove(root, "oidc", slot.stateFilename); + if (didRemove) removed += 1; + } + if (!await bridge.remove(root, "oidc", entry.name)) throw invalid(); + } return removed; } const directories = storageDirectories(root); @@ -1103,6 +1282,30 @@ export function createFileAuthSessionStore( } }); } + const slots = await storedOidcSlots(oidcEntries.filter((entry) => oidcSlotIndex(entry) !== undefined)); + for (const slot of slots) { + if (nowMs < Date.parse(slot.record.expiresAt)) continue; + await withLock(lockKey(root, "oidc", slot.record.stateFilename), async () => { + const claim = inspectOidcStateClaim(directories.oidc, slot.record.stateFilename); + if (claim && claim !== "orphan") { + if (!oidcStateExpired(claim.state.value, nowMs)) throw invalid(); + removeClaimedOidcState(directories.oidc, slot.record.stateFilename, claim); + removed += 1; + } else if (!claim) { + const trusted = readTrusted( + directories.oidc, + slot.record.stateFilename, + MAX_OIDC_STATE_RECORD_BYTES, + parseOidcStateRecord, + ); + if (trusted) { + if (!oidcStateExpired(trusted.value, nowMs)) throw invalid(); + if (removeTrusted(directories.oidc, slot.record.stateFilename, trusted.identity)) removed += 1; + } + } + await removeOidcSlot(slot); + }); + } return removed; } diff --git a/backend/src/auth/types.ts b/backend/src/auth/types.ts index 187d1092..55ec5cf2 100644 --- a/backend/src/auth/types.ts +++ b/backend/src/auth/types.ts @@ -82,6 +82,7 @@ export interface OidcStateRecord { authConfigRevision: string; issuer: string; browserTransactionDigest: string; + capacitySlot?: number; createdAt: string; expiresAt: string; } diff --git a/backend/src/auth/url-policy.ts b/backend/src/auth/url-policy.ts new file mode 100644 index 00000000..7143e996 --- /dev/null +++ b/backend/src/auth/url-policy.ts @@ -0,0 +1,42 @@ +export interface ConfiguredTransportUrlOptions { + allowLoopbackHttp: boolean; + originOnly?: boolean; +} + +function canonicalLoopbackAuthority(value: string): boolean { + const match = /^http:\/\/([^/?#]+)(?:[/?#]|$)/.exec(value); + if (!match) return false; + const authority = match[1]; + let port: string | undefined; + if (authority.startsWith("[")) { + const ipv6 = /^(\[::1\])(?::([^:]+))?$/.exec(authority); + if (!ipv6) return false; + port = ipv6[2]; + } else { + const ipv4 = /^([^:]+)(?::([^:]+))?$/.exec(authority); + if (!ipv4) return false; + const octets = ipv4[1].split("."); + if (octets.length !== 4 || octets.some((octet) => !/^(?:0|[1-9]\d{0,2})$/.test(octet) + || Number(octet) > 255) || Number(octets[0]) !== 127) return false; + port = ipv4[2]; + } + return port === undefined || (/^(?:0|[1-9]\d{0,4})$/.test(port) && Number(port) <= 65_535); +} + +export function parseConfiguredTransportUrl( + value: string, + options: ConfiguredTransportUrlOptions, +): URL | undefined { + let url: URL; + try { + url = new URL(value); + } catch { + return undefined; + } + if (url.username || url.password || url.search || url.hash || (options.originOnly && url.pathname !== "/")) { + return undefined; + } + if (url.protocol === "https:") return url; + if (options.allowLoopbackHttp && url.protocol === "http:" && canonicalLoopbackAuthority(value)) return url; + return undefined; +} diff --git a/backend/src/auth/windows-auth-storage.ts b/backend/src/auth/windows-auth-storage.ts index d840956b..c9c00332 100644 --- a/backend/src/auth/windows-auth-storage.ts +++ b/backend/src/auth/windows-auth-storage.ts @@ -12,6 +12,7 @@ const MAX_ENTRIES = 256; const TIMEOUT_MS = 5_000; const DIGEST_FILENAME = /^[a-f0-9]{64}\.json$/; const CLAIM_FILENAME = /^[a-f0-9]{64}\.claim$/; +const OIDC_SLOT_FILENAME = /^slot-(?:[0-5][0-9]|6[0-3])\.json$/; const invalid = (): Error => new Error("auth_session_store_invalid"); @@ -119,8 +120,10 @@ function validateRoot(root: string): void { || !win32.isAbsolute(root) || win32.normalize(root) !== root) throw invalid(); } -function validateFilename(filename: string, allowClaim = false): void { - if (typeof filename !== "string" || (!DIGEST_FILENAME.test(filename) && !(allowClaim && CLAIM_FILENAME.test(filename)))) throw invalid(); +function validateFilename(filename: string, allowClaim = false, allowOidcSlot = false): void { + if (typeof filename !== "string" || (!DIGEST_FILENAME.test(filename) + && !(allowClaim && CLAIM_FILENAME.test(filename)) + && !(allowOidcSlot && OIDC_SLOT_FILENAME.test(filename)))) throw invalid(); } function safeThtExecutable(value: string | undefined): string { @@ -151,7 +154,9 @@ function encodedRequest(request: BridgeRequest): Buffer { } else { if (request.filename === undefined) throw invalid(); const allowClaim = request.operation === "remove" && request.directory === "oidc"; - validateFilename(request.filename, allowClaim); + const allowOidcSlot = request.directory === "oidc" + && (request.operation === "create" || request.operation === "read" || request.operation === "remove"); + validateFilename(request.filename, allowClaim, allowOidcSlot); if (request.contentBase64 !== undefined && request.operation !== "create" && request.operation !== "replace") throw invalid(); } if ((request.operation === "claim-consume" || request.operation === "read-claim" || request.operation === "remove-claim") @@ -349,7 +354,8 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge const response = await request({ version: PROTOCOL_VERSION, operation: "list", root, directory }); if (response.entries === undefined) throw invalid(); for (const entry of response.entries) { - if (!DIGEST_FILENAME.test(entry.name) && !(directory === "oidc" && CLAIM_FILENAME.test(entry.name))) throw invalid(); + if (!DIGEST_FILENAME.test(entry.name) && !(directory === "oidc" + && (CLAIM_FILENAME.test(entry.name) || OIDC_SLOT_FILENAME.test(entry.name)))) throw invalid(); } return response.entries.map((entry) => ({ name: entry.name, modifiedUnixMs: entry.modifiedUnixMs })); }, diff --git a/backend/test/auth-config.test.ts b/backend/test/auth-config.test.ts index 707c4dd4..7c78767c 100644 --- a/backend/test/auth-config.test.ts +++ b/backend/test/auth-config.test.ts @@ -122,9 +122,28 @@ test.each([ expect(() => loadAuthenticationConfig(writeFixture(value))).toThrow("authentication configuration is invalid"); }); -test("accepts the explicit loopback HTTP OIDC exception", () => { - expect(loadAuthenticationConfig(writeFixture(oidcConfig({ publicUrl: "http://127.0.0.1:8787" }))).value.mode) - .toBe("oidc"); +test.each([ + "http://127.0.0.1:8787", + "http://127.255.255.254:8787", + "http://[::1]:8787", +])("accepts the literal loopback HTTP OIDC exception %s", (publicUrl) => { + expect(loadAuthenticationConfig(writeFixture(oidcConfig({ publicUrl }))).value.mode).toBe("oidc"); +}); + +test.each([ + "http://localhost:8787", + "http://loopback.example.test:8787", + "http://user@127.0.0.1:8787", + "http://127.1:8787", + "http://127.0.0.01:8787", + "http://0177.0.0.1:8787", + "http://0x7f000001:8787", + "http://2130706433:8787", + "http://[::ffff:127.0.0.1]:8787", + "http://128.0.0.1:8787", +])("rejects non-canonical or non-loopback HTTP public URL %s", (publicUrl) => { + expect(() => loadAuthenticationConfig(writeFixture(oidcConfig({ publicUrl })))) + .toThrow("authentication configuration is invalid"); }); test("rejects unknown roles and requires exactly one admin group", () => { diff --git a/backend/test/auth-routes-oidc.test.ts b/backend/test/auth-routes-oidc.test.ts index af757929..f433939b 100644 --- a/backend/test/auth-routes-oidc.test.ts +++ b/backend/test/auth-routes-oidc.test.ts @@ -1,6 +1,6 @@ import Fastify from "fastify"; import cookie from "@fastify/cookie"; -import { afterEach, expect, test } from "vitest"; +import { afterEach, expect, test, vi } from "vitest"; import { registerAuthRoutes } from "../src/auth/routes.js"; import type { LoadedAuthConfig, OidcStateRecord } from "../src/auth/types.js"; import type { AuthSessionStore } from "../src/auth/session-store.js"; @@ -76,8 +76,10 @@ function fixture(options: { const creates: Array> = []; const createTimes: Array = []; const callbacks: URL[] = []; + let authorizationRequests = 0; const protocol: OidcProtocol = { authorizationUrl: async ({ state: received, nonce: receivedNonce, codeVerifier }) => { + authorizationRequests += 1; expect(received).toBe(state); expect(receivedNonce).toHaveLength(43); expect(codeVerifier).toHaveLength(43); @@ -134,6 +136,7 @@ function fixture(options: { createdApps.push(app); return { app, creates, createTimes, callbacks, stateInputs, + authorizationRequests: () => authorizationRequests, setConfig(next: LoadedAuthConfig) { loaded = next; }, setProtocolAvailable(available: boolean) { protocolAvailable = available; }, stateWasConsumed: () => storedState === undefined, @@ -161,6 +164,42 @@ async function finishOidcLogin( }); } +test("rate limits OIDC initiation before state creation and provider discovery", async () => { + const subject = fixture(); + + for (let attempt = 0; attempt < 20; attempt += 1) { + expect((await beginOidcLogin(subject)).response.statusCode).toBe(302); + } + const limited = await beginOidcLogin(subject); + + expect(limited.response.statusCode).toBe(429); + expect(limited.response.json()).toEqual({ + code: "login_rate_limited", + error: "Too many login attempts", + }); + expect(subject.stateInputs).toHaveLength(20); + expect(subject.authorizationRequests()).toBe(20); +}); + +test("OIDC initiation rate-limit capacity expires after ten minutes", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2030-01-02T03:04:05.000Z")); + const subject = fixture(); + try { + for (let attempt = 0; attempt < 20; attempt += 1) { + expect((await beginOidcLogin(subject)).response.statusCode).toBe(302); + } + expect((await beginOidcLogin(subject)).response.statusCode).toBe(429); + + vi.advanceTimersByTime(10 * 60_000 + 1); + + expect((await beginOidcLogin(subject)).response.statusCode).toBe(302); + expect(subject.authorizationRequests()).toBe(21); + } finally { + vi.useRealTimers(); + } +}); + test("creates digest-only bound state, maps exact groups, creates a cookie session, and redirects safely", async () => { const subject = fixture(); const { response: start, cookie } = await beginOidcLogin(subject); @@ -298,6 +337,20 @@ test.each([ expect(subject.callbacks).toEqual([]); }); +test("boundedly burns a canonical state from an oversized callback URL", async () => { + const subject = fixture(); + const { cookie } = await beginOidcLogin(subject); + const oversized = `state=${state}&code=good&padding=${"x".repeat(4096)}`; + + const failed = await finishOidcLogin(subject, cookie, oversized); + + expect(failed.statusCode).toBe(401); + expectTransactionCleared(failed); + expect(subject.stateWasConsumed()).toBe(true); + expect(subject.callbacks).toEqual([]); + expect((await finishOidcLogin(subject, cookie)).statusCode).toBe(401); +}); + test("rejects an empty direct groups claim without creating a session cookie", async () => { const subject = fixture({ identity: { issuer, subject: "user-123", groups: [], tokenExpiresAt: new Date(Date.now() + 60_000), diff --git a/backend/test/auth-session-store.test.ts b/backend/test/auth-session-store.test.ts index 8c800134..d6d3fbb3 100644 --- a/backend/test/auth-session-store.test.ts +++ b/backend/test/auth-session-store.test.ts @@ -8,6 +8,7 @@ import { mkdirSync, mkdtempSync, readFileSync, + readdirSync, realpathSync, renameSync, rmSync, @@ -60,6 +61,7 @@ import { createFileAuthSessionStore, deriveCsrfToken, type AuthSessionStore, + type FileAuthSessionStoreOptions, type SessionCreateInput, } from "../src/auth/session-store.js"; import { createWindowsAuthStorageBridge } from "../src/auth/windows-auth-storage.js"; @@ -104,11 +106,11 @@ function claimPath(rootPath: string, rawState: string): string { return join(rootPath, "oidc", `${createHash("sha256").update(rawState).digest("hex")}.claim`); } -function validStore(storageRoot: string): AuthSessionStore { +function validStore(storageRoot: string, options: FileAuthSessionStoreOptions = {}): AuthSessionStore { return createFileAuthSessionStore(storageRoot, { currentAuthConfigRevision: () => revision, findLocalUser: async () => validLocalUser, - }); + }, options); } async function create( @@ -180,6 +182,52 @@ async function isolatedOidcConsumer(storageRoot: string, state: string): Promise return { start: () => child.send("consume"), result }; } +async function isolatedOidcCreator(storageRoot: string, attempts: number): Promise<{ + start(): void; + result: Promise; +}> { + const child = fork(new URL("./fixtures/oidc-state-creator.mts", import.meta.url), [], { + cwd: process.cwd(), + execArgv: ["--import", "tsx"], + env: { + ...process.env, + THT_TEST_SESSION_ROOT: storageRoot, + THT_TEST_OIDC_ATTEMPTS: String(attempts), + THT_TEST_OIDC_NOW: base.toISOString(), + THT_TEST_OIDC_CAPACITY: "8", + }, + silent: true, + }); + const ready = new Promise((resolve, reject) => { + child.once("message", (message) => { + if (message === "ready") resolve(); + else reject(new Error("OIDC creator did not become ready")); + }); + child.once("error", reject); + child.once("exit", (code) => { + if (code !== null && code !== 0) reject(new Error("OIDC creator exited before ready")); + }); + }); + const result = new Promise((resolve, reject) => { + let outcome: { created: unknown; failed?: unknown; error?: unknown } | undefined; + child.on("message", (message) => { + if (message && typeof message === "object" && "created" in message) { + outcome = message as { created: unknown; failed?: unknown; error?: unknown }; + } + }); + child.on("error", reject); + child.on("exit", (code) => { + if (code !== 0 || outcome?.failed === true || typeof outcome?.created !== "number") { + reject(new Error(`OIDC creator failed: ${String(outcome?.error)}`)); + } else { + resolve(outcome.created); + } + }); + }); + await ready; + return { start: () => child.send("create"), result }; +} + describe("file-backed auth session store", () => { test("fails closed and revokes a session when constructed without validity dependencies", async () => { const storageRoot = root(); @@ -301,6 +349,49 @@ describe("file-backed auth session store", () => { .resolves.toBeUndefined(); }); + test("rejects an OIDC-state flood without deleting any valid live state", async () => { + const storageRoot = root(); + const store = validStore(storageRoot, { oidcStateCapacity: 8 }); + const created = await Promise.all(Array.from({ length: 8 }, (_unused, index) => + store.createOidcState(oidcInput(`n${String(index).padStart(42, "0")}`, `v${String(index).padStart(42, "0")}`), base))); + + await expect(store.createOidcState(oidcInput("x".repeat(43), "y".repeat(43)), base)) + .rejects.toThrow("auth_oidc_state_capacity"); + await expect(store.consumeOidcState(created[0].state, new Date(base.getTime() + 60_000))) + .resolves.toMatchObject({ nonce: "n" + "0".repeat(42) }); + expect(created.slice(1).every(({ state }) => existsSync(digestPath(storageRoot, "oidc", state)))).toBe(true); + }); + + test("prunes expired OIDC states before admitting a new transaction at capacity", async () => { + const storageRoot = root(); + const store = validStore(storageRoot, { oidcStateCapacity: 8 }); + const expired = await Promise.all(Array.from({ length: 8 }, (_unused, index) => + store.createOidcState(oidcInput(`n${String(index).padStart(42, "0")}`, `v${String(index).padStart(42, "0")}`), base))); + + const admitted = await store.createOidcState( + oidcInput("x".repeat(43), "y".repeat(43)), + new Date(base.getTime() + 10 * 60_000), + ); + + expect(expired.every(({ state }) => !existsSync(digestPath(storageRoot, "oidc", state)))).toBe(true); + expect(existsSync(digestPath(storageRoot, "oidc", admitted.state))).toBe(true); + }); + + test("bounds OIDC state creation across concurrent Node processes", async () => { + const storageRoot = root(); + const creators = await Promise.all([ + isolatedOidcCreator(storageRoot, 6), + isolatedOidcCreator(storageRoot, 6), + ]); + + creators.forEach(({ start }) => start()); + const winners = await Promise.all(creators.map(({ result }) => result)); + + expect(winners.reduce((sum, value) => sum + value, 0)).toBe(8); + const names = readdirSync(join(storageRoot, "oidc")); + expect(names.filter((name) => /^[a-f0-9]{64}\.json$/.test(name))).toHaveLength(8); + }); + test("fails closed when an OIDC state already has an atomic filesystem claim", async () => { const storageRoot = root(); const store = validStore(storageRoot); diff --git a/backend/test/fixtures/oidc-state-creator.mts b/backend/test/fixtures/oidc-state-creator.mts new file mode 100644 index 00000000..58acd0db --- /dev/null +++ b/backend/test/fixtures/oidc-state-creator.mts @@ -0,0 +1,41 @@ +import { createFileAuthSessionStore } from "../../src/auth/session-store.js"; + +const root = process.env.THT_TEST_SESSION_ROOT; +const attempts = Number(process.env.THT_TEST_OIDC_ATTEMPTS); +const now = new Date(process.env.THT_TEST_OIDC_NOW ?? "invalid"); +const capacity = Number(process.env.THT_TEST_OIDC_CAPACITY); +if (!root || !Number.isSafeInteger(attempts) || attempts < 1 || Number.isNaN(now.getTime()) + || !Number.isSafeInteger(capacity) || capacity < 1) process.exit(2); + +const store = createFileAuthSessionStore(root, undefined, { oidcStateCapacity: capacity }); +process.send?.("ready"); +process.once("message", async (message) => { + if (message !== "create") process.exit(3); + let created = 0; + try { + for (let attempt = 0; attempt < attempts; attempt += 1) { + try { + await store.createOidcState({ + nonce: `n${String(process.pid).padStart(10, "0")}${String(attempt).padStart(32, "0")}`, + codeVerifier: `v${String(process.pid).padStart(10, "0")}${String(attempt).padStart(32, "0")}`, + returnTo: "/", + authConfigRevision: "a".repeat(64), + issuer: "https://issuer.example.test", + browserTransactionDigest: "b".repeat(64), + }, now); + created += 1; + } catch (error) { + if (!(error instanceof Error) || error.message !== "auth_oidc_state_capacity") throw error; + } + } + process.send?.({ created }); + process.exit(0); + } catch (error) { + process.send?.({ + created, + failed: true, + error: error instanceof Error ? error.message : "non-error failure", + }); + process.exit(4); + } +}); diff --git a/backend/test/oidc-client.test.ts b/backend/test/oidc-client.test.ts index a800335c..3af76ff2 100644 --- a/backend/test/oidc-client.test.ts +++ b/backend/test/oidc-client.test.ts @@ -30,6 +30,7 @@ function protocol(options: { seen?: URL[]; jwksResponse?: (init?: RequestInit) => Response | Promise; jwksTimeoutMs?: number; + discoveryMetadata?: Record; } = {}) { const now = Math.floor(Date.now() / 1000); const claims = { @@ -56,6 +57,7 @@ function protocol(options: { grant_types_supported: ["authorization_code"], subject_types_supported: ["public"], id_token_signing_alg_values_supported: ["RS256"], + ...options.discoveryMetadata, }); } if (url.pathname === "/jwks") return options.jwksResponse ? await options.jwksResponse(init) : Response.json({ keys: [jwk] }); @@ -116,6 +118,47 @@ test("rejects non-HTTPS issuer configuration and a discovery issuer mismatch", a .rejects.toThrow(OidcProtocolError); }); +test.each([ + ["authorization", { authorization_endpoint: "http://127.0.0.1/authorize" }], + ["token", { token_endpoint: "http://127.0.0.1/token" }], + ["JWKS", { jwks_uri: "http://127.0.0.1/jwks" }], +])("keeps the discovered %s endpoint HTTPS-only", async (_label, discoveryMetadata) => { + const subject = protocol({ discoveryMetadata }); + await expect(subject.authorizationUrl({ state, nonce, codeVerifier: verifier })) + .rejects.toThrow(OidcProtocolError); +}); + +test.each([ + "http://127.0.0.1:3000/api/auth/oidc/callback", + "http://127.255.255.254/api/auth/oidc/callback", + "http://[::1]:3000/api/auth/oidc/callback", + "https://thothii.example.test/api/auth/oidc/callback", +])("accepts the configured callback URL %s", (configuredCallbackUrl) => { + expect(() => createOidcProtocol({ + issuer, clientId, clientSecret: "secret", callbackUrl: configuredCallbackUrl, + scopes: ["openid"], groupsClaim: "groups", + })).not.toThrow(); +}); + +test.each([ + "http://localhost/api/auth/oidc/callback", + "http://loopback.example.test/api/auth/oidc/callback", + "http://user@127.0.0.1/api/auth/oidc/callback", + "http://127.1/api/auth/oidc/callback", + "http://127.0.0.01/api/auth/oidc/callback", + "http://0177.0.0.1/api/auth/oidc/callback", + "http://0x7f000001/api/auth/oidc/callback", + "http://2130706433/api/auth/oidc/callback", + "http://[::ffff:127.0.0.1]/api/auth/oidc/callback", + "http://128.0.0.1/api/auth/oidc/callback", + "http://192.168.1.1/api/auth/oidc/callback", +])("rejects the non-canonical or non-loopback HTTP callback URL %s", (configuredCallbackUrl) => { + expect(() => createOidcProtocol({ + issuer, clientId, clientSecret: "secret", callbackUrl: configuredCallbackUrl, + scopes: ["openid"], groupsClaim: "groups", + })).toThrow(OidcProtocolError); +}); + test.each([ ["state", new URL(`${callbackUrl}?code=good&state=wrong`), {}], ["nonce", new URL(`${callbackUrl}?code=good&state=${state}`), { nonce: "wrong" }], @@ -151,6 +194,7 @@ test("aborts a hanging JWKS request at the configured timeout", async () => { test("rejects an oversized JWKS Content-Length before reading the body", async () => { let pulls = 0; + let cancelled = false; const body = new ReadableStream({ type: "bytes", pull(controller) { @@ -158,12 +202,43 @@ test("rejects an oversized JWKS Content-Length before reading the body", async ( controller.enqueue(new TextEncoder().encode("{}")); controller.close(); }, + cancel() { cancelled = true; }, }); const subject = protocol({ jwksResponse: () => new Response(body, { headers: { "content-length": String(1024 * 1024 + 1) } }), }); await expect(callback(subject)).rejects.toThrow(OidcProtocolError); expect(pulls).toBe(0); + expect(cancelled).toBe(true); + expect(body.locked).toBe(false); +}); + +test("cancels and releases a JWKS stream with an invalid Content-Length", async () => { + let cancelled = false; + const body = new ReadableStream({ + pull() { /* remains pending until the response is rejected and cancelled */ }, + cancel() { cancelled = true; }, + }); + const subject = protocol({ + jwksResponse: () => new Response(body, { headers: { "content-length": "not-a-number" } }), + }); + + await expect(callback(subject)).rejects.toThrow(OidcProtocolError); + expect(cancelled).toBe(true); + expect(body.locked).toBe(false); +}); + +test("cancels and releases a non-success JWKS response body", async () => { + let cancelled = false; + const body = new ReadableStream({ + pull() { /* remains pending until the response is rejected and cancelled */ }, + cancel() { cancelled = true; }, + }); + const subject = protocol({ jwksResponse: () => new Response(body, { status: 503 }) }); + + await expect(callback(subject)).rejects.toThrow(OidcProtocolError); + expect(cancelled).toBe(true); + expect(body.locked).toBe(false); }); test("stops streaming a JWKS response as soon as the byte limit is exceeded", async () => { @@ -186,6 +261,7 @@ test("stops streaming a JWKS response as soon as the byte limit is exceeded", as await expect(callback(subject)).rejects.toThrow(OidcProtocolError); expect(pulls).toBe(2); expect(cancelled).toBe(true); + expect(body.locked).toBe(false); }); test.each([ diff --git a/backend/test/windows-auth-storage.test.ts b/backend/test/windows-auth-storage.test.ts index 72460463..fb9a39bc 100644 --- a/backend/test/windows-auth-storage.test.ts +++ b/backend/test/windows-auth-storage.test.ts @@ -80,6 +80,42 @@ function bridgeForChild(child: FakeBridgeChild) { } describe("Windows auth-storage bridge", () => { + test("permits reservation slots only for OIDC record operations", async () => { + const requests: Array> = []; + const bridge = createWindowsAuthStorageBridge({ + thtExecutable: "C:\\tht.exe", + invoke: async ({ input }) => { + const request = JSON.parse(input.toString("utf8")) as Record; + requests.push(request); + const operation = request.operation; + const body = operation === "create" + ? { created: true } + : operation === "read" + ? { found: true, contentBase64: Buffer.from("slot").toString("base64") } + : operation === "remove" + ? { removed: true } + : { entries: [{ name: "slot-00.json", modifiedUnixMs: 1 }] }; + return { + code: 0, + stdout: Buffer.from(`${JSON.stringify({ version: 1, ok: true, ...body })}\n`), + stderr: Buffer.alloc(0), + }; + }, + }); + + await expect(bridge.create("C:\\auth", "oidc", "slot-00.json", Buffer.from("slot"))).resolves.toBe(true); + await expect(bridge.read("C:\\auth", "oidc", "slot-00.json")).resolves.toEqual(Buffer.from("slot")); + await expect(bridge.list("C:\\auth", "oidc")).resolves.toEqual([ + { name: "slot-00.json", modifiedUnixMs: 1 }, + ]); + await expect(bridge.remove("C:\\auth", "oidc", "slot-00.json")).resolves.toBe(true); + await expect(bridge.create("C:\\auth", "sessions", "slot-00.json", Buffer.from("slot"))) + .rejects.toThrow("auth_session_store_invalid"); + await expect(bridge.create("C:\\auth", "oidc", "slot-64.json", Buffer.from("slot"))) + .rejects.toThrow("auth_session_store_invalid"); + expect(requests).toHaveLength(4); + }); + test("uses hidden tht argv and sends record bytes only over bounded stdin", async () => { const calls: Array<{ executable: string; args: readonly string[]; input: Buffer; timeoutMs: number }> = []; const bridge = createWindowsAuthStorageBridge({ diff --git a/tools/tht/internal/authstorage/storage.go b/tools/tht/internal/authstorage/storage.go index 4766bf0c..427b3111 100644 --- a/tools/tht/internal/authstorage/storage.go +++ b/tools/tht/internal/authstorage/storage.go @@ -26,9 +26,10 @@ const ( ) var ( - digestFilename = regexp.MustCompile(`^[a-f0-9]{64}\.json$`) - claimFilename = regexp.MustCompile(`^[a-f0-9]{64}\.claim$`) - errInvalid = errors.New("auth storage request invalid") + digestFilename = regexp.MustCompile(`^[a-f0-9]{64}\.json$`) + claimFilename = regexp.MustCompile(`^[a-f0-9]{64}\.claim$`) + oidcSlotFilename = regexp.MustCompile(`^slot-(?:[0-5][0-9]|6[0-3])\.json$`) + errInvalid = errors.New("auth storage request invalid") ) type request struct { @@ -141,7 +142,7 @@ func execute(input request) (response, error) { return response{}, errInvalid } for _, entry := range entries { - if !digestFilename.MatchString(entry.Name) && !(input.Directory == "oidc" && claimFilename.MatchString(entry.Name)) { + if !digestFilename.MatchString(entry.Name) && !(input.Directory == "oidc" && (claimFilename.MatchString(entry.Name) || oidcSlotFilename.MatchString(entry.Name))) { return response{}, errInvalid } } @@ -176,11 +177,11 @@ func validOperationShape(input request) bool { noContents := input.ContentBase64 == "" switch input.Operation { case "create", "replace": - return digestFilename.MatchString(input.Filename) + return digestFilename.MatchString(input.Filename) || (input.Operation == "create" && input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename)) case "read": - return noContents && digestFilename.MatchString(input.Filename) + return noContents && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename))) case "remove": - return noContents && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && claimFilename.MatchString(input.Filename))) + return noContents && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && (claimFilename.MatchString(input.Filename) || oidcSlotFilename.MatchString(input.Filename)))) case "list": return input.Filename == "" && noContents case "claim-consume", "read-claim", "remove-claim": diff --git a/tools/tht/internal/authstorage/storage_test.go b/tools/tht/internal/authstorage/storage_test.go index 3111a0d9..434cdfe0 100644 --- a/tools/tht/internal/authstorage/storage_test.go +++ b/tools/tht/internal/authstorage/storage_test.go @@ -44,6 +44,28 @@ func TestProtocolCreatesReadsReplacesListsAndRemovesPrivateRecord(t *testing.T) } } +func TestProtocolPermitsBoundedReservationSlotsOnlyForOIDCRecords(t *testing.T) { + root := filepath.Join(privateTestRoot(t), "auth") + slot := "slot-00.json" + contents := base64.StdEncoding.EncodeToString([]byte("reservation")) + + if created := runRequest(t, request{Version: 1, Operation: "create", Root: root, Directory: "oidc", Filename: slot, ContentBase64: contents}); !created.Created { + t.Fatal("OIDC slot create did not report success") + } + if read := runRequest(t, request{Version: 1, Operation: "read", Root: root, Directory: "oidc", Filename: slot}); !read.Found || decodeContent(t, read) != "reservation" { + t.Fatalf("OIDC slot read = %#v", read) + } + listed := runRequest(t, request{Version: 1, Operation: "list", Root: root, Directory: "oidc"}) + if listed.Entries == nil || len(*listed.Entries) != 1 || (*listed.Entries)[0].Name != slot { + t.Fatalf("OIDC slot list = %#v", listed.Entries) + } + if removed := runRequest(t, request{Version: 1, Operation: "remove", Root: root, Directory: "oidc", Filename: slot}); !removed.Removed { + t.Fatal("OIDC slot remove did not report success") + } + runRejected(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: slot, ContentBase64: contents}) + runRejected(t, request{Version: 1, Operation: "create", Root: root, Directory: "oidc", Filename: "slot-64.json", ContentBase64: contents}) +} + func TestProtocolListSerializesLowerCamelBridgeDTO(t *testing.T) { root := filepath.Join(privateTestRoot(t), "auth") filename := "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc.json"