From a09c13153974498f9ceea4738519794165f894ce Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Thu, 2 Jul 2026 16:41:18 +0200 Subject: [PATCH] =?UTF-8?q?fix(relay-auth):=20close=205=20pre-production?= =?UTF-8?q?=20security=20findings=20(F1=E2=80=93F5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 (HIGH): bind step-up freshness to the required method (stepUpMethod) so a login-time or weaker (TOTP) factor can no longer satisfy a passkey requirement. F2 (HIGH): make the step-up gate host-driven and fail-closed on connect+reattach (deny when policy.required and the principal is missing/stale/wrong-method). F3 (MED): bind the WebAuthn assertion to the stored credential (credentialId cross-check + forward publicKey/credentialId to the verifier). F4 (LOW): make verifyDpopProof totally fail-safe (no unhandled throw), wrap the authz boundary into a clean audited 401, and validate cnfJkt format at issue. F5 (LOW): return newSignCount from finishAuthentication so the caller can persist the advanced counter (WebAuthn clone detection). All fixes ship regression tests. relay-auth: 122 tests green, tsc clean. --- relay-auth/src/authz/decide.ts | 8 ++- relay-auth/src/capability/errors.ts | 1 + relay-auth/src/capability/issue.ts | 2 + relay-auth/src/capability/verify.ts | 63 ++++++++++--------- relay-auth/src/enforce/onUpgrade.ts | 12 ++-- relay-auth/src/human/stepup/stepup.ts | 25 +++++--- relay-auth/src/human/webauthn/authenticate.ts | 8 ++- relay-auth/src/human/webauthn/verifier.ts | 3 + relay-auth/src/types.ts | 4 ++ relay-auth/test/capability.test.ts | 27 ++++++++ relay-auth/test/enforce.test.ts | 24 +++++-- relay-auth/test/stepup.test.ts | 45 ++++++++++--- relay-auth/test/tripwire/cross-tenant.test.ts | 2 +- relay-auth/test/types.test.ts | 60 ++++++++++++++++++ relay-auth/test/webauthn.test.ts | 24 +++++-- 15 files changed, 248 insertions(+), 60 deletions(-) create mode 100644 relay-auth/test/types.test.ts diff --git a/relay-auth/src/authz/decide.ts b/relay-auth/src/authz/decide.ts index 4a92fd5..a1374b6 100644 --- a/relay-auth/src/authz/decide.ts +++ b/relay-auth/src/authz/decide.ts @@ -54,7 +54,13 @@ async function coreAuthorize( } catch { return { outcome: deny(401, 'invalid_capability_token') } } - if (!(await verifyDpopProof(token, dpop, now))) { + // verifyDpopProof is fail-safe, but guard defensively so an unexpected throw becomes a single + // audited deny (emitted once by onUpgrade) rather than an unhandled exception (Finding-4). + try { + if (!(await verifyDpopProof(token, dpop, now))) { + return { outcome: deny(401, 'dpop_proof_failed') } + } + } catch { return { outcome: deny(401, 'dpop_proof_failed') } } if (await revocation.isRevoked(token.jti)) return { outcome: deny(403, 'token_revoked') } diff --git a/relay-auth/src/capability/errors.ts b/relay-auth/src/capability/errors.ts index 208b1a4..5ea3fab 100644 --- a/relay-auth/src/capability/errors.ts +++ b/relay-auth/src/capability/errors.ts @@ -11,6 +11,7 @@ export type CapabilityErrorReason = | 'bad_signature' | 'replayed' | 'no_cnf' + | 'bad_cnf' export class CapabilityError extends Error { readonly reason: CapabilityErrorReason diff --git a/relay-auth/src/capability/issue.ts b/relay-auth/src/capability/issue.ts index ff7cfa9..c5d91e8 100644 --- a/relay-auth/src/capability/issue.ts +++ b/relay-auth/src/capability/issue.ts @@ -49,6 +49,8 @@ export async function issueCapabilityToken( for (const r of a.rights) CapabilityRightSchema.parse(r) if (a.rights.length === 0) throw new CapabilityError('malformed', 'rights must be non-empty') if (a.cnfJkt.length === 0) throw new CapabilityError('no_cnf') + // A base64url SHA-256 JWK thumbprint is exactly 43 chars of [A-Za-z0-9_-] (hardening, Finding-4). + if (!/^[A-Za-z0-9_-]{43}$/.test(a.cnfJkt)) throw new CapabilityError('bad_cnf') const ttl = Math.max(a.ttlSeconds, CONNECT_TOKEN_MIN_TTL_SEC) const body: TokenBody = { diff --git a/relay-auth/src/capability/verify.ts b/relay-auth/src/capability/verify.ts index d05a907..1b21df1 100644 --- a/relay-auth/src/capability/verify.ts +++ b/relay-auth/src/capability/verify.ts @@ -116,38 +116,45 @@ export async function verifyDpopProof( dpop: DpopContext, now: number, ): Promise { - const expectedJkt = readCnfJkt(token) - const parts = dpop.proofJws.split('.') - if (parts.length !== 3) return false - const [h, p, s] = parts as [string, string, string] - let header: DpopHeader - let payload: DpopPayload + // TOTALLY fail-safe (Finding-4): any thrown error — from readCnfJkt, jwkThumbprint, + // importEd25519PublicRaw, or any decode — becomes `false`, never a rejected promise, so the + // INV15 gate always denies through an audited path instead of an unhandled exception. try { - header = JSON.parse(new TextDecoder().decode(decodeBase64UrlBytes(h))) as DpopHeader - payload = JSON.parse(new TextDecoder().decode(decodeBase64UrlBytes(p))) as DpopPayload + const expectedJkt = readCnfJkt(token) + const parts = dpop.proofJws.split('.') + if (parts.length !== 3) return false + const [h, p, s] = parts as [string, string, string] + let header: DpopHeader + let payload: DpopPayload + try { + header = JSON.parse(new TextDecoder().decode(decodeBase64UrlBytes(h))) as DpopHeader + payload = JSON.parse(new TextDecoder().decode(decodeBase64UrlBytes(p))) as DpopPayload + } catch { + return false + } + if (header.jwk?.kty !== 'OKP' || header.jwk?.crv !== 'Ed25519') return false + let rawPub: Uint8Array + try { + rawPub = decodeBase64UrlBytes(header.jwk.x) + } catch { + return false + } + if ((await jwkThumbprint(rawPub)) !== expectedJkt) return false + if (payload.htu !== dpop.htu || payload.htm !== dpop.htm) return false + if (typeof payload.iat !== 'number' || Math.abs(now - payload.iat) > DPOP_MAX_AGE_SEC) return false + const signed = new TextEncoder().encode(`${h}.${p}`) + const pubKey = await importEd25519PublicRaw(rawPub) + let sig: Uint8Array + try { + sig = decodeBase64UrlBytes(s) + } catch { + return false + } + if (!(await verifyEd25519(pubKey, sig, signed))) return false + return rememberDpop(payload.jti, payload.iat + DPOP_MAX_AGE_SEC, now) } catch { return false } - if (header.jwk?.kty !== 'OKP' || header.jwk?.crv !== 'Ed25519') return false - let rawPub: Uint8Array - try { - rawPub = decodeBase64UrlBytes(header.jwk.x) - } catch { - return false - } - if ((await jwkThumbprint(rawPub)) !== expectedJkt) return false - if (payload.htu !== dpop.htu || payload.htm !== dpop.htm) return false - if (typeof payload.iat !== 'number' || Math.abs(now - payload.iat) > DPOP_MAX_AGE_SEC) return false - const signed = new TextEncoder().encode(`${h}.${p}`) - const pubKey = await importEd25519PublicRaw(rawPub) - let sig: Uint8Array - try { - sig = decodeBase64UrlBytes(s) - } catch { - return false - } - if (!(await verifyEd25519(pubKey, sig, signed))) return false - return rememberDpop(payload.jti, payload.iat + DPOP_MAX_AGE_SEC, now) } /** Build a DPoP proof (client-side helper; also used by tests). Signs with the ephemeral key. */ diff --git a/relay-auth/src/enforce/onUpgrade.ts b/relay-auth/src/enforce/onUpgrade.ts index 51885a1..843fef7 100644 --- a/relay-auth/src/enforce/onUpgrade.ts +++ b/relay-auth/src/enforce/onUpgrade.ts @@ -124,10 +124,14 @@ export async function runEnforcement( await emitDeny(deps, ctx, base, 'too_many_sessions', now) return deny(403, 'too_many_sessions') } - // 5. Step-up gate (Finding-3, v0.10) — only when a session principal is present. - if (ctx.principal !== null) { - const host = await deps.hosts.getById(outcome.hostId) - if (host !== null && needsStepUp(ctx.principal, deps.stepUpPolicyFor(host), now)) { + // 5. Step-up gate (Finding-3/F1/F2) — HOST-DRIVEN and FAIL-CLOSED on connect AND reattach. + // When a host REQUIRES step-up, DENY unless an authenticated principal proves a FRESH step-up + // of the required method. A missing host, a null principal, or a stale/wrong-method step-up all + // fail closed. When the policy does not require step-up, this gate is a no-op. + const host = await deps.hosts.getById(outcome.hostId) + const policy = deps.stepUpPolicyFor(host as HostRecord) + if (policy.required) { + if (host === null || ctx.principal === null || needsStepUp(ctx.principal, policy, now)) { await emitDeny(deps, ctx, 'stepup', 'step_up_required', now) return deny(403, 'step_up_required') } diff --git a/relay-auth/src/human/stepup/stepup.ts b/relay-auth/src/human/stepup/stepup.ts index 2d4c750..1f85dd2 100644 --- a/relay-auth/src/human/stepup/stepup.ts +++ b/relay-auth/src/human/stepup/stepup.ts @@ -14,14 +14,22 @@ import type { AuthMethod, AuthenticatedPrincipal, StepUpPolicy } from '../../typ export const DEFAULT_STEPUP_MAX_AGE_SECONDS = 300 as const -/** Per-host step-up freshness requirement (default: recent passkey step-up). */ +/** Per-host step-up freshness requirement (default: recent passkey step-up REQUIRED). */ export function policyForHost(_host: HostRecord): StepUpPolicy { - return { maxAgeSeconds: DEFAULT_STEPUP_MAX_AGE_SECONDS, requiredMethod: 'passkey' } + return { required: true, maxAgeSeconds: DEFAULT_STEPUP_MAX_AGE_SECONDS, requiredMethod: 'passkey' } +} + +/** v0.9 "never required" default: step-up gate is off (used until per-host tiers exist). */ +export const NO_STEPUP_POLICY: StepUpPolicy = { + required: false, + maxAgeSeconds: DEFAULT_STEPUP_MAX_AGE_SECONDS, + requiredMethod: 'passkey', } /** - * true when step-up is required: never stepped up, OR the last step-up is stale, OR the required - * method is not present in `amr` — i.e. a fresh login alone does NOT satisfy it. + * true when step-up is required: never stepped up, OR the last step-up is stale, OR the LAST step-up + * was not performed with the required method (method-binding, F1) — i.e. neither a fresh login nor a + * weaker (e.g. TOTP) step-up satisfies a passkey requirement. */ export function needsStepUp( principal: AuthenticatedPrincipal, @@ -30,16 +38,19 @@ export function needsStepUp( ): boolean { if (principal.stepUpAt === null) return true if (now - principal.stepUpAt > policy.maxAgeSeconds) return true - if (!principal.amr.includes(policy.requiredMethod)) return true + if ((principal.stepUpMethod ?? null) !== policy.requiredMethod) return true return false } -/** Returns a NEW principal with `stepUpAt = now` and `method` added to `amr` (immutable). */ +/** + * Returns a NEW principal recording a fresh step-up: `stepUpAt = now`, `stepUpMethod = method`, and + * `method` appended to `amr` (audit trail). Immutable — the original is untouched. + */ export function recordStepUp( principal: AuthenticatedPrincipal, method: AuthMethod, now: number, ): AuthenticatedPrincipal { const amr = principal.amr.includes(method) ? principal.amr : [...principal.amr, method] - return { ...principal, amr, stepUpAt: now } + return { ...principal, amr, stepUpAt: now, stepUpMethod: method } } diff --git a/relay-auth/src/human/webauthn/authenticate.ts b/relay-auth/src/human/webauthn/authenticate.ts index 3b4816f..69c2b8e 100644 --- a/relay-auth/src/human/webauthn/authenticate.ts +++ b/relay-auth/src/human/webauthn/authenticate.ts @@ -32,16 +32,19 @@ export async function finishAuthentication( origin: string, verifier: WebAuthnVerifier, now: number, -): Promise { +): Promise<{ principal: AuthenticatedPrincipal; newSignCount: number }> { if (resp.clientChallenge !== expectedChallenge) throw new WebAuthnError('challenge mismatch') if (resp.origin !== origin) throw new WebAuthnError('origin mismatch') if (resp.rpId !== rpId) throw new WebAuthnError('rpId mismatch') + if (resp.credentialId !== cred.credentialId) throw new WebAuthnError('credential id mismatch') const result = await verifier.verifyAuthentication( resp, expectedChallenge, rpId, origin, + cred.publicKey, + cred.credentialId, cred.signCount, ) if (!result.verified) throw new WebAuthnError('assertion not verified') @@ -49,7 +52,7 @@ export async function finishAuthentication( throw new WebAuthnError('signCount regression (cloned authenticator)') } - return { + const principal: AuthenticatedPrincipal = { kind: 'human', accountId: cred.accountId, principalId: cred.credentialId, @@ -57,4 +60,5 @@ export async function finishAuthentication( authAt: now, stepUpAt: null, } + return { principal, newSignCount: result.newSignCount } } diff --git a/relay-auth/src/human/webauthn/verifier.ts b/relay-auth/src/human/webauthn/verifier.ts index bd702f5..7ed3254 100644 --- a/relay-auth/src/human/webauthn/verifier.ts +++ b/relay-auth/src/human/webauthn/verifier.ts @@ -28,6 +28,7 @@ export interface AuthenticationResponse { readonly clientChallenge: string readonly origin: string readonly rpId: string + readonly credentialId: string } export interface RegistrationVerification { @@ -54,6 +55,8 @@ export interface WebAuthnVerifier { expectedChallenge: string, rpId: string, origin: string, + credentialPublicKey: Uint8Array, + expectedCredentialId: string, storedSignCount: number, ): Promise } diff --git a/relay-auth/src/types.ts b/relay-auth/src/types.ts index 4b1a8ba..71e21ad 100644 --- a/relay-auth/src/types.ts +++ b/relay-auth/src/types.ts @@ -34,6 +34,7 @@ export interface AuthenticatedPrincipal { readonly amr: readonly AuthMethod[] // methods satisfied this session (for step-up, T8) readonly authAt: number // epoch seconds of last successful auth (login freshness) readonly stepUpAt: number | null // epoch seconds of last step-up (T8 freshness); null = never + readonly stepUpMethod?: AuthMethod | null // method used for the LAST step-up (F1 method-binding); absent/null ⇒ none } /** @@ -44,6 +45,7 @@ export interface AuthenticatedPrincipal { export const CAP_TOKEN_SUB_IS_ACCOUNT_ID = true as const export interface StepUpPolicy { + readonly required: boolean readonly maxAgeSeconds: number readonly requiredMethod: AuthMethod } @@ -141,11 +143,13 @@ export const AuthenticatedPrincipalSchema = z amr: z.array(AuthMethodSchema).readonly(), authAt: z.number().int().nonnegative(), stepUpAt: z.number().int().nonnegative().nullable(), + stepUpMethod: AuthMethodSchema.nullable().optional(), }) .strict() export const StepUpPolicySchema = z .object({ + required: z.boolean(), maxAgeSeconds: z.number().int().nonnegative(), requiredMethod: AuthMethodSchema, }) diff --git a/relay-auth/test/capability.test.ts b/relay-auth/test/capability.test.ts index 2faeae1..d9ae232 100644 --- a/relay-auth/test/capability.test.ts +++ b/relay-auth/test/capability.test.ts @@ -11,6 +11,7 @@ import { } from '../src/capability/verify.js' import { jwkThumbprint } from '../src/crypto/thumbprint.js' import { CapabilityError } from '../src/capability/errors.js' +import { encodeBase64UrlBytes } from 'relay-contracts' import { setupP5SigningKey, makeEphemeral, principal, uuid } from './_helpers.js' const NOW = 1_700_000_000 @@ -150,5 +151,31 @@ describe('capability token (§4.3)', () => { expect(await verifyDpopProof(tok, { proofJws: proof, htu, htm: 'GET' }, NOW)).toBe(true) expect(await verifyDpopProof(tok, { proofJws: proof, htu, htm: 'GET' }, NOW)).toBe(false) }) + + // F4: a proof whose jwk.x decodes to a NON-32-byte blob makes importEd25519PublicRaw throw. + // verifyDpopProof must be TOTALLY fail-safe and RESOLVE to false, never reject. + it('resolves false (never throws) when the proof key is not a valid 32-byte Ed25519 key', async () => { + // A 16-byte "key" — importEd25519PublicRaw will reject this raw length. + const shortBlob = new Uint8Array(16).fill(7) + // cnf.jkt is computed over the SAME 16-byte blob so the thumbprint check passes and + // execution reaches the risky importEd25519PublicRaw call. + const cnfJkt = await jwkThumbprint(shortBlob) + const raw = await issueFor(signingKey, { cnfJkt }) + const tok = await verifyCapabilityToken(raw, AUD, NOW) + + const htu = 'https://alice.term.example.com/ws' + const enc = (o: unknown) => encodeBase64UrlBytes(new TextEncoder().encode(JSON.stringify(o))) + const h = enc({ typ: 'dpop+ed25519', jwk: { crv: 'Ed25519', kty: 'OKP', x: encodeBase64UrlBytes(shortBlob) } }) + const p = enc({ htu, htm: 'GET', jti: uuid(), iat: NOW }) + const s = encodeBase64UrlBytes(new Uint8Array(64)) // any signature bytes + const proofJws = `${h}.${p}.${s}` + + const verify = verifyDpopProof(tok, { proofJws, htu, htm: 'GET' }, NOW) + await expect(verify).resolves.toBe(false) + }) + }) + + it('rejects a malformed cnf.jkt at issue (not a 43-char base64url thumbprint)', async () => { + await expect(issueFor(signingKey, { cnfJkt: 'short' })).rejects.toMatchObject({ reason: 'bad_cnf' }) }) }) diff --git a/relay-auth/test/enforce.test.ts b/relay-auth/test/enforce.test.ts index ba36bc3..b75f473 100644 --- a/relay-auth/test/enforce.test.ts +++ b/relay-auth/test/enforce.test.ts @@ -20,7 +20,7 @@ const NOW = 1_700_000_000 const AUD_A = 'alice.term.example.com' const ORIGIN_A = `https://${AUD_A}` const ALLOWED = [ORIGIN_A] -const NEVER_REQUIRED: StepUpPolicy = { maxAgeSeconds: Number.MAX_SAFE_INTEGER, requiredMethod: 'passkey' } +const NEVER_REQUIRED: StepUpPolicy = { required: false, maxAgeSeconds: Number.MAX_SAFE_INTEGER, requiredMethod: 'passkey' } describe('enforcement onUpgrade/onReattach (T12)', () => { let signingKey: CryptoKey @@ -126,8 +126,8 @@ describe('enforcement onUpgrade/onReattach (T12)', () => { expect(out).toMatchObject({ ok: false, status: 403, reason: 'cross_tenant_session' }) }) - describe('v0.10 step-up augmentation (Finding-3)', () => { - const STRICT: StepUpPolicy = { maxAgeSeconds: 300, requiredMethod: 'passkey' } + describe('v0.10 step-up augmentation (Finding-3, F1/F2)', () => { + const STRICT: StepUpPolicy = { required: true, maxAgeSeconds: 300, requiredMethod: 'passkey' } it('fresh login but stale step-up → 403 step_up_required at onUpgrade', async () => { deps = { ...deps, stepUpPolicyFor: () => STRICT } @@ -141,10 +141,26 @@ describe('enforcement onUpgrade/onReattach (T12)', () => { it('after a fresh step-up the same request → ok:true', async () => { deps = { ...deps, stepUpPolicyFor: () => STRICT } const b = await issueWithDpop(signingKey, { accountId: 'acct-A', host: hostA, aud: AUD_A, now: NOW }) - const steppedUp = principal('acct-A', { authAt: NOW, stepUpAt: NOW, amr: ['passkey', 'stepup'] }) + const steppedUp = principal('acct-A', { authAt: NOW, stepUpAt: NOW, amr: ['passkey', 'stepup'], stepUpMethod: 'passkey' }) expect(needsStepUp(steppedUp, STRICT, NOW)).toBe(false) const out = await onUpgrade(ctxFor(b, { principal: steppedUp }), deps, ALLOWED, NOW) expect(out.ok).toBe(true) }) + + it('STRICT (required) policy + principal:null → 403 step_up_required (F2 fail-closed)', async () => { + deps = { ...deps, stepUpPolicyFor: () => STRICT } + const b = await issueWithDpop(signingKey, { accountId: 'acct-A', host: hostA, aud: AUD_A, now: NOW }) + const out = await onUpgrade(ctxFor(b, { principal: null }), deps, ALLOWED, NOW) + expect(out).toMatchObject({ ok: false, status: 403, reason: 'step_up_required' }) + expect(audit.events.some((e) => e.action === 'stepup' && e.outcome === 'deny')).toBe(true) + }) + + it('required:false policy + principal:null → allow (non-step-up host preserved)', async () => { + deps = { ...deps, stepUpPolicyFor: () => NEVER_REQUIRED } + const b = await issueWithDpop(signingKey, { accountId: 'acct-A', host: hostA, aud: AUD_A, now: NOW }) + const out = await onUpgrade(ctxFor(b, { principal: null }), deps, ALLOWED, NOW) + expect(out.ok).toBe(true) + expect(audit.events.some((e) => e.outcome === 'allow')).toBe(true) + }) }) }) diff --git a/relay-auth/test/stepup.test.ts b/relay-auth/test/stepup.test.ts index a7d41b6..3860dae 100644 --- a/relay-auth/test/stepup.test.ts +++ b/relay-auth/test/stepup.test.ts @@ -1,10 +1,10 @@ import { describe, it, expect } from 'vitest' -import { needsStepUp, recordStepUp, policyForHost } from '../src/human/stepup/stepup.js' +import { needsStepUp, recordStepUp, policyForHost, NO_STEPUP_POLICY } from '../src/human/stepup/stepup.js' import type { StepUpPolicy } from '../src/types.js' import { principal, makeHost, uuid } from './_helpers.js' const NOW = 1_700_000_000 -const POLICY: StepUpPolicy = { maxAgeSeconds: 300, requiredMethod: 'passkey' } +const POLICY: StepUpPolicy = { required: true, maxAgeSeconds: 300, requiredMethod: 'passkey' } describe('step-up before session open (T8, Finding-3)', () => { it('fresh login but stepUpAt=null → needsStepUp true (login ≠ step-up)', () => { @@ -13,34 +13,61 @@ describe('step-up before session open (T8, Finding-3)', () => { }) it('stale step-up (older than maxAgeSeconds) → true', () => { - const p = principal('acct-A', { stepUpAt: NOW - 301, amr: ['passkey', 'stepup'] }) + const p = principal('acct-A', { stepUpAt: NOW - 301, amr: ['passkey', 'stepup'], stepUpMethod: 'passkey' }) expect(needsStepUp(p, POLICY, NOW)).toBe(true) }) - it('required method absent from amr → true', () => { - const p = principal('acct-A', { stepUpAt: NOW, amr: ['totp'] }) + it('required method not the LAST step-up method → true', () => { + const p = principal('acct-A', { stepUpAt: NOW, amr: ['totp'], stepUpMethod: 'totp' }) expect(needsStepUp(p, POLICY, NOW)).toBe(true) }) - it('fresh passkey step-up → false', () => { - const p = principal('acct-A', { stepUpAt: NOW, amr: ['passkey'] }) + it('fresh passkey step-up satisfies passkey policy → false (F1)', () => { + const p = principal('acct-A', { stepUpAt: NOW, amr: ['passkey'], stepUpMethod: 'passkey' }) expect(needsStepUp(p, POLICY, NOW)).toBe(false) }) + it('fresh TOTP step-up does NOT satisfy passkey policy → true (F1 factor-downgrade regression)', () => { + // A weaker (TOTP) step-up performed just now must not satisfy a passkey requirement, even though + // it is fresh — the required method itself must be the LAST step-up. + const p = principal('acct-A', { stepUpAt: NOW, amr: ['passkey', 'totp'], stepUpMethod: 'totp' }) + expect(needsStepUp(p, POLICY, NOW)).toBe(true) + }) + + it('login-time passkey in amr but no step-up (stepUpMethod null) → true (login ≠ step-up)', () => { + const p = principal('acct-A', { stepUpAt: NOW, amr: ['passkey'], stepUpMethod: null }) + expect(needsStepUp(p, POLICY, NOW)).toBe(true) + }) + it('recordStepUp returns a NEW principal (immutable), original untouched', () => { - const p = principal('acct-A', { stepUpAt: null, amr: ['totp'] }) + const p = principal('acct-A', { stepUpAt: null, amr: ['totp'], stepUpMethod: 'totp' }) const stepped = recordStepUp(p, 'passkey', NOW) expect(stepped).not.toBe(p) expect(p.stepUpAt).toBeNull() expect(p.amr).toEqual(['totp']) + expect(p.stepUpMethod).toBe('totp') expect(stepped.stepUpAt).toBe(NOW) expect(stepped.amr).toContain('passkey') expect(needsStepUp(stepped, POLICY, NOW)).toBe(false) }) - it('policyForHost yields a passkey freshness requirement', () => { + it('recordStepUp stamps stepUpMethod with the method used (F1)', () => { + const p = principal('acct-A', { stepUpAt: null, amr: ['passkey'], stepUpMethod: null }) + const stepped = recordStepUp(p, 'passkey', NOW) + expect(stepped.stepUpMethod).toBe('passkey') + expect(stepped.stepUpAt).toBe(NOW) + }) + + it('policyForHost yields a REQUIRED passkey freshness requirement', () => { const policy = policyForHost(makeHost('acct-A', uuid())) + expect(policy.required).toBe(true) expect(policy.requiredMethod).toBe('passkey') expect(policy.maxAgeSeconds).toBeGreaterThan(0) }) + + it('NO_STEPUP_POLICY is the v0.9 never-required default', () => { + expect(NO_STEPUP_POLICY.required).toBe(false) + expect(NO_STEPUP_POLICY.requiredMethod).toBe('passkey') + expect(NO_STEPUP_POLICY.maxAgeSeconds).toBeGreaterThan(0) + }) }) diff --git a/relay-auth/test/tripwire/cross-tenant.test.ts b/relay-auth/test/tripwire/cross-tenant.test.ts index fcb9418..dc1beef 100644 --- a/relay-auth/test/tripwire/cross-tenant.test.ts +++ b/relay-auth/test/tripwire/cross-tenant.test.ts @@ -29,7 +29,7 @@ const NOW = 1_700_000_000 const AUD_A = 'alice.term.example.com' const AUD_B = 'bob.term.example.com' const ORIGIN_A = `https://${AUD_A}` -const NEVER: StepUpPolicy = { maxAgeSeconds: Number.MAX_SAFE_INTEGER, requiredMethod: 'passkey' } +const NEVER: StepUpPolicy = { required: false, maxAgeSeconds: Number.MAX_SAFE_INTEGER, requiredMethod: 'passkey' } describe('PERMANENT TRIPWIRE — device A can never reach host B (INV1)', () => { let signingKey: CryptoKey diff --git a/relay-auth/test/types.test.ts b/relay-auth/test/types.test.ts new file mode 100644 index 0000000..045c5cb --- /dev/null +++ b/relay-auth/test/types.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest' +import { + AuthenticatedPrincipalSchema, + StepUpPolicySchema, + type AuthenticatedPrincipal, + type StepUpPolicy, +} from '../src/types.js' + +const basePrincipal = { + kind: 'human' as const, + accountId: 'acct-A', + principalId: 'cred-1', + amr: ['passkey' as const], + authAt: 1_700_000_000, + stepUpAt: null, +} + +describe('StepUpPolicy.required (host-driven, fail-closed)', () => { + it('parses a policy with required=true', () => { + const policy: StepUpPolicy = { required: true, maxAgeSeconds: 300, requiredMethod: 'passkey' } + expect(StepUpPolicySchema.parse(policy)).toEqual(policy) + }) + + it('parses a policy with required=false', () => { + const policy: StepUpPolicy = { required: false, maxAgeSeconds: 300, requiredMethod: 'passkey' } + expect(StepUpPolicySchema.parse(policy)).toEqual(policy) + }) + + it('rejects a policy missing required', () => { + const bad = { maxAgeSeconds: 300, requiredMethod: 'passkey' } + expect(StepUpPolicySchema.safeParse(bad).success).toBe(false) + }) + + it('rejects a non-boolean required', () => { + const bad = { required: 'yes', maxAgeSeconds: 300, requiredMethod: 'passkey' } + expect(StepUpPolicySchema.safeParse(bad).success).toBe(false) + }) +}) + +describe('AuthenticatedPrincipal.stepUpMethod (F1 method-binding, optional)', () => { + it('parses a principal that omits stepUpMethod (backward compatible)', () => { + const parsed = AuthenticatedPrincipalSchema.parse(basePrincipal) + expect(parsed.stepUpMethod).toBeUndefined() + }) + + it('parses a principal with an explicit stepUpMethod', () => { + const p: AuthenticatedPrincipal = { ...basePrincipal, stepUpAt: 1_700_000_100, stepUpMethod: 'passkey' } + expect(AuthenticatedPrincipalSchema.parse(p).stepUpMethod).toBe('passkey') + }) + + it('parses a principal with stepUpMethod=null (no step-up)', () => { + const parsed = AuthenticatedPrincipalSchema.parse({ ...basePrincipal, stepUpMethod: null }) + expect(parsed.stepUpMethod).toBeNull() + }) + + it('rejects an invalid stepUpMethod value', () => { + const bad = { ...basePrincipal, stepUpMethod: 'sms' } + expect(AuthenticatedPrincipalSchema.safeParse(bad).success).toBe(false) + }) +}) diff --git a/relay-auth/test/webauthn.test.ts b/relay-auth/test/webauthn.test.ts index 1696031..af919fc 100644 --- a/relay-auth/test/webauthn.test.ts +++ b/relay-auth/test/webauthn.test.ts @@ -21,14 +21,14 @@ const verifier: WebAuthnVerifier = { signCount: 0, transports: ['internal'], }), - verifyAuthentication: async (_r, _c, _rp, _o, stored) => ({ verified: true, newSignCount: stored + 1 }), + verifyAuthentication: async (_r, _c, _rp, _o, _pk, _cid, stored) => ({ verified: true, newSignCount: stored + 1 }), } function regResp(over: Partial = {}): RegistrationResponse { return { clientChallenge: 'chal', origin: ORIGIN, rpId: RP_ID, ...over } } function authResp(over: Partial = {}): AuthenticationResponse { - return { clientChallenge: 'chal', origin: ORIGIN, rpId: RP_ID, ...over } + return { clientChallenge: 'chal', origin: ORIGIN, rpId: RP_ID, credentialId: 'cred-1', ...over } } const cred: WebAuthnCredential = { credentialId: 'cred-1', @@ -68,16 +68,32 @@ describe('WebAuthn / Passkey (T5, primary)', () => { it('happy path auth yields a principal with amr:[passkey]', async () => { const startOpts = await startAuthentication('acct-A', RP_ID) expect(startOpts.rpId).toBe(RP_ID) - const p = await finishAuthentication(cred, authResp(), 'chal', RP_ID, ORIGIN, verifier, NOW) + const { principal: p } = await finishAuthentication(cred, authResp(), 'chal', RP_ID, ORIGIN, verifier, NOW) expect(p.accountId).toBe('acct-A') expect(p.amr).toEqual(['passkey']) expect(p.authAt).toBe(NOW) }) + it('F3: rejects an assertion whose credentialId does not match the stored credential', async () => { + const forgiving: WebAuthnVerifier = { + ...verifier, + verifyAuthentication: async (_r, _c, _rp, _o, _pk, _cid, stored) => ({ verified: true, newSignCount: stored + 1 }), + } + await expect( + finishAuthentication(cred, authResp({ credentialId: 'other-cred' }), 'chal', RP_ID, ORIGIN, forgiving, NOW), + ).rejects.toThrow('credential id mismatch') + }) + + it('F5: returns the verifier-advanced newSignCount so the caller can persist it', async () => { + const { principal: p, newSignCount } = await finishAuthentication(cred, authResp(), 'chal', RP_ID, ORIGIN, verifier, NOW) + expect(p.accountId).toBe('acct-A') + expect(newSignCount).toBe(cred.signCount + 1) + }) + it('rejects a signCount regression (cloned authenticator signal)', async () => { const regressing: WebAuthnVerifier = { ...verifier, - verifyAuthentication: async () => ({ verified: true, newSignCount: 3 }), // < stored 5 + verifyAuthentication: async () => ({ verified: true, newSignCount: 3 }), // < stored 5 (adapts to {principal,newSignCount} return) } await expect( finishAuthentication(cred, authResp(), 'chal', RP_ID, ORIGIN, regressing, NOW),