From 289f408d239ea43b93b54bf131ea885f8d9ceeab Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sat, 5 Sep 2026 23:01:53 +0200 Subject: [PATCH] perf(dpop): collapse the SW nonce stampede to a single challenge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DPoP proof carries a server-issued nonce. With none cached the server answers `401 use_dpop_nonce`, the client harvests the nonce and retries. `signAndFetch` already absorbed that, so it was invisible — but it did it PER REQUEST, with no coordination. The Service Worker always cold-starts without a nonce. Both mechanisms that pre-seed the page are unreachable from worker scope: workers have no `sessionStorage`, and `seedNonceFromCookie` early-returns on `typeof document === 'undefined'`. The SW also skips requests that already carry a `DPoP` header, so it never observes the page's responses and cannot harvest from them either. Browsers terminate idle workers after ~30s, so this happens routinely, not once. Uncoordinated, every request issued in that window discovers the nonce independently: N parallel requests → N challenges → 2N requests. That is precisely the photo grid, and serving `` is the SW's main job — those requests cannot sign themselves, which is why the worker exists. Each wasted challenge also costs the server a full ECDSA P-256 verify, because `verify_proof` runs before the nonce check. Now the first request through owns the discovery and the rest await it, so N challenges collapse to 1. The wait is capped (5s) and released in a `finally`: the SW is on the critical path for every thumbnail, so a hung discovery must degrade to the old behaviour rather than stall the grid behind a promise that never settles. Measured on a 763-line e2e server log: 115 `dpop.nonce_challenged` events across 97 logins. Scope, deliberately: this does NOT remove the one challenge per worker lifetime. Doing that needs the nonce persisted where a worker can read it — IndexedDB already holds the keypair — and it can never replace the challenge path anyway, since a persisted nonce can be stale. Left out until the audit line shows it is worth it; the numbers above are now legible enough to tell. Not a correctness fix. Nothing was broken and no test failed over this. The argument is waste, plus signal: 115 challenges per run is noise that would bury a real one. `hasNonce()` is exported for the gate — deliberately "will the next proof carry a nonce", not "is it still valid", since only the server knows the latter and the challenge path already handles it. Its tests assert it agrees with what `buildDpopProof` actually emits, because a wrong answer either reinstates the stampede or stalls every request behind a bootstrap that is not happening. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/auth/dpop-proof.test.ts | 43 ++++++++++++ frontend/src/lib/auth/dpop-proof.ts | 17 +++++ frontend/src/service-worker.ts | 87 ++++++++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/frontend/src/lib/auth/dpop-proof.test.ts b/frontend/src/lib/auth/dpop-proof.test.ts index 9e29add4..2dcab922 100644 --- a/frontend/src/lib/auth/dpop-proof.test.ts +++ b/frontend/src/lib/auth/dpop-proof.test.ts @@ -17,6 +17,7 @@ import { buildDpopProof, canonicalHtu, clearNonce, + hasNonce, isDpopNonceChallenge, updateNonceFromResponse } from './dpop-proof'; @@ -153,3 +154,45 @@ describe('updateNonceFromResponse', () => { expect(b64uDecodeJson(proof!.split('.')[1]).nonce).toBe('v2'); }); }); + +describe('hasNonce', () => { + // The Service Worker's single-flight gate keys off this: `false` + // means the next proof carries no nonce and WILL be challenged, so + // concurrent requests should let one of them discover it rather than + // each taking its own 401. A wrong answer here either reinstates the + // stampede (false negative) or stalls every request behind a + // bootstrap that is not happening (false positive). + it('is false before any nonce has been seen', () => { + expect(hasNonce()).toBe(false); + }); + + it('is true once a nonce has been harvested', () => { + updateNonceFromResponse( + new Response(null, { status: 200, headers: { 'DPoP-Nonce': 'nonce-1' } }) + ); + expect(hasNonce()).toBe(true); + }); + + it('agrees with what the proof actually carries', async () => { + expect(hasNonce()).toBe(false); + const before = await buildDpopProof('GET', '/api/x'); + expect(b64uDecodeJson(before!.split('.')[1]).nonce).toBeUndefined(); + + updateNonceFromResponse( + new Response(null, { status: 200, headers: { 'DPoP-Nonce': 'nonce-2' } }) + ); + + expect(hasNonce()).toBe(true); + const after = await buildDpopProof('GET', '/api/x'); + expect(b64uDecodeJson(after!.split('.')[1]).nonce).toBe('nonce-2'); + }); + + it('is false again after logout clears the nonce', () => { + updateNonceFromResponse( + new Response(null, { status: 200, headers: { 'DPoP-Nonce': 'nonce-3' } }) + ); + expect(hasNonce()).toBe(true); + clearNonce(); + expect(hasNonce()).toBe(false); + }); +}); diff --git a/frontend/src/lib/auth/dpop-proof.ts b/frontend/src/lib/auth/dpop-proof.ts index c55dda5a..5460d6e8 100644 --- a/frontend/src/lib/auth/dpop-proof.ts +++ b/frontend/src/lib/auth/dpop-proof.ts @@ -82,6 +82,23 @@ export function seedNonceFromCookie(): void { document.cookie = 'oxicloud_dpop_nonce=; SameSite=Strict; Path=/; Max-Age=0'; } +/** + * Whether a nonce is cached, i.e. whether the next proof will carry one. + * + * Exists for the Service Worker's single-flight gate: `false` means the + * next request WILL be answered with a nonce challenge, so concurrent + * callers should let one request discover the nonce rather than each + * discovering it separately. See `signAndFetch` in `service-worker.ts`. + * + * Deliberately not "is the nonce still valid" — only the server knows + * that, and a stale nonce is handled by the challenge/retry path. This + * answers the cheaper question that avoids a stampede. + */ +export function hasNonce(): boolean { + loadNonceOnce(); + return currentNonce !== null; +} + /** Wipe the current nonce — called on logout so a new session bootstraps fresh. */ export function clearNonce(): void { currentNonce = null; diff --git a/frontend/src/service-worker.ts b/frontend/src/service-worker.ts index 3e30141b..a51e93b0 100644 --- a/frontend/src/service-worker.ts +++ b/frontend/src/service-worker.ts @@ -43,6 +43,7 @@ import { buildDpopProof, + hasNonce, isDpopNonceChallenge, updateNonceFromResponse } from '$lib/auth/dpop-proof'; @@ -79,7 +80,93 @@ self.addEventListener('fetch', (event) => { event.respondWith(signAndFetch(req)); }); +/** + * In-flight nonce discovery, or `null` when a nonce is already cached. + * + * Single-flight gate — see [`signAndFetch`]. + */ +let nonceBootstrap: Promise | null = null; + +/** + * Cap on how long a request will wait for someone else's bootstrap. + * + * The SW sits on the critical path for every thumbnail, so a hung or + * pathologically slow discovery must not stall the grid behind it. On + * expiry the waiter proceeds unsigned-by-nonce and takes its own + * challenge — exactly the pre-gate behaviour, so the worst case is what + * we had before rather than a stall. + */ +const NONCE_BOOTSTRAP_WAIT_MS = 5_000; + +/** + * DPoP-sign one request, with a **single-flight gate on the first + * nonce**. + * + * ## Why the gate + * + * A DPoP proof carries a server-issued nonce. With none cached the + * server answers `401 use_dpop_nonce`, the client harvests the nonce + * from the response and retries — one extra round trip, absorbed here + * so the caller never sees it. + * + * The SW's nonce cache is per-worker and **in memory only**: workers + * have no `sessionStorage`, and `seedNonceFromCookie` needs `document`, + * so neither mechanism that pre-seeds the page reaches this scope. A + * worker therefore always cold-starts with no nonce — and browsers + * terminate idle workers after ~30s, so that happens often. + * + * Without a gate, every request issued in that window discovers the + * nonce *independently*: N parallel requests → N challenges → 2N + * requests. The photo grid is exactly this shape, and it is the SW's + * main job (`` thumbnails can't sign themselves). Each wasted + * challenge also costs the server a full ECDSA verify, since + * `verify_proof` runs before the nonce check. + * + * So: the first request through discovers the nonce; the rest await it + * and then sign normally. N challenges collapse to 1. + * + * ## What this is not + * + * Not a correctness fix — the retry already made this invisible. It + * removes waste, and it keeps `dpop.nonce_challenged` rare enough in + * the audit log to be worth reading. + * + * Not a cure for cold starts. One challenge per worker lifetime + * remains; removing that needs the nonce persisted somewhere the worker + * can reach (IndexedDB already holds the keypair). Deliberately left + * out — it cannot replace the challenge path anyway, since a persisted + * nonce can be stale. + */ async function signAndFetch(req: Request): Promise { + if (!hasNonce()) { + if (nonceBootstrap) { + // Someone else is already discovering it. Wait — but never + // indefinitely; on timeout fall through and take our own + // challenge. + await Promise.race([ + nonceBootstrap, + new Promise((resolve) => setTimeout(resolve, NONCE_BOOTSTRAP_WAIT_MS)) + ]); + } else { + // We own the discovery. `finally` releases on every path, + // including a thrown fetch — a waiter blocked on a promise + // that never settles would be worse than the stampede. + let release!: () => void; + nonceBootstrap = new Promise((resolve) => { + release = resolve; + }); + try { + return await signAndFetchOnce(req); + } finally { + nonceBootstrap = null; + release(); + } + } + } + return signAndFetchOnce(req); +} + +async function signAndFetchOnce(req: Request): Promise { const firstProof = await buildDpopProof(req.method, req.url).catch(() => null); // No keypair (IndexedDB blocked, SubtleCrypto missing, etc.) — pass // through unsigned. Unbound sessions still work; bound sessions in