From 61b22ec60df229b8cdc352b3ca3098f4ea3c4a73 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Sun, 19 May 2019 11:51:52 -0700 Subject: [PATCH] Fix #192 ExpressReceiver support for rawBody for signature verification --- src/ExpressReceiver.ts | 80 +++++++++++++-------------------- src/ExpressSignatureVerifier.ts | 66 +++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 48 deletions(-) create mode 100644 src/ExpressSignatureVerifier.ts diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 1667caaf3..0df752a73 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -4,10 +4,9 @@ import { createServer, Server } from 'http'; import express, { Request, Response, Application, RequestHandler, NextFunction } from 'express'; import axios from 'axios'; import rawBody from 'raw-body'; -import crypto from 'crypto'; -import tsscmp from 'tsscmp'; import querystring from 'querystring'; -import { ErrorCode, errorWithCode } from './errors'; +import { ErrorCode } from './errors'; +import ExpressSignatureVerifier from './ExpressSignatureVerifier'; // TODO: we throw away the key names for endpoints, so maybe we should use this interface. is it better for migrations? // if that's the reason, let's document that with a comment. @@ -40,7 +39,7 @@ export default class ExpressReceiver extends EventEmitter implements Receiver { this.server = createServer(this.app); const expressMiddleware: RequestHandler[] = [ - verifySlackRequest(signingSecret), + ExpressSignatureVerifier.create(signingSecret), parseBody, respondToSslCheck, respondToUrlVerification, @@ -151,54 +150,39 @@ const respondToUrlVerification: RequestHandler = (req, res, next) => { next(); }; -// TODO: this should be imported from another package -function verifySlackRequest(signingSecret: string): RequestHandler { - return async (req , _res, next) => { - try { - const body: string = (await rawBody(req)).toString(); - const signature = req.headers['x-slack-signature'] as string; - const ts = Number(req.headers['x-slack-request-timestamp']); - - // Divide current date to match Slack ts format - // Subtract 5 minutes from current time - const fiveMinutesAgo = Math.floor(Date.now() / 1000) - (60 * 5); - - if (ts < fiveMinutesAgo) { - const error = errorWithCode( - 'Slack request signing verification failed. Timestamp is too old.', - ErrorCode.ExpressReceiverAuthenticityError, - ); - next(error); - } - - const hmac = crypto.createHmac('sha256', signingSecret); - const [version, hash] = signature.split('='); - hmac.update(`${version}:${ts}:${body}`); - - if (!tsscmp(hash, hmac.digest('hex'))) { - const error = errorWithCode( - 'Slack request signing verification failed. Signature mismatch.', - ErrorCode.ExpressReceiverAuthenticityError, - ); - next(error); - } +/** + * Bolt's own Express body-parser. + * + * This handler parses `req.body` or `req.rawBody`(on Google Could Platform) + * and overwrites `req.body` with the parsed JS object. + * Following middlewares can expect `req.body` is no long a string value. + */ +const parseBody: RequestHandler = (req, _res, next) => { - // Verification passed, assign string body back to request and resume - req.body = body; - next(); - } catch (error) { - next(error); - } - }; -} + let stringBody: string; + // On some environments like GCP (Google Cloud Platform), + // req.body can be pre-parsed and be passed as req.rawBody here + const preparsedRawBody: any = (req as any).rawBody; + if (preparsedRawBody) { + stringBody = preparsedRawBody.toString(); + } else { + stringBody = rawBody(req).toString(); + } -const parseBody: RequestHandler = (req, _res, next) => { - if (req.headers['content-type'] === 'application/x-www-form-urlencoded') { - const parsedBody = querystring.parse(req.body); + const contentType = req.headers['content-type']; + if (contentType === 'application/x-www-form-urlencoded') { + const parsedBody = querystring.parse(stringBody); req.body = (typeof parsedBody.payload === 'string') ? JSON.parse(parsedBody.payload) : parsedBody; + } else if (contentType === 'application/json') { + req.body = JSON.parse(stringBody); } else { - // TODO: should we check the content type header to make sure its JSON here? - req.body = JSON.parse(req.body); + console.warn(`Unexpected content-type detected: ${contentType}`); + try { + // Parse this body anyway + req.body = JSON.parse(stringBody); + } catch (e) { + console.error(`Failed to parse body as JSON data for content-type: ${contentType}`) + } } next(); }; diff --git a/src/ExpressSignatureVerifier.ts b/src/ExpressSignatureVerifier.ts new file mode 100644 index 000000000..71860a80c --- /dev/null +++ b/src/ExpressSignatureVerifier.ts @@ -0,0 +1,66 @@ +import { RequestHandler } from 'express'; +import rawBody from 'raw-body'; +import crypto from 'crypto'; +import tsscmp from 'tsscmp'; +import { ErrorCode, errorWithCode } from './errors'; + +export default class ExpressSignatureVerifier { + + public static create(signingSecret: string): RequestHandler { + return async (req, _res, next) => { + try { + let stringBody: string; + // On some environments like GCP (Google Cloud Platform), + // req.body can be pre-parsed and be passed as req.rawBody here + const preparsedRawBody: any = (req as any).rawBody; + if (preparsedRawBody) { + stringBody = preparsedRawBody.toString(); + } else { + stringBody = (await rawBody(req)).toString(); + } + + const signature = req.headers['x-slack-signature'] as string; + const ts = Number(req.headers['x-slack-request-timestamp']); + if (!signature && !ts) { + const error = errorWithCode( + 'Slack request signing verification failed. Some headers are missing.', + ErrorCode.ExpressReceiverAuthenticityError, + ); + return next(error); + } + + // Divide current date to match Slack ts format + // Subtract 5 minutes from current time + const fiveMinutesAgo = Math.floor(Date.now() / 1000) - (60 * 5); + + if (ts < fiveMinutesAgo) { + const error = errorWithCode( + 'Slack request signing verification failed. Timestamp is too old.', + ErrorCode.ExpressReceiverAuthenticityError, + ); + return next(error); + } + + const hmac = crypto.createHmac('sha256', signingSecret); + const [version, hash] = signature.split('='); + hmac.update(`${version}:${ts}:${stringBody}`); + + if (!tsscmp(hash, hmac.digest('hex'))) { + const error = errorWithCode( + 'Slack request signing verification failed. Signature mismatch.', + ErrorCode.ExpressReceiverAuthenticityError, + ); + return next(error); + } + + // Verification passed, assign string body back to request and resume + req.body = stringBody; + + next(); + } catch (error) { + next(error); + } + }; + } + +} \ No newline at end of file