From b7f53a3a8b4086567c2583a7ce86665180c550fa Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:16:51 +0200 Subject: [PATCH] fix: bind a purpose into encrypted payloads (GHSA-cqqx-mjcf-mh55) --- app/api/auth/reauth/sso/complete/route.ts | 2 +- app/api/auth/sso/complete/route.ts | 2 +- app/api/auth/sso/start/route.ts | 2 +- lib/__tests__/auth-crypto.test.ts | 82 ++++++++++++-- .../wopi-token-not-a-session.test.ts | 95 ++++++++++++++++ lib/auth/crypto.ts | 102 +++++++++++++++--- lib/auth/pair-reauth.ts | 4 +- lib/stalwart/auth-context.ts | 19 +++- lib/wopi/token.ts | 4 +- 9 files changed, 277 insertions(+), 35 deletions(-) create mode 100644 lib/__tests__/wopi-token-not-a-session.test.ts diff --git a/app/api/auth/reauth/sso/complete/route.ts b/app/api/auth/reauth/sso/complete/route.ts index e51b3a18f..2f222db6c 100644 --- a/app/api/auth/reauth/sso/complete/route.ts +++ b/app/api/auth/reauth/sso/complete/route.ts @@ -34,7 +34,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'No pending re-auth session' }, { status: 400 }); } - const pending = decryptPayload(pendingCookie); + const pending = decryptPayload(pendingCookie, 'sso-pending'); cookieStore.delete(SSO_PENDING_COOKIE); if (!pending) { return NextResponse.json({ error: 'Invalid re-auth session' }, { status: 400 }); diff --git a/app/api/auth/sso/complete/route.ts b/app/api/auth/sso/complete/route.ts index 60b1b917d..72f96b2e8 100644 --- a/app/api/auth/sso/complete/route.ts +++ b/app/api/auth/sso/complete/route.ts @@ -41,7 +41,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'No pending SSO session. Please start the login flow again.' }, { status: 400 }); } - const pending = decryptPayload(pendingCookie); + const pending = decryptPayload(pendingCookie, 'sso-pending'); if (!pending) { cookieStore.delete(SSO_PENDING_COOKIE); return NextResponse.json({ error: 'Invalid SSO session' }, { status: 400 }); diff --git a/app/api/auth/sso/start/route.ts b/app/api/auth/sso/start/route.ts index c5484aee7..59bfee9e2 100644 --- a/app/api/auth/sso/start/route.ts +++ b/app/api/auth/sso/start/route.ts @@ -100,7 +100,7 @@ export async function POST(request: NextRequest) { ...(isReauth ? { purpose: 'reauth' } : {}), }; - const encrypted = encryptPayload(pendingData); + const encrypted = encryptPayload(pendingData, 'sso-pending'); const cookieStore = await cookies(); const baseCookieOpts = getCookieOptions(); cookieStore.set(SSO_PENDING_COOKIE, encrypted, { diff --git a/lib/__tests__/auth-crypto.test.ts b/lib/__tests__/auth-crypto.test.ts index 133aac23a..2fa3c854d 100644 --- a/lib/__tests__/auth-crypto.test.ts +++ b/lib/__tests__/auth-crypto.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { createCipheriv, createHash, randomBytes } from 'node:crypto'; import { encryptSession, decryptSession, @@ -53,12 +54,12 @@ describe('encryptSession / decryptSession', () => { }); it('returns null when the version is not 1', () => { - const token = encryptPayload({ v: 2, serverUrl: 'https://x', username: 'u', password: 'p' }); + const token = encryptPayload({ v: 2, serverUrl: 'https://x', username: 'u', password: 'p' }, 'session-context'); expect(decryptSession(token)).toBeNull(); }); it('returns null when a required field is missing', () => { - const token = encryptPayload({ v: 1, serverUrl: 'https://x', username: 'u' }); + const token = encryptPayload({ v: 1, serverUrl: 'https://x', username: 'u' }, 'session-context'); expect(decryptSession(token)).toBeNull(); }); @@ -74,22 +75,83 @@ describe('encryptSession / decryptSession', () => { }); describe('encryptPayload / decryptPayload', () => { - it('round-trips an arbitrary object', () => { - const token = encryptPayload({ a: 1, b: 'two', c: { nested: true } }); - expect(decryptPayload(token)).toEqual({ a: 1, b: 'two', c: { nested: true } }); + it('round-trips an arbitrary object and stamps the purpose', () => { + const token = encryptPayload({ a: 1, b: 'two', c: { nested: true } }, 'sso-pending'); + expect(decryptPayload(token, 'sso-pending')).toEqual({ + a: 1, b: 'two', c: { nested: true }, p: 'sso-pending', + }); }); it('does NOT enforce the version/field guard that decryptSession applies', () => { // CHARACTERISATION: decryptPayload returns whatever JSON parsed, with no - // v===1 / required-field validation (unlike decryptSession). - const token = encryptPayload({ v: 2, anything: 'goes' }); - expect(decryptPayload(token)).toEqual({ v: 2, anything: 'goes' }); + // v===1 / required-field validation (unlike decryptSession). The purpose + // is the only thing it enforces. + const token = encryptPayload({ v: 2, anything: 'goes' }, 'sso-pending'); + expect(decryptPayload(token, 'sso-pending')).toEqual({ v: 2, anything: 'goes', p: 'sso-pending' }); }); it('returns null on a tampered token', () => { - const token = encryptPayload({ a: 1 }); + const token = encryptPayload({ a: 1 }, 'session-context'); const buf = Buffer.from(token, 'base64'); buf[20] ^= 0xff; - expect(decryptPayload(buf.toString('base64'))).toBeNull(); + expect(decryptPayload(buf.toString('base64'), 'session-context')).toBeNull(); + }); + + it('refuses a blob minted for another purpose', () => { + // GHSA-cqqx-mjcf-mh55: an office editor token is authentic under the same + // key as the session cookie, so only the bound purpose separates them. + const editorToken = encryptPayload( + { serverUrl: 'https://mail.example.com', username: 'alice', authHeader: 'Basic abc', fileId: 'd' }, + 'wopi-token', + ); + expect(decryptPayload(editorToken, 'wopi-token')).not.toBeNull(); + expect(decryptPayload(editorToken, 'session-context')).toBeNull(); + expect(decryptPayload(editorToken, 'pair-reauth')).toBeNull(); + expect(decryptPayload(editorToken, 'sso-pending')).toBeNull(); + }); + + it('refuses a session cookie presented on any other path', () => { + const session = encryptPayload( + { serverUrl: 'https://mail.example.com', username: 'alice', authHeader: 'Basic abc' }, + 'session-context', + ); + expect(decryptPayload(session, 'wopi-token')).toBeNull(); + expect(decryptPayload(session, 'pair-reauth')).toBeNull(); + }); + + describe('payloads minted before purposes were bound', () => { + // Blobs from <= 1.9.3 carry no AAD. They are still opened on the path that + // minted them - otherwise upgrading logs everyone out - but only there. + function mintLegacy(payload: Record): string { + const key = createHash('sha256').update(SECRET).digest(); + const iv = randomBytes(12); + const cipher = createCipheriv('aes-256-gcm', key, iv); + const body = Buffer.concat([cipher.update(JSON.stringify(payload), 'utf8'), cipher.final()]); + return Buffer.concat([iv, cipher.getAuthTag(), body]).toString('base64'); + } + + it('still opens a legacy session context', () => { + const legacy = mintLegacy({ serverUrl: 'https://mail.example.com', username: 'alice', authHeader: 'Basic abc' }); + expect(decryptPayload(legacy, 'session-context')).toMatchObject({ username: 'alice' }); + }); + + it('still opens a legacy pairing proof and SSO pending state', () => { + expect(decryptPayload(mintLegacy({ purpose: 'pair', created_at: 1 }), 'pair-reauth')).toMatchObject({ purpose: 'pair' }); + expect(decryptPayload(mintLegacy({ state: 's', code_verifier: 'v' }), 'sso-pending')).toMatchObject({ state: 's' }); + expect(decryptPayload(mintLegacy({ state: 's', purpose: 'reauth' }), 'sso-pending')).toMatchObject({ purpose: 'reauth' }); + }); + + it('does not let a legacy editor token in through the session path', () => { + const legacyWopi = mintLegacy({ + v: 1, t: 'wopi', serverUrl: 'https://mail.example.com', username: 'alice', + authHeader: 'Basic abc', fileId: 'd', exp: Date.now() + 1000, + }); + expect(decryptPayload(legacyWopi, 'session-context')).toBeNull(); + expect(decryptPayload(legacyWopi, 'wopi-token')).toMatchObject({ t: 'wopi' }); + }); + + it('does not let a legacy pairing proof in through the session path', () => { + expect(decryptPayload(mintLegacy({ purpose: 'pair', created_at: 1 }), 'session-context')).toBeNull(); + }); }); }); diff --git a/lib/__tests__/wopi-token-not-a-session.test.ts b/lib/__tests__/wopi-token-not-a-session.test.ts new file mode 100644 index 000000000..e7b017060 --- /dev/null +++ b/lib/__tests__/wopi-token-not-a-session.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, vi } from 'vitest'; +import { mintWopiToken, verifyWopiToken } from '@/lib/wopi/token'; +import { encryptPayload } from '@/lib/auth/crypto'; +import { + readStalwartAuthContextFromStore, + setStalwartAuthContextInStore, + stalwartAuthContextCookieName, +} from '@/lib/stalwart/auth-context'; + +// GHSA-cqqx-mjcf-mh55. The WOPI access token handed to the office editor and +// the jmap_stalwart_ctx session cookie are the same AES-256-GCM envelope under +// the same SESSION_SECRET-derived key. The editor token used to be accepted as +// a session context, which handed whoever held it - an editor log, a proxy +// access log - the user's whole mailbox through the JMAP passthrough, with the +// token's file scope and six-hour expiry both dropped on that path. + +vi.mock('@/lib/auth/session-secret', () => ({ + getSessionSecret: () => 'x'.repeat(32), + hasSessionSecret: () => true, +})); +vi.mock('@/lib/logger', () => ({ + logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} }, +})); + +const CREDENTIALS = { + serverUrl: 'https://mail.example.com', + username: 'alice@example.org', + authHeader: 'Basic YWxpY2U6czNjcmV0', +}; + +type CookieStore = Parameters[0]; + +/** Minimal stand-in for the next/headers cookie store. */ +function cookieStore(initial: Record = {}): CookieStore { + const jar = new Map(Object.entries(initial)); + return { + get: (name: string) => (jar.has(name) ? { name, value: jar.get(name)! } : undefined), + set: (name: string, value: string) => { jar.set(name, value); }, + delete: (name: string) => { jar.delete(name); }, + } as unknown as CookieStore; +} + +describe('editor token presented as a session cookie', () => { + const { token } = mintWopiToken({ + ...CREDENTIALS, + accountId: 'c', + fileId: 'd', + canWrite: true, + origin: 'https://mail.example.com', + }); + + it('is rejected by the session context reader', () => { + const store = cookieStore({ [stalwartAuthContextCookieName(0)]: token }); + expect(readStalwartAuthContextFromStore(store, 0)).toBeNull(); + }); + + it('is rejected even after base64url is decoded back to standard base64', () => { + // The token is issued in the URL-safe alphabet; an attacker would be free + // to convert it back before pasting it into the cookie. + const standard = token.replace(/-/g, '+').replace(/_/g, '/'); + const store = cookieStore({ [stalwartAuthContextCookieName(0)]: standard }); + expect(readStalwartAuthContextFromStore(store, 0)).toBeNull(); + }); + + it('still works for the file it was scoped to', () => { + expect(verifyWopiToken(token, 'd')).toMatchObject({ fileId: 'd', username: CREDENTIALS.username }); + expect(verifyWopiToken(token, 'other')).toBeNull(); + }); +}); + +describe('session cookie presented as an editor token', () => { + it('is rejected by the WOPI verifier', () => { + const store = cookieStore(); + setStalwartAuthContextInStore(store, 0, CREDENTIALS); + const sessionCookie = store.get(stalwartAuthContextCookieName(0))!.value; + + expect(verifyWopiToken(sessionCookie, 'd')).toBeNull(); + }); + + it('round-trips on its own path', () => { + const store = cookieStore(); + setStalwartAuthContextInStore(store, 0, CREDENTIALS); + expect(readStalwartAuthContextFromStore(store, 0)).toEqual(CREDENTIALS); + }); +}); + +describe('pairing proof presented as a session cookie', () => { + it('is rejected', () => { + // Same shape lib/auth/pair-reauth.ts mints, without dragging next/headers + // into the test. + const proof = encryptPayload({ purpose: 'pair', created_at: Date.now() }, 'pair-reauth'); + const store = cookieStore({ [stalwartAuthContextCookieName(0)]: proof }); + expect(readStalwartAuthContextFromStore(store, 0)).toBeNull(); + }); +}); diff --git a/lib/auth/crypto.ts b/lib/auth/crypto.ts index 38eb24a40..4e3032d85 100644 --- a/lib/auth/crypto.ts +++ b/lib/auth/crypto.ts @@ -58,37 +58,109 @@ export function decryptSession(token: string): { serverUrl: string; username: st } } -export function encryptPayload(payload: Record): string { +/** + * The purpose an encrypted payload was minted for. + * + * Several unrelated things travel as AES-256-GCM blobs under the same + * SESSION_SECRET-derived key: the session context cookie, the OAuth pending + * state, the pairing step-up proof, the office editor token. The purpose is + * bound into the GCM tag as additional authenticated data, so a blob minted + * for one of them cannot be opened on the path of another - the confusion + * fails as a decryption error rather than as a validation check somebody has + * to remember to write (GHSA-cqqx-mjcf-mh55). + */ +export type PayloadPurpose = + /** jmap_stalwart_ctx - the signed-in user's Stalwart credentials. */ + | 'session-context' + /** OAuth state + PKCE verifier, held between /sso/start and /sso/complete. */ + | 'sso-pending' + /** Fresh-IdP-login proof required before a device pairing code is issued. */ + | 'pair-reauth' + /** WOPI access token, scoped to one file node and a six-hour expiry. */ + | 'wopi-token'; + +/** + * Whether a blob minted before purposes were bound may be re-admitted for + * `purpose`, recognised by the plaintext markers that version wrote. + * + * Without this, upgrading would log every user out; with it, an editor token + * still cannot pass as a session cookie, because the WOPI mint has always + * stamped `t: 'wopi'` and the session context has never carried a marker at + * all. Removable once the pre-upgrade cookies have expired. + */ +const LEGACY_PAYLOAD_MARKERS: Record) => boolean> = { + 'session-context': (p) => p.t === undefined && p.purpose === undefined, + // `purpose: 'reauth'` distinguishes step-up from plain login within the SSO + // flow; it predates these envelope purposes and is unrelated to them. + 'sso-pending': (p) => p.t === undefined && (p.purpose === undefined || p.purpose === 'reauth'), + 'pair-reauth': (p) => p.purpose === 'pair', + 'wopi-token': (p) => p.t === 'wopi', +}; + +export function encryptPayload(payload: Record, purpose: PayloadPurpose): string { const key = getKey(); const iv = randomBytes(IV_LENGTH); const cipher = createCipheriv(ALGORITHM, key, iv); + cipher.setAAD(Buffer.from(purpose, 'utf8')); - const json = JSON.stringify(payload); + // `p` restates the bound purpose in the plaintext. The GCM tag is what + // actually confines the blob; this is the readable copy, checked on the way + // back out so a mismatch surfaces even if a future caller opens the + // envelope by hand. + const json = JSON.stringify({ ...payload, p: purpose }); const encrypted = Buffer.concat([cipher.update(json, 'utf8'), cipher.final()]); const tag = cipher.getAuthTag(); return Buffer.concat([iv, tag, encrypted]).toString('base64'); } -export function decryptPayload(token: string): Record | null { +function openEnvelope(data: Buffer, aad: PayloadPurpose | null): Record | null { + if (data.length < IV_LENGTH + TAG_LENGTH) return null; + + const key = getKey(); + const iv = data.subarray(0, IV_LENGTH); + const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); + const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); + + const decipher = createDecipheriv(ALGORITHM, key, iv); + if (aad !== null) decipher.setAAD(Buffer.from(aad, 'utf8')); + decipher.setAuthTag(tag); + + const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); + const parsed: unknown = JSON.parse(decrypted.toString('utf8')); + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return null; + return parsed as Record; +} + +/** + * Decrypt a payload minted for `purpose`. A blob minted for any other purpose + * returns null, even though it is authentic under the same key. + */ +export function decryptPayload(token: string, purpose: PayloadPurpose): Record | null { + const data = Buffer.from(token, 'base64'); + try { - const key = getKey(); - const data = Buffer.from(token, 'base64'); - if (data.length < IV_LENGTH + TAG_LENGTH) return null; + const bound = openEnvelope(data, purpose); + if (bound) return bound.p === purpose ? bound : null; + } catch { + // Not a blob bound to this purpose: either it belongs to another purpose, + // it predates the binding, or it is forged. The legacy read below tells + // the second case apart from the other two. + } - const iv = data.subarray(0, IV_LENGTH); - const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); - const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); - - const decipher = createDecipheriv(ALGORITHM, key, iv); - decipher.setAuthTag(tag); - - const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); - return JSON.parse(decrypted.toString('utf8')); + try { + const legacy = openEnvelope(data, null); + if (legacy && legacy.p === undefined && LEGACY_PAYLOAD_MARKERS[purpose](legacy)) { + return legacy; + } } catch (error) { logger.warn('Payload decryption failed', { + purpose, error: error instanceof Error ? error.message : 'Unknown error', }); return null; } + + logger.warn('Payload rejected: not minted for this purpose', { purpose }); + return null; } diff --git a/lib/auth/pair-reauth.ts b/lib/auth/pair-reauth.ts index 8124c473f..51d1ea049 100644 --- a/lib/auth/pair-reauth.ts +++ b/lib/auth/pair-reauth.ts @@ -13,7 +13,7 @@ const PAIR_REAUTH_TTL_MS = 5 * 60 * 1000; // 5 minutes — long enough to render export async function setPairReauth(): Promise { const cookieStore = await cookies(); - const value = encryptPayload({ purpose: 'pair', created_at: Date.now() }); + const value = encryptPayload({ purpose: 'pair', created_at: Date.now() }, 'pair-reauth'); cookieStore.set(PAIR_REAUTH_COOKIE, value, { ...getCookieOptions(), maxAge: Math.floor(PAIR_REAUTH_TTL_MS / 1000), @@ -24,7 +24,7 @@ export async function hasValidPairReauth(): Promise { const cookieStore = await cookies(); const raw = cookieStore.get(PAIR_REAUTH_COOKIE)?.value; if (!raw) return false; - const data = decryptPayload(raw); + const data = decryptPayload(raw, 'pair-reauth'); if (!data || data.purpose !== 'pair') return false; const createdAt = typeof data.created_at === 'number' ? data.created_at : 0; // Belt-and-suspenders alongside the cookie maxAge: a forged/old proof whose diff --git a/lib/stalwart/auth-context.ts b/lib/stalwart/auth-context.ts index f2879d347..a58759b5e 100644 --- a/lib/stalwart/auth-context.ts +++ b/lib/stalwart/auth-context.ts @@ -22,6 +22,14 @@ function isValidContext(payload: unknown): payload is StalwartAuthContext { } const candidate = payload as Record; + // A session context is exactly these three strings. The envelope purpose + // already keeps blobs minted elsewhere out; rejecting the markers those + // blobs carry is the second lock, so an office editor token or a pairing + // proof cannot be read back as a signed-in session even if it reaches this + // function by some other route (GHSA-cqqx-mjcf-mh55). + if (candidate.t !== undefined || candidate.purpose !== undefined) { + return false; + } return typeof candidate.serverUrl === 'string' && typeof candidate.username === 'string' && typeof candidate.authHeader === 'string'; @@ -39,8 +47,13 @@ export function readStalwartAuthContextFromStore( const token = cookieStore.get(stalwartAuthContextCookieName(slot))?.value; if (!token) return null; - const payload = decryptPayload(token); - return isValidContext(payload) ? payload : null; + const payload = decryptPayload(token, 'session-context'); + if (!isValidContext(payload)) return null; + + // Hand back exactly the declared shape: the envelope's own bookkeeping + // stays in the envelope rather than riding along into the credentials. + const { serverUrl, username, authHeader } = payload; + return { serverUrl, username, authHeader }; } export async function readStalwartAuthContext(slot: number): Promise { @@ -55,7 +68,7 @@ export function setStalwartAuthContextInStore( ): void { cookieStore.set( stalwartAuthContextCookieName(slot), - encryptPayload(context as unknown as Record), + encryptPayload(context as unknown as Record, 'session-context'), getSessionCookieOptions(), ); } diff --git a/lib/wopi/token.ts b/lib/wopi/token.ts index 92268a93d..218454b60 100644 --- a/lib/wopi/token.ts +++ b/lib/wopi/token.ts @@ -43,7 +43,7 @@ function fromBase64Url(token: string): string { export function mintWopiToken(payload: Omit): { token: string; expiresAt: number } { const expiresAt = Date.now() + WOPI_TOKEN_TTL_MS; - const token = toBase64Url(encryptPayload({ v: 1, t: 'wopi', ...payload, exp: expiresAt })); + const token = toBase64Url(encryptPayload({ v: 1, t: 'wopi', ...payload, exp: expiresAt }, 'wopi-token')); return { token, expiresAt }; } @@ -53,7 +53,7 @@ export function mintWopiToken(payload: Omit): { token: */ export function verifyWopiToken(token: string | null, fileId: string): WopiTokenPayload | null { if (!token) return null; - const raw = decryptPayload(fromBase64Url(token)); + const raw = decryptPayload(fromBase64Url(token), 'wopi-token'); if (!raw || raw.v !== 1 || raw.t !== 'wopi') return null; const p = raw as unknown as WopiTokenPayload; if (