fix(signature-v4): add secrets to signing key cache key (#1776)

* fix(signature-v4): add secrets to signing key cache key
aws · Dec 19, 2020 · 8785ad4 · 8785ad4
1 parent 388b180
commit 8785ad4
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 28 deletions.
diff --git a/packages/signature-v4/src/credentialDerivation.spec.ts b/packages/signature-v4/src/credentialDerivation.spec.ts
@@ -46,27 +46,31 @@ describe("getSigningKey", () => {
   });
 
   describe("caching", () => {
-    it("should return the same promise when called with the same date, region, service, and credentials", () => {
-      const promise1 = getSigningKey(Sha256, credentials, shortDate, region, service);
-      const promise2 = getSigningKey(Sha256, credentials, shortDate, region, service);
-      expect(promise1).toBe(promise2);
+    it("should return the same signing key when called with the same date, region, service, and credentials", async () => {
+      const mockSha256Constructor = jest.fn().mockImplementation((args) => {
+        return new Sha256(args);
+      });
+      const key1 = await getSigningKey(mockSha256Constructor, credentials, shortDate, region, service);
+      const key2 = await getSigningKey(mockSha256Constructor, credentials, shortDate, region, service);
+      expect(key1).toBe(key2);
+      expect(mockSha256Constructor).toHaveBeenCalledTimes(6);
     });
 
-    it("should cache a maximum of 50 entries", () => {
-      const keyPromises: Array<Promise<Uint8Array>> = new Array(50);
+    it("should cache a maximum of 50 entries", async () => {
+      const keys: Array<Uint8Array> = new Array(50);
       // fill the cache
       for (let i = 0; i < 50; i++) {
-        keyPromises[i] = getSigningKey(Sha256, credentials, shortDate, `us-foo-${i.toString(10)}`, service);
+        keys[i] = await getSigningKey(Sha256, credentials, shortDate, `us-foo-${i.toString(10)}`, service);
       }
 
       // evict the oldest member from the cache
-      getSigningKey(Sha256, credentials, shortDate, `us-foo-50`, service);
+      await getSigningKey(Sha256, credentials, shortDate, `us-foo-50`, service);
 
       // the second oldest member should still be in cache
-      expect(keyPromises[1]).toBe(getSigningKey(Sha256, credentials, shortDate, `us-foo-1`, service));
+      await expect(getSigningKey(Sha256, credentials, shortDate, `us-foo-1`, service)).resolves.toStrictEqual(keys[1]);
 
       // the oldest member should not be in the cache
-      expect(keyPromises[0]).not.toBe(getSigningKey(Sha256, credentials, shortDate, `us-foo-0`, service));
+      await expect(getSigningKey(Sha256, credentials, shortDate, `us-foo-0`, service)).resolves.not.toBe(keys[0]);
     });
   });
 });
diff --git a/packages/signature-v4/src/credentialDerivation.ts b/packages/signature-v4/src/credentialDerivation.ts
@@ -1,8 +1,9 @@
 import { Credentials, HashConstructor, SourceData } from "@aws-sdk/types";
+import { toHex } from "@aws-sdk/util-hex-encoding";
 
 import { KEY_TYPE_IDENTIFIER, MAX_CACHE_SIZE } from "./constants";
 
-const signingKeyCache: { [key: string]: Promise<Uint8Array> } = {};
+const signingKeyCache: { [key: string]: Uint8Array } = {};
 const cacheQueue: Array<string> = [];
 
 /**
@@ -28,14 +29,15 @@ export function createScope(shortDate: string, region: string, service: string):
  * @param service           The service to which the signed request is being
  *                          sent.
  */
-export function getSigningKey(
+export const getSigningKey = async (
   sha256Constructor: HashConstructor,
   credentials: Credentials,
   shortDate: string,
   region: string,
   service: string
-): Promise<Uint8Array> {
-  const cacheKey = `${shortDate}:${region}:${service}:` + `${credentials.accessKeyId}:${credentials.sessionToken}`;
+): Promise<Uint8Array> => {
+  const credsHash = await hmac(sha256Constructor, credentials.secretAccessKey, credentials.accessKeyId);
+  const cacheKey = `${shortDate}:${region}:${service}:${toHex(credsHash)}:${credentials.sessionToken}`;
   if (cacheKey in signingKeyCache) {
     return signingKeyCache[cacheKey];
   }
@@ -45,20 +47,12 @@ export function getSigningKey(
     delete signingKeyCache[cacheQueue.shift() as string];
   }
 
-  return (signingKeyCache[cacheKey] = new Promise((resolve, reject) => {
-    let keyPromise: Promise<SourceData> = Promise.resolve(`AWS4${credentials.secretAccessKey}`);
-
-    for (const signable of [shortDate, region, service, KEY_TYPE_IDENTIFIER]) {
-      keyPromise = keyPromise.then<Uint8Array>((intermediateKey) => hmac(sha256Constructor, intermediateKey, signable));
-      keyPromise.catch(() => {});
-    }
-
-    (keyPromise as Promise<Uint8Array>).then(resolve, (reason) => {
-      delete signingKeyCache[cacheKey];
-      reject(reason);
-    });
-  }));
-}
+  let key: SourceData = `AWS4${credentials.secretAccessKey}`;
+  for (const signable of [shortDate, region, service, KEY_TYPE_IDENTIFIER]) {
+    key = await hmac(sha256Constructor, key, signable);
+  }
+  return (signingKeyCache[cacheKey] = key as Uint8Array);
+};
 
 /**
  * @internal