Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
const crypto = require('crypto');
const hmac = crypto.createHmac('SHA256', 'my-webhook-secret');
hmac.update('{ ... }'); // request body
const correctHash = hmac.digest().toString('hex');
const receivedHash = '...'; // e.g. req.get('x-impala-signature');
/*
* It's important to perform a constant time equality comparison of the
* two HMACs to avoid timing attacks.
*
* See: https://en.wikipedia.org/wiki/Timing_attack
*/
if (
crypto.timingSafeEqual(
Buffer.from(correctHash),
Buffer.from(receivedHash)
)
) {
// Request is valid
} else {
throw new Error('Authentication failed.');
}
@adrianvellamlt

This comment has been minimized.

Copy link

adrianvellamlt commented Jun 5, 2019

I think that comparing the lengths of the hashes defeats the purpose of comparing with timingSafeEqual. There is a very good answer on StackOverflow that explains just this https://stackoverflow.com/a/16953301/4305074. What's your opinion on this?

@tobyurff

This comment has been minimized.

Copy link
Owner Author

tobyurff commented Jun 5, 2019

@adrianvellamlt Thanks for the feedback and link, Adrian. The StackOverflow link you sent is correct although applies to a slightly different situation to ours. Whilst the examples there deal with different hash lengths we use SHA256 to hash the message body and as a result the length of the hash will be constant. The only information an attacker could determine from timing the length check would be the length of correctHash which is always the same and is publicly available in our documentation. For this reason doing the check wouldn’t compromise security in any way.

I’ve removed the length check from the example as it wasn’t necessary and made the code less clear. The purpose it served was to speed up the process of rejecting inputs known to be incorrect and thus reduce the chances of a DoS attack. Having given it some more though it also wouldn’t massively reduce the DoS attack vector (assuming a relatively sophisticated attack) as an attacker could easily determine the correct length of the hash to trigger the constant time equality check.

@PeterKottas

This comment has been minimized.

Copy link

PeterKottas commented Aug 29, 2019

Hi @tobyurff, do you have any examples for dot net (core) by any chance?

@tobyurff

This comment has been minimized.

Copy link
Owner Author

tobyurff commented Aug 29, 2019

Hi @PeterKottas! No, unfortunately we don't have anything in .NET. Hope you'll find the equivalent on the above example in .NET! Let us know if there's anything else we can help with, ideally on support@getimpala.com as that's monitored more regularly.

@PeterKottas

This comment has been minimized.

Copy link

PeterKottas commented Aug 29, 2019

No problem, we figured it out in the meantime.

@PeterKottas

This comment has been minimized.

Copy link

PeterKottas commented Aug 29, 2019

In case anybody is looking for implementation: https://gist.github.com/PeterKottas/d83906865a42f521586523fd54e7a6dc

@tobyurff

This comment has been minimized.

Copy link
Owner Author

tobyurff commented Aug 29, 2019

Great, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.