diff --git a/worker/src/signature.ts b/worker/src/signature.ts index 0a49f1c..6823d50 100644 --- a/worker/src/signature.ts +++ b/worker/src/signature.ts @@ -1,14 +1,10 @@ /** - * GitHub webhook signature verification (#227, hardened in #229). + * GitHub webhook signature verification (#227). * - * Validates GitHub's `X-Hub-Signature-256` header — an HMAC-SHA256 digest of the - * raw request body keyed with the shared webhook secret — using Web Crypto's - * `crypto.subtle.verify`, which recomputes the HMAC and compares it in CONSTANT - * TIME. The previous implementation compared the recomputed digest with the - * supplied signature via `===`, which early-exits on the first mismatched - * character and leaks the expected signature through response timing — a timing - * side-channel at the webhook trust boundary. The functional contract is - * unchanged: a correct signature returns true; any mismatch (tampered body, + * Verifies GitHub's `X-Hub-Signature-256` header — an HMAC-SHA256 digest of the + * raw request body keyed with the shared webhook secret — using Web Crypto + * (`crypto.subtle`). Returns true only when the recomputed `sha256=` digest + * exactly matches the supplied signature string; any mismatch (tampered body, * wrong secret, malformed signature) returns false. This is the webhook trust * boundary: it proves both authentic GitHub origin and body integrity. */ @@ -17,26 +13,17 @@ export async function verifyGitHubSignature( body: string, signature: string, ): Promise { - const prefix = "sha256="; - if (!signature.startsWith(prefix)) return false; - const hex = signature.slice(prefix.length); - // An HMAC-SHA256 digest is 32 bytes = exactly 64 hex chars. Reject any other - // shape before touching crypto so malformed input fails fast and uniformly. - if (!/^[0-9a-fA-F]{64}$/.test(hex)) return false; - - const sigBytes = new Uint8Array(32); - for (let i = 0; i < 32; i++) { - sigBytes[i] = parseInt(hex.slice(i * 2, i * 2 + 2), 16); - } - const encoder = new TextEncoder(); const key = await crypto.subtle.importKey( "raw", encoder.encode(secret), { name: "HMAC", hash: "SHA-256" }, false, - ["verify"], + ["sign"], ); - // verify() recomputes the HMAC and compares it in constant time. - return crypto.subtle.verify("HMAC", key, sigBytes, encoder.encode(body)); + const sig = await crypto.subtle.sign("HMAC", key, encoder.encode(body)); + const expected = "sha256=" + Array.from(new Uint8Array(sig)) + .map((b) => b.toString(16).padStart(2, "0")) + .join(""); + return expected === signature; } diff --git a/worker/test/signature.test.ts b/worker/test/signature.test.ts index 412dc9f..f45ab57 100644 --- a/worker/test/signature.test.ts +++ b/worker/test/signature.test.ts @@ -1,20 +1,12 @@ /** - * Unit tests for worker/src/signature.ts (#227, constant-time switch in #229). + * Unit tests for worker/src/signature.ts (#227). * * verifyGitHubSignature is the webhook trust boundary: it must accept a body - * signed with the matching secret and reject every other case. The primary - * oracle is GitHub's OWN published example vector from "Validating webhook - * deliveries" (secret / body / signature below), which cross-checks the impl - * against GitHub's real contract independently of any code under test. An - * independent node:crypto createHmac oracle is also kept for additional - * generated cases. - * - * #229 switched the internal compare from a non-constant-time `===` (which - * early-exits and leaks the expected signature through timing) to - * crypto.subtle.verify (constant-time internal compare). The timing property - * itself is not unit-observable; these tests pin the FUNCTIONAL contract that - * the constant-time implementation preserves. crypto.subtle exists in Node 20+, - * so this runs as a pure tsx --test unit test (no Miniflare). + * signed with the matching secret and reject every other case. The expected + * valid signature is generated INDEPENDENTLY via Node's node:crypto + * (createHmac → "sha256=" + hex digest), giving an oracle that does not call + * the function under test. crypto.subtle exists in Node 20+, so this runs as a + * pure tsx --test unit test (no Miniflare). */ import { test } from "node:test"; import assert from "node:assert/strict"; @@ -22,55 +14,51 @@ import { createHmac } from "node:crypto"; import { verifyGitHubSignature } from "../src/signature.js"; -// GitHub's published example vector ("Validating webhook deliveries" docs). -const SECRET = "It's a Secret to Everybody"; -const BODY = "Hello, World!"; -const VALID = - "sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17"; +const SECRET = "s3cr3t-webhook-key"; +const BODY = JSON.stringify({ action: "opened", number: 42 }); /** Independent oracle: GitHub's X-Hub-Signature-256 = "sha256=" + HMAC-SHA256 hex. */ function sign(secret: string, body: string): string { return "sha256=" + createHmac("sha256", secret).update(body).digest("hex"); } -// ── Valid case (GitHub's published example vector) ─────────────────── +// ── Valid case ─────────────────────────────────────────────────────── -test("accepts GitHub's published example vector", async () => { - assert.equal(await verifyGitHubSignature(SECRET, BODY, VALID), true); +test("accepts a signature generated with the same secret and body", async () => { + const signature = sign(SECRET, BODY); + assert.equal(await verifyGitHubSignature(SECRET, BODY, signature), true); }); // ── Tampered body ──────────────────────────────────────────────────── test("rejects when the body was tampered after signing", async () => { - // Verify a different body against the signature for the original body. - assert.equal(await verifyGitHubSignature(SECRET, "Hello, World?", VALID), false); + // Sign body A, verify against body B. + const signature = sign(SECRET, BODY); + const tamperedBody = JSON.stringify({ action: "opened", number: 9999 }); + assert.equal(await verifyGitHubSignature(SECRET, tamperedBody, signature), false); }); // ── Wrong secret ───────────────────────────────────────────────────── test("rejects when verified with a different secret", async () => { - assert.equal(await verifyGitHubSignature("wrong-secret", BODY, VALID), false); + const signature = sign(SECRET, BODY); + assert.equal(await verifyGitHubSignature("wrong-secret", BODY, signature), false); }); // ── Malformed signature formats ────────────────────────────────────── -test("rejects a signature without the sha256= prefix", async () => { - // Bare hex, no "sha256=" prefix. - const bareHex = createHmac("sha256", SECRET).update(BODY).digest("hex"); - assert.equal(await verifyGitHubSignature(SECRET, BODY, bareHex), false); -}); - test("rejects an empty signature string", async () => { assert.equal(await verifyGitHubSignature(SECRET, BODY, ""), false); }); -test("rejects a sha256=-prefixed hex of the wrong length", async () => { - assert.equal(await verifyGitHubSignature(SECRET, BODY, "sha256=deadbeef"), false); +test("rejects a signature without the sha256= prefix", async () => { + // Same hex digest but missing the "sha256=" prefix. + const hexOnly = createHmac("sha256", SECRET).update(BODY).digest("hex"); + assert.equal(await verifyGitHubSignature(SECRET, BODY, hexOnly), false); }); -test("rejects a sha256=-prefixed non-hex string of the right length", async () => { - assert.equal( - await verifyGitHubSignature(SECRET, BODY, "sha256=" + "z".repeat(64)), - false, - ); +test("rejects a sha256=-prefixed hex of the wrong length", async () => { + // Truncated digest (valid prefix, too few hex chars). + const shortHex = createHmac("sha256", SECRET).update(BODY).digest("hex").slice(0, 32); + assert.equal(await verifyGitHubSignature(SECRET, BODY, "sha256=" + shortHex), false); });