perf(dpop): collapse the SW nonce stampede to a single challenge
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 `<img src>` 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) <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,7 @@ import {
|
|||||||
buildDpopProof,
|
buildDpopProof,
|
||||||
canonicalHtu,
|
canonicalHtu,
|
||||||
clearNonce,
|
clearNonce,
|
||||||
|
hasNonce,
|
||||||
isDpopNonceChallenge,
|
isDpopNonceChallenge,
|
||||||
updateNonceFromResponse
|
updateNonceFromResponse
|
||||||
} from './dpop-proof';
|
} from './dpop-proof';
|
||||||
@@ -153,3 +154,45 @@ describe('updateNonceFromResponse', () => {
|
|||||||
expect(b64uDecodeJson(proof!.split('.')[1]).nonce).toBe('v2');
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -82,6 +82,23 @@ export function seedNonceFromCookie(): void {
|
|||||||
document.cookie = 'oxicloud_dpop_nonce=; SameSite=Strict; Path=/; Max-Age=0';
|
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. */
|
/** Wipe the current nonce — called on logout so a new session bootstraps fresh. */
|
||||||
export function clearNonce(): void {
|
export function clearNonce(): void {
|
||||||
currentNonce = null;
|
currentNonce = null;
|
||||||
|
|||||||
@@ -43,6 +43,7 @@
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
buildDpopProof,
|
buildDpopProof,
|
||||||
|
hasNonce,
|
||||||
isDpopNonceChallenge,
|
isDpopNonceChallenge,
|
||||||
updateNonceFromResponse
|
updateNonceFromResponse
|
||||||
} from '$lib/auth/dpop-proof';
|
} from '$lib/auth/dpop-proof';
|
||||||
@@ -79,7 +80,93 @@ self.addEventListener('fetch', (event) => {
|
|||||||
event.respondWith(signAndFetch(req));
|
event.respondWith(signAndFetch(req));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* In-flight nonce discovery, or `null` when a nonce is already cached.
|
||||||
|
*
|
||||||
|
* Single-flight gate — see [`signAndFetch`].
|
||||||
|
*/
|
||||||
|
let nonceBootstrap: Promise<void> | 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 (`<img src>` 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<Response> {
|
async function signAndFetch(req: Request): Promise<Response> {
|
||||||
|
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<void>((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<void>((resolve) => {
|
||||||
|
release = resolve;
|
||||||
|
});
|
||||||
|
try {
|
||||||
|
return await signAndFetchOnce(req);
|
||||||
|
} finally {
|
||||||
|
nonceBootstrap = null;
|
||||||
|
release();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return signAndFetchOnce(req);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function signAndFetchOnce(req: Request): Promise<Response> {
|
||||||
const firstProof = await buildDpopProof(req.method, req.url).catch(() => null);
|
const firstProof = await buildDpopProof(req.method, req.url).catch(() => null);
|
||||||
// No keypair (IndexedDB blocked, SubtleCrypto missing, etc.) — pass
|
// No keypair (IndexedDB blocked, SubtleCrypto missing, etc.) — pass
|
||||||
// through unsigned. Unbound sessions still work; bound sessions in
|
// through unsigned. Unbound sessions still work; bound sessions in
|
||||||
|
|||||||
Reference in New Issue
Block a user