diff --git a/modules/raw-aes-keyring-node/src/raw_aes_keyring_node.ts b/modules/raw-aes-keyring-node/src/raw_aes_keyring_node.ts index 698d94f16..855a06bd2 100644 --- a/modules/raw-aes-keyring-node/src/raw_aes_keyring_node.ts +++ b/modules/raw-aes-keyring-node/src/raw_aes_keyring_node.ts @@ -61,13 +61,17 @@ export class RawAesKeyringNode extends KeyringNode { } = input /* Precondition: AesKeyringNode needs identifying information for encrypt and decrypt. */ needs(keyName && keyNamespace, 'Identifying information must be defined.') + /* The material takes ownership of the key it is given and zeros it once it is + * moved into a KeyObject, so the material is given a copy. + * The caller keeps ownership of unencryptedMasterKey and is responsible for zeroing it. + */ /* Precondition: RawAesKeyringNode requires wrappingSuite to be a valid RawAesWrappingSuite. */ const wrappingMaterial = new NodeRawAesMaterial(wrappingSuite) /* Precondition: unencryptedMasterKey must correspond to the NodeAlgorithmSuite specification. * Note: the KeyringTrace and flag are _only_ set because I am reusing an existing implementation. * See: raw_aes_material.ts in @aws-crypto/raw-keyring for details */ - .setUnencryptedDataKey(unencryptedMasterKey, { + .setUnencryptedDataKey(copyOfMasterKey(unencryptedMasterKey), { keyNamespace, keyName, flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY, @@ -147,6 +151,21 @@ export class RawAesKeyringNode extends KeyringNode { } immutableClass(RawAesKeyringNode) +/** + * Returns an isolated copy of the caller supplied master key. + * The isolation precondition is checked against the caller's buffer + * because the copy always starts at offset 0 of its own ArrayBuffer. + */ +function copyOfMasterKey(unencryptedMasterKey: Uint8Array): Uint8Array { + /* Precondition: The caller's master key must be an isolated buffer. */ + needs( + unencryptedMasterKey instanceof Uint8Array && + unencryptedMasterKey.byteOffset === 0, + 'Unencrypted Master Key must be an isolated buffer.' + ) + return new Uint8Array(unencryptedMasterKey) +} + const encryptFlags = KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX diff --git a/modules/raw-aes-keyring-node/test/raw_aes_keyring_node.test.ts b/modules/raw-aes-keyring-node/test/raw_aes_keyring_node.test.ts index ce513a331..aa666051a 100644 --- a/modules/raw-aes-keyring-node/test/raw_aes_keyring_node.test.ts +++ b/modules/raw-aes-keyring-node/test/raw_aes_keyring_node.test.ts @@ -118,6 +118,89 @@ describe('RawAesKeyringNode::constructor', () => { }) }) +describe('RawAesKeyringNode::constructor does not consume the caller key (#970)', () => { + const wrappingSuite = + RawAesWrappingSuiteIdentifier.AES128_GCM_IV12_TAG16_NO_PADDING + const keyNamespace = 'keyNamespace' + const keyName = 'keyName' + const suite = new NodeAlgorithmSuite( + AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA256 + ) + + it('leaves the unencryptedMasterKey passed in by the caller intact', () => { + const unencryptedMasterKey = new Uint8Array(128 / 8).fill(7) + new RawAesKeyringNode({ + keyName, + keyNamespace, + unencryptedMasterKey, + wrappingSuite, + }) + expect(Array.from(unencryptedMasterKey)).to.deep.equal( + new Array(128 / 8).fill(7) + ) + }) + + it('keeps working after the caller zeros its own unencryptedMasterKey', async () => { + const unencryptedMasterKey = new Uint8Array(128 / 8).fill(7) + const keyring = new RawAesKeyringNode({ + keyName, + keyNamespace, + unencryptedMasterKey, + wrappingSuite, + }) + unencryptedMasterKey.fill(0) + + const encryptionMaterial = await keyring.onEncrypt( + new NodeEncryptionMaterial(suite, {}) + ) + const [edk] = encryptionMaterial.encryptedDataKeys + const decryptionMaterial = await keyring.onDecrypt( + new NodeDecryptionMaterial(suite, {}), + [edk] + ) + expect( + unwrapDataKey(decryptionMaterial.getUnencryptedDataKey()) + ).to.deep.equal(unwrapDataKey(encryptionMaterial.getUnencryptedDataKey())) + }) + + it('two keyrings can be built from the same unencryptedMasterKey', async () => { + const unencryptedMasterKey = new Uint8Array(128 / 8).fill(7) + const first = new RawAesKeyringNode({ + keyName, + keyNamespace, + unencryptedMasterKey, + wrappingSuite, + }) + const second = new RawAesKeyringNode({ + keyName, + keyNamespace, + unencryptedMasterKey, + wrappingSuite, + }) + const encryptionMaterial = await first.onEncrypt( + new NodeEncryptionMaterial(suite, {}) + ) + const decryptionMaterial = await second.onDecrypt( + new NodeDecryptionMaterial(suite, {}), + encryptionMaterial.encryptedDataKeys + ) + expect(decryptionMaterial.hasValidKey()).to.equal(true) + }) + + it('Precondition: unencryptedMasterKey must be an isolated buffer.', () => { + const shared = new Uint8Array(new ArrayBuffer(64), 16, 128 / 8) + expect( + () => + new RawAesKeyringNode({ + keyName, + keyNamespace, + unencryptedMasterKey: shared, + wrappingSuite, + }) + ).to.throw('Unencrypted Master Key must be an isolated buffer.') + }) +}) + describe('RawAesKeyringNode::_filter', () => { const wrappingSuite = RawAesWrappingSuiteIdentifier.AES128_GCM_IV12_TAG16_NO_PADDING