From 5a19a33e03d1ba7db3e2da8d28269aa97776866a Mon Sep 17 00:00:00 2001 From: Ran Vaknin <50976344+RanVaknin@users.noreply.github.com> Date: Mon, 22 Jan 2024 14:21:47 -0800 Subject: [PATCH] =?UTF-8?q?fix(middleware-ssec):=20add=20logic=20to=20hand?= =?UTF-8?q?le=20string=20input=20as=20specified=20b=E2=80=A6=20(#5676)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(middleware-ssec): add logic to handle string input as specified by api model * fix(middleware-ssec): removing buffer to support browser environment * fix(middleware-ssec): adding support both for browser and nodejs * fix(middleware-ssec): using previously defined implementation for decoding base64 * fix(middleware-ssec): adding an integration test * fix(middleware-ssec): removing console logs * fix(middleware-ssec): remove unused imports --------- Co-authored-by: Ran Vaknin --- packages/middleware-ssec/src/index.spec.ts | 54 +++++++++++++++++-- packages/middleware-ssec/src/index.ts | 33 +++++++----- .../src/middleware-ssec.integ.spec.ts | 27 ++++++++++ 3 files changed, 97 insertions(+), 17 deletions(-) diff --git a/packages/middleware-ssec/src/index.spec.ts b/packages/middleware-ssec/src/index.spec.ts index a333412e126c..167442c367d8 100644 --- a/packages/middleware-ssec/src/index.spec.ts +++ b/packages/middleware-ssec/src/index.spec.ts @@ -1,11 +1,14 @@ import { ChecksumConstructor } from "@smithy/types"; +import * as crypto from "crypto"; import { ssecMiddleware } from "./"; describe("ssecMiddleware", () => { const next = jest.fn(); const decoder = jest.fn().mockResolvedValue(new Uint8Array(0)); - const encoder = jest.fn().mockReturnValue("base64"); + const base64Decoder = jest.fn(); + const encoder1 = jest.fn(); + const encoder2 = jest.fn(); const mockHashUpdate = jest.fn(); const mockHashReset = jest.fn(); const mockHashDigest = jest.fn().mockReturnValue(new Uint8Array(0)); @@ -17,13 +20,54 @@ describe("ssecMiddleware", () => { beforeEach(() => { next.mockClear(); decoder.mockClear(); - encoder.mockClear(); + encoder1.mockClear(); + encoder2.mockClear(); mockHashUpdate.mockClear(); mockHashDigest.mockClear(); mockHashReset.mockClear(); }); it("should base64 encode input keys and set respective MD5 inputs", async () => { + encoder1.mockReturnValue("/+JF8FMG8UVMWSaNz0s6Wg=="); + const key = "TestKey123"; + const binaryRepresentationOfKey = Buffer.from(key); + const base64Key = binaryRepresentationOfKey.toString("base64"); + const md5Hash = crypto.createHash("md5").update(binaryRepresentationOfKey).digest(); + const base64Md5Hash = Buffer.from(md5Hash).toString("base64"); + + const args = { + input: { + SSECustomerKey: base64Key, + CopySourceSSECustomerKey: base64Key, + }, + }; + + const handler = ssecMiddleware({ + base64Encoder: encoder1, + utf8Decoder: decoder, + md5: MockHash, + base64Decoder: base64Decoder, + })(next, {} as any); + + await handler(args); + + expect(next.mock.calls.length).toBe(1); + expect(next).toHaveBeenCalledWith({ + input: { + SSECustomerKey: base64Key, + SSECustomerKeyMD5: base64Md5Hash, + CopySourceSSECustomerKey: base64Key, + CopySourceSSECustomerKeyMD5: base64Md5Hash, + }, + }); + expect(decoder.mock.calls.length).toBe(0); + expect(encoder1.mock.calls.length).toBe(2); + expect(mockHashUpdate.mock.calls.length).toBe(2); + expect(mockHashDigest.mock.calls.length).toBe(2); + encoder1.mockClear(); + }); + it("should base64 encode input keys and set respective MD5 inputs", async () => { + encoder2.mockReturnValue("base64"); const args = { input: { SSECustomerKey: "foo", @@ -32,9 +76,10 @@ describe("ssecMiddleware", () => { }; const handler = ssecMiddleware({ - base64Encoder: encoder, + base64Encoder: encoder2, utf8Decoder: decoder, md5: MockHash, + base64Decoder: base64Decoder, })(next, {} as any); await handler(args); @@ -49,8 +94,9 @@ describe("ssecMiddleware", () => { }, }); expect(decoder.mock.calls.length).toBe(2); - expect(encoder.mock.calls.length).toBe(4); + expect(encoder2.mock.calls.length).toBe(4); expect(mockHashUpdate.mock.calls.length).toBe(2); expect(mockHashDigest.mock.calls.length).toBe(2); + encoder2.mockClear(); }); }); diff --git a/packages/middleware-ssec/src/index.ts b/packages/middleware-ssec/src/index.ts index 63c245eb988a..330e2abae535 100644 --- a/packages/middleware-ssec/src/index.ts +++ b/packages/middleware-ssec/src/index.ts @@ -16,12 +16,13 @@ interface PreviouslyResolved { base64Encoder: Encoder; md5: ChecksumConstructor | HashConstructor; utf8Decoder: Decoder; + base64Decoder: Decoder; } export function ssecMiddleware(options: PreviouslyResolved): InitializeMiddleware { return (next: InitializeHandler): InitializeHandler => async (args: InitializeHandlerArguments): Promise> => { - let input = { ...args.input }; + const input = { ...args.input }; const properties = [ { target: "SSECustomerKey", @@ -36,19 +37,25 @@ export function ssecMiddleware(options: PreviouslyResolved): InitializeMiddlewar for (const prop of properties) { const value: SourceData | undefined = (input as any)[prop.target]; if (value) { - const valueView: Uint8Array = ArrayBuffer.isView(value) - ? new Uint8Array(value.buffer, value.byteOffset, value.byteLength) - : typeof value === "string" - ? options.utf8Decoder(value) - : new Uint8Array(value); - const encoded = options.base64Encoder(valueView); + let valueForHash: Uint8Array; + if (typeof value === "string") { + const isBase64Encoded = /^(?:[A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/.test(value); + if (isBase64Encoded) { + valueForHash = options.base64Decoder(value); + } else { + valueForHash = options.utf8Decoder(value); + input[prop.target] = options.base64Encoder(valueForHash); + } + } else { + valueForHash = ArrayBuffer.isView(value) + ? new Uint8Array(value.buffer, value.byteOffset, value.byteLength) + : new Uint8Array(value); + input[prop.target] = options.base64Encoder(valueForHash); + } + const hash = new options.md5(); - hash.update(valueView); - input = { - ...(input as any), - [prop.target]: encoded, - [prop.hash]: options.base64Encoder(await hash.digest()), - }; + hash.update(valueForHash); + input[prop.hash] = options.base64Encoder(await hash.digest()); } } diff --git a/packages/middleware-ssec/src/middleware-ssec.integ.spec.ts b/packages/middleware-ssec/src/middleware-ssec.integ.spec.ts index b5d0a5544046..337f61b1b98f 100644 --- a/packages/middleware-ssec/src/middleware-ssec.integ.spec.ts +++ b/packages/middleware-ssec/src/middleware-ssec.integ.spec.ts @@ -1,4 +1,5 @@ import { S3 } from "@aws-sdk/client-s3"; +import * as crypto from "crypto"; import { requireRequestsFrom } from "../../../private/aws-util-test/src"; @@ -29,5 +30,31 @@ describe("middleware-ssec", () => { Key: "k", }); }); + + it("verifies headers for PutObject with base64-encoded SSECustomerKey", async () => { + const client = new S3({ region: "us-east-1" }); + requireRequestsFrom(client).toMatch({ + method: "PUT", + hostname: "testbucket.s3.us-east-1.amazonaws.com", + query: { "x-id": "PutObject" }, + headers: { + "x-amz-server-side-encryption-customer-algorithm": "AES256", + "x-amz-server-side-encryption-customer-key": "UNhY4JhezH9gQYqvDMWrWH9CwlcKiECVqejMrND2VFw=", + "x-amz-server-side-encryption-customer-key-md5": "SwoBWUcJBbc/WRhR6hZGCA==", + "content-length": "14", + }, + body: "This is a test", + protocol: "https:", + path: "/foo", + }); + const exampleKey = crypto.createHash("sha256").update("example").digest(); + await client.putObject({ + Bucket: "testbucket", + Body: "This is a test", + Key: "foo", + SSECustomerKey: exampleKey.toString("base64"), + SSECustomerAlgorithm: "AES256", + }); + }); }); });