From 6ed6d8fa0dbd2ac8d7700cfea4bd7e7bbe1accce Mon Sep 17 00:00:00 2001 From: Heimdall Date: Fri, 13 Nov 2026 20:19:41 +0000 Subject: [PATCH] Step-up assertion: pin per-app not-permitted budget and the 10.5 min prune horizon, name the tripped budget in the audit row, correct validation-order and audit docs Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 27 +++++++++++++++++---------- src/app.ts | 15 +++++++++------ tests/step-up-assertion.test.ts | 29 +++++++++++++++++++++++------ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index 32b402a..2266c0d 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -649,10 +649,13 @@ Nonce: 16 to 128 characters of the base64url alphabet (`A-Za-z0-9_-`), so a random token fits and spaces, controls and lone surrogates are a 400 schema refusal. `method` is 1 to 64 characters and then must be in the profile's set. -Order of refusals, each a closed `error` code: +Schema validation (body shape, `method` length, the nonce pattern and length, +the app slug) runs before the secret is checked and answers `400` with +Fastify's fixed schema error, the same for every caller whatever the secret and +never audited. Then the order of refusals, each a closed `error` code: -- `401 app_auth_required`: missing, wrong or another app's secret. Checked - first; nothing is written. +- `401 app_auth_required`: missing, wrong or another app's secret. The first + check of the handler; nothing is written. - `403 step_up_assertion_not_permitted`: the app's profile has no `stepUpAssertion` grant. Only `bifrost` has one; every other app is refused with its own secret. This branch has its own small budget (5 per 60 s per @@ -678,7 +681,8 @@ Order of refusals, each a closed `error` code: The claim is the store's `oauth_state_uses` single-use primitive with phase `step_up_assert` and the key a SHA-256 of session id and nonce, so it holds across restarts and instances. **Retention is the session's expiry as of the - claim, plus the store's prune margin.** A refresh slides the session's expiry + claim plus 10.5 minutes (the 10 minute prune margin and 30 s clock skew), and + pruning is lazy: each successful claim deletes at most 50 expired rows.** A refresh slides the session's expiry without bound, so a nonce can be pruned while its session is still live and then be accepted again. Replaying it still needs the secret and a live bearer, which is the power to assert with a fresh nonce; the nonce guards a @@ -698,13 +702,16 @@ the token's own `exp`. Receipts: each assertion writes the audit event `step_up_asserted`, and each refusal after the secret verified writes `step_up_assertion_refused`, except -that a flood is not recorded row by row: a rate-limited refusal is recorded -once per window per budget (its first), and an app without the grant once per -window, with the +that two kinds are not recorded row by row: a rate-limited refusal is recorded +once per window per budget (its first; the row's `budget` names `app`, +`account` or `notPermitted`), and an app without the grant once per window. +Every other refusal under budget still writes a row each, up to about 121 a +minute for a holder of the secret, and `audit_events` has no retention: that is +an open follow-up. Rows carry the app, the account and session when the bearer resolved, `method` (null when it -was not in the allowlist), `outcome`, `code` and `nonceDigest`. Never the -secret, the bearer, the minted token or the nonce. A request without the secret -writes nothing. +was not in the allowlist), `outcome`, `code`, `budget` (rate-limit rows) and `nonceDigest`. Never the +secret, the bearer, the minted token or the nonce. A request without the secret, +or failing schema validation, writes nothing. Precondition: the route is usable only once the app holds the member's Heimdall bearer. Bifrost deliberately does not hold it today. Putting the bearer in a diff --git a/src/app.ts b/src/app.ts index ac2f97a..9e8c334 100644 --- a/src/app.ts +++ b/src/app.ts @@ -998,28 +998,31 @@ export async function assertStepUp( const permitted = getAppProfile(appSlug)?.stepUpAssertion; let known: { accountId: string; sessionId: string } | undefined; let nonceDigest: string | undefined; + let tripped: string | undefined; const refuse = async (statusCode: number, error: string, extra: Record = {}, record = true): Promise => { if (!record) return { statusCode, body: { error, ...extra } }; await store.createAuditEvent({ ...(known ? { accountId: known.accountId, sessionId: known.sessionId } : {}), appSlug, eventType: "step_up_assertion_refused", - eventPayloadJson: { outcome: "refused", code: error, method: permitted?.methods.includes(input.method) ? input.method : null, nonceDigest: nonceDigest ?? null }, + eventPayloadJson: { outcome: "refused", code: error, budget: tripped ?? null, method: permitted?.methods.includes(input.method) ? input.method : null, nonceDigest: nonceDigest ?? null }, createdAt: new Date().toISOString(), }); return { statusCode, body: { error, ...extra } }; }; // Audit only the first denial of a window: the rows record that a budget tripped, not every request behind it. - const limited = (spent: { n: number; retryAfterSeconds: number }, max: number) => - refuse(429, "step_up_rate_limited", { retryAfterSeconds: spent.retryAfterSeconds }, spent.n === max + 1); + const limited = (budget: string, spent: { n: number; retryAfterSeconds: number }, max: number) => { + tripped = budget; + return refuse(429, "step_up_rate_limited", { retryAfterSeconds: spent.retryAfterSeconds }, spent.n === max + 1); + }; if (!permitted) { const spent = budgets.notPermitted.spend(appSlug); - if (spent.retryAfterSeconds) return limited(spent, STEP_UP_ASSERT_LIMITS.notPermitted.max); + if (spent.retryAfterSeconds) return limited("notPermitted", spent, STEP_UP_ASSERT_LIMITS.notPermitted.max); return refuse(403, "step_up_assertion_not_permitted", { detail: `${appSlug} may not assert step-up proofs.` }, spent.n === 1); } const appSpent = budgets.app.spend(appSlug); - if (appSpent.retryAfterSeconds) return limited(appSpent, STEP_UP_ASSERT_LIMITS.app.max); + if (appSpent.retryAfterSeconds) return limited("app", appSpent, STEP_UP_ASSERT_LIMITS.app.max); const bearer = await resolveBearer(ctx, headers.authorization as string | undefined); if (!bearer) { @@ -1031,7 +1034,7 @@ export async function assertStepUp( return refuse(403, "bearer_app_mismatch", { detail: `The access token was not issued to ${appSlug}.` }); } const accountSpent = budgets.account.spend(bearer.accountId); - if (accountSpent.retryAfterSeconds) return limited(accountSpent, STEP_UP_ASSERT_LIMITS.account.max); + if (accountSpent.retryAfterSeconds) return limited("account", accountSpent, STEP_UP_ASSERT_LIMITS.account.max); nonceDigest = createHash("sha256").update(`heimdall-step-up-nonce\0${sessionId}\0${input.nonce}`).digest("hex"); if (!permitted.methods.includes(input.method)) { diff --git a/tests/step-up-assertion.test.ts b/tests/step-up-assertion.test.ts index b3115f1..f064e00 100644 --- a/tests/step-up-assertion.test.ts +++ b/tests/step-up-assertion.test.ts @@ -278,7 +278,7 @@ describe("the method and the nonce", () => { expect((await assertStepUp(r, { token: second.token })).statusCode).toBe(201); }); - it("a nonce is retained to the session's expiry as of the claim: a refresh slides the session past it, and then the nonce can be reused (documented limit)", async () => { + it("a nonce is retained to the session's expiry as of the claim plus the 10.5 min prune horizon; a refresh slides the session past that and the nonce can be reused (documented limit)", async () => { const r = await rig(); const m = await member(r); vi.useFakeTimers({ toFake: ["Date"] }); @@ -289,11 +289,14 @@ describe("the method and the nonce", () => { const refreshed = await r.app.inject({ method: "POST", url: "/v1/apps/bifrost/sessions/refresh", payload: { refreshToken: m.issued.refreshToken } }); expect(refreshed.statusCode).toBe(201); const token = refreshed.json().accessToken as string; - // Inside the retained horizon the replay is still refused, refreshed session or not. - expect(refused(await assertStepUp(r, { token, nonce: nonce("N") }))).toEqual([409, "step_up_nonce_replayed"]); - // Past the old expiry plus the store's prune margin, the next claim prunes N while the session lives on. - vi.setSystemTime(t0 + R + 11 * 60_000); + expect(refused(await assertStepUp(r, { token, nonce: nonce("N") })).join()).toBe("409,step_up_nonce_replayed"); + // 10 min past the old expiry: a claim runs the prune, and N is still inside the 10.5 min horizon (10 min margin + 30 s skew). + vi.setSystemTime(t0 + R + 10 * 60_000); expect((await assertStepUp(r, { token, nonce: nonce("M") })).statusCode).toBe(201); + expect(refused(await assertStepUp(r, { token, nonce: nonce("N") }))).toEqual([409, "step_up_nonce_replayed"]); + // Past the horizon the next claim prunes N (lazily, with the claim) while the session lives on. + vi.setSystemTime(t0 + R + 10 * 60_000 + 31_000 + 2_000); + expect((await assertStepUp(r, { token, nonce: nonce("K") })).statusCode).toBe(201); expect((await assertStepUp(r, { token, nonce: nonce("N") })).statusCode).toBe(201); }); @@ -343,6 +346,18 @@ describe("rate limits", () => { }); describe("flood handling", () => { + it("the not-permitted budget is per app: one app's flood neither blocks bifrost's own call nor another ungranted app's first refusal", async () => { + const r = await rig(); + const sp = await member(r, "streampixels"); + const rp = await member(r, "repixelizer"); + const b = await member(r); + const as = (app: string, appSecret: string, token: string, i: number) => assertStepUp(r, { app, headers: { "x-heimdall-app-secret": appSecret }, token, nonce: nonce(`i${i}`) }); + for (let i = 0; i < 50; i += 1) await as("streampixels", "streampixels-secret", sp.token, i); + expect((await as("streampixels", "streampixels-secret", sp.token, 99)).statusCode).toBe(429); + expect(refused(await as("repixelizer", "repixelizer-secret", rp.token, 1))).toEqual([403, "step_up_assertion_not_permitted"]); + expect((await assertStepUp(r, { token: b.token })).statusCode).toBe(201); + }); + it("a flood is not an audit flood: an app without the grant is audited once per window, a tripped budget once per window", async () => { const r = await rig(); const sp = await member(r, "streampixels"); @@ -356,6 +371,7 @@ describe("flood handling", () => { const rows = (code: string) => auditOf(r, "step_up_assertion_refused").filter((e) => e.eventPayloadJson.code === code); expect(rows("step_up_assertion_not_permitted")).toHaveLength(1); expect(rows("step_up_rate_limited")).toHaveLength(1); + expect(rows("step_up_rate_limited")[0]!.eventPayloadJson.budget).toBe("notPermitted"); vi.setSystemTime(Date.now() + STEP_UP_ASSERT_LIMITS.notPermitted.windowMs + 1000); await assertStepUp(r, { app: "streampixels", headers: { "x-heimdall-app-secret": "streampixels-secret" }, token: sp.token }); expect(rows("step_up_assertion_not_permitted")).toHaveLength(2); @@ -370,7 +386,8 @@ describe("flood handling", () => { for (let i = 0; i < 30; i += 1) await assertStepUp(r, { token: m.token, nonce: nonce(`a${i}`) }); const accountRows = r.audit.slice(mark); expect(accountRows.filter((e) => e.eventType === "step_up_asserted")).toHaveLength(STEP_UP_ASSERT_LIMITS.account.max); - expect(accountRows.filter((e) => e.eventPayloadJson.code === "step_up_rate_limited")).toHaveLength(1); + const trips = accountRows.filter((e) => e.eventPayloadJson.code === "step_up_rate_limited"); + expect(trips.map((e) => e.eventPayloadJson.budget)).toEqual(["account"]); }); });