Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an example implementation for verifying webhook signatures #8

Open
mphasize opened this issue Aug 31, 2021 · 7 comments
Open
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mphasize
Copy link

mphasize commented Aug 31, 2021

The documentation ( https://developer.8x8.com/jaas/docs/webhooks-signatures ) describes how we can check the signature from a webhook call, but a real code example would help with some of the questions we might have after reading the documentation.

For example:

"The X-Jaas-Signature header included in each signed event contains a timestamp and one or more signatures."

Why one or more signatures? Does another signature always mean another version/protocol or could the message body somehow be broken in several parts with separate signatures?

Split the header, using the comma “,” symbol as the separator, to get a list of elements. Then split each element, using the equals sign “=” as the separator, to get a prefix and value pair.

As the example value contains an equal sign (=) as part of the signature value, I'm wondering how to avoid removing that by accident with a simple String.split...

The signed_payload string is created by concatenating:

  • the timestamp obtained from the header in the previous step (as a string).
  • the character .
  • the actual JSON payload (i.e., the request body).

Since this seems to refer to string concatenation, I'm wondering which specific options to use with JSON.stringify on the request body. Could I break the signature validation by using the wrong stringify settings?

To protect against timing attacks, use a constant-time string comparison to compare the expected signature to each of the received signatures.

Ummm.. what is constant-time string comparison and how do I do that? 😅

Anyway, a bit of sample code would be really fantastic. Thank you.

@mphasize
Copy link
Author

mphasize commented Sep 2, 2021

Based on the documentation provided, I came up with this implementation in Node.js:

exports.validateJaasSignature = (req, secret) => {
  const header = req.headers["x-jaas-signature"];

  const signatureElements = header.split(",");
  let timestamp = null;
  let signature = null;

  signatureElements.forEach((element) => {
    const key = element.substring(0, element.indexOf("="));
    const value = element.substring(element.indexOf("=") + 1); // we can't use split as the value can contain (=) characters
    switch (key) {
      case "t":
        timestamp = value;
        break;
      case "v1":
        signature = value;
        break;
    }
  });

  if (timestamp === null || signature === null) return false; // signature doesn't match expected format

  const hmac = crypto.createHmac("sha256", secret);
  const signedPayload = timestamp.concat(".", JSON.stringify(req.body));

  hmac.update(signedPayload);

  const generatedSignature = hmac.digest("base64");

  console.log("Compare JaaS signature:", req.headers["x-jaas-signature"], generatedSignature);
  // if comparison checks out
  return true;
};

Now, this generates the correct signature MOST of the time, but NOT ALL of the time. I tried to use req.rawBody.toString() (instead of JSON.stringify) for the signedPayload, but that produces exactly the same generated signature.

Any ideas what could cause the mismatch in the signature?

@iwaffles iwaffles added good first issue Good for newcomers help wanted Extra attention is needed labels Sep 14, 2021
@horymury
Copy link
Contributor

horymury commented Sep 22, 2021

@mphasize I made it work as expected by replacing hmac.update(signedPayload); with : hmac.update(new TextEncoder("utf-8").encode(signedPayload)); in your codebase.
Also, you need to send a 200 response after receiving the webhook, otherwise the webhook will be retried indefinitely (with the same idempotency key)

We will update the documentation to specify that the hmac message should be the utf-8 encoded byte array of the string.
Very sorry for the trouble this had caused.

@iwaffles
Copy link
Member

Thanks for taking a look at this, @horymury!

Also thank you for submitting your first PR and contribution, @mphasize awesome stuff! 🎉

@mphasize
Copy link
Author

@horymury Thank you for looking into it, I will try your solution.

@mphasize
Copy link
Author

@horymury so I tried to implement your solution like this:

  // Generate the signature of JSON body for comparison
  const hmac = crypto.createHmac("sha256", secret);
  const signedPayload = timestamp.concat(".", JSON.stringify(req.body));
  const signedPayloadUTF8 = new TextEncoder("utf-8").encode(signedPayload);
  hmac.update(signedPayloadUTF8);
  const generatedSignature = hmac.digest("base64");

  // Generate second signature of raw body for comparison
  const rawBodyString = req.rawBody.toString();
  const hmac2 = crypto.createHmac("sha256", secret);
  const signedPayload2 = timestamp.concat(".", rawBodyString);
  const signedPayload2UTF8 = new TextEncoder("utf-8").encode(signedPayload2);
  hmac2.update(signedPayload2UTF8);
  const generatedSignature2 = hmac2.digest("base64");

Both of these generated signatures are the same, but unfortunately they are sometimes (not always) still different from the signature I received in the request header.

I also tried a version with const rawBodyString = new TextEncoder("utf-8").encode(req.rawBody); but this always generates a wrong signature.

I'm also wondering what the TextEncoder would really do, as it is my current understanding that JSON.stringify would produce a UTF-8 encoded string (but maybe I'm mistaken here).

So in the end, I'm still at at a loss.

Also, you need to send a 200 response after receiving the webhook, otherwise the webhook will be retried indefinitely (with the same idempotency key)

Yes, the function that calls validateJaasSignature will then also return a status 200 via res.

@horymury
Copy link
Contributor

@mphasize utf-8 might be the default so it might not be needed explicitely on new TextEncoder instantiation, but .encode returns an Uint8Array, which is then passed as message on hmac update call, so it is not a converter to UTF-8 text.

I also tried a version with const rawBodyString = new TextEncoder("utf-8").encode(req.rawBody); but this always generates a wrong signature.

This is because req.rawBody is not a string, but a byte array I think. Also the encode needs to be done on the whole string which includes the timestamp.<payload_json_stringify>

During our testing we noticed that if a webhook is missed which results in re-sending it with the same idempotency key, the body slightly differs the second time but the signature from the header remains the same as the original, which causes the resulting signature to not correspond anymore with the computed signature on the webhook listening endpoint.
We have a fix for that but it was not yet deployed. Could this be the cause for signatures not always matching in your case?

CC @lstirb8x8

@mphasize
Copy link
Author

@horymury As far as I can tell, the signature mismatch already happens the first time that I see the request in our logs, but I will keep an eye out for this. Since our webhook handler is deployed as a Cloud Function on Firebase, I'm wondering if this might have something to do with a cold-start scenario. Maybe your first request get's a timeout and then already re-tries before I see it. I will go through the logs to see if this might be an explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants