sendgrid/sendgrid-nodejs

Unable to verify headers using @sendgrid/eventwebhook lib

Closed this issue · 6 comments

Issue Summary

When attempting to use the @sendgrid/eventwebhook methods to verify Sendgrid signed event headers the method verifySignature() always returns false. Below code is written as an expressJS middleware function to run when the webhook sends a POST call to our endpoint.

Steps to Reproduce

  1. Set up a server to receive POST data from the Sendgrid webhooks and enable signed event headers
  2. Send a Webhook test event to your handler and note the false result

Expected response from method is true

Code Snippet

const { EventWebhook, EventWebhookHeader } = require('@sendgrid/eventwebhook');

const pubkey = 'string pubkey';

const verifySendgridWebhook = (req, res, next) => {
  console.log('Verifying webhook headers...')

  const webhook = new EventWebhook();
  const webhookHeaders = new EventWebhookHeader();

  const headers = req.headers;
  const headerSig = headers[EventWebhookHeader.SIGNATURE().toLowerCase()];
  const headerTimestamp = headers[EventWebhookHeader.TIMESTAMP().toLowerCase()];

  const buffer = Buffer.from(JSON.stringify(req.body));
  const ecpk = webhook.convertPublicKeyToECDSA(pubkey);
  const verified = webhook.verifySignature(ecpk, buffer.toString(), headerSig, headerTimestamp)

  if (verified) {
    console.log('Verified headers');
    next();
  } else {
    console.log('Invalid headers. 401 unauthorized.');
    res.status(401).send("Unauthorized");
  }
}

Technical details:

  • sendgrid-nodejs version: "@sendgrid/eventwebhook": "^7.4.5"
  • node version: v12.18.2 OR v16.13.1 (tried both)

I am having this same problem - I am on the same version of sendgrid-nodejs and I used the sample code from https://www.twilio.com/docs/runtime/quickstart/validate-webhook-requests-from-sendgrid

Has anyone found a fix for this? I'm also having the same issues.

Do the changes in the linked PR help to validate the payload?

From my experience, it is very important that the request body is not parsed as JSON or string, and is the raw Buffer!

This is the reason that the "hack" with JSON.stringify() and new line replacements from the linked pr seems to work, but it is not 100% correct!
The signature sent by the server is computed on the raw body. Any json parsing and stringify operations are not guaranteed to produce identical results!

The correct way to do it is to make sure that body-parser.json does not parse this webhook endpoint at all.

For example, this is the setup I have and is working 100% (I cleaned it up a bit to share it here).
Note: I use express-unless to make sure that body-parser.json does not process this endpoint, and I use body-parser.raw instead.

const unless = require("express-unless");
const bodyParser = require("body-parser");
const { EventWebhook, EventWebhookHeader } = require("@sendgrid/eventwebhook");

// Important: apply body-parser json for all endpoints EXCEPT the webhook!
const parse_json = bodyParser.json();
parse_json.unless = unless;
app.use(
  parse_json.unless({
    path: ["/api/webhooks/sendgrid"],
  })
);

app.post(
  "/api/webhooks/sendgrid",
  bodyParser.raw({ type: "application/json" }), // Important: parse body as raw Buffer
  (req, res) => {
    console.log("Is Buffer?", Buffer.isBuffer(req.body)); // must return true !

    const signature = req.get(EventWebhookHeader.SIGNATURE());
    const timestamp = req.get(EventWebhookHeader.TIMESTAMP());
    const eventWebhook = new EventWebhook();
    const key = eventWebhook.convertPublicKeyToECDSA(pubkey);

    if (eventWebhook.verifySignature(key, req.body, signature, timestamp)) {
      const events = JSON.parse(req.body);
      // TODO: handle events
      res.sendStatus(204);
    } else {
      res.status(401).json({ message: "Invalid signature" });
    }
  }
);

@danmana
Hi, just a question,
Is bodyParser.raw({ type: "application/json" }) the middleware supposed to override app.use(bodyParser.json())that was used before the endpoint? Seems like it does not override it from your solution.

And also do we have to use the body-parser package? It seems it is deprecated and is now included in express package.express.raw() could be used as well, I guess?
Could close the PR if you create a new one with your solution, but seems they are not really checking it.

@eroo36

Yes, we are in fact "overriding" the middleware for the webhook.

This is a global middleware that applies to all paths, except the webhook - see unless.
Most people use incorrectly app.use(bodyParser.json()) which applies it to all routes, and is impossible to override. you should use this instead:

app.use(
  parse_json.unless({
    path: ["/api/webhooks/sendgrid"],
  })
);

This is a middleware applied just to the webhook path (nothing to override as we excluded this path from the global middleware using unless).

app.post(
  "/api/webhooks/sendgrid",
  bodyParser.raw({ type: "application/json" }), // Important: parse body as raw Buffer

Yes you can replace bodyParser.json() with express.json() and bodyparser.raw() with express.raw().
In fact, that's what I'm using, I just figured most people were still using the old version (which is not wrong in any way)