Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions app/__tests__/api/gh1692-timing-safe-keeper-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
// "<timestamp>.<rawBody>". 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 };
}
Expand Down Expand Up @@ -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 "<timestamp>.<rawBody>" — 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);
});
});
10 changes: 10 additions & 0 deletions app/app/api/markets/[slab]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
5 changes: 4 additions & 1 deletion app/app/api/markets/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
11 changes: 10 additions & 1 deletion app/app/api/oracle-keeper/register/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ export async function POST(req: NextRequest) {
// LAUNCH-16: HMAC-SHA256(REGISTER_SECRET, "<timestamp>.<rawBody>") 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 });
Expand Down Expand Up @@ -131,7 +134,13 @@ export async function POST(req: NextRequest) {
// The keeper verifies: HMAC-SHA256(REGISTER_SECRET, "<timestamp>.<body>") == 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",
Expand Down
86 changes: 81 additions & 5 deletions app/lib/keeper-hmac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,107 @@ 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 };
}

/**
* 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 `"<timestamp>.<rawBody>"` 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;
}
Loading