From bfe95b29eac33094891038bdc4864dc9f4275ba8 Mon Sep 17 00:00:00 2001 From: Noah Guillory Date: Wed, 1 May 2024 15:13:26 -0500 Subject: [PATCH 1/3] Add flag to `AwsLambdaReceiver` to enable/disable signature verification --- src/receivers/AwsLambdaReceiver.spec.ts | 36 +++++++++++++++++++++++++ src/receivers/AwsLambdaReceiver.ts | 23 ++++++++++------ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/receivers/AwsLambdaReceiver.spec.ts b/src/receivers/AwsLambdaReceiver.spec.ts index 813b32758..b6035147a 100644 --- a/src/receivers/AwsLambdaReceiver.spec.ts +++ b/src/receivers/AwsLambdaReceiver.spec.ts @@ -542,6 +542,42 @@ describe('AwsLambdaReceiver', function () { ); assert.equal(response.statusCode, 401); }); + + it('does not perform signature verification if signature verification flag is set to false', async () => { + const awsReceiver = new AwsLambdaReceiver({ + signingSecret: '', + signatureVerification: false, + logger: noopLogger, + }); + const handler = awsReceiver.toHandler(); + const awsEvent = { + resource: '/slack/events', + path: '/slack/events', + httpMethod: 'POST', + headers: { + Accept: 'application/json,*/*', + 'Content-Type': 'application/json', + Host: 'xxx.execute-api.ap-northeast-1.amazonaws.com', + 'User-Agent': 'Slackbot 1.0 (+https://api.slack.com/robots)', + 'X-Slack-Request-Timestamp': 'far back dude', + 'X-Slack-Signature': 'very much invalid', + }, + multiValueHeaders: {}, + queryStringParameters: null, + multiValueQueryStringParameters: null, + pathParameters: null, + stageVariables: null, + requestContext: {}, + body: urlVerificationBody, + isBase64Encoded: false, + }; + const response = await handler( + awsEvent, + {}, + (_error, _result) => {}, + ); + assert.equal(response.statusCode, 200); + }); }); // Composable overrides diff --git a/src/receivers/AwsLambdaReceiver.ts b/src/receivers/AwsLambdaReceiver.ts index 717201a34..6b6c0739c 100644 --- a/src/receivers/AwsLambdaReceiver.ts +++ b/src/receivers/AwsLambdaReceiver.ts @@ -40,9 +40,10 @@ export interface AwsResponse { export type AwsHandler = (event: AwsEvent, context: any, callback: AwsCallback) => Promise; export interface AwsLambdaReceiverOptions { - signingSecret: string; + signingSecret?: string; logger?: Logger; logLevel?: LogLevel; + signatureVerification?: boolean; customPropertiesExtractor?: (request: AwsEvent) => StringIndexed; } @@ -59,16 +60,20 @@ export default class AwsLambdaReceiver implements Receiver { private logger: Logger; + private signatureVerification: boolean; + private customPropertiesExtractor: (request: AwsEvent) => StringIndexed; public constructor({ - signingSecret, + signingSecret = '', logger = undefined, logLevel = LogLevel.INFO, + signatureVerification = true, customPropertiesExtractor = (_) => ({}), }: AwsLambdaReceiverOptions) { // Initialize instance variables, substituting defaults for each value this.signingSecret = signingSecret; + this.signatureVerification = signatureVerification; this.logger = logger ?? (() => { const defaultLogger = new ConsoleLogger(); @@ -130,12 +135,14 @@ export default class AwsLambdaReceiver implements Receiver { return Promise.resolve({ statusCode: 200, body: '' }); } - // request signature verification - const signature = this.getHeaderValue(awsEvent.headers, 'X-Slack-Signature') as string; - const ts = Number(this.getHeaderValue(awsEvent.headers, 'X-Slack-Request-Timestamp')); - if (!this.isValidRequestSignature(this.signingSecret, rawBody, signature, ts)) { - this.logger.info(`Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`); - return Promise.resolve({ statusCode: 401, body: '' }); + if (this.signatureVerification) { + // request signature verification + const signature = this.getHeaderValue(awsEvent.headers, 'X-Slack-Signature') as string; + const ts = Number(this.getHeaderValue(awsEvent.headers, 'X-Slack-Request-Timestamp')); + if (!this.isValidRequestSignature(this.signingSecret, rawBody, signature, ts)) { + this.logger.info(`Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`); + return Promise.resolve({ statusCode: 401, body: '' }); + } } // url_verification (Events API) From 3fd0b3dffef0d3d89c062d4a6d98728ba1bed754 Mon Sep 17 00:00:00 2001 From: Noah Guillory Date: Mon, 6 May 2024 20:31:37 +0000 Subject: [PATCH 2/3] Address PR comments --- src/receivers/AwsLambdaReceiver.ts | 32 ++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/receivers/AwsLambdaReceiver.ts b/src/receivers/AwsLambdaReceiver.ts index 6b6c0739c..799220e90 100644 --- a/src/receivers/AwsLambdaReceiver.ts +++ b/src/receivers/AwsLambdaReceiver.ts @@ -40,10 +40,38 @@ export interface AwsResponse { export type AwsHandler = (event: AwsEvent, context: any, callback: AwsCallback) => Promise; export interface AwsLambdaReceiverOptions { - signingSecret?: string; + /** + * The Slack Signing secret to be used as an input to signature verification to ensure that requests are coming from + * Slack. + * + * @see {@link https://api.slack.com/authentication/verifying-requests-from-slack#about} for details about signing secrets + */ + signingSecret: string; + /** + * The {@link Logger} for the receiver + * + * @default ConsoleLogger + */ logger?: Logger; + /** + * The {@link LogLevel} to be used for the logger. + * + * @default LogLevel.INFO + */ logLevel?: LogLevel; + /** + * Flag that determines whether Bolt should {@link https://api.slack.com/authentication/verifying-requests-from-slack|verify Slack's signature on incoming requests}. + * + * @default true + */ signatureVerification?: boolean; + /** + * Optional `function` that can extract custom properties from an incoming receiver event + * @param request The API Gateway event {@link AwsEvent} + * @returns An object containing custom properties + * + * @default noop + */ customPropertiesExtractor?: (request: AwsEvent) => StringIndexed; } @@ -65,7 +93,7 @@ export default class AwsLambdaReceiver implements Receiver { private customPropertiesExtractor: (request: AwsEvent) => StringIndexed; public constructor({ - signingSecret = '', + signingSecret, logger = undefined, logLevel = LogLevel.INFO, signatureVerification = true, From dd5bf1d029c818038a20796af9a24a4023b608e1 Mon Sep 17 00:00:00 2001 From: Noah Guillory Date: Mon, 6 May 2024 20:36:07 +0000 Subject: [PATCH 3/3] Add blurb to `signingSecret` JSDoc saying that the value of it doesn't matter if sig verification is disabled --- src/receivers/AwsLambdaReceiver.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/receivers/AwsLambdaReceiver.ts b/src/receivers/AwsLambdaReceiver.ts index 799220e90..5fe9a6d85 100644 --- a/src/receivers/AwsLambdaReceiver.ts +++ b/src/receivers/AwsLambdaReceiver.ts @@ -44,6 +44,9 @@ export interface AwsLambdaReceiverOptions { * The Slack Signing secret to be used as an input to signature verification to ensure that requests are coming from * Slack. * + * If the {@link signatureVerification} flag is set to `false`, this can be set to any value as signature verification + * using this secret will not be performed. + * * @see {@link https://api.slack.com/authentication/verifying-requests-from-slack#about} for details about signing secrets */ signingSecret: string;