GitHub Webhook Secret Never Validates

Question

I'm using a GitHub webhook to pipe events to an application of mine (an instance of GitHub's Hubot) and it is secured with an sha1 secret.

I'm using the following code to validate hashes on incoming webhooks

crypto    = require('crypto')
signature = "sha1=" + crypto.createHmac('sha1', process.env.HUBOT_GITHUB_SECRET).update( new Buffer request.body ).digest('hex')
unless request.headers['x-hub-signature'] is signature
  response.send "Signature not valid"
  return

The X-Hub-Signature header passed through in the webhook looks like this

X-Hub-Signature: sha1=1cffc5d4c77a3f696ecd9c19dbc2575d22ffebd4

I am passing in the key and data accurately as per GitHub's documentation, but the hash always ends up different.

Here is GitHub's documentation. https://developer.github.com/v3/repos/hooks/#example

and this is the section which I am most likely misinterpreting

secret: An optional string that’s passed with the HTTP requests as an X-Hub-Signature header. The value of this header is computed as the HMAC hex digest of the body, using the secret as the key.

Can anyone see where I'm going wrong?

Answer 1

Seems to not work with a Buffer, but JSON.stringify(); Here's my working code:

var
  hmac,
  calculatedSignature,
  payload = req.body;

hmac = crypto.createHmac('sha1', config.github.secret);
hmac.update(JSON.stringify(payload));
calculatedSignature = 'sha1=' + hmac.digest('hex');

if (req.headers['x-hub-signature'] === calculatedSignature) {
  console.log('all good');
} else {
  console.log('not good');
}

Answer 2

Adding to Patrick's answer. It's good to use crypto.timingSafeEqual for comparing HMAC digests or secret values. Here's how:

const blob = JSON.stringify(req.body);  
const hmac = crypto.createHmac('sha1', process.env.GITHUB_WEBHOOK_SECRET);
const ourSignature = `sha1=${hmac.update(blob).digest('hex')}`;

const theirSignature = req.get('X-Hub-Signature');

const bufferA = Buffer.from(ourSignature, 'utf8');
const bufferB = Buffer.from(theirSignature, 'utf8');

const safe = crypto.timingSafeEqual(bufferA, bufferB);

if (safe) {
  console.log('Valid signature');
} else {
  console.log('Invalid signature');
}

To know more about the difference between a secure compare like timingEqual and a simple === check this thread here.

crypto.timingSafeEqual was added in Node.js v6.6.0