fix(middleware-ssec): ssecMiddleware with strict base64 validation (#…

…5875) * fix(middleware-ssec): ssecMiddleware with strict base64 validation * fix(middleware-ssec): ssecMiddleware with strict base64 validation, using serde decoder * fix(middleware-ssec): ssecMiddleware with strict base64 validation with tests --------- Co-authored-by: RanVaknin <[email protected]>
aws · Mar 18, 2024 · 28dcfa3 · 28dcfa3
1 parent 69ecf8b
commit 28dcfa3
Show file tree

Hide file tree

Showing 2 changed files with 141 additions and 37 deletions.
diff --git a/packages/middleware-ssec/src/index.spec.ts b/packages/middleware-ssec/src/index.spec.ts
@@ -1,7 +1,7 @@
 import { ChecksumConstructor } from "@smithy/types";
 import * as crypto from "crypto";
 
-import { ssecMiddleware } from "./";
+import { isValidBase64EncodedSSECustomerKey, ssecMiddleware } from "./";
 
 describe("ssecMiddleware", () => {
   const next = jest.fn();
@@ -28,22 +28,16 @@ describe("ssecMiddleware", () => {
   });
 
   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");
-
+    encoder2.mockReturnValue("base64");
     const args = {
       input: {
-        SSECustomerKey: base64Key,
-        CopySourceSSECustomerKey: base64Key,
+        SSECustomerKey: "foo",
+        CopySourceSSECustomerKey: "bar",
       },
     };
 
     const handler = ssecMiddleware({
-      base64Encoder: encoder1,
+      base64Encoder: encoder2,
       utf8Decoder: decoder,
       md5: MockHash,
       base64Decoder: base64Decoder,
@@ -54,49 +48,147 @@ describe("ssecMiddleware", () => {
     expect(next.mock.calls.length).toBe(1);
     expect(next).toHaveBeenCalledWith({
       input: {
-        SSECustomerKey: base64Key,
-        SSECustomerKeyMD5: base64Md5Hash,
-        CopySourceSSECustomerKey: base64Key,
-        CopySourceSSECustomerKeyMD5: base64Md5Hash,
+        SSECustomerKey: "base64",
+        SSECustomerKeyMD5: "base64",
+        CopySourceSSECustomerKey: "base64",
+        CopySourceSSECustomerKeyMD5: "base64",
       },
     });
-    expect(decoder.mock.calls.length).toBe(0);
-    expect(encoder1.mock.calls.length).toBe(2);
+    expect(decoder.mock.calls.length).toBe(2);
+    expect(encoder2.mock.calls.length).toBe(4);
     expect(mockHashUpdate.mock.calls.length).toBe(2);
     expect(mockHashDigest.mock.calls.length).toBe(2);
-    encoder1.mockClear();
+    encoder2.mockClear();
   });
-  it("should base64 encode input keys and set respective MD5 inputs", async () => {
-    encoder2.mockReturnValue("base64");
+
+  it("should handle valid base64 encoded 32byte key input for SSECustomerKey and CopySourceSSECustomerKey", async () => {
+    const validBase64EncodedKey = "QUIwMTIzNDU2Nzg5QUJDREVGQUJDREVGQUJDREVGQUI=";
+    const decodedBytes = Buffer.from(validBase64EncodedKey, "base64");
+    const md5Hash = crypto.createHash("md5").update(decodedBytes).digest();
+    const base64EncodedMD5Hash = Buffer.from(md5Hash).toString("base64");
+
+    base64Decoder.mockReturnValue(decodedBytes);
+    const mockMD5Instance = {
+      update: jest.fn().mockReturnThis(),
+      digest: jest.fn().mockReturnValue(md5Hash),
+    };
+    const mockMD5Constructor = jest.fn().mockReturnValue(mockMD5Instance);
+    const base64Encoder = jest.fn().mockReturnValue(base64EncodedMD5Hash);
+
+    const handler = ssecMiddleware({
+      base64Encoder,
+      utf8Decoder: decoder,
+      md5: mockMD5Constructor,
+      base64Decoder,
+    })(next, {} as any);
+
     const args = {
       input: {
-        SSECustomerKey: "foo",
-        CopySourceSSECustomerKey: "bar",
+        SSECustomerKey: validBase64EncodedKey,
+        CopySourceSSECustomerKey: validBase64EncodedKey,
       },
     };
 
+    await handler(args);
+
+    expect(next).toHaveBeenCalledTimes(1);
+    expect(next).toHaveBeenCalledWith({
+      input: {
+        SSECustomerKey: validBase64EncodedKey,
+        SSECustomerKeyMD5: base64EncodedMD5Hash,
+        CopySourceSSECustomerKey: validBase64EncodedKey,
+        CopySourceSSECustomerKeyMD5: base64EncodedMD5Hash,
+      },
+    });
+
+    expect(base64Decoder).toHaveBeenCalledTimes(4);
+    expect(base64Decoder).toHaveBeenCalledWith(validBase64EncodedKey);
+    expect(mockMD5Constructor).toHaveBeenCalledTimes(2);
+    expect(mockMD5Instance.update).toHaveBeenCalledTimes(2);
+    expect(mockMD5Instance.update).toHaveBeenCalledWith(decodedBytes);
+    expect(mockMD5Instance.digest).toHaveBeenCalledTimes(2);
+    expect(base64Encoder).toHaveBeenCalledTimes(2);
+    expect(base64Encoder).toHaveBeenCalledWith(md5Hash);
+  });
+
+  it("should handle 32-byte binary key input for SSECustomerKey and CopySourceSSECustomerKey", async () => {
+    const binaryKey = crypto.randomBytes(32);
+    const md5Hash = crypto.createHash("md5").update(binaryKey).digest();
+    const base64EncodedKey = binaryKey.toString("base64");
+    const base64EncodedMD5Hash = md5Hash.toString("base64");
+
+    const mockMD5Constructor = jest.fn().mockReturnValue({
+      update: jest.fn().mockReturnThis(),
+      digest: jest.fn().mockReturnValueOnce(md5Hash).mockReturnValueOnce(md5Hash),
+    });
+
+    const base64Encoder = jest
+      .fn()
+      .mockReturnValueOnce(base64EncodedKey)
+      .mockReturnValueOnce(base64EncodedMD5Hash)
+      .mockReturnValueOnce(base64EncodedKey)
+      .mockReturnValueOnce(base64EncodedMD5Hash);
+
     const handler = ssecMiddleware({
-      base64Encoder: encoder2,
+      base64Encoder,
       utf8Decoder: decoder,
-      md5: MockHash,
-      base64Decoder: base64Decoder,
+      md5: mockMD5Constructor,
+      base64Decoder,
     })(next, {} as any);
 
+    const args = {
+      input: {
+        SSECustomerKey: binaryKey,
+        CopySourceSSECustomerKey: binaryKey,
+      },
+    };
+
     await handler(args);
 
-    expect(next.mock.calls.length).toBe(1);
+    expect(next).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledWith({
       input: {
-        SSECustomerKey: "base64",
-        SSECustomerKeyMD5: "base64",
-        CopySourceSSECustomerKey: "base64",
-        CopySourceSSECustomerKeyMD5: "base64",
+        SSECustomerKey: base64EncodedKey,
+        SSECustomerKeyMD5: base64EncodedMD5Hash,
+        CopySourceSSECustomerKey: base64EncodedKey,
+        CopySourceSSECustomerKeyMD5: base64EncodedMD5Hash,
       },
     });
-    expect(decoder.mock.calls.length).toBe(2);
-    expect(encoder2.mock.calls.length).toBe(4);
-    expect(mockHashUpdate.mock.calls.length).toBe(2);
-    expect(mockHashDigest.mock.calls.length).toBe(2);
-    encoder2.mockClear();
+
+    expect(mockMD5Constructor).toHaveBeenCalledTimes(2);
+    expect(base64Encoder).toHaveBeenCalledTimes(4);
+  });
+
+  it("should return false for an invalid base64 string", () => {
+    const invalidBase64 = "invalid!@#$%";
+    const base64Decoder = jest.fn();
+    const options = { base64Decoder };
+
+    const result = isValidBase64EncodedSSECustomerKey(invalidBase64, options as any);
+    expect(result).toBe(false);
+    expect(base64Decoder).not.toHaveBeenCalled();
+  });
+
+  it("should return true for a valid base64 string and 32 bytes", () => {
+    const validBase64EncodedSSECustomerKey = "QUIwMTIzNDU2Nzg5QUJDREVGQUJDREVGQUJDREVGQUI=";
+    const decodedBytes = new Uint8Array(32);
+    const base64Decoder = jest.fn().mockReturnValue(decodedBytes);
+    const options = { base64Decoder };
+
+    const result = isValidBase64EncodedSSECustomerKey(validBase64EncodedSSECustomerKey, options as any);
+    expect(result).toBe(true);
+    expect(base64Decoder).toHaveBeenCalledTimes(1);
+    expect(base64Decoder).toHaveBeenCalledWith(validBase64EncodedSSECustomerKey);
+  });
+
+  it("should return false for a valid base64 string but not 32 bytes", () => {
+    const validBase64NonThirtyTwoBytes = "SGVsbG8=";
+    const base64Decoder = jest.fn().mockReturnValue(new Uint8Array([72, 101, 108, 108, 111]));
+    const options = { base64Decoder };
+
+    const result = isValidBase64EncodedSSECustomerKey(validBase64NonThirtyTwoBytes, options as any);
+    expect(result).toBe(false);
+    expect(base64Decoder).toHaveBeenCalledTimes(1);
+    expect(base64Decoder).toHaveBeenCalledWith(validBase64NonThirtyTwoBytes);
   });
 });
diff --git a/packages/middleware-ssec/src/index.ts b/packages/middleware-ssec/src/index.ts
@@ -39,8 +39,7 @@ export function ssecMiddleware(options: PreviouslyResolved): InitializeMiddlewar
         if (value) {
           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) {
+            if (isValidBase64EncodedSSECustomerKey(value, options)) {
               valueForHash = options.base64Decoder(value);
             } else {
               valueForHash = options.utf8Decoder(value);
@@ -78,3 +77,16 @@ export const getSsecPlugin = (config: PreviouslyResolved): Pluggable<any, any> =
     clientStack.add(ssecMiddleware(config), ssecMiddlewareOptions);
   },
 });
+
+export function isValidBase64EncodedSSECustomerKey(str: string, options: PreviouslyResolved) {
+  const base64Regex = /^(?:[A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/;
+
+  if (!base64Regex.test(str)) return false;
+
+  try {
+    const decodedBytes = options.base64Decoder(str);
+    return decodedBytes.length === 32;
+  } catch {
+    return false;
+  }
+}