diff --git a/README.md b/README.md index 641c602..0f5f8bd 100644 --- a/README.md +++ b/README.md @@ -71,9 +71,6 @@ const { await sign("mysecret", eventPayloadString); // resolves with a string like "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3" -await sign({ secret: "mysecret", algorithm: "sha1" }, eventPayloadString); -// resolves with a string like "sha1=d03207e4b030cf234e3447bac4d93add4c6643d8" - await verify("mysecret", eventPayloadString, "sha256=486d27..."); // resolves with true or false @@ -87,7 +84,6 @@ await verifyWithFallback("mysecret", eventPayloadString, "sha256=486d27...", ["o ```js await sign(secret, eventPayloadString); -await sign({ secret, algorithm }, eventPayloadString); ``` @@ -103,23 +99,6 @@ await sign({ secret, algorithm }, eventPayloadString); Secret as configured in GitHub Settings. - - - -
- - algorithm - - - (String) - - - -Algorithm to calculate signature. Can be set to `sha1` or `sha256`. `sha1` is supported for legacy reasons. GitHub Enterprise Server 2.22 and older do not send the `X-Hub-Signature-256` header. Defaults to `sha256`. - -Learn more at [Validating payloads from GitHub](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks#validating-payloads-from-github) - -
diff --git a/src/index.ts b/src/index.ts index 5756ce1..d405d20 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,14 +1,23 @@ -export { sign } from "./node/sign.js"; -import { verify } from "./node/verify.js"; -export { verify }; +export { sign, signSync } from "./node/sign.js"; +import { verifySync } from "./node/verify.js"; +export { verify, verifySync } from "./node/verify.js"; export async function verifyWithFallback( secret: string, payload: string, signature: string, additionalSecrets: undefined | string[], -): Promise { - const firstPass = await verify(secret, payload, signature); +): Promise { + return verifyWithFallbackSync(secret, payload, signature, additionalSecrets); +} + +export function verifyWithFallbackSync( + secret: string, + payload: string, + signature: string, + additionalSecrets: undefined | string[], +): boolean { + const firstPass = verifySync(secret, payload, signature); if (firstPass) { return true; @@ -16,7 +25,7 @@ export async function verifyWithFallback( if (additionalSecrets !== undefined) { for (const s of additionalSecrets) { - const v: boolean = await verify(s, payload, signature); + const v: boolean = verifySync(s, payload, signature); if (v) { return v; } diff --git a/src/node/sign.ts b/src/node/sign.ts index 254ddb9..dd5fa11 100644 --- a/src/node/sign.ts +++ b/src/node/sign.ts @@ -1,34 +1,25 @@ import { createHmac } from "node:crypto"; -import { Algorithm, type SignOptions } from "../types.js"; import { VERSION } from "../version.js"; export async function sign( - options: SignOptions | string, - payload: string, + secret: string | Buffer, + payload: string | Buffer, ): Promise { - const { secret, algorithm } = - typeof options === "object" - ? { - secret: options.secret, - algorithm: options.algorithm || Algorithm.SHA256, - } - : { secret: options, algorithm: Algorithm.SHA256 }; + return signSync(secret, payload); +} +export function signSync( + secret: string | Buffer, + payload: string | Buffer, +): string { if (!secret || !payload) { throw new TypeError( "[@octokit/webhooks-methods] secret & payload required for sign()", ); } - if (!Object.values(Algorithm).includes(algorithm as Algorithm)) { - throw new TypeError( - `[@octokit/webhooks] Algorithm ${algorithm} is not supported. Must be 'sha1' or 'sha256'`, - ); - } - - return `${algorithm}=${createHmac(algorithm, secret) - .update(payload) - .digest("hex")}`; + return `sha256=${createHmac("sha256", secret).update(payload).digest("hex")}`; } sign.VERSION = VERSION; +signSync.VERSION = VERSION; diff --git a/src/node/verify.ts b/src/node/verify.ts index 5d89d66..37d8cf4 100644 --- a/src/node/verify.ts +++ b/src/node/verify.ts @@ -1,32 +1,41 @@ -import { timingSafeEqual } from "node:crypto"; +import { createHmac, timingSafeEqual } from "node:crypto"; import { Buffer } from "node:buffer"; -import { sign } from "./sign.js"; import { VERSION } from "../version.js"; -import { getAlgorithm } from "../utils.js"; +import { isValidSignaturePrefix } from "../utils.js"; export async function verify( - secret: string, - eventPayload: string, + secret: string | Buffer, + eventPayload: string | Buffer, signature: string, ): Promise { + return verifySync(secret, eventPayload, signature); +} + +export function verifySync( + secret: string | Buffer, + eventPayload: string | Buffer, + signature: string, +): boolean { if (!secret || !eventPayload || !signature) { throw new TypeError( "[@octokit/webhooks-methods] secret, eventPayload & signature required", ); } - const signatureBuffer = Buffer.from(signature); - const algorithm = getAlgorithm(signature); - - const verificationBuffer = Buffer.from( - await sign({ secret, algorithm }, eventPayload), - ); + if (isValidSignaturePrefix(signature) === false) { + return false; + } + const signatureBuffer = Buffer.from(signature.slice(7), "hex"); - if (signatureBuffer.length !== verificationBuffer.length) { + if (signatureBuffer.length !== 32) { return false; } + const verificationBuffer = createHmac("sha256", secret) + .update(eventPayload) + .digest().buffer as Buffer; + // constant time comparison to prevent timing attacks // https://stackoverflow.com/a/31096242/206879 // https://en.wikipedia.org/wiki/Timing_attack @@ -34,3 +43,4 @@ export async function verify( } verify.VERSION = VERSION; +verifySync.VERSION = VERSION; diff --git a/src/types.ts b/src/types.ts index ce72b46..e45c341 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,11 +1,3 @@ -export enum Algorithm { - SHA1 = "sha1", - SHA256 = "sha256", -} - -export type AlgorithmLike = Algorithm | "sha1" | "sha256"; - export type SignOptions = { secret: string; - algorithm?: AlgorithmLike; }; diff --git a/src/utils.ts b/src/utils.ts index d1e6444..b5e58b0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,3 +1,12 @@ -export const getAlgorithm = (signature: string) => { - return signature.startsWith("sha256=") ? "sha256" : "sha1"; +export const isValidSignaturePrefix = (signature: string) => { + return ( + signature.length === 71 && + signature[0] === "s" && + signature[1] === "h" && + signature[2] === "a" && + signature[3] === "2" && + signature[4] === "5" && + signature[5] === "6" && + signature[6] === "=" + ); }; diff --git a/src/web.ts b/src/web.ts index 403aa69..42923b5 100644 --- a/src/web.ts +++ b/src/web.ts @@ -1,6 +1,3 @@ -import { Algorithm, type AlgorithmLike, type SignOptions } from "./types.js"; -import { getAlgorithm } from "./utils.js"; - const enc = new TextEncoder(); function hexToUInt8Array(string: string) { @@ -21,16 +18,7 @@ function UInt8ArrayToHex(signature: ArrayBuffer) { .join(""); } -function getHMACHashName(algorithm: AlgorithmLike) { - return ( - { - [Algorithm.SHA1]: "SHA-1", - [Algorithm.SHA256]: "SHA-256", - } as { [key in Algorithm]: string } - )[algorithm]; -} - -async function importKey(secret: string, algorithm: AlgorithmLike) { +async function importKey(secret: string) { // ref: https://developer.mozilla.org/en-US/docs/Web/API/HmacImportParams return crypto.subtle.importKey( "raw", // raw format of the key - should be Uint8Array @@ -38,41 +26,27 @@ async function importKey(secret: string, algorithm: AlgorithmLike) { { // algorithm details name: "HMAC", - hash: { name: getHMACHashName(algorithm) }, + hash: { name: "SHA-256" }, }, false, // export = false ["sign", "verify"], // what this key can do ); } -export async function sign(options: SignOptions | string, payload: string) { - const { secret, algorithm } = - typeof options === "object" - ? { - secret: options.secret, - algorithm: options.algorithm || Algorithm.SHA256, - } - : { secret: options, algorithm: Algorithm.SHA256 }; - +export async function sign(secret: string, payload: string) { if (!secret || !payload) { throw new TypeError( "[@octokit/webhooks-methods] secret & payload required for sign()", ); } - if (!Object.values(Algorithm).includes(algorithm as Algorithm)) { - throw new TypeError( - `[@octokit/webhooks] Algorithm ${algorithm} is not supported. Must be 'sha1' or 'sha256'`, - ); - } - const signature = await crypto.subtle.sign( "HMAC", - await importKey(secret, algorithm), + await importKey(secret), enc.encode(payload), ); - return `${algorithm}=${UInt8ArrayToHex(signature)}`; + return `sha256=${UInt8ArrayToHex(signature)}`; } export async function verify( @@ -86,11 +60,10 @@ export async function verify( ); } - const algorithm = getAlgorithm(signature); return await crypto.subtle.verify( "HMAC", - await importKey(secret, algorithm), - hexToUInt8Array(signature.replace(`${algorithm}=`, "")), + await importKey(secret), + hexToUInt8Array(signature.replace(`sha256=`, "")), enc.encode(eventPayload), ); } diff --git a/test/sign.test.ts b/test/sign.test.ts index 94432f3..c0804b4 100644 --- a/test/sign.test.ts +++ b/test/sign.test.ts @@ -1,4 +1,4 @@ -import { sign } from "../src/index.ts"; +import { sign, signSync } from "../src/index.ts"; const eventPayload = { foo: "bar", @@ -35,45 +35,51 @@ describe("sign", () => { ); }); - test("sign({secret, algorithm}) throws with invalid algorithm", async () => { - await expect(() => - // @ts-expect-error - sign({ secret, algorithm: "sha2" }, eventPayload), - ).rejects.toThrow( - "[@octokit/webhooks] Algorithm sha2 is not supported. Must be 'sha1' or 'sha256'", - ); - }); - - describe("with eventPayload as string", () => { - describe("returns expected sha1 signature", () => { + describe("with secret as Buffer", () => { + describe("returns expected sha256 signature", () => { test("sign(secret, eventPayload)", async () => { - const signature = await sign(secret, JSON.stringify(eventPayload)); + const signature = await sign( + Buffer.from(secret), + JSON.stringify(eventPayload), + ); expect(signature).toBe( "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3", ); }); + }); + }); - test("sign({secret}, eventPayload)", async () => { - const signature = await sign({ secret }, JSON.stringify(eventPayload)); + describe("with eventPayload as string", () => { + describe("returns expected sha256 signature", () => { + test("sign(secret, eventPayload)", async () => { + const signature = await sign(secret, JSON.stringify(eventPayload)); expect(signature).toBe( "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3", ); }); + }); + }); - test("sign({secret, algorithm: 'sha1'}, eventPayload)", async () => { + describe("with eventPayload as Buffer", () => { + describe("returns expected sha256 signature", () => { + test("sign(secret, eventPayload)", async () => { const signature = await sign( - { secret, algorithm: "sha1" }, - JSON.stringify(eventPayload), + secret, + Buffer.from(JSON.stringify(eventPayload)), + ); + expect(signature).toBe( + "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3", ); - expect(signature).toBe("sha1=d03207e4b030cf234e3447bac4d93add4c6643d8"); }); }); + }); + describe("with eventPayload and secret as Buffer", () => { describe("returns expected sha256 signature", () => { - test("sign({secret, algorithm: 'sha256'}, eventPayload)", async () => { + test("sign(secret, eventPayload)", async () => { const signature = await sign( - { secret, algorithm: "sha256" }, - JSON.stringify(eventPayload), + Buffer.from(secret), + Buffer.from(JSON.stringify(eventPayload)), ); expect(signature).toBe( "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3", @@ -82,3 +88,20 @@ describe("sign", () => { }); }); }); + +describe("signSync", () => { + it("is a function", () => { + expect(signSync).toBeInstanceOf(Function); + }); + + it("signSync.VERSION is set", () => { + expect(signSync.VERSION).toEqual("0.0.0-development"); + }); + + test("signSync(secret, eventPayload)", () => { + const signature = signSync(secret, JSON.stringify(eventPayload)); + expect(signature).toBe( + "sha256=4864d2759938a15468b5df9ade20bf161da9b4f737ea61794142f3484236bda3", + ); + }); +}); diff --git a/test/verify.test.ts b/test/verify.test.ts index fcdf480..df2bab4 100644 --- a/test/verify.test.ts +++ b/test/verify.test.ts @@ -1,4 +1,4 @@ -import { verify, verifyWithFallback } from "../src/index.ts"; +import { verify, verifySync, verifyWithFallback } from "../src/index.ts"; function toNormalizedJsonString(payload: object) { // GitHub sends its JSON with an indentation of 2 spaces and a line break at the end @@ -10,8 +10,7 @@ function toNormalizedJsonString(payload: object) { const eventPayload = toNormalizedJsonString({ foo: "bar" }); const secret = "mysecret"; -const signatureSHA1 = "sha1=640c0ea7402a3f74e1767338fa2dba243b1f2d9c"; -const signatureSHA256 = +const signature = "sha256=e3eccac34c43c7dc1cbb905488b1b81347fcc700a7b025697a9d07862256023f"; describe("verify", () => { @@ -51,69 +50,58 @@ describe("verify", () => { ); }); - test("verify(secret, eventPayload, signatureSHA1) returns true for correct signature", async () => { - const signatureMatches = await verify(secret, eventPayload, signatureSHA1); + test("verify(secret, eventPayload, signature) returns true for correct signature", async () => { + const signatureMatches = await verify(secret, eventPayload, signature); expect(signatureMatches).toBe(true); }); - test("verify(secret, eventPayload, signatureSHA1) returns false for incorrect signature", async () => { - const signatureMatches = await verify(secret, eventPayload, "foo"); - expect(signatureMatches).toBe(false); + test("verify(secret, eventPayload, signature) returns true for secret provided as Buffer", async () => { + const signatureMatches = await verify( + Buffer.from(secret), + eventPayload, + signature, + ); + expect(signatureMatches).toBe(true); }); - test("verify(secret, eventPayload, signatureSHA1) returns false for correct secret", async () => { - const signatureMatches = await verify("foo", eventPayload, signatureSHA1); - expect(signatureMatches).toBe(false); + test("verify(secret, eventPayload, signature) returns true for payload provided as Buffer", async () => { + const signatureMatches = await verify( + secret, + Buffer.from(eventPayload), + signature, + ); + expect(signatureMatches).toBe(true); }); - test("verify(secret, eventPayload, signatureSHA1) returns true if eventPayload contains special characters (#71)", async () => { - // https://github.com/octokit/webhooks.js/issues/71 - const signatureMatchesLowerCaseSequence = await verify( - "development", - toNormalizedJsonString({ - foo: "Foo\n\u001b[34mbar: ♥♥♥♥♥♥♥♥\nthis-is-lost\u001b[0m\u001b[2K", - }), - "sha1=82a91c5aacc9cdc2eea893bc828bd03d218df79c", - ); - expect(signatureMatchesLowerCaseSequence).toBe(true); - const signatureMatchesUpperCaseSequence = await verify( - "development", - toNormalizedJsonString({ - foo: "Foo\n\u001B[34mbar: ♥♥♥♥♥♥♥♥\nthis-is-lost\u001B[0m\u001B[2K", - }), - "sha1=82a91c5aacc9cdc2eea893bc828bd03d218df79c", - ); - expect(signatureMatchesUpperCaseSequence).toBe(true); - const signatureMatchesEscapedSequence = await verify( - "development", - toNormalizedJsonString({ - foo: "\\u001b", - }), - "sha1=bdae4705bdd827d026bb227817ca025b5b3a6756", + test("verify(secret, eventPayload, signature) returns true for payload and secret provided as Buffer", async () => { + const signatureMatches = await verify( + Buffer.from(secret), + Buffer.from(eventPayload), + signature, ); - expect(signatureMatchesEscapedSequence).toBe(true); + expect(signatureMatches).toBe(true); }); - test("verify(secret, eventPayload, signatureSHA256) returns true for correct signature", async () => { + test("verify(secret, eventPayload, signature) returns false for incorrect signature", async () => { const signatureMatches = await verify( secret, eventPayload, - signatureSHA256, + "sha256=xxxccac34c43c7dc1cbb905488b1b81347fcc700a7b025697a9d07862256023f", ); - expect(signatureMatches).toBe(true); + expect(signatureMatches).toBe(false); }); - test("verify(secret, eventPayload, signatureSHA256) returns false for incorrect signature", async () => { + test("verify(secret, eventPayload, signature) returns false for incorrect signature", async () => { const signatureMatches = await verify(secret, eventPayload, "foo"); expect(signatureMatches).toBe(false); }); - test("verify(secret, eventPayload, signatureSHA256) returns false for incorrect secret", async () => { - const signatureMatches = await verify("foo", eventPayload, signatureSHA256); + test("verify(secret, eventPayload, signature) returns false for incorrect secret", async () => { + const signatureMatches = await verify("foo", eventPayload, signature); expect(signatureMatches).toBe(false); }); - test("verify(secret, eventPayload, signatureSHA256) returns true if eventPayload contains special characters (#71)", async () => { + test("verify(secret, eventPayload, signature) returns true if eventPayload contains special characters (#71)", async () => { // https://github.com/octokit/webhooks.js/issues/71 const signatureMatchesLowerCaseSequence = await verify( "development", @@ -147,33 +135,48 @@ describe("verifyWithFallback", () => { expect(verifyWithFallback).toBeInstanceOf(Function); }); - test("verifyWithFallback(secret, eventPayload, signatureSHA256, [bogus]) returns true", async () => { + test("verifyWithFallback(secret, eventPayload, signature, [bogus]) returns true", async () => { const signatureMatches = await verifyWithFallback( secret, eventPayload, - signatureSHA256, + signature, ["foo"], ); expect(signatureMatches).toBe(true); }); - test("verifyWithFallback(bogus, eventPayload, signatureSHA256, [secret]) returns true", async () => { + test("verifyWithFallback(bogus, eventPayload, signature, [secret]) returns true", async () => { const signatureMatches = await verifyWithFallback( "foo", eventPayload, - signatureSHA256, + signature, [secret], ); expect(signatureMatches).toBe(true); }); - test("verify(bogus, eventPayload, signatureSHA256, [bogus]) returns false", async () => { + test("verify(bogus, eventPayload, signature, [bogus]) returns false", async () => { const signatureMatches = await verifyWithFallback( "foo", eventPayload, - signatureSHA256, + signature, ["foo"], ); expect(signatureMatches).toBe(false); }); }); + +describe("verifySync", () => { + it("is a function", () => { + expect(verifySync).toBeInstanceOf(Function); + }); + + it("verifySync.VERSION is set", () => { + expect(verifySync.VERSION).toEqual("0.0.0-development"); + }); + + test("verifySync(secret, eventPayload, signature) returns true for correct signature", () => { + const signatureMatches = verifySync(secret, eventPayload, signature); + expect(signatureMatches).toBe(true); + }); +});