From d4602fce8869de9020cd05bae75fa48ec694fb39 Mon Sep 17 00:00:00 2001 From: Markus Horst Becker Date: Mon, 14 Mar 2022 15:18:58 +0100 Subject: [PATCH 1/2] Use 'Expires' header to time-limit URLs Co-Authored-By: Markus Horst Becker --- source/image-handler/image-request.ts | 26 ++++++++++++ .../image-handler/test/image-request.spec.ts | 40 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 54b6fd901..049f744e9 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -41,6 +41,7 @@ export class ImageRequest { imageRequestInfo = { ...imageRequestInfo, ...originalImage }; imageRequestInfo.headers = this.parseImageHeaders(event, imageRequestInfo.requestType); + this.validateRequestExpires(imageRequestInfo); // If the original image is SVG file and it has any edits but no output format, change the format to WebP. if (imageRequestInfo.contentType === 'image/svg+xml' && imageRequestInfo.edits && Object.keys(imageRequestInfo.edits).length > 0 && !imageRequestInfo.edits.toFormat) { @@ -443,4 +444,29 @@ export class ImageRequest { } } } + + private validateRequestExpires(requestInfo: ImageRequestInfo): void { + try { + const expires = requestInfo.headers?.expires; + if (expires !== undefined) { + const parsedDate = new Date(expires); + if (isNaN(parsedDate.getTime())) { + throw new ImageHandlerError(StatusCodes.BAD_REQUEST, 'ImageRequestExpiryFormat', 'Request has invalid expiry date.'); + } + const now = new Date(); + if (now > parsedDate) { + throw new ImageHandlerError(StatusCodes.FORBIDDEN, 'ImageRequestExpired', 'Request has expired.'); + } + } + } catch (error) { + if (error.code === 'ImageRequestExpired') { + throw error; + } + if (error.code === 'ImageRequestExpiryFormat') { + throw error; + } + console.error('Error occurred while checking expiry.', error); + throw new ImageHandlerError(StatusCodes.INTERNAL_SERVER_ERROR, 'ExpiryDateCheckFailure', 'Expiry date check failed.'); + } + } } diff --git a/source/image-handler/test/image-request.spec.ts b/source/image-handler/test/image-request.spec.ts index c7535c528..344e40100 100644 --- a/source/image-handler/test/image-request.spec.ts +++ b/source/image-handler/test/image-request.spec.ts @@ -853,6 +853,46 @@ describe('setup()', () => { expect(imageRequestInfo).toEqual(expectedResult); }); }); + + describe('011/expiryDate', () => { + it.each([ + { + path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJUaHUsIDAxIEphbiAxOTcwIDAwOjAwOjAwIEdNVCJ9fQ==', + error: { + code: 'ImageRequestExpired', + message: 'Request has expired.', + status: StatusCodes.FORBIDDEN, + }, + }, + { + path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJpbnZhbGlkS2V5In19', + error: { + code: 'ImageRequestExpiryFormat', + message: 'Request has invalid expiry date.', + status: StatusCodes.BAD_REQUEST, + } + } + ])("Should throw an error when $error.message", (async ({ path, error: expectedError }) => { + // Arrange + const event = { + path, + }; + // Mock + mockAwsS3.getObject.mockImplementationOnce(() => ({ + promise() { + return Promise.resolve({ Body: Buffer.from('SampleImageContent\n') }); + } + })); + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + try { + await imageRequest.setup(event); + } catch (error) { + // Assert + expect(error).toMatchObject(expectedError); + } + })); + }); }); describe('getOriginalImage()', () => { From 38671a53e643f654b7561589718fab6345cca913 Mon Sep 17 00:00:00 2001 From: Markus Horst Becker Date: Fri, 25 Mar 2022 11:30:05 +0100 Subject: [PATCH 2/2] Refactor to queryStringParams --- source/image-handler/image-request.ts | 27 ++++++-- source/image-handler/lib/interfaces.ts | 3 +- .../image-handler/test/image-request.spec.ts | 69 ++++++++++++------- 3 files changed, 70 insertions(+), 29 deletions(-) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 049f744e9..25b1fcb42 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -19,7 +19,7 @@ type OriginalImageInfo = Partial<{ export class ImageRequest { private static readonly DEFAULT_REDUCTION_EFFORT = 4; - constructor(private readonly s3Client: S3, private readonly secretProvider: SecretProvider) {} + constructor(private readonly s3Client: S3, private readonly secretProvider: SecretProvider) { } /** * Initializer function for creating a new image request, used by the image handler to perform image modifications. @@ -29,6 +29,7 @@ export class ImageRequest { public async setup(event: ImageHandlerEvent): Promise { try { await this.validateRequestSignature(event); + this.validateRequestExpires(event); let imageRequestInfo: ImageRequestInfo = {}; @@ -41,7 +42,6 @@ export class ImageRequest { imageRequestInfo = { ...imageRequestInfo, ...originalImage }; imageRequestInfo.headers = this.parseImageHeaders(event, imageRequestInfo.requestType); - this.validateRequestExpires(imageRequestInfo); // If the original image is SVG file and it has any edits but no output format, change the format to WebP. if (imageRequestInfo.contentType === 'image/svg+xml' && imageRequestInfo.edits && Object.keys(imageRequestInfo.edits).length > 0 && !imageRequestInfo.edits.toFormat) { @@ -407,6 +407,19 @@ export class ImageRequest { } } + /** + * Creates a query string similar to API Gateway 2.0 payload's $.rawQueryString + * @param queryStringParameters Request's query parameters + * @returns URL encoded queryString + */ + private recreateQueryString(queryStringParameters: ImageHandlerEvent['queryStringParameters']): string { + return Object + .entries(queryStringParameters) + .filter(([key]) => key !== 'signature') + .map(([key, value]) => [key, value].join('=')) + .join('&'); + } + /** * Validates the request's signature. * @param event Lambda request body. @@ -424,11 +437,14 @@ export class ImageRequest { throw new ImageHandlerError(StatusCodes.BAD_REQUEST, 'AuthorizationQueryParametersError', 'Query-string requires the signature parameter.'); } + const queryString = this.recreateQueryString(queryStringParameters); + try { const { signature } = queryStringParameters; const secret = JSON.parse(await this.secretProvider.getSecret(SECRETS_MANAGER)); const key = secret[SECRET_KEY]; - const hash = createHmac('sha256', key).update(path).digest('hex'); + const stringToSign = queryString !== '' ? [path, queryString].join('?') : path; + const hash = createHmac('sha256', key).update(stringToSign).digest('hex'); // Signature should be made with the full path. if (signature !== hash) { @@ -445,9 +461,10 @@ export class ImageRequest { } } - private validateRequestExpires(requestInfo: ImageRequestInfo): void { + private validateRequestExpires(event: ImageHandlerEvent): void { try { - const expires = requestInfo.headers?.expires; + const { queryStringParameters } = event; + const expires = queryStringParameters?.expires; if (expires !== undefined) { const parsedDate = new Date(expires); if (isNaN(parsedDate.getTime())) { diff --git a/source/image-handler/lib/interfaces.ts b/source/image-handler/lib/interfaces.ts index 1c96aeb56..d6c146336 100644 --- a/source/image-handler/lib/interfaces.ts +++ b/source/image-handler/lib/interfaces.ts @@ -9,7 +9,8 @@ import { Headers, ImageEdits } from './types'; export interface ImageHandlerEvent { path?: string; queryStringParameters?: { - signature: string; + signature?: string; + expires?: string; }; requestContext?: { elb?: unknown; diff --git a/source/image-handler/test/image-request.spec.ts b/source/image-handler/test/image-request.spec.ts index 344e40100..6a4618d16 100644 --- a/source/image-handler/test/image-request.spec.ts +++ b/source/image-handler/test/image-request.spec.ts @@ -7,7 +7,7 @@ import SecretsManager from 'aws-sdk/clients/secretsmanager'; import S3 from 'aws-sdk/clients/s3'; import { ImageRequest } from '../image-request'; -import { ImageHandlerError, RequestTypes, StatusCodes } from '../lib'; +import { ImageHandlerError, ImageHandlerEvent, RequestTypes, StatusCodes } from '../lib'; import { SecretProvider } from '../secret-provider'; describe('setup()', () => { @@ -434,6 +434,22 @@ describe('setup()', () => { expect(imageRequestInfo).toEqual(expectedResult); }); + it('Should pass when the image signature is correct with expires query string', async () => { + jest.useFakeTimers('modern').setSystemTime(0); + + // TODO write test + + expect(true).toBe(true); + }); + + it('Should throw when the image signature is incorrect with expires query string', async () => { + jest.useFakeTimers('modern').setSystemTime(0); + + // TODO write test + + expect(true).toBe(true); + }); + it('Should throw an error when queryStringParameters are missing', async () => { // Arrange const event = { @@ -855,9 +871,15 @@ describe('setup()', () => { }); describe('011/expiryDate', () => { + const baseRequest = { + bucket: 'test', + requestType: 'Default', + key: 'test.png', + }; + const path = `/${Buffer.from(JSON.stringify(baseRequest)).toString('base64')}`; it.each([ { - path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJUaHUsIDAxIEphbiAxOTcwIDAwOjAwOjAwIEdNVCJ9fQ==', + expires: 'Thu, 01 Jan 1970 00:00:00 GMT', error: { code: 'ImageRequestExpired', message: 'Request has expired.', @@ -865,33 +887,34 @@ describe('setup()', () => { }, }, { - path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJpbnZhbGlkS2V5In19', + expires: 'invalidKey', error: { code: 'ImageRequestExpiryFormat', message: 'Request has invalid expiry date.', status: StatusCodes.BAD_REQUEST, } } - ])("Should throw an error when $error.message", (async ({ path, error: expectedError }) => { - // Arrange - const event = { - path, - }; - // Mock - mockAwsS3.getObject.mockImplementationOnce(() => ({ - promise() { - return Promise.resolve({ Body: Buffer.from('SampleImageContent\n') }); - } - })); - // Act - const imageRequest = new ImageRequest(s3Client, secretProvider); - try { - await imageRequest.setup(event); - } catch (error) { - // Assert - expect(error).toMatchObject(expectedError); - } - })); + ] as { expires: ImageHandlerEvent['queryStringParameters']['expires'], error: object, }[])( + "Should throw an error when $error.message", + (async ({ error: expectedError, expires }) => { + // Arrange + const event: ImageHandlerEvent = { + path, + queryStringParameters: { + expires, + }, + }; + // Mock + mockAwsS3.getObject.mockImplementationOnce(() => ({ + promise() { + return Promise.resolve({ Body: Buffer.from('SampleImageContent\n') }); + } + })); + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + await expect(imageRequest.setup(event)).rejects.toMatchObject(expectedError); + }) + ); }); });