36

I'm coding a Webhook for GitHub, and implemented secure verification in KOA.js as:

function sign(tok, blob) {
  var hmac;

  hmac = crypto
    .createHmac('sha1', tok)
    .update(blob)
    .digest('hex');

  return 'sha1=' + hmac;
}

...

key = this.request.headers['x-hub-signature'];
blob = JSON.stringify(this.request.body);

if (!key || !blob) {
  this.status = 400;
  this.body = 'Bad Request';
}

lock = sign(settings.api_secret, blob);

if (lock !== key) {
  console.log(symbols.warning, 'Unauthorized');
  this.status = 403;
  this.body = 'Unauthorized';
  return;
}

...

for pull_requests and create events this works ok, even pushing new branches works, but for push commits events the x-hub-signature and the computed hash from the payload don't match, so it always get 403 unauthorized.

Update

I've noticed that for this kind of push payloads the commits and head_commit are added to the payload. I've tried removing the commits and the head_commit from the body but it didn't work.

Update

For more information please review these example payloads. I've also included url for the test repo and token info: https://gist.github.com/marcoslhc/ec581f1a5ccdd80f8b33

dmulter
  • 2,608
  • 3
  • 15
  • 24
marcoslhc
  • 1,194
  • 1
  • 17
  • 29
  • What does "fails miserably" mean? What error(s) do you get? – ChrisGPT was on strike May 15 '15 at 22:46
  • The 'x-hub-signature' doesn't equals to the calculated hash. – marcoslhc May 16 '15 at 06:06
  • 2
    A total stab in the dark, but are you sure that `this.request.body` is not already a string? If it is, it will be double-encoded (e.g., the three-character string `foo` will be JSON encoded into the five-character string `"foo"`). See what `typeof this.request.body` produces. – apsillers May 20 '15 at 14:14
  • Are you able to provide a sample problematic event payload (from the events API), the value of `x-hub-signature`, your calculated HMAC, and the secret? Obviously this would want to be from a test repository. – javabrett May 22 '15 at 11:55
  • 1
    You can see example data along with token details here: https://gist.github.com/marcoslhc/ec581f1a5ccdd80f8b33 – marcoslhc May 22 '15 at 14:11

1 Answers1

11

The default encoding of Crypto hash.update() is binary as detailed in the answer to Node JS crypto, cannot create hmac on chars with accents. This causes a problem in your push-event payload, which contains the character U+00E1 LATIN SMALL LETTER A WITH ACUTE in Hernández four times, and GitHub services is hashing the payload as utf-8 encoded. Note that your Gist shows these incorrectly-encoded in ISO-8859-1, so also make sure that you are handling the incoming request character-encoding properly (but this should happen by-default).

To fix this you need to either use a Buffer:

hmac = crypto.createHmac('sha1', tok).update(new Buffer(blob, 'utf-8')).digest('hex');

... or pass the encoding directly to update:

hmac = crypto.createHmac('sha1', tok).update(blob, 'utf-8').digest('hex');

The correct hash of 7f9e6014b7bddf5533494eff6a2c71c4ec7c042d will then be calculated.

Community
  • 1
  • 1
javabrett
  • 7,020
  • 4
  • 51
  • 73