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}`));
I see two issues:
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);
};
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.