From 6d438f4c7e09f7ff0825fdf424d44646dac3f87c Mon Sep 17 00:00:00 2001 From: mptyl Date: Mon, 17 Aug 2026 13:04:15 +0200 Subject: [PATCH] fix(auth): close diagnostic filesystem races --- backend/src/auth/diagnostics.ts | 10 +- backend/src/auth/oidc-client.ts | 27 +++-- backend/src/auth/session-store.ts | 107 ++++++++++-------- backend/src/auth/windows-auth-storage.ts | 61 +++++++--- backend/test/auth-diagnostics.test.ts | 95 +++++++++++++++- backend/test/auth-dynamic-registry.test.ts | 5 +- backend/test/auth-request-snapshot.test.ts | 5 +- backend/test/auth-routes-local.test.ts | 3 + backend/test/auth-session-store.test.ts | 69 +++++++++++ backend/test/auth-test-fixtures.ts | 16 ++- backend/test/windows-auth-storage.test.ts | 36 +++++- tools/tht/internal/authstorage/storage.go | 68 ++++++++++- .../tht/internal/authstorage/storage_test.go | 48 ++++++++ .../authstorage/storage_windows_test.go | 16 +++ tools/tht/internal/safeio/files_unix_test.go | 52 +++++++++ tools/tht/internal/safeio/private_unix.go | 17 +++ 16 files changed, 545 insertions(+), 90 deletions(-) diff --git a/backend/src/auth/diagnostics.ts b/backend/src/auth/diagnostics.ts index b97452d1..242f1f4a 100644 --- a/backend/src/auth/diagnostics.ts +++ b/backend/src/auth/diagnostics.ts @@ -1,8 +1,11 @@ import type { AuthenticationConfigProvider, AuthMode } from "./types.js"; import type { LocalUserRegistry } from "./local-registry.js"; import { OidcIssuerMismatchError, OidcJwksUnavailableError, type OidcProtocol } from "./oidc-client.js"; -import { validateAuthSessionRoot } from "./session-store.js"; -import { createWindowsAuthStorageBridge, type WindowsAuthStorageBridge } from "./windows-auth-storage.js"; +import { + createPosixAuthStorageBridge, + createWindowsAuthStorageBridge, + type WindowsAuthStorageBridge, +} from "./windows-auth-storage.js"; import { isUsableAuthenticationSecret, type AuthenticationSecretReference } from "./secret-policy.js"; import type { AuthDiagnostic, AuthDiagnosticCode, AuthDiagnostics, GroupCatalog } from "./group-catalog.js"; @@ -20,6 +23,7 @@ export interface AuthDiagnoserDependencies { /** Platform integrations may inject an equivalent side-effect-free owner/ACL validator. */ sessionRootValidator?: (root: string) => void | Promise; windowsStorageBridge?: Pick; + posixStorageBridge?: Pick; authentication?: AuthenticationConfigProvider; secrets?: ReadonlyMap; localUserRegistry?: LocalUserRegistry; @@ -101,7 +105,7 @@ async function localRegistryIsUsable(deps: AuthDiagnoserDependencies): Promise (deps.windowsStorageBridge ?? createWindowsAuthStorageBridge()).validateRoot(root) - : validateAuthSessionRoot); + : (root: string) => (deps.posixStorageBridge ?? createPosixAuthStorageBridge()).validateRoot(root)); return { async inspect(options): Promise { const checks: AuthDiagnostic[] = []; diff --git a/backend/src/auth/oidc-client.ts b/backend/src/auth/oidc-client.ts index 56fc5d41..85dd39e4 100644 --- a/backend/src/auth/oidc-client.ts +++ b/backend/src/auth/oidc-client.ts @@ -88,13 +88,24 @@ function discoveryStringList(value: unknown): value is readonly string[] { function schemaValidDiscoveryMetadata(value: unknown): value is Record & { issuer: string } { if (!value || typeof value !== "object" || Array.isArray(value)) return false; const metadata = value as Record; - return text(metadata.issuer, 2048) - && text(metadata.authorization_endpoint, 2048) - && text(metadata.token_endpoint, 2048) - && text(metadata.jwks_uri, 2048) - && discoveryStringList(metadata.response_types_supported) - && discoveryStringList(metadata.subject_types_supported) - && discoveryStringList(metadata.id_token_signing_alg_values_supported); + const issuer = metadata.issuer; + const authorizationEndpoint = metadata.authorization_endpoint; + const tokenEndpoint = metadata.token_endpoint; + const jwksUri = metadata.jwks_uri; + if (!text(issuer, 2048) || !text(authorizationEndpoint, 2048) + || !text(tokenEndpoint, 2048) || !text(jwksUri, 2048) + || !discoveryStringList(metadata.response_types_supported) + || !discoveryStringList(metadata.subject_types_supported) + || !discoveryStringList(metadata.id_token_signing_alg_values_supported)) return false; + try { + configuredHttpsUrl(issuer); + httpsEndpoint(authorizationEndpoint); + httpsEndpoint(tokenEndpoint); + httpsEndpoint(jwksUri); + return true; + } catch { + return false; + } } function configuredHttpsUrl(value: string): URL { @@ -526,11 +537,11 @@ export function createOidcProtocol(options: OidcProtocolOptions): OidcProtocol { { [customFetch]: issuerCheckingFetch, timeout: httpTimeoutMs / 1000 }, ); const metadata = config.serverMetadata(); - if (certifiedIssuerMismatch) throw new OidcIssuerMismatchError(); if (metadata.issuer !== options.issuer) throw new OidcProtocolError(); httpsEndpoint(metadata.authorization_endpoint); httpsEndpoint(metadata.token_endpoint); httpsEndpoint(metadata.jwks_uri); + if (certifiedIssuerMismatch) throw new OidcIssuerMismatchError(); return config; } catch (error) { throw protocolFailure(error); diff --git a/backend/src/auth/session-store.ts b/backend/src/auth/session-store.ts index f24c936c..b5c338b6 100644 --- a/backend/src/auth/session-store.ts +++ b/backend/src/auth/session-store.ts @@ -8,7 +8,6 @@ import { fsyncSync, lstatSync, linkSync, - mkdirSync, openSync, opendirSync, readSync, @@ -29,6 +28,7 @@ import type { Role, } from "./types.js"; import { + createPosixAuthStorageBridge, createWindowsAuthStorageBridge, type WindowsAuthStorageBridge, } from "./windows-auth-storage.js"; @@ -137,9 +137,11 @@ export interface AuthSessionStore { consumeOidcState(state: string, now?: Date): Promise; } -/** Narrow test seam for the native Windows tht-backed storage adaptor. */ +/** Narrow test seams for the native tht-backed storage adaptors. */ export interface FileAuthSessionStoreOptions { windowsStorageBridge?: WindowsAuthStorageBridge; + /** Test seam for POSIX layout creation; production uses the bounded hidden tht bridge. */ + posixStorageBridge?: Pick; /** Test-only capacity seam; production always uses the fixed 64-state bound. */ oidcStateCapacity?: number; } @@ -297,10 +299,6 @@ function isNotFound(error: unknown): boolean { return (error as NodeJS.ErrnoException | undefined)?.code === "ENOENT"; } -function isAlreadyExists(error: unknown): boolean { - return (error as NodeJS.ErrnoException | undefined)?.code === "EEXIST"; -} - function canonicalRawValue(value: string): boolean { if (typeof value !== "string" || !TOKEN_PATTERN.test(value)) return false; try { @@ -387,6 +385,7 @@ interface SessionRootAncestor { dev: number; ino: number; uid: number; + mode: number; } function validateSessionRootSyntax(root: string): void { @@ -396,7 +395,7 @@ function validateSessionRootSyntax(root: string): void { function sameAncestor(ancestor: SessionRootAncestor, info: Stats): boolean { return info.isDirectory() && !info.isSymbolicLink() && ancestor.dev === info.dev - && ancestor.ino === info.ino && ancestor.uid === info.uid; + && ancestor.ino === info.ino && ancestor.uid === info.uid && ancestor.mode === info.mode; } function withSessionRootPreflight(root: string, use: (exists: boolean) => T): T { @@ -428,11 +427,19 @@ function withSessionRootPreflight(root: string, use: (exists: boolean) => T): const descriptor = openSync(current, constants.O_RDONLY | (constants.O_DIRECTORY ?? 0) | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); const opened = fstatSync(descriptor) as Stats; - if (!opened.isDirectory() || opened.dev !== info.dev || opened.ino !== info.ino || opened.uid !== info.uid) { + if (!opened.isDirectory() || opened.dev !== info.dev || opened.ino !== info.ino + || opened.uid !== info.uid || opened.mode !== info.mode) { closeSync(descriptor); throw invalid(); } - ancestors.push({ path: current, descriptor, dev: info.dev, ino: info.ino, uid: info.uid }); + ancestors.push({ + path: current, + descriptor, + dev: info.dev, + ino: info.ino, + uid: info.uid, + mode: info.mode, + }); } directoryIdentity(root); const result = use(true); @@ -450,34 +457,16 @@ function withSessionRootPreflight(root: string, use: (exists: boolean) => T): } } -function privateDirectory(path: string): void { - withSessionRootPreflight(path, (exists) => { - if (exists) return; - try { - mkdirSync(path, { recursive: false, mode: PRIVATE_DIRECTORY_MODE }); - } catch (error) { - if (!isAlreadyExists(error)) throw invalid(); - directoryIdentity(path); - return; - } - const descriptor = openSync(path, constants.O_RDONLY | (constants.O_DIRECTORY ?? 0) - | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); - try { - fchmodSync(descriptor, PRIVATE_DIRECTORY_MODE); - const opened = fstatSync(descriptor) as Stats; - const current = directoryIdentity(path); - if (opened.dev !== current.dev || opened.ino !== current.ino || opened.uid !== current.uid - || (opened.mode & 0o7777) !== current.mode) throw invalid(); - } finally { - try { closeSync(descriptor); } catch { /* creation already fails closed */ } - } - }); -} - /** Side-effect-free POSIX validator shared by runtime storage and static diagnostics. */ export function validateAuthSessionRoot(root: string): void { try { - withSessionRootPreflight(root, () => undefined); + const exists = withSessionRootPreflight(root, (currentExists) => currentExists); + if (!exists) return; + for (const child of ["sessions", "oidc"]) { + const path = join(root, child); + if (dirname(path) !== root) throw invalid(); + withSessionRootPreflight(path, () => undefined); + } } catch { throw invalid(); } @@ -487,12 +476,11 @@ function storageDirectories(root: string): StorageDirectories { // Native Windows calls must dispatch to the tht DACL-capable bridge before reaching this // POSIX-only helper. Keep this guard so an un-routed caller cannot fall back to chmod. validateSessionRootSyntax(root); - privateDirectory(root); validateAuthSessionRoot(root); const sessions = join(root, "sessions"); const oidc = join(root, "oidc"); - privateDirectory(sessions); - privateDirectory(oidc); + directoryIdentity(sessions); + directoryIdentity(oidc); return { root, sessions, oidc }; } @@ -1020,6 +1008,9 @@ export function createFileAuthSessionStore( const windowsStorage = process.platform === "win32" ? options.windowsStorageBridge ?? createWindowsAuthStorageBridge() : undefined; + const posixStorage = process.platform === "win32" + ? undefined + : options.posixStorageBridge ?? createPosixAuthStorageBridge(); let sessionPruneCursor: string | undefined; function requiredWindowsStorage(): WindowsAuthStorageBridge { @@ -1027,9 +1018,25 @@ export function createFileAuthSessionStore( return windowsStorage; } + async function posixStorageDirectories(): Promise { + if (process.platform === "win32" || posixStorage === undefined) throw invalid(); + try { + return storageDirectories(root); + } catch { + // A missing safe layout is the only case the helper can repair. Unsafe layouts are + // rejected by the same native primitive without path-based fallback in this process. + } + try { + await posixStorage.ensureLayout(root); + return storageDirectories(root); + } catch { + throw invalid(); + } + } + async function ordinarySessionPage(after: string | undefined): Promise { if (process.platform !== "win32") { - return boundedSessionDirectoryPage(storageDirectories(root).sessions, after, MAX_SESSION_PRUNE_ENTRIES); + return boundedSessionDirectoryPage((await posixStorageDirectories()).sessions, after, MAX_SESSION_PRUNE_ENTRIES); } const page = await requiredWindowsStorage().listPage(root, "sessions", after, MAX_SESSION_PRUNE_ENTRIES); if (!page || !Array.isArray(page.entries) || typeof page.more !== "boolean") throw invalid(); @@ -1066,7 +1073,7 @@ export function createFileAuthSessionStore( if (sessionExpired(record, nowMs) && await bridge.remove(root, "sessions", filename)) removed += 1; } } else { - const directory = storageDirectories(root).sessions; + const directory = (await posixStorageDirectories()).sessions; for (const filename of page.entries) { await withLock(lockKey(root, "sessions", filename), async () => { const trusted = readTrusted(directory, filename, MAX_SESSION_RECORD_BYTES, parseSessionRecord); @@ -1082,7 +1089,7 @@ export function createFileAuthSessionStore( async function oidcStorageEntries(): Promise { const entries = process.platform === "win32" ? (await requiredWindowsStorage().list(root, "oidc", MAX_OIDC_STORAGE_ENTRIES)).map((entry) => entry.name) - : boundedDirectoryNames(storageDirectories(root).oidc, MAX_OIDC_STORAGE_ENTRIES); + : boundedDirectoryNames((await posixStorageDirectories()).oidc, MAX_OIDC_STORAGE_ENTRIES); if (entries.length > MAX_OIDC_STORAGE_ENTRIES) throw invalid(); if (entries.some((entry) => !DIGEST_FILENAME_PATTERN.test(entry) && !CLAIM_FILENAME_PATTERN.test(entry) && oidcSlotIndex(entry) === undefined)) throw invalid(); @@ -1111,7 +1118,7 @@ export function createFileAuthSessionStore( if (retryDelayMs > 0) await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); try { trusted = readTrusted( - storageDirectories(root).oidc, + (await posixStorageDirectories()).oidc, filename, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord, @@ -1140,7 +1147,7 @@ export function createFileAuthSessionStore( || !await requiredWindowsStorage().remove(root, "oidc", slot.filename)) throw invalid(); return; } - if (!slot.identity || !removeTrusted(storageDirectories(root).oidc, slot.filename, slot.identity)) throw invalid(); + if (!slot.identity || !removeTrusted((await posixStorageDirectories()).oidc, slot.filename, slot.identity)) throw invalid(); } async function releaseOidcSlot(index: number | undefined, stateFilename: string): Promise { @@ -1172,7 +1179,7 @@ export function createFileAuthSessionStore( const filename = oidcSlotFilename(index); const created = process.platform === "win32" ? await requiredWindowsStorage().create(root, "oidc", filename, contents) - : writeExclusive(storageDirectories(root).oidc, filename, contents); + : writeExclusive((await posixStorageDirectories()).oidc, filename, contents); if (created) return index; } throw new OidcStateCapacityError(); @@ -1216,7 +1223,7 @@ export function createFileAuthSessionStore( } throw invalid(); } - const directories = storageDirectories(root); + const directories = await posixStorageDirectories(); for (let attempt = 0; attempt < 8; attempt += 1) { const token = randomBytes(TOKEN_BYTES).toString("base64url"); const filename = digestFilename(token); @@ -1255,7 +1262,7 @@ export function createFileAuthSessionStore( await bridge.remove(root, "sessions", filename); return undefined; } - const directories = storageDirectories(root); + const directories = await posixStorageDirectories(); const trusted = readTrusted(directories.sessions, filename, MAX_SESSION_RECORD_BYTES, parseSessionRecord); if (!trusted) return undefined; if (sessionExpired(trusted.value, nowMs)) { @@ -1300,7 +1307,7 @@ export function createFileAuthSessionStore( await bridge.replace(root, "sessions", filename, serialize(touched, MAX_SESSION_RECORD_BYTES)); return; } - const directories = storageDirectories(root); + const directories = await posixStorageDirectories(); const trusted = readTrusted(directories.sessions, filename, MAX_SESSION_RECORD_BYTES, parseSessionRecord); if (!trusted) return; if (sessionExpired(trusted.value, nowMs)) { @@ -1333,7 +1340,7 @@ export function createFileAuthSessionStore( await requiredWindowsStorage().remove(root, "sessions", filename); return; } - const directories = storageDirectories(root); + const directories = await posixStorageDirectories(); removeTrusted(directories.sessions, filename); }); } @@ -1394,7 +1401,7 @@ export function createFileAuthSessionStore( return removed; } - const directory = storageDirectories(root).oidc; + const directory = (await posixStorageDirectories()).oidc; const oidcEntries = boundedDirectoryNames(directory, MAX_OIDC_STORAGE_ENTRIES); const stateFilenames = new Set(oidcEntries.filter((entry) => DIGEST_FILENAME_PATTERN.test(entry))); let removed = 0; @@ -1485,7 +1492,7 @@ export function createFileAuthSessionStore( 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); + : writeExclusive((await posixStorageDirectories()).oidc, filename, contents); if (created) return { state, record }; await releaseOidcSlot(capacitySlot, filename); } @@ -1505,7 +1512,7 @@ export function createFileAuthSessionStore( await releaseOidcSlot(record.capacitySlot, filename); return oidcStateExpired(record, nowMs) ? undefined : record; } - const directories = storageDirectories(root); + const directories = await posixStorageDirectories(); const claim = claimOidcState(directories.oidc, filename); // An installed claim belongs to another process/store instance. Only the process which // created the hard link is allowed to receive the record. diff --git a/backend/src/auth/windows-auth-storage.ts b/backend/src/auth/windows-auth-storage.ts index 9448ebd4..bfbb3487 100644 --- a/backend/src/auth/windows-auth-storage.ts +++ b/backend/src/auth/windows-auth-storage.ts @@ -1,5 +1,5 @@ import { spawn, spawnSync } from "node:child_process"; -import { win32 } from "node:path"; +import { posix, win32 } from "node:path"; import type { Readable, Writable } from "node:stream"; import { z } from "zod"; @@ -36,6 +36,7 @@ export interface WindowsAuthStoragePage { /** Internal adapter boundary for the file-session store's native Windows path. */ export interface WindowsAuthStorageBridge { validateRoot(root: string): Promise; + ensureLayout(root: string): Promise; readAuthConfig(path: string): Buffer; create(root: string, directory: WindowsAuthStorageDirectory, filename: string, contents: Buffer): Promise; read(root: string, directory: WindowsAuthStorageDirectory, filename: string): Promise; @@ -100,6 +101,8 @@ export interface WindowsAuthStorageBridgeOptions { beforeInputForTest?: () => Promise; } +type AuthStoragePathStyle = "posix" | "windows"; + const responseSchema = z.strictObject({ version: z.literal(PROTOCOL_VERSION), ok: z.literal(true), @@ -114,13 +117,14 @@ const responseSchema = z.strictObject({ })).max(MAX_ENTRIES).optional(), more: z.boolean().optional(), validated: z.boolean().optional(), + prepared: z.boolean().optional(), }); type BridgeResponse = z.infer; interface BridgeRequest { version: typeof PROTOCOL_VERSION; - operation: "validate-root" | "read-auth-config" | "create" | "read" | "replace" | "remove" | "list" | "claim-consume" | "read-claim" | "remove-claim"; + operation: "validate-root" | "ensure-layout" | "read-auth-config" | "create" | "read" | "replace" | "remove" | "list" | "claim-consume" | "read-claim" | "remove-claim"; root: string; directory?: WindowsAuthStorageDirectory; filename?: string; @@ -145,9 +149,10 @@ function canonicalBase64(value: string, maximum: number): Buffer { } } -function validateRoot(root: string): void { +function validateRoot(root: string, pathStyle: AuthStoragePathStyle): void { + const paths = pathStyle === "windows" ? win32 : posix; if (typeof root !== "string" || root.length === 0 || /[\u0000-\u001f\u007f]/.test(root) - || !win32.isAbsolute(root) || win32.normalize(root) !== root) throw invalid(); + || !paths.isAbsolute(root) || paths.normalize(root) !== root) throw invalid(); } function validateFilename(filename: string, allowClaim = false, allowOidcSlot = false): void { @@ -156,11 +161,13 @@ function validateFilename(filename: string, allowClaim = false, allowOidcSlot = && !(allowOidcSlot && OIDC_SLOT_FILENAME.test(filename)))) throw invalid(); } -function safeThtExecutable(value: string | undefined): string { +function safeThtExecutable(value: string | undefined, pathStyle: AuthStoragePathStyle): string { const executable = value ?? process.env.THT_BIN ?? "tht"; if (typeof executable !== "string" || executable.length === 0 || /[\u0000-\u001f\u007f]/.test(executable)) throw invalid(); - if (executable === "tht" || executable === "tht.exe") return executable; - if (win32.isAbsolute(executable) && win32.normalize(executable) === executable && /\.exe$/i.test(executable)) return executable; + if (executable === "tht" || (pathStyle === "windows" && executable === "tht.exe")) return executable; + const paths = pathStyle === "windows" ? win32 : posix; + if (paths.isAbsolute(executable) && paths.normalize(executable) === executable + && (pathStyle === "posix" || /\.exe$/i.test(executable))) return executable; throw invalid(); } @@ -178,9 +185,9 @@ function parseResponse(result: WindowsAuthStorageInvocationResult, maximumOutput } } -function encodedRequest(request: BridgeRequest): Buffer { - validateRoot(request.root); - if (request.operation === "validate-root") { +function encodedRequest(request: BridgeRequest, pathStyle: AuthStoragePathStyle): Buffer { + validateRoot(request.root, pathStyle); + if (request.operation === "validate-root" || request.operation === "ensure-layout") { if (request.directory !== undefined || request.filename !== undefined || request.contentBase64 !== undefined || request.maximumEntries !== undefined || request.afterName !== undefined || request.continuation !== undefined) throw invalid(); } else if (request.operation === "read-auth-config") { @@ -390,8 +397,11 @@ function listedEntries( return response.entries.map((entry) => ({ name: entry.name, modifiedUnixMs: entry.modifiedUnixMs })); } -export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridgeOptions = {}): WindowsAuthStorageBridge { - const executable = safeThtExecutable(options.thtExecutable); +function createAuthStorageBridge( + pathStyle: AuthStoragePathStyle, + options: WindowsAuthStorageBridgeOptions = {}, +): WindowsAuthStorageBridge { + const executable = safeThtExecutable(options.thtExecutable, pathStyle); const invoke = options.invoke ?? ((invocation: WindowsAuthStorageInvocation) => invokeTht( invocation, options.spawnChild, @@ -404,7 +414,7 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge const response = await invoke({ executable, args: ["_auth-storage"], - input: encodedRequest(value), + input: encodedRequest(value, pathStyle), timeoutMs: TIMEOUT_MS, maximumOutputBytes, }); @@ -418,7 +428,7 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge const response = invokeSync({ executable, args: ["_auth-storage"], - input: encodedRequest(value), + input: encodedRequest(value, pathStyle), timeoutMs: TIMEOUT_MS, maximumOutputBytes: MAX_AUTH_CONFIG_RESPONSE_BYTES, }); @@ -442,12 +452,18 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge if (response.validated !== true || Object.keys(response).some((key) => !["version", "ok", "validated"].includes(key))) throw invalid(); }, + async ensureLayout(root) { + const response = await request({ version: PROTOCOL_VERSION, operation: "ensure-layout", root }); + if (response.prepared !== true + || Object.keys(response).some((key) => !["version", "ok", "prepared"].includes(key))) throw invalid(); + }, readAuthConfig(path) { + const paths = pathStyle === "windows" ? win32 : posix; if (typeof path !== "string" || path.length === 0 || /[\u0000-\u001f\u007f]/.test(path) - || !win32.isAbsolute(path) || win32.normalize(path) !== path) throw invalid(); - const root = win32.dirname(path); - const filename = win32.basename(path); - if (!AUTH_CONFIG_FILENAME.test(filename) || win32.join(root, filename) !== path) throw invalid(); + || !paths.isAbsolute(path) || paths.normalize(path) !== path) throw invalid(); + const root = paths.dirname(path); + const filename = paths.basename(path); + if (!AUTH_CONFIG_FILENAME.test(filename) || paths.join(root, filename) !== path) throw invalid(); const response = syncRequest({ version: PROTOCOL_VERSION, operation: "read-auth-config", root, filename }); if (Object.keys(response).some((key) => !["version", "ok", "found", "contentBase64"].includes(key))) throw invalid(); const contents = contentFrom(response, MAX_AUTH_CONFIG_BYTES); @@ -520,3 +536,12 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge }, }; } + +export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridgeOptions = {}): WindowsAuthStorageBridge { + return createAuthStorageBridge("windows", options); +} + +/** POSIX uses the same single hidden tht protocol and bounds, with native canonical path rules. */ +export function createPosixAuthStorageBridge(options: WindowsAuthStorageBridgeOptions = {}): WindowsAuthStorageBridge { + return createAuthStorageBridge("posix", options); +} diff --git a/backend/test/auth-diagnostics.test.ts b/backend/test/auth-diagnostics.test.ts index f4a09d12..58a74292 100644 --- a/backend/test/auth-diagnostics.test.ts +++ b/backend/test/auth-diagnostics.test.ts @@ -7,6 +7,7 @@ import { createAuthenticationConfigProvider } from "../src/auth/config.js"; import { createAuthentikGroupCatalog } from "../src/auth/authentik-group-catalog.js"; import { createLocalUserRegistry } from "../src/auth/local-registry.js"; import { createOidcProtocol, OidcJwksUnavailableError } from "../src/auth/oidc-client.js"; +import { validateAuthSessionRoot } from "../src/auth/session-store.js"; import type { LoadedAuthConfig } from "../src/auth/types.js"; const sentinels = [ @@ -151,6 +152,7 @@ test("distinguishes a valid registry without an enabled admin from a malformed r chmodSync(validUsers, 0o600); const validReport = await createAuthDiagnoser({ authMode: "local", authStateRoot: validRoot, + sessionRootValidator: validateAuthSessionRoot, authentication: { current: () => localConfig(join(validRoot, "auth.yaml")) }, localUserRegistry: createLocalUserRegistry(validUsers), }).inspect({ live: false }); @@ -162,6 +164,7 @@ test("distinguishes a valid registry without an enabled admin from a malformed r chmodSync(malformedUsers, 0o600); const malformedReport = await createAuthDiagnoser({ authMode: "local", authStateRoot: malformedRoot, + sessionRootValidator: validateAuthSessionRoot, authentication: { current: () => localConfig(join(malformedRoot, "auth.yaml")) }, localUserRegistry: createLocalUserRegistry(malformedUsers), }).inspect({ live: false }); @@ -179,6 +182,7 @@ test("maps unsafe auth.yaml storage from the real provider to a redacted config chmodSync(unsafePath, 0o640); const report = await createAuthDiagnoser({ authMode: "local", authStateRoot: root, + sessionRootValidator: validateAuthSessionRoot, authentication: createAuthenticationConfigProvider(unsafePath), }).inspect({ live: false }); @@ -367,6 +371,42 @@ test.each([ id_token_signing_alg_values_supported: ["RS256"], }], ["schema-invalid metadata", 200, { issuer: "https://different-issuer.example.test" }], + ["an unsafe foreign issuer", 200, { + issuer: "http://different-issuer.example.test", + authorization_endpoint: "https://issuer.example.test/authorize", + token_endpoint: "https://issuer.example.test/token", + jwks_uri: "https://issuer.example.test/jwks", + response_types_supported: ["code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + }], + ["an unsafe authorization endpoint", 200, { + issuer: "https://different-issuer.example.test", + authorization_endpoint: "http://127.0.0.1/authorize", + token_endpoint: "https://issuer.example.test/token", + jwks_uri: "https://issuer.example.test/jwks", + response_types_supported: ["code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + }], + ["an unsafe token endpoint", 200, { + issuer: "https://different-issuer.example.test", + authorization_endpoint: "https://issuer.example.test/authorize", + token_endpoint: "https://operator:secret@issuer.example.test/token", + jwks_uri: "https://issuer.example.test/jwks", + response_types_supported: ["code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + }], + ["an unsafe JWKS endpoint", 200, { + issuer: "https://different-issuer.example.test", + authorization_endpoint: "https://issuer.example.test/authorize", + token_endpoint: "https://issuer.example.test/token", + jwks_uri: "https://issuer.example.test/jwks#fragment", + response_types_supported: ["code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + }], ])("does not classify %s containing an issuer as an issuer mismatch", async (_label, status, body) => { const loaded = oidcConfig(); const fetch = vi.fn(async () => Response.json(body, { status })); @@ -403,8 +443,13 @@ test("redacts exceptional configuration, registry, protocol, and catalog errors" }); test.skipIf(process.platform === "win32")("uses the runtime validator for canonical, private session roots", async () => { + const dependencies = (authStateRoot: string) => ({ + authMode: "none" as const, + authStateRoot, + sessionRootValidator: validateAuthSessionRoot, + }); const valid = privateRoot(); - await expect(createAuthDiagnoser({ authMode: "none", authStateRoot: valid }).inspect({ live: false })) + await expect(createAuthDiagnoser(dependencies(valid)).inspect({ live: false })) .resolves.toMatchObject({ ready: true, checks: [expect.objectContaining({ code: "auth_ready" })] }); const realRoot = join(privateRoot(), "real-auth"); @@ -419,22 +464,64 @@ test.skipIf(process.platform === "win32")("uses the runtime validator for canoni writeFileSync(blockedParent, "blocked", { mode: 0o600 }); const traversal = `${valid}/../${basename(valid)}`; - const missingReport = await createAuthDiagnoser({ authMode: "none", authStateRoot: absent }).inspect({ live: false }); + const missingReport = await createAuthDiagnoser(dependencies(absent)).inspect({ live: false }); expect(missingReport).toMatchObject({ ready: true, checks: [expect.objectContaining({ code: "auth_ready" })] }); expect(existsSync(absent)).toBe(false); for (const unsafe of [traversal, linkedRoot, absentNested, join(blockedParent, "auth")]) { - const report = await createAuthDiagnoser({ authMode: "none", authStateRoot: unsafe }).inspect({ live: false }); + const report = await createAuthDiagnoser(dependencies(unsafe)).inspect({ live: false }); expect(report).toMatchObject({ ready: false, checks: [expect.objectContaining({ code: "auth_session_store_invalid" })] }); expect(JSON.stringify(report)).not.toContain(unsafe); } expect(existsSync(absentParent)).toBe(false); chmodSync(valid, 0o750); - await expect(createAuthDiagnoser({ authMode: "none", authStateRoot: valid }).inspect({ live: false })) + await expect(createAuthDiagnoser(dependencies(valid)).inspect({ live: false })) .resolves.toMatchObject({ ready: false, checks: [expect.objectContaining({ code: "auth_session_store_invalid" })] }); }); +test.skipIf(process.platform === "win32")("diagnoses unsafe existing session-store children without creating missing children", async () => { + const root = privateRoot(); + const outside = privateRoot(); + symlinkSync(outside, join(root, "sessions")); + + const linked = await createAuthDiagnoser({ + authMode: "none", authStateRoot: root, sessionRootValidator: validateAuthSessionRoot, + }).inspect({ live: false }); + expect(linked).toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "auth_session_store_invalid" })], + }); + expect(existsSync(join(root, "oidc"))).toBe(false); + expect(JSON.stringify(linked)).not.toContain(root); + + rmSync(join(root, "sessions")); + mkdirSync(join(root, "sessions"), { mode: 0o700 }); + chmodSync(join(root, "sessions"), 0o700); + mkdirSync(join(root, "oidc"), { mode: 0o700 }); + chmodSync(join(root, "oidc"), 0o750); + const nonPrivate = await createAuthDiagnoser({ + authMode: "none", authStateRoot: root, sessionRootValidator: validateAuthSessionRoot, + }).inspect({ live: false }); + expect(nonPrivate).toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "auth_session_store_invalid" })], + }); +}); + +test.skipIf(process.platform === "win32")("routes production POSIX static validation through the native auth-storage bridge", async () => { + const validateRoot = vi.fn(async () => undefined); + const report = await createAuthDiagnoser({ + authMode: "none", + authStateRoot: "/var/lib/thothii/auth", + posixStorageBridge: { validateRoot }, + }).inspect({ live: false }); + + expect(report).toMatchObject({ ready: true }); + expect(validateRoot).toHaveBeenCalledOnce(); + expect(validateRoot).toHaveBeenCalledWith("/var/lib/thothii/auth"); +}); + test("routes native Windows static session-root validation through the auth-storage bridge", async () => { const originalPlatform = process.platform; const validateRoot = vi.fn(async () => undefined); diff --git a/backend/test/auth-dynamic-registry.test.ts b/backend/test/auth-dynamic-registry.test.ts index 6abcd0ef..854e912a 100644 --- a/backend/test/auth-dynamic-registry.test.ts +++ b/backend/test/auth-dynamic-registry.test.ts @@ -6,6 +6,7 @@ import { stringify } from "yaml"; import { buildApp, type AppWithAuthSessionStore } from "../src/app.js"; import { loadAuthenticationConfig } from "../src/auth/config.js"; import { loadConfig } from "../src/config.js"; +import { prepareAuthStateRoot } from "./auth-test-fixtures.js"; const password = "correct horse battery staple"; const passwordHash = "$argon2id$v=19$m=65536,t=3,p=1$AAECAwQFBgcICQoLDA0ODw$DRo8ZSPI8G5OCvnFFapbVEjP69aDjy1Sw9i2743cPC4"; @@ -47,9 +48,11 @@ test("each login and session resolve uses the current config snapshot users file chmodSync(authFile, 0o600); chmodSync(usersAFile, 0o600); chmodSync(usersBFile, 0o600); + const authStateRoot = join(directory, "auth-state"); + prepareAuthStateRoot(authStateRoot); const app = buildApp(loadConfig({ THT_AUTH_CONFIG_FILE: authFile, - THT_AUTH_STATE_ROOT: join(directory, "auth-state"), + THT_AUTH_STATE_ROOT: authStateRoot, THT_HARNESS_DIR: "/tmp/h", })); cleanups.push(async () => { diff --git a/backend/test/auth-request-snapshot.test.ts b/backend/test/auth-request-snapshot.test.ts index a38ae9a1..836344b5 100644 --- a/backend/test/auth-request-snapshot.test.ts +++ b/backend/test/auth-request-snapshot.test.ts @@ -7,6 +7,7 @@ import { buildApp, type AppWithAuthSessionStore } from "../src/app.js"; import { loadAuthenticationConfig } from "../src/auth/config.js"; import type { AuthenticationConfigProvider, LoadedAuthConfig } from "../src/auth/types.js"; import { loadConfig } from "../src/config.js"; +import { prepareAuthStateRoot } from "./auth-test-fixtures.js"; const password = "correct horse battery staple"; const passwordHash = "$argon2id$v=19$m=65536,t=3,p=1$AAECAwQFBgcICQoLDA0ODw$DRo8ZSPI8G5OCvnFFapbVEjP69aDjy1Sw9i2743cPC4"; @@ -78,9 +79,11 @@ async function createFixture(first: "A" | "B", later: "A" | "B" | "oidc") { return calls === 1 ? snapshots[first] : snapshots[later === "oidc" ? "B" : later]; }, }; + const authStateRoot = join(directory, "auth-state"); + prepareAuthStateRoot(authStateRoot); const config = loadConfig({ THT_AUTH_CONFIG_FILE: authA, - THT_AUTH_STATE_ROOT: join(directory, "auth-state"), + THT_AUTH_STATE_ROOT: authStateRoot, THT_HARNESS_DIR: "/tmp/h", }); config.authentication = provider; diff --git a/backend/test/auth-routes-local.test.ts b/backend/test/auth-routes-local.test.ts index 5e2bfa84..1dc40e15 100644 --- a/backend/test/auth-routes-local.test.ts +++ b/backend/test/auth-routes-local.test.ts @@ -6,6 +6,7 @@ import { stringify } from "yaml"; import { buildApp } from "../src/app.js"; import { loadConfig } from "../src/config.js"; import { LoginFailureLimiter } from "../src/auth/routes.js"; +import { prepareAuthStateRoot } from "./auth-test-fixtures.js"; const password = "correct horse battery staple"; const passwordHash = "$argon2id$v=19$m=65536,t=3,p=1$AAECAwQFBgcICQoLDA0ODw$DRo8ZSPI8G5OCvnFFapbVEjP69aDjy1Sw9i2743cPC4"; @@ -82,6 +83,7 @@ async function createLocalApp(options: { writeFileSync(usersFile, usersYaml({ enabled: options.enabled }), { encoding: "utf8", mode: 0o600 }); chmodSync(authConfigFile, 0o600); chmodSync(usersFile, 0o600); + prepareAuthStateRoot(authStateRoot); const app = buildApp(loadConfig({ THT_AUTH_CONFIG_FILE: authConfigFile, THT_AUTH_STATE_ROOT: authStateRoot, @@ -151,6 +153,7 @@ test("remembered login uses a persistent secure cookie under an HTTPS public URL writeFileSync(usersFile, usersYaml(), { encoding: "utf8", mode: 0o600 }); chmodSync(authConfigFile, 0o600); chmodSync(usersFile, 0o600); + prepareAuthStateRoot(authStateRoot); const config = () => loadConfig({ THT_AUTH_CONFIG_FILE: authConfigFile, THT_AUTH_STATE_ROOT: authStateRoot, THT_HARNESS_DIR: "/tmp/h" }); const first = buildApp(config()); try { diff --git a/backend/test/auth-session-store.test.ts b/backend/test/auth-session-store.test.ts index 1b68594b..1fa8c652 100644 --- a/backend/test/auth-session-store.test.ts +++ b/backend/test/auth-session-store.test.ts @@ -107,6 +107,10 @@ afterEach(() => { function root(): string { const path = mkdtempSync(join(realpathSync(tmpdir()), "thothii-auth-session-")); chmodSync(path, 0o700); + for (const child of ["sessions", "oidc"]) { + mkdirSync(join(path, child), { mode: 0o700 }); + chmodSync(join(path, child), 0o700); + } roots.push(path); return path; } @@ -265,10 +269,75 @@ describe("file-backed auth session store", () => { } expect(existsSync(join(outer, "absent-parent"))).toBe(false); + for (const child of ["sessions", "oidc"] as const) { + const childRoot = join(outer, `child-${child}`); + mkdirSync(childRoot, { mode: 0o700 }); + chmodSync(childRoot, 0o700); + const childPath = join(childRoot, child); + const outside = root(); + symlinkSync(outside, childPath); + expect(() => validateAuthSessionRoot(childRoot)).toThrow("auth_session_store_invalid"); + expect(readdirSync(outside).sort()).toEqual(["oidc", "sessions"]); + unlinkSync(childPath); + mkdirSync(childPath, { mode: 0o700 }); + chmodSync(childPath, 0o750); + expect(() => validateAuthSessionRoot(childRoot)).toThrow("auth_session_store_invalid"); + } + + const missingChildren = join(outer, "missing-children"); + mkdirSync(missingChildren, { mode: 0o700 }); + chmodSync(missingChildren, 0o700); + expect(() => validateAuthSessionRoot(missingChildren)).not.toThrow(); + expect(existsSync(join(missingChildren, "sessions"))).toBe(false); + expect(existsSync(join(missingChildren, "oidc"))).toBe(false); + chmodSync(valid, 0o750); expect(() => validateAuthSessionRoot(valid)).toThrow("auth_session_store_invalid"); }); + test.skipIf(process.platform === "win32")("delegates missing layout creation and then enforces static/runtime parity", async () => { + const storageRoot = join(root(), "auth"); + const ensureLayout = vi.fn(async (requestedRoot: string) => { + expect(requestedRoot).toBe(storageRoot); + mkdirSync(requestedRoot, { mode: 0o700 }); + chmodSync(requestedRoot, 0o700); + for (const child of ["sessions", "oidc"]) { + mkdirSync(join(requestedRoot, child), { mode: 0o700 }); + chmodSync(join(requestedRoot, child), 0o700); + } + }); + const store = validStore(storageRoot, { posixStorageBridge: { ensureLayout } }); + + await expect(create(store)).resolves.toMatchObject({ record: { method: "local" } }); + expect(ensureLayout).toHaveBeenCalledOnce(); + expect(() => validateAuthSessionRoot(storageRoot)).not.toThrow(); + + chmodSync(join(storageRoot, "oidc"), 0o750); + expect(() => validateAuthSessionRoot(storageRoot)).toThrow("auth_session_store_invalid"); + await expectStoreInvalid(store.createOidcState(oidcInput("n".repeat(16), "v".repeat(43)), base)); + }); + + test.skipIf(process.platform === "win32")("does not perform a path-based fallback when bridge layout creation loses an ancestor race", async () => { + const outer = root(); + const outside = root(); + const parent = join(outer, "parent"); + const movedParent = join(outer, "parent-original"); + mkdirSync(parent, { mode: 0o700 }); + chmodSync(parent, 0o700); + const storageRoot = join(parent, "auth"); + const ensureLayout = vi.fn(async () => { + renameSync(parent, movedParent); + symlinkSync(outside, parent); + throw new Error(`${storageRoot} rejected`); + }); + const store = validStore(storageRoot, { posixStorageBridge: { ensureLayout } }); + + await expectStoreInvalid(create(store)); + expect(ensureLayout).toHaveBeenCalledOnce(); + expect(existsSync(join(outside, "auth"))).toBe(false); + expect(existsSync(join(movedParent, "auth"))).toBe(false); + }); + test.skipIf(process.platform === "win32")("never follows a symlinked ancestor while creating a missing session root", async () => { const outer = root(); const outside = root(); diff --git a/backend/test/auth-test-fixtures.ts b/backend/test/auth-test-fixtures.ts index 863620a7..d9a4cf15 100644 --- a/backend/test/auth-test-fixtures.ts +++ b/backend/test/auth-test-fixtures.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { FastifyInstance } from "fastify"; @@ -37,6 +37,16 @@ function cookiePair(setCookie: string): string { return setCookie.split(";", 1)[0] ?? ""; } +/** Unit fixtures which bypass the installed tht binary start from an already-safe native layout. */ +export function prepareAuthStateRoot(root: string): void { + mkdirSync(root, { mode: 0o700 }); + chmodSync(root, 0o700); + for (const child of ["sessions", "oidc"]) { + mkdirSync(join(root, child), { mode: 0o700 }); + chmodSync(join(root, child), 0o700); + } +} + /** Creates a production-local app and authenticates through the real login/session boundary. */ export async function createLocalAuthFixture( deps?: BuildAppDeps, @@ -70,9 +80,11 @@ export async function createLocalAuthFixture( chmodSync(authConfigFile, 0o600); chmodSync(usersFile, 0o600); + const authStateRoot = join(directory, "auth-state"); + prepareAuthStateRoot(authStateRoot); const app = buildApp(loadConfig({ THT_AUTH_CONFIG_FILE: authConfigFile, - THT_AUTH_STATE_ROOT: join(directory, "auth-state"), + THT_AUTH_STATE_ROOT: authStateRoot, THT_HARNESS_DIR: "/tmp/h", }), deps); let downstream = 0; diff --git a/backend/test/windows-auth-storage.test.ts b/backend/test/windows-auth-storage.test.ts index f0917f1f..a08be585 100644 --- a/backend/test/windows-auth-storage.test.ts +++ b/backend/test/windows-auth-storage.test.ts @@ -6,7 +6,10 @@ import { fileURLToPath } from "node:url"; import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; import { afterEach, describe, expect, test, vi } from "vitest"; -import { createWindowsAuthStorageBridge } from "../src/auth/windows-auth-storage.js"; +import { + createPosixAuthStorageBridge, + createWindowsAuthStorageBridge, +} from "../src/auth/windows-auth-storage.js"; const root = "C:\\ProgramData\\ThothII\\auth"; const filename = "a".repeat(64) + ".json"; @@ -80,6 +83,37 @@ function bridgeForChild(child: FakeBridgeChild) { } describe("Windows auth-storage bridge", () => { + test("uses the same bounded hidden bridge to ensure a POSIX session layout", async () => { + const calls: Array<{ executable: string; args: readonly string[]; input: Buffer; timeoutMs: number }> = []; + const bridge = createPosixAuthStorageBridge({ + thtExecutable: "/opt/thothii/bin/tht", + invoke: async (call) => { + calls.push(call); + return { + code: 0, + stdout: Buffer.from('{"version":1,"ok":true,"prepared":true}\n'), + stderr: Buffer.alloc(0), + }; + }, + }); + + await expect(bridge.ensureLayout("/var/lib/thothii/auth")).resolves.toBeUndefined(); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ + executable: "/opt/thothii/bin/tht", + args: ["_auth-storage"], + timeoutMs: 5_000, + }); + expect(JSON.parse(calls[0]!.input.toString("utf8"))).toEqual({ + version: 1, + operation: "ensure-layout", + root: "/var/lib/thothii/auth", + }); + expect(JSON.stringify(calls[0]!.args)).not.toContain("/var/lib/thothii/auth"); + await expect(bridge.ensureLayout("/var/lib/thothii/../auth")) + .rejects.toThrow("auth_session_store_invalid"); + }); + test("permits reservation slots only for OIDC record operations", async () => { const requests: Array> = []; const bridge = createWindowsAuthStorageBridge({ diff --git a/tools/tht/internal/authstorage/storage.go b/tools/tht/internal/authstorage/storage.go index ff5f4276..77f1a9c7 100644 --- a/tools/tht/internal/authstorage/storage.go +++ b/tools/tht/internal/authstorage/storage.go @@ -60,6 +60,7 @@ type response struct { Entries *[]safeio.PrivateDirectoryEntry `json:"entries,omitempty"` More *bool `json:"more,omitempty"` Validated bool `json:"validated,omitempty"` + Prepared bool `json:"prepared,omitempty"` } // Run accepts exactly one strict JSON request on stdin and emits exactly one JSON response on @@ -110,11 +111,17 @@ func execute(input request) (response, error) { return response{}, errInvalid } if input.Operation == "validate-root" { - if _, err := preflightRoot(input.Root); err != nil { + if err := validateStorageLayout(input.Root); err != nil { return response{}, errInvalid } return response{Version: protocolVersion, OK: true, Validated: true}, nil } + if input.Operation == "ensure-layout" { + if err := ensureStorageLayout(input.Root); err != nil { + return response{}, errInvalid + } + return response{Version: protocolVersion, OK: true, Prepared: true}, nil + } if input.Operation == "read-auth-config" { root, err := existingPrivateRoot(input.Root) if err != nil { @@ -225,7 +232,7 @@ func validOperationShape(input request) bool { noAfterName := input.AfterName == "" noContinuation := !input.Continuation switch input.Operation { - case "validate-root": + case "validate-root", "ensure-layout": return input.Directory == "" && input.Filename == "" && noContents && noMaximumEntries && noAfterName && noContinuation case "read-auth-config": return input.Directory == "" && authConfigFilename.MatchString(input.Filename) && noContents && noMaximumEntries && noAfterName && noContinuation @@ -260,6 +267,63 @@ func existingPrivateRoot(root string) (string, error) { return root, nil } +func validateStorageLayout(root string) error { + exists, err := preflightRoot(root) + if err != nil { + return errInvalid + } + if !exists { + return nil + } + existingChildren := make([]string, 0, 2) + for _, directory := range []string{"sessions", "oidc"} { + path := filepath.Join(root, directory) + if filepath.Dir(path) != root { + return errInvalid + } + childExists, err := safeio.PreflightPrivateDirectory(path) + if err != nil { + return errInvalid + } + if childExists { + existingChildren = append(existingChildren, path) + } + } + // Close permission/identity races between the individual side-effect-free preflights. + if safeio.ValidatePrivateDirectory(root) != nil { + return errInvalid + } + for _, directory := range []string{"sessions", "oidc"} { + if _, err := safeio.PreflightPrivateDirectory(filepath.Join(root, directory)); err != nil { + return errInvalid + } + } + for _, path := range existingChildren { + if safeio.ValidatePrivateDirectory(path) != nil { + return errInvalid + } + } + return nil +} + +func ensureStorageLayout(root string) error { + if _, err := preflightRoot(root); err != nil || safeio.EnsurePrivateDirectory(root) != nil { + return errInvalid + } + for _, directory := range []string{"sessions", "oidc"} { + path := filepath.Join(root, directory) + if filepath.Dir(path) != root || safeio.EnsurePrivateDirectory(path) != nil { + return errInvalid + } + } + if safeio.ValidatePrivateDirectory(root) != nil || + safeio.ValidatePrivateDirectory(filepath.Join(root, "sessions")) != nil || + safeio.ValidatePrivateDirectory(filepath.Join(root, "oidc")) != nil { + return errInvalid + } + return nil +} + func contentResponse(found bool, contents []byte) response { if !found { return response{Version: protocolVersion, OK: true} diff --git a/tools/tht/internal/authstorage/storage_test.go b/tools/tht/internal/authstorage/storage_test.go index e673a80b..0181a832 100644 --- a/tools/tht/internal/authstorage/storage_test.go +++ b/tools/tht/internal/authstorage/storage_test.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "sync" "testing" @@ -119,6 +120,53 @@ func TestProtocolPreflightsRootWithoutCreatingOrFollowingLinks(t *testing.T) { runRejected(t, request{Version: 1, Operation: "validate-root", Root: filepath.Join(parent, "auth\n")}) } +func TestProtocolValidatesTheCompleteSessionLayoutWithoutCreatingIt(t *testing.T) { + root := filepath.Join(privateTestRoot(t), "auth") + if err := safeio.EnsurePrivateDirectory(root); err != nil { + t.Fatal(err) + } + validated := runRequest(t, request{Version: 1, Operation: "validate-root", Root: root}) + if !validated.Validated { + t.Fatal("layout with safely creatable children was not validated") + } + for _, child := range []string{"sessions", "oidc"} { + if _, err := os.Lstat(filepath.Join(root, child)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("validate-root created %s: %v", child, err) + } + } + + outside := privateTestRoot(t) + testsupport.SymlinkOrSkip(t, outside, filepath.Join(root, "sessions")) + runRejected(t, request{Version: 1, Operation: "validate-root", Root: root}) + if entries, err := os.ReadDir(outside); err != nil || len(entries) != 0 { + t.Fatalf("linked child target was mutated: entries=%v error=%v", entries, err) + } + if err := os.Remove(filepath.Join(root, "sessions")); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(root, "sessions"), 0o700); err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + return // Native DACL coverage lives in storage_windows_test.go. + } + if err := os.Chmod(filepath.Join(root, "sessions"), 0o750); err != nil { + t.Fatal(err) + } + runRejected(t, request{Version: 1, Operation: "validate-root", Root: root}) +} + +func TestProtocolEnsuresTheCompletePrivateSessionLayout(t *testing.T) { + root := filepath.Join(privateTestRoot(t), "auth") + runRequest(t, request{Version: 1, Operation: "ensure-layout", Root: root}) + for _, path := range []string{root, filepath.Join(root, "sessions"), filepath.Join(root, "oidc")} { + if err := safeio.ValidatePrivateDirectory(path); err != nil { + t.Fatalf("private layout path %q: %v", filepath.Base(path), err) + } + } + runRejected(t, request{Version: 1, Operation: "ensure-layout", Root: root, Directory: "sessions"}) +} + func TestProtocolReadsOnlyBoundedPrivateAuthConfig(t *testing.T) { root := privateTestRoot(t) filename := "auth.yaml" diff --git a/tools/tht/internal/authstorage/storage_windows_test.go b/tools/tht/internal/authstorage/storage_windows_test.go index dd0510a2..fbd76bb9 100644 --- a/tools/tht/internal/authstorage/storage_windows_test.go +++ b/tools/tht/internal/authstorage/storage_windows_test.go @@ -55,6 +55,22 @@ func TestProtocolRejectsRecordCreationAfterSessionsDirectoryDACLBecomesPermissiv runRejected(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))}) } +func TestProtocolValidatesAndCreatesTheCompleteWindowsSessionLayout(t *testing.T) { + root := filepath.Join(t.TempDir(), "auth") + runRequest(t, request{Version: 1, Operation: "ensure-layout", Root: root}) + runRequest(t, request{Version: 1, Operation: "validate-root", Root: root}) + for _, directory := range []string{"sessions", "oidc"} { + if err := safeio.ValidatePrivateDirectory(filepath.Join(root, directory)); err != nil { + t.Fatalf("%s DACL = %v", directory, err) + } + } + + if err := setPermissiveDACL(filepath.Join(root, "oidc")); err != nil { + t.Fatal(err) + } + runRejected(t, request{Version: 1, Operation: "validate-root", Root: root}) +} + func setPermissiveDACL(path string) error { world, err := windows.StringToSid("S-1-1-0") if err != nil { diff --git a/tools/tht/internal/safeio/files_unix_test.go b/tools/tht/internal/safeio/files_unix_test.go index d0a966f6..c644dbbd 100644 --- a/tools/tht/internal/safeio/files_unix_test.go +++ b/tools/tht/internal/safeio/files_unix_test.go @@ -107,3 +107,55 @@ func TestReplaceCanonicalRegularRejectsSymlinkedPathComponents(t *testing.T) { t.Fatalf("final symlink replacement error = %v, want ErrUnsafeFile", err) } } + +func TestPrivateDirectoryCreationUsesThePinnedParentAfterAncestorSwap(t *testing.T) { + temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-mkdirat-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + for _, target := range []string{"root", "sessions", "oidc"} { + t.Run(target, func(t *testing.T) { + caseRoot := filepath.Join(root, target) + parent := filepath.Join(caseRoot, "parent") + if err := os.MkdirAll(parent, 0o700); err != nil { + t.Fatal(err) + } + path := filepath.Join(parent, "auth") + swappedAncestor := parent + if target != "root" { + if err := os.Mkdir(path, 0o700); err != nil { + t.Fatal(err) + } + swappedAncestor = path + path = filepath.Join(path, target) + } + outside := filepath.Join(caseRoot, "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + movedAncestor := swappedAncestor + "-original" + + if err := createPrivateDirectoryAfterParentOpen(path, func() { + if err := os.Rename(swappedAncestor, movedAncestor); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, swappedAncestor); err != nil { + t.Fatal(err) + } + }); err != nil { + t.Fatal(err) + } + if err := ValidatePrivateDirectory(filepath.Join(movedAncestor, filepath.Base(path))); err != nil { + t.Fatalf("pinned-parent creation failed: %v", err) + } + if _, err := os.Lstat(filepath.Join(outside, filepath.Base(path))); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("outside target was mutated: %v", err) + } + }) + } +} diff --git a/tools/tht/internal/safeio/private_unix.go b/tools/tht/internal/safeio/private_unix.go index 895e57c1..2e3d8769 100644 --- a/tools/tht/internal/safeio/private_unix.go +++ b/tools/tht/internal/safeio/private_unix.go @@ -11,11 +11,28 @@ import ( ) func createPrivateDirectory(path string) error { + return createPrivateDirectoryAfterParentOpen(path, nil) +} + +func createPrivateDirectoryAfterParentOpen(path string, afterOpen func()) error { parents, err := openCanonicalUnixParent(path) if err != nil { return ErrUnsafeFile } defer parents.Close() + if afterOpen != nil { + afterOpen() + } + return createPrivateDirectoryAt(parents) +} + +// createPrivateDirectoryAt performs every mutating operation relative to the already-opened +// parent. An attacker can rename or replace any lexical ancestor after the open without +// redirecting mkdir or chmod into a different directory tree. +func createPrivateDirectoryAt(parents *unixParentHandles) error { + if parents == nil || parents.parent < 0 || parents.target == "" { + return ErrUnsafeFile + } if err := unix.Mkdirat(parents.parent, parents.target, 0o700); err != nil { if errors.Is(err, unix.EEXIST) { return os.ErrExist