fix(kms-keyring-node): copy branch key material on read

modules/kms-keyring-node/src/kms_hkeyring_node_helpers.ts

@@ -252,7 +252,22 @@ export async function getBranchKeyMaterials(
     branchKeyMaterials = cacheEntry.response
   }
 
-  return branchKeyMaterials
+  /* Hand back a copy the cache can never touch. The CMC zeroes a material's
+   * buffer in place on eviction; callers read the branch key AFTER this await,
+   * so a concurrent eviction (overwrite, TTL, or tail) could otherwise zero the
+   * buffer mid-derivation. */
+  return deepCopyBranchKeyMaterial(branchKeyMaterials)
+}
+
+function deepCopyBranchKeyMaterial(
+  material: NodeBranchKeyMaterial
+): NodeBranchKeyMaterial {
+  return new NodeBranchKeyMaterial(
+    Buffer.from(material.branchKey()),
+    material.branchKeyIdentifier,
+    material.branchKeyVersion.toString('utf-8'),
+    { ...material.encryptionContext }
+  )
 }
 
 //= aws-encryption-sdk-specification/framework/aws-kms/aws-kms-hierarchical-keyring.md#onencrypt

modules/kms-keyring-node/test/kms_hkeyring_node.concurrency.test.ts

@@ -0,0 +1,145 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import {
+  EncryptedDataKey,
+  NodeBranchKeyMaterial,
+  NodeDecryptionMaterial,
+  NodeEncryptionMaterial,
+  unwrapDataKey,
+} from '@aws-crypto/material-management'
+import {
+  BRANCH_KEY_ID_A,
+  BRANCH_KEY_ID_B,
+  EC_A,
+  EC_B,
+  KEYSTORE,
+  TEST_ESDK_ALG_SUITE,
+  TTL,
+} from './fixtures'
+import { KmsHierarchicalKeyRingNode } from '../src/kms_hkeyring_node'
+import {
+  BRANCH_KEY_ID_SUPPLIER,
+  deepCopyBranchKeyMaterial,
+} from './kms_hkeyring_node.test'
+import { expect } from 'chai'
+import Sinon from 'sinon'
+import {
+  BranchKeyStoreNode,
+  KeyStoreInfoOutput,
+} from '@aws-crypto/branch-keystore-node'
+
+const CONCURRENCY = 25
+
+let activeMaterialA: NodeBranchKeyMaterial
+let activeMaterialB: NodeBranchKeyMaterial
+before(async function () {
+  activeMaterialA = await KEYSTORE.getActiveBranchKey(BRANCH_KEY_ID_A)
+  activeMaterialB = await KEYSTORE.getActiveBranchKey(BRANCH_KEY_ID_B)
+})
+
+// A keystore stub that returns fresh material (its own buffer, as a real
+// keystore would) and resolves on the microtask queue.
+function stubKeyStore(): BranchKeyStoreNode {
+  const keyStore = Sinon.createStubInstance(BranchKeyStoreNode)
+  const forId = async (branchKeyId: string) =>
+    deepCopyBranchKeyMaterial(
+      branchKeyId === BRANCH_KEY_ID_A ? activeMaterialA : activeMaterialB
+    )
+  keyStore.getActiveBranchKey.callsFake(forId)
+  // The two active versions are the only versions used here, so map by id.
+  keyStore.getBranchKeyVersion.callsFake(forId)
+  keyStore.getKeyStoreInfo.callsFake(
+    (): KeyStoreInfoOutput => ({
+      keystoreId: 'keyStoreId',
+      keystoreTableName: 'keystoreTableName',
+      logicalKeyStoreName: 'logicalKeyStoreName',
+      grantTokens: [],
+      kmsConfiguration: null as any,
+    })
+  )
+  return keyStore
+}
+
+function newKeyring(maxCacheSize?: number): KmsHierarchicalKeyRingNode {
+  return new KmsHierarchicalKeyRingNode({
+    branchKeyIdSupplier: BRANCH_KEY_ID_SUPPLIER,
+    keyStore: stubKeyStore(),
+    cacheLimitTtl: TTL,
+    maxCacheSize,
+  })
+}
+
+describe('KmsHierarchicalKeyRingNode: concurrent cold-cache operations (#1691)', () => {
+  it('concurrent onEncrypt does not wrap data keys under an evicted (zeroed) branch key', async () => {
+    // maxCacheSize=1 + two branch keys => every alternating encrypt evicts the
+    // other entry, zeroing its buffer.
+    const hkr = newKeyring(1)
+    const materials = Array.from(
+      { length: CONCURRENCY },
+      (_, i) =>
+        new NodeEncryptionMaterial(
+          TEST_ESDK_ALG_SUITE,
+          i % 2 === 0 ? EC_A : EC_B
+        )
+    )
+
+    await Promise.all(
+      materials.map(async (m) => {
+        await hkr.onEncrypt(m)
+      })
+    )
+
+    const verifier = newKeyring()
+    for (const m of materials) {
+      const expectedPdk = unwrapDataKey(m.getUnencryptedDataKey())
+      const decryptionMaterial = new NodeDecryptionMaterial(
+        TEST_ESDK_ALG_SUITE,
+        m.encryptionContext
+      )
+      await verifier.onDecrypt(decryptionMaterial, m.encryptedDataKeys)
+      expect(
+        unwrapDataKey(decryptionMaterial.getUnencryptedDataKey())
+      ).to.deep.equal(expectedPdk)
+    }
+  })
+
+  it('concurrent onDecrypt does not derive from an evicted (zeroed) branch key', async () => {
+    // Build valid ciphertexts for both branch keys (sequential = uncorrupted).
+    const setup = newKeyring()
+    const encA = new NodeEncryptionMaterial(TEST_ESDK_ALG_SUITE, EC_A)
+    const encB = new NodeEncryptionMaterial(TEST_ESDK_ALG_SUITE, EC_B)
+    await setup.onEncrypt(encA)
+    await setup.onEncrypt(encB)
+
+    const cases = [
+      {
+        edks: encA.encryptedDataKeys,
+        ec: EC_A,
+        pdk: unwrapDataKey(encA.getUnencryptedDataKey()),
+      },
+      {
+        edks: encB.encryptedDataKeys,
+        ec: EC_B,
+        pdk: unwrapDataKey(encB.getUnencryptedDataKey()),
+      },
+    ]
+
+    const hkr = newKeyring(1)
+    const recovered = await Promise.all(
+      Array.from({ length: CONCURRENCY }, async (_, i) => {
+        const { edks, ec } = cases[i % 2]
+        const decryptionMaterial = new NodeDecryptionMaterial(
+          TEST_ESDK_ALG_SUITE,
+          ec
+        )
+        await hkr.onDecrypt(decryptionMaterial, edks as EncryptedDataKey[])
+        return unwrapDataKey(decryptionMaterial.getUnencryptedDataKey())
+      })
+    )
+
+    for (let i = 0; i < recovered.length; i++) {
+      expect(recovered[i]).to.deep.equal(cases[i % 2].pdk)
+    }
+  })
+})