From be1724890a902adc7056438ce4dbc57514d41804 Mon Sep 17 00:00:00 2001 From: mptyl Date: Mon, 17 Aug 2026 12:19:19 +0200 Subject: [PATCH] fix(auth): make diagnostics bounded and portable --- backend/src/app.ts | 3 +- backend/src/auth/authentik-group-catalog.ts | 26 ++- backend/src/auth/config.ts | 40 +++- backend/src/auth/diagnostics.ts | 117 +++++++--- backend/src/auth/oidc-client.ts | 50 +++-- backend/src/auth/secret-policy.ts | 19 ++ backend/src/auth/session-store.ts | 115 ++++++++-- backend/src/auth/windows-auth-storage.ts | 96 +++++++- backend/src/config/secret-bundle.ts | 13 +- backend/test/auth-config.test.ts | 35 +++ backend/test/auth-diagnostics.test.ts | 208 +++++++++++++++++- backend/test/auth-session-store.test.ts | 54 ++++- backend/test/authentik-group-catalog.test.ts | 37 ++++ backend/test/oidc-client.test.ts | 11 + backend/test/secret-bundle.test.ts | 11 + backend/test/windows-auth-storage.test.ts | 36 +++ tools/tht/internal/authstorage/storage.go | 69 ++++-- .../tht/internal/authstorage/storage_test.go | 60 +++++ tools/tht/internal/safeio/files.go | 21 +- tools/tht/internal/safeio/files_test.go | 42 ++++ tools/tht/internal/safeio/preflight_unix.go | 75 +++++++ .../tht/internal/safeio/preflight_windows.go | 39 ++++ tools/tht/internal/safeio/private_unix.go | 27 ++- 23 files changed, 1088 insertions(+), 116 deletions(-) create mode 100644 backend/src/auth/secret-policy.ts create mode 100644 tools/tht/internal/safeio/preflight_unix.go create mode 100644 tools/tht/internal/safeio/preflight_windows.go diff --git a/backend/src/app.ts b/backend/src/app.ts index 6a26e9e2..a6339f17 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -15,6 +15,7 @@ import { createCurrentLocalUserRegistryResolver, type LocalUserRegistry } from " import { AuthSessionOperationalError, createFileAuthSessionStore, type AuthSessionStore, type AuthSessionValidity } from "./auth/session-store.js"; import { registerAuthRoutes } from "./auth/routes.js"; import { createOidcProtocol, type OidcProtocol } from "./auth/oidc-client.js"; +import { isUsableAuthenticationSecret } from "./auth/secret-policy.js"; import { sessionRoutes } from "./routes/sessions.js"; import { sqlRoutes } from "./routes/sql.js"; import { metaRoutes, type ListModelsFn } from "./routes/meta.js"; @@ -198,7 +199,7 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc if (deps?.oidcProtocol) return deps.oidcProtocol; if (loaded.value.mode !== "oidc") return undefined; const clientSecret = process.env.THT_OIDC_CLIENT_SECRET; - if (typeof clientSecret !== "string" || clientSecret.length === 0 || clientSecret.length > 4096 || /\p{Cc}/u.test(clientSecret)) { + if (!isUsableAuthenticationSecret("THT_OIDC_CLIENT_SECRET", clientSecret)) { return undefined; } try { diff --git a/backend/src/auth/authentik-group-catalog.ts b/backend/src/auth/authentik-group-catalog.ts index 8dd9bbf3..6d9f5180 100644 --- a/backend/src/auth/authentik-group-catalog.ts +++ b/backend/src/auth/authentik-group-catalog.ts @@ -1,4 +1,5 @@ import type { AuthDiagnostic, GroupCatalog } from "./group-catalog.js"; +import { isUsableAuthenticationSecret } from "./secret-policy.js"; import { parseConfiguredTransportUrl } from "./url-policy.js"; const MAX_RESPONSE_BYTES = 1024 * 1024; @@ -34,10 +35,6 @@ function ambiguous(name: string): AuthDiagnostic { return diagnostic("oidc_mapped_group_ambiguous", "A configured authorization group is ambiguous.", name); } -function safeApiToken(value: string): boolean { - return typeof value === "string" && value.length > 0 && value.length <= 16 * 1024 && !/\p{Cc}/u.test(value); -} - function stableCompare(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0; } @@ -143,10 +140,21 @@ type GroupResult = "present" | "missing" | "ambiguous" | "unauthorized" | "unrea function exactResult(name: string, parsed: unknown): GroupResult { if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return "unreachable"; const record = parsed as { results?: unknown; pagination?: unknown }; - if (!Array.isArray(record.results) || !record.pagination || typeof record.pagination !== "object" + if (!Array.isArray(record.results) || record.results.length > 2 + || !record.pagination || typeof record.pagination !== "object" || Array.isArray(record.pagination)) return "unreachable"; - const next = (record.pagination as { next?: unknown }).next; - if (next !== null && next !== undefined) return "ambiguous"; + if (!Object.prototype.hasOwnProperty.call(record.pagination, "next")) return "unreachable"; + const next = (record.pagination as { next: unknown }).next; + if (next !== null) { + if (typeof next !== "string" || next.length === 0 || next.length > 2048 || /\p{Cc}/u.test(next)) return "unreachable"; + try { + const continuation = new URL(next); + if (continuation.protocol !== "https:" || continuation.username || continuation.password || continuation.hash) return "unreachable"; + } catch { + return "unreachable"; + } + return "ambiguous"; + } const resultNames: string[] = []; for (const result of record.results) { if (!result || typeof result !== "object" || Array.isArray(result) @@ -161,7 +169,9 @@ function exactResult(name: string, parsed: unknown): GroupResult { export function createAuthentikGroupCatalog(options: AuthentikGroupCatalogOptions): GroupCatalog { const origin = parseConfiguredTransportUrl(options.baseUrl, { allowLoopbackHttp: false, originOnly: true }); const fetchImplementation = options.fetch ?? globalThis.fetch; - const valid = origin !== undefined && safeApiToken(options.apiToken) && typeof fetchImplementation === "function"; + const valid = origin !== undefined + && isUsableAuthenticationSecret("THT_AUTHENTIK_API_TOKEN", options.apiToken) + && typeof fetchImplementation === "function"; async function verify(name: string, signal: AbortSignal): Promise { if (!origin || !valid || signal.aborted) return "unreachable"; diff --git a/backend/src/auth/config.ts b/backend/src/auth/config.ts index 792470e5..f3c97aa6 100644 --- a/backend/src/auth/config.ts +++ b/backend/src/auth/config.ts @@ -21,6 +21,7 @@ import type { Role, } from "./types.js"; import { parseConfiguredTransportUrl } from "./url-policy.js"; +import { createWindowsAuthStorageBridge, type WindowsAuthStorageBridge } from "./windows-auth-storage.js"; export type { AuthenticationConfig, @@ -32,6 +33,8 @@ export type { } from "./types.js"; const MAX_AUTH_CONFIG_BYTES = 1024 * 1024; +// Keep live catalog work within the same deterministic bound as the mandatory direct groups claim. +const MAX_MAPPED_GROUPS = 128; const ROLES = ["user", "admin"] as const; export const PERMISSION_CATALOG: readonly Permission[] = [ "session.use", "session.read_all", "session.manage_all", "settings.manage", @@ -52,7 +55,8 @@ const sessionSchema = z.strictObject({ }); const roleSchema = z.enum(ROLES); const groupNameSchema = nonEmptyText.max(256); -const groupRolesSchema = z.record(groupNameSchema, z.array(roleSchema).min(1)); +const groupRolesSchema = z.record(groupNameSchema, z.array(roleSchema).min(1)) + .refine((value) => Object.keys(value).length <= MAX_MAPPED_GROUPS); const localSchema = z.strictObject({ version: z.literal(1), mode: z.literal("local"), publicUrl: nonEmptyText, session: sessionSchema.optional(), @@ -94,6 +98,11 @@ interface StorageIdentity { directory: DirectoryIdentity; } +export interface AuthenticationConfigLoadOptions { + /** Test seam; production creates the existing bounded internal tht auth-storage bridge. */ + windowsStorageBridge?: Pick; +} + function validateCanonicalPath(path: string): void { if (typeof path !== "string" || path.length === 0 || path.trim() !== path || path.includes("\0") || !isAbsolute(path) || normalize(path) !== path @@ -249,11 +258,36 @@ function loadAuthenticationConfigWithIdentity(path: string): { loaded: LoadedAut return { loaded: { value, revision: canonicalRevision(value), sourcePath: path }, identity: read.identity }; } -export function loadAuthenticationConfig(path: string): LoadedAuthConfig { +function loadWindowsAuthenticationConfig( + path: string, + bridge: Pick, +): LoadedAuthConfig { + try { + const contents = bridge.readAuthConfig(path); + if (!Buffer.isBuffer(contents) || contents.length === 0 || contents.length > MAX_AUTH_CONFIG_BYTES) throw invalid(); + const source = new TextDecoder("utf-8", { fatal: true }).decode(contents); + const value = parseAuthenticationConfig(source); + return { value, revision: canonicalRevision(value), sourcePath: path }; + } catch { + throw invalid(); + } +} + +export function loadAuthenticationConfig(path: string, options: AuthenticationConfigLoadOptions = {}): LoadedAuthConfig { + if (process.platform === "win32") { + return loadWindowsAuthenticationConfig(path, options.windowsStorageBridge ?? createWindowsAuthStorageBridge()); + } return loadAuthenticationConfigWithIdentity(path).loaded; } -export function createAuthenticationConfigProvider(path: string): AuthenticationConfigProvider { +export function createAuthenticationConfigProvider( + path: string, + options: AuthenticationConfigLoadOptions = {}, +): AuthenticationConfigProvider { + if (process.platform === "win32") { + const bridge = options.windowsStorageBridge ?? createWindowsAuthStorageBridge(); + return { current: () => loadWindowsAuthenticationConfig(path, bridge) }; + } let cached: { identity: StorageIdentity; loaded: LoadedAuthConfig } | undefined; return { current(): LoadedAuthConfig { const before = storageIdentity(path); diff --git a/backend/src/auth/diagnostics.ts b/backend/src/auth/diagnostics.ts index f91ccdf5..b97452d1 100644 --- a/backend/src/auth/diagnostics.ts +++ b/backend/src/auth/diagnostics.ts @@ -2,10 +2,14 @@ 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 { isUsableAuthenticationSecret, type AuthenticationSecretReference } from "./secret-policy.js"; import type { AuthDiagnostic, AuthDiagnosticCode, AuthDiagnostics, GroupCatalog } from "./group-catalog.js"; export type { AuthDiagnostic, AuthDiagnosticCode, AuthDiagnostics } from "./group-catalog.js"; +const LIVE_DIAGNOSTIC_TIMEOUT_MS = 30_000; + export interface AuthDiagnoser { inspect(options: { live: boolean; interactive?: boolean; signal?: AbortSignal }): Promise; } @@ -15,6 +19,7 @@ export interface AuthDiagnoserDependencies { authStateRoot: string; /** Platform integrations may inject an equivalent side-effect-free owner/ACL validator. */ sessionRootValidator?: (root: string) => void | Promise; + windowsStorageBridge?: Pick; authentication?: AuthenticationConfigProvider; secrets?: ReadonlyMap; localUserRegistry?: LocalUserRegistry; @@ -26,11 +31,6 @@ function check(code: AuthDiagnosticCode, message: string, field?: string): AuthD return { level: "error", code, message, ...(field === undefined ? {} : { field }) }; } -function secretPresent(secrets: ReadonlyMap | undefined, name: string): boolean { - const value = secrets?.get(name); - return typeof value === "string" && value.length > 0 && value.length <= 16 * 1024 && !/\p{Cc}/u.test(value); -} - function ordered(checks: readonly AuthDiagnostic[]): readonly AuthDiagnostic[] { const unique = new Map(); for (const item of checks) unique.set(`${item.code}\u0000${item.field ?? ""}`, item); @@ -45,6 +45,45 @@ function stableCompare(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0; } +function abortReason(signal: AbortSignal): unknown { + return signal.reason ?? new DOMException("The operation was aborted", "AbortError"); +} + +function awaitWithAbort(operation: Promise, signal: AbortSignal): Promise { + return new Promise((resolve, reject) => { + let settled = false; + const abort = () => { + if (settled) return; + settled = true; + signal.removeEventListener("abort", abort); + reject(abortReason(signal)); + }; + if (signal.aborted) abort(); + else signal.addEventListener("abort", abort, { once: true }); + operation.then( + (value) => { + if (settled) return; + settled = true; + signal.removeEventListener("abort", abort); + resolve(value); + }, + (error: unknown) => { + if (settled) return; + settled = true; + signal.removeEventListener("abort", abort); + reject(error); + }, + ); + }); +} + +function startBeforeAbort(signal: AbortSignal, operation: () => Promise): Promise { + return Promise.resolve().then(() => { + if (signal.aborted) throw abortReason(signal); + return operation(); + }); +} + async function localRegistryIsUsable(deps: AuthDiagnoserDependencies): Promise { try { if (!deps.localUserRegistry) { @@ -60,12 +99,15 @@ async function localRegistryIsUsable(deps: AuthDiagnoserDependencies): Promise (deps.windowsStorageBridge ?? createWindowsAuthStorageBridge()).validateRoot(root) + : validateAuthSessionRoot); return { async inspect(options): Promise { const checks: AuthDiagnostic[] = []; const signal = options.signal ?? new AbortController().signal; try { - await (deps.sessionRootValidator ?? validateAuthSessionRoot)(deps.authStateRoot); + await validateSessionRoot(deps.authStateRoot); } catch { checks.push(check("auth_session_store_invalid", "The authentication session store is invalid.")); } @@ -103,8 +145,8 @@ export function createAuthDiagnoser(deps: AuthDiagnoserDependencies): AuthDiagno : { ready: false, mode: "local", checks: result }; } - const requiredSecrets = ["THT_OIDC_CLIENT_SECRET", "THT_AUTHENTIK_API_TOKEN"]; - if (requiredSecrets.some((name) => !secretPresent(deps.secrets, name))) { + const requiredSecrets: readonly AuthenticationSecretReference[] = ["THT_OIDC_CLIENT_SECRET", "THT_AUTHENTIK_API_TOKEN"]; + if (requiredSecrets.some((name) => !isUsableAuthenticationSecret(name, deps.secrets?.get(name)))) { checks.push(check("oidc_secret_missing", "A required OIDC or group catalog secret is unavailable.")); } if (!options.live || checks.length > 0) { @@ -113,33 +155,44 @@ export function createAuthDiagnoser(deps: AuthDiagnoserDependencies): AuthDiagno ? { ready: true, mode: "oidc", checks: [{ level: "info", code: "auth_ready", message: "Authentication is ready." }] } : { ready: false, mode: "oidc", checks: result }; } + const mappedGroupNames = Object.keys(loaded.value.authorization.groupRoles).sort(stableCompare); - if (!deps.oidcProtocol) { - checks.push(check("oidc_discovery_unreachable", "The OIDC provider is unavailable.")); - } else { - try { - await deps.oidcProtocol.diagnose(signal); - } catch (error) { - checks.push(check( - error instanceof OidcIssuerMismatchError - ? "oidc_issuer_mismatch" - : error instanceof OidcJwksUnavailableError - ? "oidc_jwks_unreachable" - : "oidc_discovery_unreachable", - "The OIDC provider could not be validated.", - )); + const deadline = new AbortController(); + const deadlineTimer = setTimeout(() => deadline.abort(), LIVE_DIAGNOSTIC_TIMEOUT_MS); + deadlineTimer.unref(); + const liveSignal = AbortSignal.any([signal, deadline.signal]); + try { + if (!deps.oidcProtocol) { + checks.push(check("oidc_discovery_unreachable", "The OIDC provider is unavailable.")); + } else { + try { + await awaitWithAbort(startBeforeAbort(liveSignal, () => deps.oidcProtocol!.diagnose(liveSignal)), liveSignal); + } catch (error) { + checks.push(check( + error instanceof OidcIssuerMismatchError + ? "oidc_issuer_mismatch" + : error instanceof OidcJwksUnavailableError + ? "oidc_jwks_unreachable" + : "oidc_discovery_unreachable", + "The OIDC provider could not be validated.", + )); + } } - } - if (!deps.groupCatalog) { - checks.push(check("oidc_group_catalog_unreachable", "The configured group catalog cannot be certified.")); - } else { - try { - checks.push(...await deps.groupCatalog.verifyConfiguredGroups( - Object.keys(loaded.value.authorization.groupRoles).sort(stableCompare), signal, - )); - } catch { - checks.push(check("oidc_group_catalog_unreachable", "The configured group catalog is unavailable.")); + if (!liveSignal.aborted) { + if (!deps.groupCatalog) { + checks.push(check("oidc_group_catalog_unreachable", "The configured group catalog cannot be certified.")); + } else { + try { + checks.push(...await awaitWithAbort(startBeforeAbort(liveSignal, () => deps.groupCatalog!.verifyConfiguredGroups( + mappedGroupNames, liveSignal, + )), liveSignal)); + } catch { + checks.push(check("oidc_group_catalog_unreachable", "The configured group catalog is unavailable.")); + } + } } + } finally { + clearTimeout(deadlineTimer); } const result = ordered(checks); return result.length === 0 diff --git a/backend/src/auth/oidc-client.ts b/backend/src/auth/oidc-client.ts index e7c1cd60..56fc5d41 100644 --- a/backend/src/auth/oidc-client.ts +++ b/backend/src/auth/oidc-client.ts @@ -9,6 +9,7 @@ import { } from "openid-client"; import { constants, createPublicKey, verify as verifySignature } from "node:crypto"; import { parseConfiguredTransportUrl } from "./url-policy.js"; +import { isUsableAuthenticationSecret } from "./secret-policy.js"; export interface OidcIdentity { issuer: string; @@ -79,6 +80,23 @@ 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 discoveryStringList(value: unknown): value is readonly string[] { + return Array.isArray(value) && value.length > 0 && value.length <= 128 + && value.every((item) => text(item, 128)); +} + +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); +} + function configuredHttpsUrl(value: string): URL { const url = parseConfiguredTransportUrl(value, { allowLoopbackHttp: false }); if (!url) throw new OidcProtocolError(); @@ -463,7 +481,7 @@ async function verifyJwksAvailability( export function createOidcProtocol(options: OidcProtocolOptions): OidcProtocol { const issuerUrl = configuredHttpsUrl(options.issuer); const callbackUrl = configuredCallbackUrl(options.callbackUrl); - if (!text(options.clientId, 512) || !text(options.clientSecret, 4096) + if (!text(options.clientId, 512) || !isUsableAuthenticationSecret("THT_OIDC_CLIENT_SECRET", options.clientSecret) || !text(options.groupsClaim, 128) || options.scopes.length === 0 || options.scopes.length > 16 || options.scopes.some((scope) => !text(scope, 128)) || (options.httpTimeoutMs !== undefined && (!Number.isSafeInteger(options.httpTimeoutMs) @@ -478,19 +496,24 @@ export function createOidcProtocol(options: OidcProtocolOptions): OidcProtocol { const configuration = async (): Promise => { if (!discovered) { discovered = (async () => { - let inspectingDiscovery = true; + let certifiedIssuerMismatch = false; const issuerCheckingFetch: CustomFetch = async (input, init) => { const response = await transport.customFetch(input, init); - if (inspectingDiscovery) { - try { - const metadata = await response.clone().json() as { issuer?: unknown }; - if (typeof metadata?.issuer === "string" && metadata.issuer !== options.issuer) { - throw new OidcIssuerMismatchError(); - } - } catch (error) { - if (error instanceof OidcIssuerMismatchError) throw error; - // The OIDC library owns all other discovery-document validation. + if (!response.ok) return response; + try { + const metadata: unknown = await response.clone().json(); + if (schemaValidDiscoveryMetadata(metadata) && metadata.issuer !== options.issuer) { + certifiedIssuerMismatch = true; + const headers = new Headers(response.headers); + headers.delete("content-length"); + return new Response(JSON.stringify({ ...metadata, issuer: options.issuer }), { + status: response.status, + statusText: response.statusText, + headers, + }); } + } catch { + // The OIDC library owns malformed discovery-document classification. } return response; }; @@ -503,15 +526,14 @@ export function createOidcProtocol(options: OidcProtocolOptions): OidcProtocol { { [customFetch]: issuerCheckingFetch, timeout: httpTimeoutMs / 1000 }, ); const metadata = config.serverMetadata(); - if (metadata.issuer !== options.issuer) throw new OidcIssuerMismatchError(); + 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); return config; } catch (error) { throw protocolFailure(error); - } finally { - inspectingDiscovery = false; } })(); } diff --git a/backend/src/auth/secret-policy.ts b/backend/src/auth/secret-policy.ts new file mode 100644 index 00000000..e7d400d4 --- /dev/null +++ b/backend/src/auth/secret-policy.ts @@ -0,0 +1,19 @@ +export const AUTHENTICATION_SECRET_LIMITS = Object.freeze({ + THT_OIDC_CLIENT_SECRET: 4096, + THT_AUTHENTIK_API_TOKEN: 16 * 1024, +} as const); + +export type AuthenticationSecretReference = keyof typeof AUTHENTICATION_SECRET_LIMITS; + +export function isAuthenticationSecretReference(value: string): value is AuthenticationSecretReference { + return Object.prototype.hasOwnProperty.call(AUTHENTICATION_SECRET_LIMITS, value); +} + +/** One policy shared by bundle loading, runtime adapters, and static diagnostics. */ +export function isUsableAuthenticationSecret( + name: AuthenticationSecretReference, + value: unknown, +): value is string { + return typeof value === "string" && value.length > 0 + && value.length <= AUTHENTICATION_SECRET_LIMITS[name] && !/\p{Cc}/u.test(value); +} diff --git a/backend/src/auth/session-store.ts b/backend/src/auth/session-store.ts index 229d0dbe..f24c936c 100644 --- a/backend/src/auth/session-store.ts +++ b/backend/src/auth/session-store.ts @@ -1,6 +1,6 @@ import { createHash, hkdfSync, randomBytes } from "node:crypto"; import { - chmodSync, + accessSync, closeSync, constants, fchmodSync, @@ -297,6 +297,10 @@ 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 { @@ -377,26 +381,12 @@ function directoryScanIdentity(path: string): DirectoryScanIdentity { return { ...identity, mtimeMs: info.mtimeMs, ctimeMs: info.ctimeMs }; } -function privateDirectory(path: string): void { - let created = false; - try { - lstatSync(path); - } catch (error) { - if (!isNotFound(error)) throw invalid(); - try { - mkdirSync(path, { recursive: true, mode: PRIVATE_DIRECTORY_MODE }); - chmodSync(path, PRIVATE_DIRECTORY_MODE); - created = true; - } catch { - throw invalid(); - } - } - try { - directoryIdentity(path); - } catch { - if (!created) throw invalid(); - throw invalid(); - } +interface SessionRootAncestor { + path: string; + descriptor: number; + dev: number; + ino: number; + uid: number; } function validateSessionRootSyntax(root: string): void { @@ -404,11 +394,90 @@ function validateSessionRootSyntax(root: string): void { || root.includes("\0") || /\p{Cc}/u.test(root) || !isAbsolute(root) || normalize(root) !== root) throw invalid(); } +function sameAncestor(ancestor: SessionRootAncestor, info: Stats): boolean { + return info.isDirectory() && !info.isSymbolicLink() && ancestor.dev === info.dev + && ancestor.ino === info.ino && ancestor.uid === info.uid; +} + +function withSessionRootPreflight(root: string, use: (exists: boolean) => T): T { + validateSessionRootSyntax(root); + const ancestors: SessionRootAncestor[] = []; + try { + const components = root.split("/").filter((component) => component.length > 0); + let current = "/"; + for (let index = -1; index < components.length; index += 1) { + if (index >= 0) current = join(current, components[index]!); + let info: Stats; + try { + info = lstatSync(current) as Stats; + } catch (error) { + if (!isNotFound(error) || index !== components.length - 1) throw invalid(); + accessSync(dirname(current), constants.W_OK | constants.X_OK); + for (const ancestor of ancestors) { + const observed = lstatSync(ancestor.path) as Stats; + if (!sameAncestor(ancestor, observed) || realpathSync(ancestor.path) !== ancestor.path) throw invalid(); + } + const result = use(false); + for (const ancestor of ancestors) { + const observed = lstatSync(ancestor.path) as Stats; + if (!sameAncestor(ancestor, observed) || realpathSync(ancestor.path) !== ancestor.path) throw invalid(); + } + return result; + } + if (!info.isDirectory() || info.isSymbolicLink() || realpathSync(current) !== current) throw invalid(); + 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) { + closeSync(descriptor); + throw invalid(); + } + ancestors.push({ path: current, descriptor, dev: info.dev, ino: info.ino, uid: info.uid }); + } + directoryIdentity(root); + const result = use(true); + for (const ancestor of ancestors) { + const observed = lstatSync(ancestor.path) as Stats; + if (!sameAncestor(ancestor, observed) || realpathSync(ancestor.path) !== ancestor.path) throw invalid(); + } + return result; + } catch { + throw invalid(); + } finally { + for (const ancestor of ancestors.reverse()) { + try { closeSync(ancestor.descriptor); } catch { /* preflight has already failed closed */ } + } + } +} + +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 { - validateSessionRootSyntax(root); - directoryIdentity(root); + withSessionRootPreflight(root, () => undefined); } catch { throw invalid(); } diff --git a/backend/src/auth/windows-auth-storage.ts b/backend/src/auth/windows-auth-storage.ts index 36720739..9448ebd4 100644 --- a/backend/src/auth/windows-auth-storage.ts +++ b/backend/src/auth/windows-auth-storage.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { win32 } from "node:path"; import type { Readable, Writable } from "node:stream"; import { z } from "zod"; @@ -6,6 +6,9 @@ import { z } from "zod"; const PROTOCOL_VERSION = 1; const MAX_PROTOCOL_BYTES = 64 * 1024; const MAX_RESPONSE_BYTES = 64 * 1024; +const MAX_AUTH_CONFIG_BYTES = 1024 * 1024; +const MAX_AUTH_CONFIG_BASE64_BYTES = 4 * Math.ceil(MAX_AUTH_CONFIG_BYTES / 3); +const MAX_AUTH_CONFIG_RESPONSE_BYTES = MAX_AUTH_CONFIG_BASE64_BYTES + 1024; const MAX_SESSION_BYTES = 16 * 1024; const MAX_OIDC_BYTES = 8 * 1024; const DEFAULT_MAX_ENTRIES = 256; @@ -14,6 +17,7 @@ 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 AUTH_CONFIG_FILENAME = /^[A-Za-z0-9][A-Za-z0-9._-]{0,249}\.yaml$/; const invalid = (): Error => new Error("auth_session_store_invalid"); @@ -31,6 +35,8 @@ export interface WindowsAuthStoragePage { /** Internal adapter boundary for the file-session store's native Windows path. */ export interface WindowsAuthStorageBridge { + validateRoot(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; replace(root: string, directory: WindowsAuthStorageDirectory, filename: string, contents: Buffer): Promise; @@ -56,6 +62,7 @@ export interface WindowsAuthStorageInvocation { args: readonly string[]; input: Buffer; timeoutMs: number; + maximumOutputBytes: number; } export interface WindowsAuthStorageInvocationResult { @@ -83,6 +90,8 @@ type WindowsAuthStorageSpawn = ( export interface WindowsAuthStorageBridgeOptions { /** Test-only transport seam. Production always uses the no-shell child-process invocation. */ invoke?: (invocation: WindowsAuthStorageInvocation) => Promise; + /** Test-only synchronous seam used by the synchronous authentication-config provider. */ + invokeSync?: (invocation: WindowsAuthStorageInvocation) => WindowsAuthStorageInvocationResult; /** Optional configured tht path. Defaults to THT_BIN, then the safe bare command `tht`. */ thtExecutable?: string; /** Test-only child-launch seam; production uses the fixed no-shell Node child-process launcher. */ @@ -98,21 +107,22 @@ const responseSchema = z.strictObject({ replaced: z.boolean().optional(), removed: z.boolean().optional(), found: z.boolean().optional(), - contentBase64: z.string().max(MAX_PROTOCOL_BYTES).optional(), + contentBase64: z.string().max(MAX_AUTH_CONFIG_BASE64_BYTES).optional(), entries: z.array(z.strictObject({ name: z.string().max(128), modifiedUnixMs: z.number().int().safe().nonnegative(), })).max(MAX_ENTRIES).optional(), more: z.boolean().optional(), + validated: z.boolean().optional(), }); type BridgeResponse = z.infer; interface BridgeRequest { version: typeof PROTOCOL_VERSION; - operation: "create" | "read" | "replace" | "remove" | "list" | "claim-consume" | "read-claim" | "remove-claim"; + operation: "validate-root" | "read-auth-config" | "create" | "read" | "replace" | "remove" | "list" | "claim-consume" | "read-claim" | "remove-claim"; root: string; - directory: WindowsAuthStorageDirectory; + directory?: WindowsAuthStorageDirectory; filename?: string; contentBase64?: string; maximumEntries?: number; @@ -154,9 +164,10 @@ function safeThtExecutable(value: string | undefined): string { throw invalid(); } -function parseResponse(result: WindowsAuthStorageInvocationResult): BridgeResponse { +function parseResponse(result: WindowsAuthStorageInvocationResult, maximumOutputBytes: number): BridgeResponse { if (!Number.isInteger(result.code) || result.code !== 0 || !Buffer.isBuffer(result.stdout) - || !Buffer.isBuffer(result.stderr) || result.stdout.length === 0 || result.stdout.length > MAX_RESPONSE_BYTES) { + || !Buffer.isBuffer(result.stderr) || result.stderr.length > MAX_RESPONSE_BYTES + || result.stdout.length === 0 || result.stdout.length > maximumOutputBytes) { throw invalid(); } try { @@ -169,7 +180,15 @@ function parseResponse(result: WindowsAuthStorageInvocationResult): BridgeRespon function encodedRequest(request: BridgeRequest): Buffer { validateRoot(request.root); - if (request.operation === "list") { + if (request.operation === "validate-root") { + 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") { + if (request.directory !== undefined || request.contentBase64 !== undefined || request.maximumEntries !== undefined + || request.afterName !== undefined || request.continuation !== undefined + || request.filename === undefined || !AUTH_CONFIG_FILENAME.test(request.filename)) throw invalid(); + } else if (request.operation === "list") { + if (request.directory === undefined) throw invalid(); if (request.filename !== undefined || request.contentBase64 !== undefined) throw invalid(); if (request.maximumEntries !== undefined && (!Number.isInteger(request.maximumEntries) || request.maximumEntries < 1 || request.maximumEntries > MAX_ENTRIES)) throw invalid(); @@ -181,6 +200,7 @@ function encodedRequest(request: BridgeRequest): Buffer { throw invalid(); } } else { + if (request.directory === undefined) throw invalid(); if (request.maximumEntries !== undefined || request.afterName !== undefined || request.continuation !== undefined) throw invalid(); if (request.filename === undefined) throw invalid(); const allowClaim = request.operation === "remove" && request.directory === "oidc"; @@ -191,7 +211,10 @@ function encodedRequest(request: BridgeRequest): Buffer { } if ((request.operation === "claim-consume" || request.operation === "read-claim" || request.operation === "remove-claim") && request.directory !== "oidc") throw invalid(); - if (request.contentBase64 !== undefined) canonicalBase64(request.contentBase64, directoryMaximum(request.directory)); + if (request.contentBase64 !== undefined) { + if (request.directory === undefined) throw invalid(); + canonicalBase64(request.contentBase64, directoryMaximum(request.directory)); + } const encoded = Buffer.from(JSON.stringify(request), "utf8"); if (encoded.length === 0 || encoded.length > MAX_PROTOCOL_BYTES) throw invalid(); return encoded; @@ -208,6 +231,25 @@ function environmentForBridge(): NodeJS.ProcessEnv { const spawnTht: WindowsAuthStorageSpawn = (executable, args, options) => spawn(executable, [...args], options); +function invokeThtSync(invocation: WindowsAuthStorageInvocation): WindowsAuthStorageInvocationResult { + try { + const result = spawnSync(invocation.executable, [...invocation.args], { + shell: false, + windowsHide: true, + env: environmentForBridge(), + input: invocation.input, + timeout: invocation.timeoutMs, + maxBuffer: invocation.maximumOutputBytes, + encoding: "buffer", + }); + if (result.error || result.signal !== null || typeof result.status !== "number" + || !Buffer.isBuffer(result.stdout) || !Buffer.isBuffer(result.stderr)) throw invalid(); + return { code: result.status, stdout: result.stdout, stderr: result.stderr }; + } catch { + throw invalid(); + } +} + async function invokeTht( invocation: WindowsAuthStorageInvocation, spawnChild: WindowsAuthStorageSpawn = spawnTht, @@ -355,20 +397,37 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge options.spawnChild, options.beforeInputForTest, )); + const invokeSync = options.invokeSync ?? invokeThtSync; const request = async (value: BridgeRequest): Promise => { try { + const maximumOutputBytes = MAX_RESPONSE_BYTES; const response = await invoke({ executable, args: ["_auth-storage"], input: encodedRequest(value), timeoutMs: TIMEOUT_MS, + maximumOutputBytes, }); - return parseResponse(response); + return parseResponse(response, maximumOutputBytes); } catch { throw invalid(); } }; - const recordRequest = (operation: BridgeRequest["operation"], root: string, directory: WindowsAuthStorageDirectory, filename: string, contents?: Buffer): BridgeRequest => ({ + const syncRequest = (value: BridgeRequest): BridgeResponse => { + try { + const response = invokeSync({ + executable, + args: ["_auth-storage"], + input: encodedRequest(value), + timeoutMs: TIMEOUT_MS, + maximumOutputBytes: MAX_AUTH_CONFIG_RESPONSE_BYTES, + }); + return parseResponse(response, MAX_AUTH_CONFIG_RESPONSE_BYTES); + } catch { + throw invalid(); + } + }; + const recordRequest = (operation: "create" | "read" | "replace" | "remove" | "claim-consume" | "read-claim" | "remove-claim", root: string, directory: WindowsAuthStorageDirectory, filename: string, contents?: Buffer): BridgeRequest => ({ version: PROTOCOL_VERSION, operation, root, @@ -378,6 +437,23 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge }); return { + async validateRoot(root) { + const response = await request({ version: PROTOCOL_VERSION, operation: "validate-root", root }); + if (response.validated !== true + || Object.keys(response).some((key) => !["version", "ok", "validated"].includes(key))) throw invalid(); + }, + readAuthConfig(path) { + 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(); + 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); + if (contents === undefined) throw invalid(); + return contents; + }, async create(root, directory, filename, contents) { if (!Buffer.isBuffer(contents) || contents.length === 0 || contents.length > directoryMaximum(directory)) throw invalid(); const response = await request(recordRequest("create", root, directory, filename, contents)); diff --git a/backend/src/config/secret-bundle.ts b/backend/src/config/secret-bundle.ts index 960ac0a1..5dfe95b8 100644 --- a/backend/src/config/secret-bundle.ts +++ b/backend/src/config/secret-bundle.ts @@ -2,6 +2,11 @@ import { closeSync, constants, fstatSync, lstatSync, openSync, readFileSync, type Stats, } from "node:fs"; +import { + AUTHENTICATION_SECRET_LIMITS, + isAuthenticationSecretReference, + isUsableAuthenticationSecret, +} from "../auth/secret-policy.js"; /** Keys accepted by the deployment bundle. Keep this list intentionally explicit. */ export const SECRET_BUNDLE_KEYS = Object.freeze([ @@ -26,7 +31,10 @@ const LEGACY_FILES: Readonly> = { }; const MAX_BUNDLE_BYTES = 64 * 1024; -const MAX_LINE_BYTES = 16 * 1024; +const MAX_LINE_BYTES = Math.max( + 16 * 1024, + ...Object.entries(AUTHENTICATION_SECRET_LIMITS).map(([name, maximum]) => name.length + 1 + maximum), +); export interface SecretBundleConfig { secretsFile?: string; @@ -98,7 +106,8 @@ function parseBundle(text: string): ReadonlyMap { const match = /^([A-Z][A-Z0-9_]*)=(.*)$/.exec(line); if (!match) throw unavailable(); const [, key, value] = match; - if (!ALLOWED.has(key) || values.has(key) || value.length === 0 || /[\r\n]/.test(value)) { + if (!ALLOWED.has(key) || values.has(key) || value.length === 0 || /[\r\n]/.test(value) + || isAuthenticationSecretReference(key) && !isUsableAuthenticationSecret(key, value)) { throw unavailable(); } values.set(key, value); diff --git a/backend/test/auth-config.test.ts b/backend/test/auth-config.test.ts index 59eb7506..153755db 100644 --- a/backend/test/auth-config.test.ts +++ b/backend/test/auth-config.test.ts @@ -167,6 +167,18 @@ test("rejects unknown roles and requires exactly one admin group", () => { })))).toThrow("authentication configuration is invalid"); }); +test("caps configured group mappings at the OIDC direct-groups bound", () => { + const mappings = Object.fromEntries(Array.from({ length: 128 }, (_unused, index) => [ + `Mapped Group ${String(index).padStart(3, "0")}`, + index === 0 ? ["admin"] : ["user"], + ])); + expect(loadAuthenticationConfig(writeFixture(oidcConfig({ authorization: { groupRoles: mappings } }))).value.mode) + .toBe("oidc"); + mappings["Mapped Group overflow"] = ["user"]; + expect(() => loadAuthenticationConfig(writeFixture(oidcConfig({ authorization: { groupRoles: mappings } })))) + .toThrow("authentication configuration is invalid"); +}); + test("roles collapse duplicates and admin contains all administrative permissions", () => { expect(rolesToPermissions(["admin", "admin", "user"])).toEqual([ "session.use", @@ -235,6 +247,29 @@ test("provider reloads after an atomic configuration replacement", () => { expect(reloaded.value.publicUrl).toBe("http://127.0.0.1:9999"); }); +test("loads and reloads Windows auth.yaml through the production storage bridge boundary", () => { + const originalPlatform = process.platform; + const windowsPath = "C:\\ProgramData\\ThothII\\auth\\auth.yaml"; + let source = stringify(localConfig()); + const readAuthConfig = vi.fn(() => Buffer.from(source)); + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + try { + const options = { windowsStorageBridge: { readAuthConfig } as never }; + expect(loadAuthenticationConfig(windowsPath, options).value.publicUrl).toBe("http://127.0.0.1:8080"); + const provider = createAuthenticationConfigProvider(windowsPath, options); + const original = provider.current(); + source = stringify(localConfig({ publicUrl: "http://127.0.0.1:9999" })); + const reloaded = provider.current(); + + expect(reloaded.value.publicUrl).toBe("http://127.0.0.1:9999"); + expect(reloaded.revision).not.toBe(original.revision); + expect(readAuthConfig).toHaveBeenCalledTimes(3); + expect(readAuthConfig).toHaveBeenCalledWith(windowsPath); + } finally { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + } +}); + test("provider retries when replacement occurs between its read and cache identity check", () => { const file = writeFixture(localConfig()); const replacement = `${file}.replacement`; diff --git a/backend/test/auth-diagnostics.test.ts b/backend/test/auth-diagnostics.test.ts index 132b7239..f4a09d12 100644 --- a/backend/test/auth-diagnostics.test.ts +++ b/backend/test/auth-diagnostics.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { afterEach, expect, test, vi } from "vitest"; @@ -109,6 +109,41 @@ test("requires both fixed OIDC and Authentik secrets only in OIDC mode", async ( ] }); }); +test.each([ + ["OIDC maximum", "THT_OIDC_CLIENT_SECRET", 4096, true], + ["OIDC overflow", "THT_OIDC_CLIENT_SECRET", 4097, false], + ["Authentik maximum", "THT_AUTHENTIK_API_TOKEN", 16 * 1024, true], + ["Authentik overflow", "THT_AUTHENTIK_API_TOKEN", 16 * 1024 + 1, false], +])("uses the runtime $s secret boundary in static diagnosis", async (_label, name, length, ready) => { + const secrets = new Map([ + ["THT_OIDC_CLIENT_SECRET", "o"], + ["THT_AUTHENTIK_API_TOKEN", "a"], + ]); + secrets.set(name, "x".repeat(length)); + const report = await createAuthDiagnoser({ + authMode: "oidc", authentication: { current: oidcConfig }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, secrets, + }).inspect({ live: false }); + + expect(report.ready).toBe(ready); + expect(report.checks.map((item) => item.code)).toEqual(ready ? ["auth_ready"] : ["oidc_secret_missing"]); +}); + +test("rejects control characters in either required secret during static diagnosis", async () => { + for (const name of ["THT_OIDC_CLIENT_SECRET", "THT_AUTHENTIK_API_TOKEN"] as const) { + const secrets = new Map([ + ["THT_OIDC_CLIENT_SECRET", "oidc-secret"], + ["THT_AUTHENTIK_API_TOKEN", "authentik-token"], + ]); + secrets.set(name, "invalid\u0000secret"); + const report = await createAuthDiagnoser({ + authMode: "oidc", authentication: { current: oidcConfig }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, secrets, + }).inspect({ live: false }); + expect(report.checks).toEqual([expect.objectContaining({ code: "oidc_secret_missing" })]); + } +}); + test("distinguishes a valid registry without an enabled admin from a malformed registry", async () => { const validRoot = privateRoot(); const validUsers = join(validRoot, "users.yaml"); @@ -177,6 +212,115 @@ test("reports a JWKS validation failure through its closed diagnostic code", asy expect(report.checks).toEqual([expect.objectContaining({ code: "oidc_jwks_unreachable" })]); }); +test("bounds a live diagnosis whose OIDC dependency ignores abort and starts no later checks", async () => { + vi.useFakeTimers(); + try { + let rejectLate: ((error: Error) => void) | undefined; + const oidcDiagnose = vi.fn(() => new Promise((_resolve, reject) => { rejectLate = reject; })); + const verifyConfiguredGroups = vi.fn(async () => []); + const completion = createAuthDiagnoser({ + authMode: "oidc", authentication: { current: oidcConfig }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + secrets: new Map([["THT_OIDC_CLIENT_SECRET", "oidc"], ["THT_AUTHENTIK_API_TOKEN", "authentik"]]), + oidcProtocol: { diagnose: oidcDiagnose }, groupCatalog: { verifyConfiguredGroups }, + }).inspect({ live: true }); + const outcome = completion.then((report) => report, () => undefined); + + await vi.advanceTimersByTimeAsync(29_999); + await expect(Promise.race([outcome, Promise.resolve("pending")])).resolves.toBe("pending"); + await vi.advanceTimersByTimeAsync(1); + await expect(outcome).resolves.toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "oidc_discovery_unreachable" })], + }); + expect(oidcDiagnose).toHaveBeenCalledOnce(); + expect(verifyConfiguredGroups).not.toHaveBeenCalled(); + rejectLate?.(new Error("late-secret-detail")); + await Promise.resolve(); + } finally { + vi.useRealTimers(); + } +}); + +test("composes caller cancellation with the overall live-diagnostic deadline", async () => { + const caller = new AbortController(); + const verifyConfiguredGroups = vi.fn(async () => []); + const completion = createAuthDiagnoser({ + authMode: "oidc", authentication: { current: oidcConfig }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + secrets: new Map([["THT_OIDC_CLIENT_SECRET", "oidc"], ["THT_AUTHENTIK_API_TOKEN", "authentik"]]), + oidcProtocol: { diagnose: async () => await new Promise(() => undefined) }, + groupCatalog: { verifyConfiguredGroups }, + }).inspect({ live: true, signal: caller.signal }); + caller.abort(); + + await expect(completion).resolves.toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "oidc_discovery_unreachable" })], + }); + expect(verifyConfiguredGroups).not.toHaveBeenCalled(); +}); + +test("never reports auth_ready when cancellation lands during OIDC completion", async () => { + const caller = new AbortController(); + const verifyConfiguredGroups = vi.fn(async () => []); + const report = await createAuthDiagnoser({ + authMode: "oidc", authentication: { current: oidcConfig }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + secrets: new Map([["THT_OIDC_CLIENT_SECRET", "oidc"], ["THT_AUTHENTIK_API_TOKEN", "authentik"]]), + oidcProtocol: { diagnose: async () => { caller.abort(); } }, + groupCatalog: { verifyConfiguredGroups }, + }).inspect({ live: true, signal: caller.signal }); + + expect(report).toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "oidc_discovery_unreachable" })], + }); + expect(verifyConfiguredGroups).not.toHaveBeenCalled(); +}); + +test("stops starting mapped-group requests when the overall live deadline expires", async () => { + vi.useFakeTimers(); + try { + const loaded = oidcConfig(); + if (loaded.value.mode !== "oidc") throw new Error("test configuration is not OIDC"); + loaded.value.authorization.groupRoles = Object.fromEntries(Array.from({ length: 10 }, (_unused, index) => [ + `Mapped Group ${String(index).padStart(2, "0")}`, + index === 0 ? ["admin"] : ["user"], + ])); + const fetch = vi.fn((input, init) => new Promise((resolve, reject) => { + const timer = setTimeout(() => { + const name = new URL(String(input)).searchParams.get("name"); + resolve(Response.json({ pagination: { next: null }, results: [{ name }] })); + }, 4_000); + init?.signal?.addEventListener("abort", () => { + clearTimeout(timer); + reject(new DOMException("aborted", "AbortError")); + }, { once: true }); + })); + const completion = createAuthDiagnoser({ + authMode: "oidc", authentication: { current: () => loaded }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + secrets: new Map([["THT_OIDC_CLIENT_SECRET", "oidc"], ["THT_AUTHENTIK_API_TOKEN", "authentik"]]), + oidcProtocol: { diagnose: async () => undefined }, + groupCatalog: createAuthentikGroupCatalog({ + baseUrl: "https://authentik.example.test", apiToken: "authentik", fetch, + }), + }).inspect({ live: true }); + + await vi.advanceTimersByTimeAsync(30_000); + await expect(completion).resolves.toMatchObject({ + ready: false, + checks: [expect.objectContaining({ code: "oidc_group_catalog_unreachable" })], + }); + expect(fetch).toHaveBeenCalledTimes(8); + await vi.advanceTimersByTimeAsync(30_000); + expect(fetch).toHaveBeenCalledTimes(8); + } finally { + vi.useRealTimers(); + } +}); + test("maps a concrete discovery adapter issuer mismatch to its dedicated code", async () => { const loaded = oidcConfig(); const fetch = vi.fn(async (input) => { @@ -211,6 +355,40 @@ test("maps a concrete discovery adapter issuer mismatch to its dedicated code", expect(JSON.stringify(report)).not.toContain("different-issuer"); }); +test.each([ + ["an upstream error", 503, { + 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", + response_types_supported: ["code"], + grant_types_supported: ["authorization_code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + }], + ["schema-invalid metadata", 200, { issuer: "https://different-issuer.example.test" }], +])("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 })); + const oidcProtocol = createOidcProtocol({ + issuer: loaded.value.mode === "oidc" ? loaded.value.oidc.issuer : "", + clientId: "thothii", clientSecret: sentinels[0]!, + callbackUrl: "https://thothii.example.test/api/auth/oidc/callback", + scopes: ["openid"], groupsClaim: "groups", fetch, + }); + const report = await createAuthDiagnoser({ + authMode: "oidc", authentication: { current: () => loaded }, authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + secrets: new Map([["THT_OIDC_CLIENT_SECRET", sentinels[0]!], ["THT_AUTHENTIK_API_TOKEN", sentinels[1]!]]), + oidcProtocol, groupCatalog: { verifyConfiguredGroups: async () => [] }, + }).inspect({ live: true }); + + expect(report.checks).toEqual([expect.objectContaining({ code: "oidc_discovery_unreachable" })]); + expect(report.checks).not.toContainEqual(expect.objectContaining({ code: "oidc_issuer_mismatch" })); + expect(fetch).toHaveBeenCalledOnce(); + expect(JSON.stringify(report)).not.toContain("different-issuer"); +}); + test("redacts exceptional configuration, registry, protocol, and catalog errors", async () => { const detail = sentinels.join(" "); const report = await createAuthDiagnoser({ @@ -235,21 +413,47 @@ test.skipIf(process.platform === "win32")("uses the runtime validator for canoni const linkedRoot = join(privateRoot(), "linked-auth"); symlinkSync(realRoot, linkedRoot); const absent = join(privateRoot(), "absent-auth"); + const absentParent = join(privateRoot(), "absent-parent"); + const absentNested = join(absentParent, "auth"); const blockedParent = join(privateRoot(), "not-a-directory"); writeFileSync(blockedParent, "blocked", { mode: 0o600 }); const traversal = `${valid}/../${basename(valid)}`; - for (const unsafe of [traversal, linkedRoot, absent, join(blockedParent, "auth")]) { + const missingReport = await createAuthDiagnoser({ authMode: "none", authStateRoot: 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 }); 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 })) .resolves.toMatchObject({ ready: false, checks: [expect.objectContaining({ code: "auth_session_store_invalid" })] }); }); +test("routes native Windows static session-root validation through the auth-storage bridge", async () => { + const originalPlatform = process.platform; + const validateRoot = vi.fn(async () => undefined); + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + try { + const report = await createAuthDiagnoser({ + authMode: "none", + authStateRoot: "C:\\ProgramData\\ThothII\\auth", + windowsStorageBridge: { validateRoot } as never, + }).inspect({ live: false }); + + expect(report).toMatchObject({ ready: true }); + expect(validateRoot).toHaveBeenCalledOnce(); + expect(validateRoot).toHaveBeenCalledWith("C:\\ProgramData\\ThothII\\auth"); + } finally { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + } +}); + test("accepts a platform storage validator without exposing its root or failure", async () => { const platformRoot = "C:\\private-path-UNIQUE-6R2\\auth"; const sessionRootValidator = vi.fn(async () => { throw new Error(`${platformRoot} denied`); }); diff --git a/backend/test/auth-session-store.test.ts b/backend/test/auth-session-store.test.ts index 78e8a545..1b68594b 100644 --- a/backend/test/auth-session-store.test.ts +++ b/backend/test/auth-session-store.test.ts @@ -254,17 +254,69 @@ describe("file-backed auth session store", () => { symlinkSync(realRoot, linkedRoot); const traversal = `${valid}/../${basename(valid)}`; const absent = join(outer, "absent-auth"); + const absentNested = join(outer, "absent-parent", "auth"); const fileParent = join(outer, "not-a-directory"); writeFileSync(fileParent, "blocked", { mode: 0o600 }); - for (const unsafe of [traversal, linkedRoot, absent, join(fileParent, "auth")]) { + expect(() => validateAuthSessionRoot(absent)).not.toThrow(); + expect(existsSync(absent)).toBe(false); + for (const unsafe of [traversal, linkedRoot, absentNested, join(fileParent, "auth")]) { expect(() => validateAuthSessionRoot(unsafe)).toThrow("auth_session_store_invalid"); } + expect(existsSync(join(outer, "absent-parent"))).toBe(false); chmodSync(valid, 0o750); expect(() => validateAuthSessionRoot(valid)).toThrow("auth_session_store_invalid"); }); + test.skipIf(process.platform === "win32")("never follows a symlinked ancestor while creating a missing session root", async () => { + const outer = root(); + const outside = root(); + const linkedParent = join(outer, "linked-parent"); + symlinkSync(outside, linkedParent); + const storageRoot = join(linkedParent, "auth"); + + await expectStoreInvalid(create(validStore(storageRoot))); + expect(existsSync(join(outside, "auth"))).toBe(false); + }); + + test.skipIf(process.platform === "win32")("rejects an uncreatable missing root without side effects in static and runtime paths", async () => { + const outer = root(); + const lockedParent = join(outer, "locked-parent"); + mkdirSync(lockedParent, { mode: 0o700 }); + chmodSync(lockedParent, 0o500); + const storageRoot = join(lockedParent, "auth"); + try { + expect(() => validateAuthSessionRoot(storageRoot)).toThrow("auth_session_store_invalid"); + await expectStoreInvalid(create(validStore(storageRoot))); + expect(existsSync(storageRoot)).toBe(false); + } finally { + chmodSync(lockedParent, 0o700); + } + }); + + test.skipIf(process.platform === "win32")("detects an ancestor replacement before creating any session directory", 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); + let replaced = false; + fsHooks.afterLstat = (observed) => { + if (observed !== parent) return false; + renameSync(parent, movedParent); + symlinkSync(outside, parent); + replaced = true; + return true; + }; + + await expectStoreInvalid(create(validStore(join(parent, "auth")))); + expect(replaced).toBe(true); + expect(existsSync(join(outside, "auth"))).toBe(false); + expect(existsSync(join(movedParent, "auth"))).toBe(false); + }); + test.skipIf(process.platform === "win32")("rejects a session root owned by another identity", () => { const storageRoot = root(); fsHooks.transformLstat = (observed, info) => { diff --git a/backend/test/authentik-group-catalog.test.ts b/backend/test/authentik-group-catalog.test.ts index fab5e74f..e7dc6067 100644 --- a/backend/test/authentik-group-catalog.test.ts +++ b/backend/test/authentik-group-catalog.test.ts @@ -43,6 +43,15 @@ test.each([ expect(result).toEqual([expect.objectContaining({ level: "error", code, field: "TOT Users" })]); }); +test("rejects more than the requested two bounded group results", async () => { + const response = groups("TOT Users", "Unrelated One", "Unrelated Two"); + const result = await catalog(vi.fn(async () => response)) + .verifyConfiguredGroups(["TOT Users"], new AbortController().signal); + + expect(result).toEqual([expect.objectContaining({ code: "oidc_group_catalog_unreachable" })]); + expect(JSON.stringify(result)).not.toContain("Unrelated"); +}); + test("treats a pagination continuation as an ambiguous configured group", async () => { const response = Response.json({ pagination: { next: "https://authentik.example.test/api/v3/core/groups/?page=2" }, results: [{ name: "TOT Users" }] }); const result = await catalog(vi.fn(async () => response)) @@ -51,6 +60,34 @@ test("treats a pagination continuation as an ambiguous configured group", async expect(result).toEqual([expect.objectContaining({ code: "oidc_mapped_group_ambiguous", field: "TOT Users" })]); }); +test.each([ + ["missing next", { pagination: {}, results: [{ name: "TOT Users" }] }], + ["numeric next", { pagination: { next: 2 }, results: [{ name: "TOT Users" }] }], + ["boolean next", { pagination: { next: false }, results: [{ name: "TOT Users" }] }], + ["malformed continuation", { pagination: { next: "not a URL" }, results: [{ name: "TOT Users" }] }], +])("rejects a group response with %s as unreachable", async (_label, body) => { + const result = await catalog(vi.fn(async () => Response.json(body))) + .verifyConfiguredGroups(["TOT Users"], new AbortController().signal); + + expect(result).toEqual([expect.objectContaining({ code: "oidc_group_catalog_unreachable" })]); + expect(result).not.toContainEqual(expect.objectContaining({ code: "oidc_mapped_group_ambiguous" })); +}); + +test("enforces the exact Authentik token boundary before making a request", async () => { + const fetch = vi.fn(async () => groups("TOT Users")); + const accepted = createAuthentikGroupCatalog({ + baseUrl: "https://authentik.example.test", apiToken: "a".repeat(16 * 1024), fetch, + }); + const rejected = createAuthentikGroupCatalog({ + baseUrl: "https://authentik.example.test", apiToken: "a".repeat(16 * 1024 + 1), fetch, + }); + + await expect(accepted.verifyConfiguredGroups(["TOT Users"], new AbortController().signal)).resolves.toEqual([]); + await expect(rejected.verifyConfiguredGroups(["TOT Users"], new AbortController().signal)) + .resolves.toEqual([expect.objectContaining({ code: "oidc_group_catalog_unreachable" })]); + expect(fetch).toHaveBeenCalledOnce(); +}); + test.each([ ["unauthorized", new Response("upstream body must not escape", { status: 401 }), "oidc_group_catalog_unauthorized"], ["forbidden", new Response("upstream body must not escape", { status: 403 }), "oidc_group_catalog_unauthorized"], diff --git a/backend/test/oidc-client.test.ts b/backend/test/oidc-client.test.ts index 7cd2b3b3..bc3be1e9 100644 --- a/backend/test/oidc-client.test.ts +++ b/backend/test/oidc-client.test.ts @@ -300,6 +300,17 @@ test("rejects non-HTTPS issuer configuration and a discovery issuer mismatch", a } }); +test("enforces the exact shared OIDC client-secret boundary", () => { + expect(() => createOidcProtocol({ + issuer, clientId, clientSecret: "s".repeat(4096), callbackUrl, + scopes: ["openid"], groupsClaim: "groups", + })).not.toThrow(); + expect(() => createOidcProtocol({ + issuer, clientId, clientSecret: "s".repeat(4097), callbackUrl, + scopes: ["openid"], groupsClaim: "groups", + })).toThrow(OidcProtocolError); +}); + test.each([ ["authorization", { authorization_endpoint: "http://127.0.0.1/authorize" }], ["token", { token_endpoint: "http://127.0.0.1/token" }], diff --git a/backend/test/secret-bundle.test.ts b/backend/test/secret-bundle.test.ts index dc09b20e..a0580da8 100644 --- a/backend/test/secret-bundle.test.ts +++ b/backend/test/secret-bundle.test.ts @@ -30,6 +30,17 @@ test("accepts the fixed OIDC and Authentik secret references", () => { ])); }); +test.each([ + ["THT_OIDC_CLIENT_SECRET", 4096], + ["THT_AUTHENTIK_API_TOKEN", 16 * 1024], +])("enforces the shared exact value boundary for %s", (name, maximum) => { + expect(loadSecretBundle(bundle(`${name}=${"x".repeat(maximum)}\n`)).get(name)).toHaveLength(maximum); + expect(() => loadSecretBundle(bundle(`${name}=${"x".repeat(maximum + 1)}\n`))) + .toThrow("secret bundle is unavailable"); + expect(() => loadSecretBundle(bundle(`${name}=invalid\u0000secret\n`))) + .toThrow("secret bundle is unavailable"); +}); + test.each([ ["duplicate", "THT_MODEL_API_KEY=a\nTHT_MODEL_API_KEY=b\n"], ["unknown", "UNKNOWN_KEY=x\n"], diff --git a/backend/test/windows-auth-storage.test.ts b/backend/test/windows-auth-storage.test.ts index 20dfa3d6..f0917f1f 100644 --- a/backend/test/windows-auth-storage.test.ts +++ b/backend/test/windows-auth-storage.test.ts @@ -144,6 +144,42 @@ describe("Windows auth-storage bridge", () => { expect(calls[0].timeoutMs).toBeGreaterThan(0); }); + test("validates Windows roots and reads auth.yaml through the same bounded hidden bridge", async () => { + const asyncCalls: Array> = []; + const syncCalls: Array<{ args: readonly string[]; input: Buffer; timeoutMs: number; maximumOutputBytes: number }> = []; + const config = Buffer.from("version: 1\nmode: local\n"); + const bridge = createWindowsAuthStorageBridge({ + thtExecutable: "C:\\Program Files\\ThothII\\tht.exe", + invoke: async ({ input }) => { + asyncCalls.push(JSON.parse(input.toString("utf8")) as Record); + return { code: 0, stdout: Buffer.from('{"version":1,"ok":true,"validated":true}\n'), stderr: Buffer.alloc(0) }; + }, + invokeSync: (call: { args: readonly string[]; input: Buffer; timeoutMs: number; maximumOutputBytes: number }) => { + syncCalls.push(call); + return { + code: 0, + stdout: Buffer.from(`${JSON.stringify({ version: 1, ok: true, found: true, contentBase64: config.toString("base64") })}\n`), + stderr: Buffer.alloc(0), + }; + }, + } as never) as unknown as { + validateRoot(root: string): Promise; + readAuthConfig(path: string): Buffer; + }; + + await expect(bridge.validateRoot(root)).resolves.toBeUndefined(); + expect(bridge.readAuthConfig(`${root}\\auth.yaml`)).toEqual(config); + expect(asyncCalls).toEqual([{ version: 1, operation: "validate-root", root }]); + expect(JSON.parse(syncCalls[0]!.input.toString("utf8"))).toEqual({ + version: 1, operation: "read-auth-config", root, filename: "auth.yaml", + }); + expect(syncCalls[0]!.args).toEqual(["_auth-storage"]); + expect(syncCalls[0]!.timeoutMs).toBe(5_000); + expect(syncCalls[0]!.maximumOutputBytes).toBeGreaterThan(1024 * 1024); + expect(JSON.stringify(syncCalls[0]!.args)).not.toContain(root); + expect(JSON.stringify(syncCalls[0]!.args)).not.toContain(config.toString("utf8")); + }); + test.each([ { label: "nonzero", result: { code: 1, stdout: Buffer.from('{"version":1,"ok":true}\n'), stderr: Buffer.from("secret") } }, { label: "malformed stdout", result: { code: 0, stdout: Buffer.from("not-json"), stderr: Buffer.alloc(0) } }, diff --git a/tools/tht/internal/authstorage/storage.go b/tools/tht/internal/authstorage/storage.go index 6b18fffa..ff5f4276 100644 --- a/tools/tht/internal/authstorage/storage.go +++ b/tools/tht/internal/authstorage/storage.go @@ -13,31 +13,34 @@ import ( "path/filepath" "regexp" "strings" + "unicode" "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) const ( - protocolVersion = 1 - maximumProtocolBytes = 64 * 1024 - maximumSessionBytes = 16 * 1024 - maximumOIDCStateBytes = 8 * 1024 - defaultMaximumEntries = 256 - maximumEntries = 512 + protocolVersion = 1 + maximumProtocolBytes = 64 * 1024 + maximumSessionBytes = 16 * 1024 + maximumOIDCStateBytes = 8 * 1024 + maximumAuthConfigBytes = 1024 * 1024 + defaultMaximumEntries = 256 + maximumEntries = 512 ) var ( - 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") + 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$`) + authConfigFilename = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,249}\.yaml$`) + errInvalid = errors.New("auth storage request invalid") ) type request struct { Version int `json:"version"` Operation string `json:"operation"` Root string `json:"root"` - Directory string `json:"directory"` + Directory string `json:"directory,omitempty"` Filename string `json:"filename,omitempty"` ContentBase64 string `json:"contentBase64,omitempty"` MaximumEntries int `json:"maximumEntries,omitempty"` @@ -56,6 +59,7 @@ type response struct { ContentBase64 string `json:"contentBase64,omitempty"` Entries *[]safeio.PrivateDirectoryEntry `json:"entries,omitempty"` More *bool `json:"more,omitempty"` + Validated bool `json:"validated,omitempty"` } // Run accepts exactly one strict JSON request on stdin and emits exactly one JSON response on @@ -102,7 +106,27 @@ func fail(stderr io.Writer) int { } func execute(input request) (response, error) { - if input.Version != protocolVersion || !validDirectory(input.Directory) || !validOperationShape(input) { + if input.Version != protocolVersion || !validOperationShape(input) { + return response{}, errInvalid + } + if input.Operation == "validate-root" { + if _, err := preflightRoot(input.Root); err != nil { + return response{}, errInvalid + } + return response{Version: protocolVersion, OK: true, Validated: true}, nil + } + if input.Operation == "read-auth-config" { + root, err := existingPrivateRoot(input.Root) + if err != nil { + return response{}, errInvalid + } + contents, err := safeio.ReadCanonicalPrivateRegular(filepath.Join(root, input.Filename), maximumAuthConfigBytes) + if err != nil { + return response{}, errInvalid + } + return contentResponse(true, contents), nil + } + if !validDirectory(input.Directory) { return response{}, errInvalid } directory, err := storageDirectory(input.Root, input.Directory) @@ -201,6 +225,10 @@ func validOperationShape(input request) bool { noAfterName := input.AfterName == "" noContinuation := !input.Continuation switch input.Operation { + case "validate-root": + return input.Directory == "" && input.Filename == "" && noContents && noMaximumEntries && noAfterName && noContinuation + case "read-auth-config": + return input.Directory == "" && authConfigFilename.MatchString(input.Filename) && noContents && noMaximumEntries && noAfterName && noContinuation case "create", "replace": return noMaximumEntries && noAfterName && noContinuation && (digestFilename.MatchString(input.Filename) || (input.Operation == "create" && input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename))) case "read": @@ -217,6 +245,21 @@ func validOperationShape(input request) bool { } } +func preflightRoot(root string) (bool, error) { + if !filepath.IsAbs(root) || filepath.Clean(root) != root || strings.IndexFunc(root, unicode.IsControl) >= 0 { + return false, errInvalid + } + return safeio.PreflightPrivateDirectory(root) +} + +func existingPrivateRoot(root string) (string, error) { + exists, err := preflightRoot(root) + if err != nil || !exists || safeio.ValidatePrivateDirectory(root) != nil { + return "", errInvalid + } + return root, nil +} + func contentResponse(found bool, contents []byte) response { if !found { return response{Version: protocolVersion, OK: true} @@ -225,7 +268,7 @@ func contentResponse(found bool, contents []byte) response { } func storageDirectory(root, directory string) (string, error) { - if !filepath.IsAbs(root) || filepath.Clean(root) != root || strings.ContainsRune(root, '\x00') || safeio.EnsurePrivateDirectory(root) != nil { + if _, err := preflightRoot(root); err != nil || safeio.EnsurePrivateDirectory(root) != nil { return "", errInvalid } path := filepath.Join(root, directory) diff --git a/tools/tht/internal/authstorage/storage_test.go b/tools/tht/internal/authstorage/storage_test.go index dcf8f1cf..e673a80b 100644 --- a/tools/tht/internal/authstorage/storage_test.go +++ b/tools/tht/internal/authstorage/storage_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -91,6 +92,65 @@ func TestProtocolCreatesReadsReplacesListsAndRemovesPrivateRecord(t *testing.T) } } +func TestProtocolPreflightsRootWithoutCreatingOrFollowingLinks(t *testing.T) { + parent := privateTestRoot(t) + missing := filepath.Join(parent, "auth") + validated := runRequest(t, request{Version: 1, Operation: "validate-root", Root: missing}) + if !validated.Validated { + t.Fatal("missing final root was not validated") + } + if _, err := os.Lstat(missing); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("validate-root mutated missing root: %v", err) + } + + realRoot := filepath.Join(parent, "real-auth") + if err := safeio.EnsurePrivateDirectory(realRoot); err != nil { + t.Fatal(err) + } + linkedRoot := filepath.Join(parent, "linked-auth") + testsupport.SymlinkOrSkip(t, realRoot, linkedRoot) + runRejected(t, request{Version: 1, Operation: "validate-root", Root: linkedRoot}) + if entries, err := os.ReadDir(realRoot); err != nil || len(entries) != 0 { + t.Fatalf("linked target was mutated: entries=%v error=%v", entries, err) + } + + runRejected(t, request{Version: 1, Operation: "validate-root", Root: filepath.Join(parent, "missing", "auth")}) + runRejected(t, request{Version: 1, Operation: "validate-root", Root: missing, Directory: "sessions"}) + runRejected(t, request{Version: 1, Operation: "validate-root", Root: filepath.Join(parent, "auth\n")}) +} + +func TestProtocolReadsOnlyBoundedPrivateAuthConfig(t *testing.T) { + root := privateTestRoot(t) + filename := "auth.yaml" + path := filepath.Join(root, filename) + contents := []byte("version: 1\nmode: local\n") + if err := safeio.WriteCanonicalNewPrivateFile(path, contents, 0o600); err != nil { + t.Fatal(err) + } + read := runRequest(t, request{Version: 1, Operation: "read-auth-config", Root: root, Filename: filename}) + if !read.Found || decodeContent(t, read) != string(contents) { + t.Fatalf("read-auth-config = %#v", read) + } + + hardLink := filepath.Join(root, "auth-copy.yaml") + if err := os.Link(path, hardLink); err != nil { + t.Fatal(err) + } + runRejected(t, request{Version: 1, Operation: "read-auth-config", Root: root, Filename: filename}) + if err := os.Remove(hardLink); err != nil { + t.Fatal(err) + } + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + if err := safeio.WriteCanonicalNewPrivateFile(path, bytes.Repeat([]byte("x"), maximumAuthConfigBytes+1), 0o600); err != nil { + t.Fatal(err) + } + runRejected(t, request{Version: 1, Operation: "read-auth-config", Root: root, Filename: filename}) + runRejected(t, request{Version: 1, Operation: "read-auth-config", Root: root, Filename: "../auth.yaml"}) + runRejected(t, request{Version: 1, Operation: "read-auth-config", Root: root, Directory: "sessions", Filename: filename}) +} + func TestProtocolPermitsBoundedReservationSlotsOnlyForOIDCRecords(t *testing.T) { root := filepath.Join(privateTestRoot(t), "auth") slot := "slot-00.json" diff --git a/tools/tht/internal/safeio/files.go b/tools/tht/internal/safeio/files.go index 01ce24ce..33b0a7fd 100644 --- a/tools/tht/internal/safeio/files.go +++ b/tools/tht/internal/safeio/files.go @@ -23,11 +23,12 @@ const maximumPrivateDirectoryPageScanEntries = 16384 // EnsurePrivateDirectory creates only the final canonical directory with the platform's // owner-only protection, or validates an existing directory has that protection. func EnsurePrivateDirectory(path string) error { - if err := ValidateCanonicalPath(path); err != nil { + exists, err := PreflightPrivateDirectory(path) + if err != nil { return err } - if err := requireCanonicalDirectory(filepath.Dir(path)); err != nil { - return err + if exists { + return ValidatePrivateDirectory(path) } if err := createPrivateDirectory(path); err != nil && !errors.Is(err, os.ErrExist) { return ErrUnsafeFile @@ -35,6 +36,20 @@ func EnsurePrivateDirectory(path string) error { return ValidatePrivateDirectory(path) } +// PreflightPrivateDirectory validates every existing path component without following links or +// reparse points. A missing final component is safe to create later; missing intermediates are not. +// This function never creates, removes, chmods, or changes an ACL. +func PreflightPrivateDirectory(path string) (bool, error) { + if err := ValidateCanonicalPath(path); err != nil { + return false, ErrUnsafeFile + } + exists, err := preflightPrivateDirectory(path) + if err != nil { + return false, ErrUnsafeFile + } + return exists, nil +} + // ValidateCanonicalPath rejects relative or lexically non-canonical paths before they are opened. func ValidateCanonicalPath(path string) error { if !filepath.IsAbs(path) || filepath.Clean(path) != path || strings.Contains(path, string(filepath.Separator)+".."+string(filepath.Separator)) { diff --git a/tools/tht/internal/safeio/files_test.go b/tools/tht/internal/safeio/files_test.go index c58607bf..82170172 100644 --- a/tools/tht/internal/safeio/files_test.go +++ b/tools/tht/internal/safeio/files_test.go @@ -97,6 +97,48 @@ func TestWriteCanonicalNewFileRejectsExistingTargets(t *testing.T) { } } +func TestPreflightPrivateDirectoryAllowsOnlyAMissingFinalComponentWithoutMutation(t *testing.T) { + temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-preflight-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + + missing := filepath.Join(root, "auth") + exists, err := PreflightPrivateDirectory(missing) + if err != nil || exists { + t.Fatalf("missing final preflight exists=%v error=%v, want false/nil", exists, err) + } + if _, err := os.Lstat(missing); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("preflight created missing final: %v", err) + } + + nested := filepath.Join(root, "missing-parent", "auth") + if _, err := PreflightPrivateDirectory(nested); !errors.Is(err, ErrUnsafeFile) { + t.Fatalf("missing intermediate preflight error=%v, want ErrUnsafeFile", err) + } + if _, err := os.Lstat(filepath.Join(root, "missing-parent")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("preflight created missing intermediate: %v", err) + } + + realParent := filepath.Join(root, "real-parent") + if err := os.Mkdir(realParent, 0o700); err != nil { + t.Fatal(err) + } + linkedParent := filepath.Join(root, "linked-parent") + testsupport.SymlinkOrSkip(t, realParent, linkedParent) + if _, err := PreflightPrivateDirectory(filepath.Join(linkedParent, "auth")); !errors.Is(err, ErrUnsafeFile) { + t.Fatalf("symlink ancestor preflight error=%v, want ErrUnsafeFile", err) + } + if _, err := os.Lstat(filepath.Join(realParent, "auth")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("preflight mutated symlink target: %v", err) + } +} + func TestListCanonicalPrivateDirectoryBoundsAndSortsValidatedEntries(t *testing.T) { temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) if err != nil { diff --git a/tools/tht/internal/safeio/preflight_unix.go b/tools/tht/internal/safeio/preflight_unix.go new file mode 100644 index 00000000..aeb35504 --- /dev/null +++ b/tools/tht/internal/safeio/preflight_unix.go @@ -0,0 +1,75 @@ +//go:build !windows + +package safeio + +import ( + "os" + "strings" + + "golang.org/x/sys/unix" +) + +type unixParentHandles struct { + descriptors []int + parent int + target string +} + +func (parents *unixParentHandles) Close() { + closeUnixDescriptors(parents.descriptors) +} + +func openCanonicalUnixParent(path string) (*unixParentHandles, error) { + if err := ValidateCanonicalPath(path); err != nil { + return nil, ErrUnsafeFile + } + components := strings.Split(strings.TrimPrefix(path, string(os.PathSeparator)), string(os.PathSeparator)) + if len(components) == 0 || components[0] == "" { + return nil, ErrUnsafeFile + } + directory, err := unix.Open(string(os.PathSeparator), unix.O_RDONLY|unix.O_CLOEXEC|unix.O_DIRECTORY, 0) + if err != nil { + return nil, ErrUnsafeFile + } + parents := &unixParentHandles{descriptors: []int{directory}, parent: directory, target: components[len(components)-1]} + for _, component := range components[:len(components)-1] { + next, err := unix.Openat(directory, component, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_DIRECTORY|unix.O_NOFOLLOW, 0) + if err != nil { + parents.Close() + return nil, ErrUnsafeFile + } + directory = next + parents.descriptors = append(parents.descriptors, directory) + parents.parent = directory + } + return parents, nil +} + +func privateUnixDirectoryStat(stat *unix.Stat_t) bool { + return stat != nil && stat.Mode&unix.S_IFMT == unix.S_IFDIR && stat.Uid == uint32(os.Geteuid()) && + stat.Mode&0o7777 == 0o700 +} + +func preflightPrivateDirectory(path string) (bool, error) { + parents, err := openCanonicalUnixParent(path) + if err != nil { + return false, ErrUnsafeFile + } + defer parents.Close() + descriptor, err := unix.Openat(parents.parent, parents.target, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_DIRECTORY|unix.O_NOFOLLOW, 0) + if err != nil { + if err == unix.ENOENT { + if unix.Faccessat(parents.parent, ".", unix.W_OK|unix.X_OK, unix.AT_EACCESS) != nil { + return false, ErrUnsafeFile + } + return false, nil + } + return false, ErrUnsafeFile + } + defer unix.Close(descriptor) + var stat unix.Stat_t + if unix.Fstat(descriptor, &stat) != nil || !privateUnixDirectoryStat(&stat) { + return false, ErrUnsafeFile + } + return true, nil +} diff --git a/tools/tht/internal/safeio/preflight_windows.go b/tools/tht/internal/safeio/preflight_windows.go new file mode 100644 index 00000000..e127721a --- /dev/null +++ b/tools/tht/internal/safeio/preflight_windows.go @@ -0,0 +1,39 @@ +//go:build windows + +package safeio + +import ( + "errors" + "path/filepath" + + "golang.org/x/sys/windows" +) + +func preflightPrivateDirectory(path string) (bool, error) { + parents, target, err := openCanonicalWindowsParent(path) + if err != nil { + return false, ErrUnsafeFile + } + defer parents.Close() + handle, err := openWindowsComponent(filepath.Join(parents.directory, target), true) + if err != nil { + if errors.Is(err, windows.ERROR_FILE_NOT_FOUND) { + writableParent, accessErr := openWindowsComponentWithAccess( + parents.directory, + true, + windows.FILE_APPEND_DATA, // FILE_ADD_SUBDIRECTORY for a directory handle + ) + if accessErr != nil { + return false, ErrUnsafeFile + } + _ = windows.CloseHandle(writableParent) + return false, nil + } + return false, ErrUnsafeFile + } + defer windows.CloseHandle(handle) + if err := validateOwnerOnlyDACL(handle); err != nil { + return false, ErrUnsafeFile + } + return true, nil +} diff --git a/tools/tht/internal/safeio/private_unix.go b/tools/tht/internal/safeio/private_unix.go index e5c5a4ff..895e57c1 100644 --- a/tools/tht/internal/safeio/private_unix.go +++ b/tools/tht/internal/safeio/private_unix.go @@ -6,16 +6,35 @@ import ( "errors" "os" "path/filepath" + + "golang.org/x/sys/unix" ) func createPrivateDirectory(path string) error { - if err := os.Mkdir(path, 0o700); err != nil { - if errors.Is(err, os.ErrExist) { + parents, err := openCanonicalUnixParent(path) + if err != nil { + return ErrUnsafeFile + } + defer parents.Close() + if err := unix.Mkdirat(parents.parent, parents.target, 0o700); err != nil { + if errors.Is(err, unix.EEXIST) { return os.ErrExist } - return err + return ErrUnsafeFile } - return ProtectPrivateDirectory(path) + descriptor, err := unix.Openat(parents.parent, parents.target, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_DIRECTORY|unix.O_NOFOLLOW, 0) + if err != nil { + return ErrUnsafeFile + } + defer unix.Close(descriptor) + if unix.Fchmod(descriptor, 0o700) != nil { + return ErrUnsafeFile + } + var stat unix.Stat_t + if unix.Fstat(descriptor, &stat) != nil || !privateUnixDirectoryStat(&stat) { + return ErrUnsafeFile + } + return nil } // ProtectPrivateDirectory sets the private directory mode used for local authentication state.