octokit / webhooks.js

GitHub webhook events toolset for Node.js
MIT License
308 stars 81 forks source link

Project: Web platform compatibility (Browsers, Deno, Cloudflare Workers) #475

Closed gr2m closed 3 years ago

gr2m commented 3 years ago

@octokit/webhooks can already be used in browser's today, but the build size is huge, because Node's crypto API needs to be polyfilled. We need to create a universal library to sign a JSON payload, which uses the Web Crypto API for browser and Deno builds. This will make @octokit/webhooks and other libraries and framework that use it such as @octokit/app and probot work better with Cloudflare Workers. It is working today (example), but the build size could be reduced by 90%+. Another API change that would improve @octokit/webhooks's compatibility with browser/Deno/Cloudflare Workers is to remove the webhooks.middleware API, and replace it with a dedicated createNodeMiddleware export, similar to what we do with @octokit/app.

I created a similar package for the JSON Web Token authentication used by GitHub Apps: universal-github-app-jwt.

The challenge for this module is that different environment should receive different code. In the past, the "browse" package field was used by several build tools. But with the coming support of ES Modules by Node.js, we have a convention that the build tools are adapting now, which is to use the "exports" key, see https://docs.skypack.dev/package-authors/package-checks#export-map

It might be a good time to just create a native ES Module library without any build step at this point, so that we have full control over the package.json file published to npm. The @pika/pack build tool we are currently using in @octokit does not support the "exports" key at this point, and the module is currently not maintained by the Pika/Snowpack team

gr2m commented 3 years ago

for future reference, here is what we do today in node

const crypto = require("crypto");

function sign(secret, data) {
  return crypto.createHmac("sha256", secret).update(data).digest("hex");
}

and here is what we need to do in browsers, and soon Deno

// credit: https://stackoverflow.com/a/47332317/206879
var enc = new TextEncoder("utf-8");

async function sign(secret, data) {
  const key = await window.crypto.subtle.importKey(
    "raw", // raw format of the key - should be Uint8Array
    enc.encode(secret),
    {
      // algorithm details
      name: "HMAC",
      hash: { name: "SHA-256" },
    },
    false, // export = false
    ["sign", "verify"] // what this key can do
  );
  const signature = await window.crypto.subtle.sign(
    "HMAC",
    key,
    enc.encode(data)
  );
  var b = new Uint8Array(signature);
  return Array.prototype.map
    .call(b, (x) => ("00" + x.toString(16)).slice(-2))
    .join("");
}
gr2m commented 3 years ago

In node we do some fancy timesafe equal, I'm not sure if we need that in the browser, I don't fully understand what this is actually doing, so if someone knows better, please let me know if this naive version of signature string verification will suffice or not

const enc = new TextEncoder("utf-8");

async function importKey(secret) {
  return window.crypto.subtle.importKey(
    "raw", // raw format of the key - should be Uint8Array
    enc.encode(secret),
    {
      // algorithm details
      name: "HMAC",
      hash: { name: "SHA-256" },
    },
    false, // export = false
    ["sign", "verify"] // what this key can do
  );
}

function UInt8ArrayToHex(signature) {
  return Array.prototype.map
    .call(new Uint8Array(signature), (x) => x.toString(16).padStart(2, "0"))
    .join("");
}

async function sign(secret, data) {
  const signature = await window.crypto.subtle.sign(
    "HMAC",
    await importKey(secret),
    enc.encode(data)
  );
  return UInt8ArrayToHex(signature);
}

async function verify(secret, data, signature) {
  return signature === (await sign(secret, data));
}
gr2m commented 3 years ago

Looks like webcrypto is coming to Node, too: https://nodejs.org/api/webcrypto.html

wolfy1339 commented 3 years ago

In node we do some fancy timesafe equal, I'm not sure if we need that in the browser, I don't fully understand what this is actually doing

There is a great explanation here on StackOverflow of what a time safe equal accomplishes

gr2m commented 3 years ago

I guess this is a better way of verification? I dunno 🤷🏼

function hexToUInt8Array(string) {
  // convert string to pairs of 2 characters
  const pairs = string.match(/[\dA-F]{2}/gi);

  // convert the octets to integers
  const integers = pairs.map(function (s) {
    return parseInt(s, 16);
  });

  return new Uint8Array(integers);
}

async function verify(secret, data, signature) {
  return await window.crypto.subtle.verify(
    "HMAC",
    await importKey(secret),
    hexToUInt8Array(signature),
    enc.encode(data)
  );
}

In node we do some fancy timesafe equal, I'm not sure if we need that in the browser, I don't fully understand what this is actually doing

There is a great explanation here on StackOverflow of what a time safe equal accomplishes

this is great, I'll add a reference to the code

gr2m commented 3 years ago

I'll start by moving sign and verify into a separate module. I want to move forward with this because I'm experimenting with running webhooks in a browser, and the usage of crypto is currently breaking the import from Skypack

gr2m commented 3 years ago

https://github.com/octokit/webhooks-methods.js/#readme is ready. Not yet compatible with Deno, but that will be the fact as soon as Deno implements the crypto API

I'll use the package as part of https://github.com/octokit/webhooks.js/pull/518. sign and verify now need to be asynchronous which is a breaking change.

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 9.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: