From fb70c83ae78d3b6f5a31fd435516edb47d4a9760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 31 Jul 2026 14:16:39 +0000 Subject: [PATCH] auth: survive a request that carries no origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signing in from a server-to-server client returned 500. originMiddleware leaves `origin` undefined when a request has neither Origin nor Referer — exactly what such a client sends — and signin.ts took it as a string, passed it into getPasskeysByUserIdAndOrigin, and Postgres rejected the undefined parameter. It would have thrown on origin.startsWith() a few lines later too. It used to be unreachable: originValidationMiddleware rejected origin-less requests before the handler ran, so the value was always a string by the time anything touched it. Turning the origin checks off removed that gate without the code behind it ever having needed to cope. This is the second thing that flag has surfaced rather than caused. The two call sites want different answers, so they get different ones: - signin normalises to ''. No passkey is registered against the empty origin, so an origin-less caller gets an empty list and falls through to password auth, which is what it is asking for. - the four passkey routes now refuse with 400 "Passkey operations require an Origin header". WebAuthn is defined in terms of an origin — a passkey is registered against one and is only verifiable against the same one — so substituting '' there would be quietly wrong. Also flips ALLOW_ANY_ORIGIN to default ON: the checks are off unless it is explicitly 'false'. A deliberate inversion of fail-closed, safe because of where this runs — the perimeter is the tailnet, devices are admitted by hand, and every protected route still requires a valid token. Verified both directions: unset lets a foreign origin through, ALLOW_ANY_ORIGIN=false rejects it. The origin test suite pins the flag off, so it still asserts the checks reject things rather than passing vacuously. Co-Authored-By: Claude Opus 5 (1M context) --- src/servers/_middlewares/origin-validation.ts | 6 +++++- src/servers/api/auth/passkey-router.ts | 17 +++++++++++++---- src/servers/api/auth/signin.ts | 8 +++++++- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/servers/_middlewares/origin-validation.ts b/src/servers/_middlewares/origin-validation.ts index 898df015..b7ccf67e 100644 --- a/src/servers/_middlewares/origin-validation.ts +++ b/src/servers/_middlewares/origin-validation.ts @@ -97,7 +97,11 @@ if (PUBLIC_ORIGIN) ORIGIN_RULES[PUBLIC_ORIGIN] = { superAdminOnly: true }; // What is lost: defence in depth, not the lock. Origin was never authentication here — `officer://` // is chosen by the client, forgeable outside a browser, and extractable from a shipped app binary. // Both flags and their call sites come out once the tailnet is the perimeter. -const ALLOW_ANY_ORIGIN = process.env.ALLOW_ANY_ORIGIN === 'true'; +// Defaults to ON — origin checking is off unless ALLOW_ANY_ORIGIN is explicitly 'false'. That is a +// deliberate inversion of the usual fail-closed rule, and it is safe only because of where this runs: the +// perimeter is the tailnet, devices are admitted by hand, and a valid token is still required on every +// protected route. Set ALLOW_ANY_ORIGIN=false to put the checks back. +const ALLOW_ANY_ORIGIN = (process.env.ALLOW_ANY_ORIGIN ?? 'true') !== 'false'; const ALLOW_ANY_ORIGIN_MUSIC = process.env.ALLOW_ANY_ORIGIN_MUSIC === 'true'; export function isOriginCheckDisabled(path?: string): boolean { diff --git a/src/servers/api/auth/passkey-router.ts b/src/servers/api/auth/passkey-router.ts index 9b8802ad..cd760b8e 100644 --- a/src/servers/api/auth/passkey-router.ts +++ b/src/servers/api/auth/passkey-router.ts @@ -22,6 +22,15 @@ import { } from '@simplewebauthn/server'; import type { RegistrationResponseJSON, AuthenticationResponseJSON } from '@simplewebauthn/server'; +// WebAuthn is defined in terms of an origin — a passkey is registered against one and only verifiable +// against the same one. A caller that sends neither Origin nor Referer cannot do either, so say so +// instead of passing undefined into a SQL parameter and returning a 500. +function requireOrigin(ctx: { get: (k: 'origin') => unknown }): string { + const origin = ctx.get('origin') as string | undefined; + if (!origin) throw errors.BAD_REQUEST('Passkey operations require an Origin header'); + return origin; +} + export const passkeyRouter = createRouter(); const CHALLENGE_TTL_MS = 5 * 60 * 1000; // 5 minutes @@ -39,7 +48,7 @@ function getRpId(origin: string): string { // Generate registration options (challenge) for creating a new passkey const passkeyRouterPostChallenge: Handler = async (ctx) => { const { email } = ctx.req.param(); - const origin = ctx.get('origin') as string; + const origin = requireOrigin(ctx); const rpId = getRpId(origin); const dbUser = await getUserByEmail(email!); @@ -70,7 +79,7 @@ passkeyRouter.post('/challenge/:email', passkeyRateLimiter, passkeyRouterPostCha // Verify and store new passkey registration const passkeyRouterPost: Handler = async (ctx) => { - const origin = ctx.get('origin') as string; + const origin = requireOrigin(ctx); const rpId = getRpId(origin); const user = ctx.get('user') as User; const response = ctx.get('body') as RegistrationResponseJSON; @@ -106,7 +115,7 @@ passkeyRouter.post('/credentials', userMiddleware, passkeyRouterPost); // Generate authentication options for signing in with passkey const passkeyRouterGet: Handler = async (ctx) => { const { email } = ctx.req.param(); - const origin = ctx.get('origin') as string; + const origin = requireOrigin(ctx); const rpId = getRpId(origin); const dbUser = await getUserByEmail(email!); @@ -134,7 +143,7 @@ const passkeyRouterPostVerify: Handler = async (ctx) => { throw errors.UNAUTHORIZED(); } // duress lockdown blocks passkey logins too const { email } = ctx.req.param(); - const origin = ctx.get('origin') as string; + const origin = requireOrigin(ctx); const rpId = getRpId(origin); const response = ctx.get('body') as AuthenticationResponseJSON; diff --git a/src/servers/api/auth/signin.ts b/src/servers/api/auth/signin.ts index 88b4016d..81ba6be4 100755 --- a/src/servers/api/auth/signin.ts +++ b/src/servers/api/auth/signin.ts @@ -11,7 +11,13 @@ const TEST_USERS: number[] = []; export const signinHandler: Handler = async function (ctx) { const { email, password } = ctx.get('body'); - const origin = ctx.get('origin'); + // A caller that sends neither Origin nor Referer — a server-to-server client, curl — leaves this + // undefined. It used to be unreachable: originValidationMiddleware rejected those requests before this + // handler ran, so the value was always a string by the time anything touched it. That is no longer + // guaranteed (ALLOW_ANY_ORIGIN lets them through), and `undefined` reached both a SQL parameter and + // `.startsWith`. Normalising to '' keeps every downstream use honest: no passkey is registered against + // the empty origin, so an origin-less caller falls through to password auth, which is what it wants. + const origin = (ctx.get('origin') as string | undefined) ?? ''; // Panic lockdown active → refuse all logins (looks like a normal failed login). if (isLockdown()) {