diff --git a/src/webhook.test.ts b/src/webhook.test.ts index 9ab478a..57680e2 100644 --- a/src/webhook.test.ts +++ b/src/webhook.test.ts @@ -27,11 +27,12 @@ describe("webhook: verifyGitHubSignature (HMAC-SHA256, X-Hub-Signature-256)", () const bareHex = VALID.replace("sha256=", ""); expect(await verifyGitHubSignature(SECRET, BODY, bareHex)).toBe(false); // missing "sha256=" prefix expect(await verifyGitHubSignature(SECRET, BODY, "")).toBe(false); - expect(await verifyGitHubSignature(SECRET, BODY, "sha256=deadbeef")).toBe(false); + expect(await verifyGitHubSignature(SECRET, BODY, "sha256=deadbeef")).toBe(false); // wrong length + expect(await verifyGitHubSignature(SECRET, BODY, "sha256=" + "z".repeat(64))).toBe(false); // non-hex }); - // NOTE: verifyGitHubSignature compares with `===` (not constant-time) — tracked as - // #167. These tests pin the functional contract (accept valid / reject invalid), - // which holds regardless of the timing-safety fix; the timing property itself is - // not unit-observable. + // verifyGitHubSignature now validates via crypto.subtle.verify (constant-time), + // resolving #167. The timing property itself is not unit-observable; these tests + // pin the functional contract (accept valid / reject invalid / reject malformed), + // which the constant-time implementation preserves. }); diff --git a/src/webhook.ts b/src/webhook.ts index 4ee57f9..0d8ac80 100644 --- a/src/webhook.ts +++ b/src/webhook.ts @@ -34,29 +34,40 @@ import { deleteFtsRow } from "./fts.js"; /** * Verify GitHub webhook signature using WebCrypto HMAC-SHA256. * - * Compares the X-Hub-Signature-256 header value against the - * computed HMAC of the request body. + * Validates the X-Hub-Signature-256 header against the body's HMAC via + * crypto.subtle.verify, which compares in constant time. The previous approach + * recomputed the digest and string-compared it with `===`, which early-exits on + * the first mismatched character and leaks the expected signature through response + * timing (#167). Behavior is unchanged: a correct signature returns true, anything + * else returns false. */ export async function verifyGitHubSignature( secret: string, 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, - ["sign"], + ["verify"], ); - 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; + // verify() recomputes the HMAC and compares it in constant time. + return crypto.subtle.verify("HMAC", key, sigBytes, encoder.encode(body)); } /**