diff --git a/app/__tests__/api/gh1692-timing-safe-keeper-auth.test.ts b/app/__tests__/api/gh1692-timing-safe-keeper-auth.test.ts index 5b7dc51d9..ce3883160 100644 --- a/app/__tests__/api/gh1692-timing-safe-keeper-auth.test.ts +++ b/app/__tests__/api/gh1692-timing-safe-keeper-auth.test.ts @@ -53,7 +53,17 @@ describe("GH#1692/LAUNCH-16: oracle-keeper/register HMAC auth", () => { mainnetCA: "22222222222222222222222222222222", }); + // #2476: the signed string now binds the METHOD and PATH, not just + // ".". This helper signs what a MIGRATED caller signs; + // signUnbound below is the pre-#2476 form, kept so the legacy-rejection test + // below has something real to reject. function sign(secret: string, rawBody: string, timestamp = Date.now().toString()) { + const message = [timestamp, "POST", "/api/oracle-keeper/register", rawBody].join("\n"); + const signature = createHmac("sha256", secret).update(message).digest("hex"); + return { timestamp, signature }; + } + + function signUnbound(secret: string, rawBody: string, timestamp = Date.now().toString()) { const signature = createHmac("sha256", secret).update(`${timestamp}.${rawBody}`).digest("hex"); return { timestamp, signature }; } @@ -142,4 +152,57 @@ describe("GH#1692/LAUNCH-16: oracle-keeper/register HMAC auth", () => { expect(res.status).not.toBe(401); expect(res.status).not.toBe(503); }); + + // ── #2476: the signature binds the endpoint ──────────────────────────────── + // + // Before this, the signed string was "." — no method, no + // path — so every route sharing KEEPER_REGISTER_SECRET accepted every other + // route's signatures. The credential authenticated "someone who knows the + // secret", not "this request". + + it("rejects a signature that was not bound to this endpoint (#2476)", async () => { + // A signature minted for a DIFFERENT path, same secret, same body, same + // minute. Under the old scheme this verified here. + const rawBody = RAW_BODY; + const timestamp = Date.now().toString(); + const otherPath = createHmac("sha256", CORRECT_SECRET) + .update([timestamp, "POST", "/api/markets/SomeSlab", rawBody].join("\n")) + .digest("hex"); + + const { POST } = await import("@/app/api/oracle-keeper/register/route"); + const res = await POST( + new NextRequest("http://localhost/api/oracle-keeper/register", { + method: "POST", + headers: { + "content-type": "application/json", + "x-keeper-timestamp": timestamp, + "x-keeper-signature": otherPath, + }, + body: rawBody, + }), + ); + expect(res.status).toBe(401); + }); + + it("rejects the legacy UNBOUND signature on this endpoint (#2476)", async () => { + // This route's signer lives in the same repo and was migrated in the same + // commit, so it is strict — no legacy acceptance. (markets/[slab] is the one + // that still accepts the legacy form, because its caller is external.) + const rawBody = RAW_BODY; + const { timestamp, signature } = signUnbound(CORRECT_SECRET, rawBody); + + const { POST } = await import("@/app/api/oracle-keeper/register/route"); + const res = await POST( + new NextRequest("http://localhost/api/oracle-keeper/register", { + method: "POST", + headers: { + "content-type": "application/json", + "x-keeper-timestamp": timestamp, + "x-keeper-signature": signature, + }, + body: rawBody, + }), + ); + expect(res.status).toBe(401); + }); }); diff --git a/app/app/api/markets/[slab]/route.ts b/app/app/api/markets/[slab]/route.ts index 0dc27e021..c600ee0db 100644 --- a/app/app/api/markets/[slab]/route.ts +++ b/app/app/api/markets/[slab]/route.ts @@ -434,11 +434,21 @@ export async function PATCH( // Read the body ONCE as raw text — the HMAC covers the exact bytes sent, so it // must be verified before parsing, and re-reading the stream is not possible. const rawBody = await req.text(); + // #2476: bound to this method and path. `req.nextUrl.pathname` rather than a + // literal, because this route is parameterised by slab — a literal would be + // wrong, and reconstructing it would duplicate what the router already has. const signed = verifyKeeperSignature( secret, req.headers.get("x-keeper-timestamp"), rawBody, req.headers.get("x-keeper-signature"), + { method: req.method, path: req.nextUrl.pathname }, + // Transition: this endpoint's signer is NOT in this repo — the keeper + // service has no createHmac at all, so the caller is external and could + // not be updated in the same commit. Accepting the legacy form keeps that + // hop alive across the rollout and logs every use. Drop this argument once + // the warnings stop. See #2476. + true, ); if (!signed) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/app/api/markets/route.ts b/app/app/api/markets/route.ts index 1973033bf..3513cbafe 100644 --- a/app/app/api/markets/route.ts +++ b/app/app/api/markets/route.ts @@ -1928,7 +1928,10 @@ export async function POST(req: NextRequest) { const keeperBody = JSON.stringify({ slabAddress: slab_address, mainnetCA: canonicalMainnetCa }); // LAUNCH-16: sign instead of forwarding KEEPER_REGISTER_SECRET as a raw header — // /api/oracle-keeper/register verifies this same HMAC-SHA256 scheme. - const { timestamp, signature } = signKeeperRequest(process.env.KEEPER_REGISTER_SECRET, keeperBody); + const { timestamp, signature } = signKeeperRequest(process.env.KEEPER_REGISTER_SECRET, keeperBody, { + method: "POST", + path: "/api/oracle-keeper/register", + }); const res = await fetch(keeperRegisterUrl, { method: "POST", headers: { diff --git a/app/app/api/oracle-keeper/register/route.ts b/app/app/api/oracle-keeper/register/route.ts index 802046814..2f36cab0d 100644 --- a/app/app/api/oracle-keeper/register/route.ts +++ b/app/app/api/oracle-keeper/register/route.ts @@ -49,11 +49,14 @@ export async function POST(req: NextRequest) { // LAUNCH-16: HMAC-SHA256(REGISTER_SECRET, ".") replaces the raw // x-keeper-secret header — the credential itself never appears on the wire. Verification // is timing-safe and rejects stale/replayed signatures (see verifyKeeperSignature). + // #2476: the signature is bound to THIS method and path, so one issued for + // another route sharing the secret no longer verifies here. const signed = verifyKeeperSignature( REGISTER_SECRET, req.headers.get("x-keeper-timestamp"), rawBody, req.headers.get("x-keeper-signature"), + { method: "POST", path: "/api/oracle-keeper/register" }, ); if (!signed) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); @@ -131,7 +134,13 @@ export async function POST(req: NextRequest) { // The keeper verifies: HMAC-SHA256(REGISTER_SECRET, ".") == x-keeper-signature. // This means the raw credential is never sent on the wire regardless of URL scheme. const keeperPayload = JSON.stringify({ slabAddress, mainnetCA }); - const { timestamp: keeperTimestamp, signature: keeperSig } = signKeeperRequest(REGISTER_SECRET, keeperPayload); + const { timestamp: keeperTimestamp, signature: keeperSig } = signKeeperRequest(REGISTER_SECRET, keeperPayload, { + // #2533: the keeper service verifies x-shared-secret, NOT this HMAC, so + // this binding is currently INERT on this hop. Bound anyway, so the + // sender is already correct when the keeper adopts HMAC verification. + method: "POST", + path: "/register", + }); const keeperResp = await fetch(`${KEEPER_URL}/register`, { method: "POST", diff --git a/app/lib/keeper-hmac.ts b/app/lib/keeper-hmac.ts index e232049ae..ff5907d8d 100644 --- a/app/lib/keeper-hmac.ts +++ b/app/lib/keeper-hmac.ts @@ -12,12 +12,54 @@ import { createHmac, timingSafeEqual } from "node:crypto"; /** Reject signatures older than this — bounds the replay window. */ const MAX_SIGNATURE_AGE_MS = 5 * 60_000; +/** The request this signature is for. #2476: without it, one signature is valid + * for the same body sent to any endpoint sharing the secret. */ +export interface KeeperRequestBinding { + /** HTTP method, any casing — normalised below. */ + method: string; + /** Path only: no origin, no query string. */ + path: string; +} + +/** + * The exact string both ends HMAC. + * + * Newline-separated rather than dot-separated: a dot can appear in a path, so + * `"POST./a.b"` and `"POST./a" + ".b"` would be ambiguous under the old + * delimiter. A newline cannot appear in a method or a URL path. + */ +function signedString(timestamp: string, rawBody: string, b: KeeperRequestBinding): string { + return [timestamp, b.method.toUpperCase(), b.path, rawBody].join("\n"); +} + +/** + * #2476: the signed string now covers the HTTP METHOD and PATH, not just the + * timestamp and body. + * + * Without them a signature is valid for the same body sent to a DIFFERENT + * endpoint — every route sharing KEEPER_REGISTER_SECRET accepted each other's + * signatures, so the credential authenticated "someone who knows the secret" + * rather than "this request". Binding the target makes a captured signature + * usable only against the endpoint it was issued for. + * + * NOT fixed here, and #2476 is right that it remains: there is still no nonce, so + * a captured signature is replayable against that same endpoint within + * MAX_SIGNATURE_AGE_MS. A nonce needs a store shared across serverless instances + * — the same constraint that shaped the keeper-register proof (#2505) — so it is + * a separate design problem, not a line change. + * + * `method` is upper-cased and `path` is taken WITHOUT query or origin, so the two + * ends cannot disagree over casing or a trailing host. + */ export function signKeeperRequest( secret: string, rawBody: string, + binding: KeeperRequestBinding, ): { timestamp: string; signature: string } { const timestamp = Date.now().toString(); - const signature = createHmac("sha256", secret).update(`${timestamp}.${rawBody}`).digest("hex"); + const signature = createHmac("sha256", secret) + .update(signedString(timestamp, rawBody, binding)) + .digest("hex"); return { timestamp, signature }; } @@ -25,18 +67,52 @@ export function signKeeperRequest( * Verify a signed request. Timing-safe comparison prevents a signature-length/byte * timing oracle; the timestamp bound prevents replay of a captured signature. */ +/** + * @param allowLegacyUnbound Accept the pre-#2476 `"."` form + * as well as the bound one. + * + * Set this ONLY where the signer is outside this repo and therefore cannot be + * updated in the same commit. It keeps such a hop working across the rollout, + * and it is NOT a fix — an unbound signature is still valid for any endpoint + * sharing the secret, which is the whole of #2476. Each acceptance is logged so + * the transition can actually be finished rather than forgotten; once the logs + * go quiet for a caller, drop the flag at that call site. + */ export function verifyKeeperSignature( secret: string, timestamp: string | null, rawBody: string, signature: string | null, + binding: KeeperRequestBinding, + allowLegacyUnbound = false, ): boolean { if (!timestamp || !signature) return false; const ts = Number(timestamp); if (!Number.isFinite(ts) || Math.abs(Date.now() - ts) > MAX_SIGNATURE_AGE_MS) return false; - const expected = createHmac("sha256", secret).update(`${timestamp}.${rawBody}`).digest("hex"); - const aBytes = Buffer.from(signature, "utf8"); - const bBytes = Buffer.from(expected, "utf8"); - return aBytes.length === bBytes.length && timingSafeEqual(aBytes, bBytes); + const matches = (expected: string): boolean => { + const aBytes = Buffer.from(signature, "utf8"); + const bBytes = Buffer.from(expected, "utf8"); + return aBytes.length === bBytes.length && timingSafeEqual(aBytes, bBytes); + }; + + const bound = createHmac("sha256", secret) + .update(signedString(timestamp, rawBody, binding)) + .digest("hex"); + if (matches(bound)) return true; + + if (!allowLegacyUnbound) return false; + + // Pre-#2476 form. Deliberately checked SECOND, so a caller that has migrated is + // never evaluated against the weaker string. + const legacy = createHmac("sha256", secret).update(`${timestamp}.${rawBody}`).digest("hex"); + if (matches(legacy)) { + console.warn( + "[keeper-hmac] Accepted a LEGACY unbound signature (#2476). The caller has not " + + `migrated; it is not bound to ${binding.method.toUpperCase()} ${binding.path}. ` + + "Remove allowLegacyUnbound at this call site once these stop appearing.", + ); + return true; + } + return false; }