How to use Buffer.from() with crypto.timingSafeEqual()?

user_256439 0
Viewed 1313

For some reason I get

TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received undefined

from both arguments to crypto.timingSafeEqual(a, b).

I have also tried

const a = Buffer.from(signature, 'utf8').toString('base64');
const b = Buffer.from(expectedSignature, 'utf8').toString('base64');

and I get the same error.

Question

Can anyone figure out why the arguments aren't Buffers?

const express = require("express");
const bodyParser = require("body-parser");
const crypto = require('crypto');
const secret = "x";

const app = express();
const PORT = 8080;

app.use(bodyParser.json());

function isSigOk(request, secret) {
    // calculate the signature
    const expectedSignature = "sha256=" +
        crypto.createHmac("sha256", secret)
            .update(JSON.stringify(request.body))
            .digest("hex");

    // compare the signature against the one in the request
    const signature = request.headers["X-Hub-Signature-256"];
    const a = Buffer.from(signature);
    const b = Buffer.from(expectedSignature);
    return crypto.timingSafeEqual(a, b);
};

app.post("/", (req, res) => {
  if (isSigOk(req, secret)) {
    // Do stuff here
  } else {
    console.log('Error: Signatures does not match. Return res.status(401)');
  };
  res.status(200).end();
});

// Start express on the defined port
app.listen(PORT, () => console.log(`Github wekhook listening on port ${PORT}`));
node.js ecmascript-6 javascript
1 Answers

I see two issues:

  1. The first and main one is that isSigOk assumes there will be a value for the "X-Hub-Signature-256" header:

    const signature = request.headers["X-Hub-Signature-256"];
const a = Buffer.from(signature);

    That Buffer.from call will throw the error you've quoted if signature is undefined because the header isn't there. You probably want to return false in that case (and probably skip the overhead of working out the expected signature by reordering things a bit), see *** comments and associated lines:

    function isSigOk(request, secret) {
    // *** get the signature on this message, if any
    const signature = request.headers["X-Hub-Signature-256"];
    if (!signature) {
        // *** none
        return false;
    }
    // calculate the signature
    const expectedSignature = "sha256=" +
        crypto.createHmac("sha256", secret)
            .update(JSON.stringify(request.body))
            .digest("hex");

    // compare the signature against the one in the request
    const a = Buffer.from(signature);
    const b = Buffer.from(expectedSignature);
    return crypto.timingSafeEqual(a, b);
};

  2. Capitalization matters. According to the Node.js documentation (Express's Requset object inherits from Node.js's IncomingMessage), header names in headers are lowercased. So request.headers["X-Hub-Signature-256"] should be request.headers["x-hub-signature-256"]. (In a comment you've saying you were getting a value, but the comment used all lowercase, whereas the code uses mixed case.) So:

    function isSigOk(request, secret) {
    // *** get the signature on this message, if any
    const signature = request.headers["x-hub-signature-256"]; // *** Lowercase
    if (!signature) {
        // *** none
        return false;
    }
    // calculate the signature
    const expectedSignature = "sha256=" +
        crypto.createHmac("sha256", secret)
            .update(JSON.stringify(request.body))
            .digest("hex");

    // compare the signature against the one in the request
    const a = Buffer.from(signature);
    const b = Buffer.from(expectedSignature);
    return a.length === b.length && crypto.timingSafeEqual(a, b);
};

    Note the a.length === b.length && part of that. timingSafeEqual will throw an error if the buffers aren't the same length, but we wan to return false in that situation instead.

user_1572470
Related