From 3b81fdc11b6584fee8eeaa7cee37fe12f485fba9 Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Tue, 27 Aug 2019 21:05:51 -0700 Subject: [PATCH 1/4] feat: Suport Node.js crypto KeyObjects resolves #74 KeyObjects were introduced in Node.js v11. They wrap the Buffer in an object that has even better security properties than an isolated buffer. --- ...hing_cryptographic_materials_decorators.ts | 4 +- modules/cache-material/src/index.ts | 1 - .../test/kms_keyring_node.test.ts | 7 +- modules/kms-keyring/src/kms_keyring.ts | 5 +- .../material-management-browser/src/index.ts | 2 +- .../src/material_helpers.ts | 3 +- modules/material-management-node/src/index.ts | 2 +- .../src/material_helpers.ts | 19 ++- .../test/material_helpers.test.ts | 32 ++-- .../src/clone_cryptographic_material.ts | 37 +++-- .../src/cryptographic_material.ts | 147 +++++++++++++----- modules/material-management/src/index.ts | 2 + modules/material-management/src/types.ts | 24 +++ .../test/clone_cryptographic_material.test.ts | 17 +- .../test/cryptographic_material.test.ts | 124 +++++++++++++-- .../test/multi_keyring.test.ts | 30 ++-- .../src/raw_aes_keyring_browser.ts | 3 +- .../src/raw_aes_keyring_node.ts | 6 +- .../test/raw_aes_keyring_node.test.ts | 5 +- modules/raw-keyring/src/raw_aes_material.ts | 9 +- .../test/raw_keyring_decorators.test.ts | 16 +- .../src/raw_rsa_keyring_web_crypto.ts | 3 +- .../src/raw_rsa_keyring_node.ts | 8 +- .../test/raw_rsa_keyring_node.test.ts | 5 +- 24 files changed, 373 insertions(+), 138 deletions(-) rename modules/{cache-material => material-management}/src/clone_cryptographic_material.ts (70%) rename modules/{cache-material => material-management}/test/clone_cryptographic_material.test.ts (89%) diff --git a/modules/cache-material/src/caching_cryptographic_materials_decorators.ts b/modules/cache-material/src/caching_cryptographic_materials_decorators.ts index 16f9b817b..c17c87cb2 100644 --- a/modules/cache-material/src/caching_cryptographic_materials_decorators.ts +++ b/modules/cache-material/src/caching_cryptographic_materials_decorators.ts @@ -24,7 +24,8 @@ import { DecryptionRequest, // eslint-disable-line no-unused-vars needs, readOnlyProperty, - Keyring // eslint-disable-line no-unused-vars + Keyring, // eslint-disable-line no-unused-vars + cloneMaterial } from '@aws-crypto/material-management' import { Maximum } from '@aws-crypto/serialize' import { @@ -34,7 +35,6 @@ import { import { CryptographicMaterialsCacheKeyHelpersInterface // eslint-disable-line no-unused-vars } from './build_cryptographic_materials_cache_key_helpers' -import { cloneMaterial } from './clone_cryptographic_material' export function decorateProperties ( obj: CachingMaterialsManager, diff --git a/modules/cache-material/src/index.ts b/modules/cache-material/src/index.ts index 7cf622b61..19014f3da 100644 --- a/modules/cache-material/src/index.ts +++ b/modules/cache-material/src/index.ts @@ -16,5 +16,4 @@ export * from './cryptographic_materials_cache' export * from './caching_cryptographic_materials_decorators' export * from './build_cryptographic_materials_cache_key_helpers' -export * from './clone_cryptographic_material' export * from './get_local_cryptographic_materials_cache' diff --git a/modules/kms-keyring-node/test/kms_keyring_node.test.ts b/modules/kms-keyring-node/test/kms_keyring_node.test.ts index 7fd943b4f..8e673333b 100644 --- a/modules/kms-keyring-node/test/kms_keyring_node.test.ts +++ b/modules/kms-keyring-node/test/kms_keyring_node.test.ts @@ -24,7 +24,8 @@ import { NodeAlgorithmSuite, AlgorithmSuiteIdentifier, EncryptedDataKey, // eslint-disable-line no-unused-vars - NodeDecryptionMaterial + NodeDecryptionMaterial, + unwrapDataKey } from '@aws-crypto/material-management-node' describe('KmsKeyringNode::constructor', () => { @@ -61,7 +62,7 @@ describe('RawAesKeyringWebCrypto encrypt/decrypt', () => { const material = new NodeEncryptionMaterial(suite, {}) const test = await keyring.onEncrypt(material) expect(test.hasValidKey()).to.equal(true) - udk = test.getUnencryptedDataKey() + udk = unwrapDataKey(test.getUnencryptedDataKey()) expect(udk).to.have.lengthOf(suite.keyLengthBytes) expect(test.encryptedDataKeys).to.have.lengthOf(2) const [edk] = test.encryptedDataKeys @@ -73,6 +74,6 @@ describe('RawAesKeyringWebCrypto encrypt/decrypt', () => { const material = new NodeDecryptionMaterial(suite, {}) const test = await keyring.onDecrypt(material, [encryptedDataKey]) expect(test.hasValidKey()).to.equal(true) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk) }) }) diff --git a/modules/kms-keyring/src/kms_keyring.ts b/modules/kms-keyring/src/kms_keyring.ts index ae0223bb9..06a759b17 100644 --- a/modules/kms-keyring/src/kms_keyring.ts +++ b/modules/kms-keyring/src/kms_keyring.ts @@ -28,7 +28,8 @@ import { KeyringTraceFlag, EncryptedDataKey, // eslint-disable-line no-unused-vars immutableClass, - readOnlyProperty + readOnlyProperty, + unwrapDataKey } from '@aws-crypto/material-management' import { KMS_PROVIDER_ID, generateDataKey, encrypt, decrypt, kmsResponseToEncryptedDataKey } from './helpers' import { regionFromKmsKeyArn } from './region_from_kms_key_arn' @@ -131,7 +132,7 @@ export function KmsKeyringClass> ( ) { const { suite } = material const extractable = false - const udk = material.getUnencryptedDataKey() + const udk = unwrapDataKey(material.getUnencryptedDataKey()) if (suite.kdf) { /* For several browsers, import for a key to derive with HKDF diff --git a/modules/material-management-node/src/index.ts b/modules/material-management-node/src/index.ts index 43fe641e7..aeb622621 100644 --- a/modules/material-management-node/src/index.ts +++ b/modules/material-management-node/src/index.ts @@ -29,5 +29,5 @@ export { AlgorithmSuiteIdentifier, EncryptionContext, EncryptedDataKey, KeyringTrace, KeyringTraceFlag, needs, KeyringNode, MultiKeyringNode, immutableBaseClass, immutableClass, frozenClass, readOnlyProperty, - NodeMaterialsManager + NodeMaterialsManager, unwrapDataKey, AwsEsdkKeyObject } from '@aws-crypto/material-management' diff --git a/modules/material-management-node/src/material_helpers.ts b/modules/material-management-node/src/material_helpers.ts index 66cd8d121..81108111f 100644 --- a/modules/material-management-node/src/material_helpers.ts +++ b/modules/material-management-node/src/material_helpers.ts @@ -15,6 +15,9 @@ import { needs, NodeEncryptionMaterial, NodeDecryptionMaterial, + unwrapDataKey, + wrapWithKeyObjectIfSupported, + AwsEsdkKeyObject, // eslint-disable-line no-unused-vars NodeHash // eslint-disable-line no-unused-vars } from '@aws-crypto/material-management' import { @@ -191,12 +194,13 @@ export function getCryptoStream (material: NodeEncryptionMaterial|NodeDecryption */ needs(material.hasUnencryptedDataKey, 'Unencrypted data key has been zeroed.') - return createCryptoStream(cipherName, derivedKey, iv) + // createDecipheriv is incorrectly typed in @types/node. It should take key: CipherKey, not key: BinaryLike + return createCryptoStream(cipherName, derivedKey as any, iv) } } } -export function nodeKdf (material: NodeEncryptionMaterial|NodeDecryptionMaterial, info?: Uint8Array): Uint8Array { +export function nodeKdf (material: NodeEncryptionMaterial|NodeDecryptionMaterial, info?: Uint8Array): Uint8Array|AwsEsdkKeyObject { const dataKey = material.getUnencryptedDataKey() const { kdf, kdfHash, keyLengthBytes } = material.suite @@ -212,10 +216,17 @@ export function nodeKdf (material: NodeEncryptionMaterial|NodeDecryptionMaterial info instanceof Uint8Array, 'Invalid HKDF values.' ) + /* The unwrap is done once we *know* that a KDF in required. + * If we unwrapped before everything will work, + * but we may be creating new copies of the unencrypted data key (export). + */ + const { buffer: dkBuffer, byteOffset: dkByteOffset, byteLength: dkByteLength } = unwrapDataKey(dataKey) // info and kdfHash are now defined - const toExtract = Buffer.from(dataKey.buffer, dataKey.byteOffset, dataKey.byteLength) + const toExtract = Buffer.from(dkBuffer, dkByteOffset, dkByteLength) const { buffer, byteOffset, byteLength } = info const infoBuff = Buffer.from(buffer, byteOffset, byteLength) - return kdfIndex[kdfHash](toExtract)(keyLengthBytes, infoBuff) + const derivedBytes = kdfIndex[kdfHash](toExtract)(keyLengthBytes, infoBuff) + + return wrapWithKeyObjectIfSupported(derivedBytes) } diff --git a/modules/material-management-node/test/material_helpers.test.ts b/modules/material-management-node/test/material_helpers.test.ts index d1b662850..cc6e9dcf6 100644 --- a/modules/material-management-node/test/material_helpers.test.ts +++ b/modules/material-management-node/test/material_helpers.test.ts @@ -17,7 +17,7 @@ import { expect } from 'chai' import 'mocha' -import { NodeDecryptionMaterial, NodeEncryptionMaterial, NodeAlgorithmSuite, AlgorithmSuiteIdentifier, KeyringTraceFlag, SignatureKey, VerificationKey } from '@aws-crypto/material-management' +import { NodeDecryptionMaterial, NodeEncryptionMaterial, NodeAlgorithmSuite, AlgorithmSuiteIdentifier, KeyringTraceFlag, SignatureKey, VerificationKey, unwrapDataKey } from '@aws-crypto/material-management' import { nodeKdf, getCryptoStream, getEncryptHelper, getDecryptionHelper } from '../src/material_helpers' // @ts-ignore import { Decipheriv, Cipheriv, createECDH } from 'crypto' @@ -28,10 +28,10 @@ describe('nodeKdf', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) - const test = nodeKdf(material, new Uint8Array(5)) - expect(test === dataKey).to.equal(true) + const test = unwrapDataKey(nodeKdf(material, new Uint8Array(5))) + expect(test).to.deep.equal(dataKey) }) it('HKDF SHA256', () => { @@ -39,10 +39,10 @@ describe('nodeKdf', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) - const test = nodeKdf(material, new Uint8Array(5)) - expect(test === dataKey).to.equal(false) + const test = unwrapDataKey(nodeKdf(material, new Uint8Array(5))) + expect(test).to.not.deep.equal(dataKey) expect(test.byteLength).to.equal(suite.keyLengthBytes) }) @@ -51,10 +51,10 @@ describe('nodeKdf', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) - const test = nodeKdf(material, new Uint8Array(5)) - expect(test === dataKey).to.equal(false) + const test = unwrapDataKey(nodeKdf(material, new Uint8Array(5))) + expect(test).to.not.deep.equal(dataKey) expect(test.byteLength).to.equal(suite.keyLengthBytes) }) @@ -91,7 +91,7 @@ describe('getCryptoStream', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const test = getCryptoStream(material)() const iv = new Uint8Array(12) @@ -104,7 +104,7 @@ describe('getCryptoStream', () => { const material = new NodeDecryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const test = getCryptoStream(material)() const iv = new Uint8Array(12) @@ -122,7 +122,7 @@ describe('getCryptoStream', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const test = getCryptoStream(material)() const iv = new Uint8Array(1) @@ -134,7 +134,7 @@ describe('getCryptoStream', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const test = getCryptoStream(material)() material.zeroUnencryptedDataKey() @@ -149,7 +149,7 @@ describe('getEncryptHelper', () => { const material = new NodeEncryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const helper = getEncryptHelper(material) expect(helper).to.haveOwnProperty('kdfGetCipher').and.to.be.a('function') @@ -230,7 +230,7 @@ describe('getDecryptionHelper', () => { const material = new NodeDecryptionMaterial(suite, {}) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY } - material.setUnencryptedDataKey(dataKey, trace) + material.setUnencryptedDataKey(new Uint8Array(dataKey), trace) const helper = getDecryptionHelper(material) expect(helper).to.haveOwnProperty('kdfGetDecipher').and.to.be.a('function') diff --git a/modules/cache-material/src/clone_cryptographic_material.ts b/modules/material-management/src/clone_cryptographic_material.ts similarity index 70% rename from modules/cache-material/src/clone_cryptographic_material.ts rename to modules/material-management/src/clone_cryptographic_material.ts index a3958be2e..c73b7af42 100644 --- a/modules/cache-material/src/clone_cryptographic_material.ts +++ b/modules/material-management/src/clone_cryptographic_material.ts @@ -19,33 +19,45 @@ import { WebCryptoEncryptionMaterial, WebCryptoDecryptionMaterial, isEncryptionMaterial, - isDecryptionMaterial, + isDecryptionMaterial +} from './cryptographic_material' +import { NodeAlgorithmSuite - -} from '@aws-crypto/material-management' +} from './node_algorithms' +import { + AwsEsdkKeyObject // eslint-disable-line no-unused-vars +} from './types' +import { KeyringTraceFlag } from './keyring_trace' type Material = NodeEncryptionMaterial|NodeDecryptionMaterial|WebCryptoEncryptionMaterial|WebCryptoDecryptionMaterial export function cloneMaterial (source: M): M { const { suite, encryptionContext } = source - const clone = suite instanceof NodeAlgorithmSuite + const clone = (suite instanceof NodeAlgorithmSuite ? source instanceof NodeEncryptionMaterial ? new NodeEncryptionMaterial(suite, encryptionContext) : new NodeDecryptionMaterial(suite, encryptionContext) : source instanceof WebCryptoEncryptionMaterial ? new WebCryptoEncryptionMaterial(suite, encryptionContext) - : new WebCryptoDecryptionMaterial(suite, encryptionContext) + : new WebCryptoDecryptionMaterial(suite, encryptionContext)) + + /* The WRAPPING_KEY_GENERATED_DATA_KEY _should_ be the first trace, + * but it is better to look for it explicitly. + */ + const trace = source.keyringTrace.find(({ flags }) => flags & KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) if (source.hasUnencryptedDataKey) { - const udk = new Uint8Array(source.getUnencryptedDataKey()) - clone.setUnencryptedDataKey(udk, source.keyringTrace[0]) + const udk = cloneUnencryptedDataKey(source.getUnencryptedDataKey()) + if (!trace) throw new Error('Malformed source material.') + clone.setUnencryptedDataKey(udk, trace) } if ((source).hasCryptoKey) { const cryptoKey = (source).getCryptoKey() + if (!trace) throw new Error('Malformed source material.') ;(clone) - .setCryptoKey(cryptoKey, source.keyringTrace[0]) + .setCryptoKey(cryptoKey, trace) } if (isEncryptionMaterial(source) && isEncryptionMaterial(clone)) { @@ -67,5 +79,12 @@ export function cloneMaterial (source: M): M { throw new Error('Material mismatch') } - return clone + return clone +} + +function cloneUnencryptedDataKey (dataKey: AwsEsdkKeyObject| Uint8Array) { + if (dataKey instanceof Uint8Array) { + return new Uint8Array(dataKey) + } + return dataKey } diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 7c7e4729a..33c091f24 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -13,7 +13,15 @@ * limitations under the License. */ -import { MixedBackendCryptoKey, SupportedAlgorithmSuites, AwsEsdkJsCryptoKey, AwsEsdkJsKeyUsage, EncryptionContext } from './types' // eslint-disable-line no-unused-vars +import { + MixedBackendCryptoKey, // eslint-disable-line no-unused-vars + SupportedAlgorithmSuites, // eslint-disable-line no-unused-vars + AwsEsdkJsCryptoKey, // eslint-disable-line no-unused-vars + AwsEsdkJsKeyUsage, // eslint-disable-line no-unused-vars + EncryptionContext, // eslint-disable-line no-unused-vars + AwsEsdkKeyObject, // eslint-disable-line no-unused-vars + AwsEsdkCreateSecretKey // eslint-disable-line no-unused-vars +} from './types' import { EncryptedDataKey } from './encrypted_data_key' import { SignatureKey, VerificationKey } from './signature_key' import { frozenClass, readOnlyProperty } from './immutable_class' @@ -22,6 +30,27 @@ import { NodeAlgorithmSuite } from './node_algorithms' import { WebCryptoAlgorithmSuite } from './web_crypto_algorithms' import { needs } from './needs' +/* KeyObject were introduced in v11. + * They protect the data key better than a Buffer. + * Their use is preferred. + * When they are available, the AWS Encryption SDK will proscribe their use. + * See: https://nodejs.org/api/crypto.html#crypto_class_keyobject + */ +interface AwsEsdkKeyObjectInstanceOf { + new (): AwsEsdkKeyObject +} +type AwsEsdkCrypto = {KeyObject: AwsEsdkKeyObjectInstanceOf, createSecretKey: AwsEsdkCreateSecretKey, } +export const supportsKeyObject = (function () { + try { + const { KeyObject, createSecretKey } = require('crypto') as AwsEsdkCrypto + if (!KeyObject || !createSecretKey) return false + + return { KeyObject, createSecretKey } + } catch (ex) { + return false + } +})() + /* * This public interface to the CryptographicMaterial object is provided for * developers of CMMs and keyrings only. If you are a user of the AWS Encryption @@ -76,8 +105,8 @@ export interface FunctionalCryptographicMaterial { export interface CryptographicMaterial> { suite: SupportedAlgorithmSuites - setUnencryptedDataKey: (dataKey: Uint8Array, trace: KeyringTrace) => T - getUnencryptedDataKey: () => Uint8Array + setUnencryptedDataKey: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => T + getUnencryptedDataKey: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey: () => T hasUnencryptedDataKey: boolean unencryptedDataKeyLength: number @@ -108,8 +137,8 @@ export class NodeEncryptionMaterial implements Readonly>, FunctionalCryptographicMaterial { suite: NodeAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => NodeEncryptionMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => NodeEncryptionMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => NodeEncryptionMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number @@ -143,8 +172,8 @@ export class NodeDecryptionMaterial implements Readonly>, FunctionalCryptographicMaterial { suite: NodeAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => NodeDecryptionMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => NodeDecryptionMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => NodeDecryptionMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number @@ -177,8 +206,8 @@ export class WebCryptoEncryptionMaterial implements Readonly>, FunctionalCryptographicMaterial { suite: WebCryptoAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => WebCryptoEncryptionMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => WebCryptoEncryptionMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => WebCryptoEncryptionMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number @@ -219,8 +248,8 @@ export class WebCryptoDecryptionMaterial implements Readonly>, FunctionalCryptographicMaterial { suite: WebCryptoAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => WebCryptoDecryptionMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => WebCryptoDecryptionMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => WebCryptoDecryptionMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number @@ -264,50 +293,37 @@ export function isDecryptionMaterial (obj: any): obj is WebCryptoDecryptionMater export function decorateCryptographicMaterial> (material: T, setFlags: KeyringTraceFlag) { let unencryptedDataKeyZeroed = false - let unencryptedDataKey: Uint8Array + let unencryptedDataKey: AwsEsdkKeyObject | Uint8Array // This copy of the unencryptedDataKey is stored to insure that the // unencrypted data key is *never* modified. Since the // unencryptedDataKey is returned by reference, any change // to it would be propagated to any cached versions. let udkForVerification: Uint8Array - const setUnencryptedDataKey = (dataKey: Uint8Array, trace: KeyringTrace) => { - /* Precondition: unencryptedDataKey must not be set. Modifying the unencryptedDataKey is denied */ - needs(!unencryptedDataKey, 'unencryptedDataKey has already been set') - /* Precondition: dataKey must be Binary Data */ - needs(dataKey instanceof Uint8Array, 'dataKey must be a Uint8Array') - /* Precondition: dataKey should have an ArrayBuffer that *only* stores the key. - * This is a simple check to make sure that the key is not stored on - * a large potentially shared ArrayBuffer. - * If this was the case, it may be possible to find or manipulate. - */ - needs(dataKey.byteOffset === 0, 'Unencrypted Master Key must be an isolated buffer.') - /* Precondition: The data key length must agree with algorithm specification. - * If this is not the case, it either means ciphertext was tampered - * with or the keyring implementation is not setting the length properly. - */ - needs(dataKey.byteLength === material.suite.keyLengthBytes, 'Key length does not agree with the algorithm specification.') - - /* Precondition: Trace must be set, and the flag must indicate that the data key was generated. */ - needs(trace && trace.keyName && trace.keyNamespace, 'Malformed KeyringTrace') - /* Precondition: On set the required KeyringTraceFlag must be set. */ - needs(trace.flags & setFlags, 'Required KeyringTraceFlag not set') + const setUnencryptedDataKey = (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => { + /* Avoid making unnecessary copies of the dataKey. */ + const tempUdk = dataKey instanceof Uint8Array ? dataKey : unwrapDataKey(dataKey) + /* All security conditions are tested here and failures will throw. */ + verifyUnencryptedDataKeyForSet(tempUdk, trace) + unencryptedDataKey = wrapWithKeyObjectIfSupported(dataKey) + udkForVerification = new Uint8Array(tempUdk) material.keyringTrace.push(trace) - unencryptedDataKey = dataKey - udkForVerification = new Uint8Array(dataKey) - return material } - const getUnencryptedDataKey = (): Uint8Array => { + const getUnencryptedDataKey = (): Uint8Array|AwsEsdkKeyObject => { /* Precondition: unencryptedDataKey must be set before we can return it. */ needs(unencryptedDataKey, 'unencryptedDataKey has not been set') /* Precondition: unencryptedDataKey must not be Zeroed out. * Returning a null key would be incredibly bad. */ needs(!unencryptedDataKeyZeroed, 'unencryptedDataKey has been zeroed.') - /* Precondition: The unencryptedDataKey must not have been modified. */ - needs(timingSafeEqual(udkForVerification, unencryptedDataKey), 'unencryptedDataKey has been corrupted.') + /* Precondition: The unencryptedDataKey must not have been modified. + * If the unencryptedDataKey is a KeyObject, + * then the security around modification is handled in C. + * Do not duplicate the secret just to check... + */ + needs(!(unencryptedDataKey instanceof Uint8Array) || timingSafeEqual(udkForVerification, unwrapDataKey(unencryptedDataKey)), 'unencryptedDataKey has been corrupted.') return unencryptedDataKey } Object.defineProperty(material, 'hasUnencryptedDataKey', { @@ -334,6 +350,12 @@ export function decorateCryptographicMaterial udkForVerification = new Uint8Array() unsetCount += 1 } + /* The KeyObject manages its own ref counter. + * Once there are no more users, it will clean the memory. + */ + if (!(unencryptedDataKey instanceof Uint8Array)) { + unencryptedDataKey = new Uint8Array() + } unencryptedDataKey.fill(0) udkForVerification.fill(0) unencryptedDataKeyZeroed = true @@ -356,7 +378,7 @@ export function decorateCryptographicMaterial * to do this */ needs(!unencryptedDataKeyZeroed, 'unencryptedDataKey has been zeroed.') - return unencryptedDataKey.byteLength + return unwrapDataKey(unencryptedDataKey).byteLength }, enumerable: true }) @@ -366,6 +388,29 @@ export function decorateCryptographicMaterial readOnlyProperty(material, 'zeroUnencryptedDataKey', zeroUnencryptedDataKey) return material + + function verifyUnencryptedDataKeyForSet (dataKey: Uint8Array, trace: KeyringTrace) { + /* Precondition: unencryptedDataKey must not be set. Modifying the unencryptedDataKey is denied */ + needs(!unencryptedDataKey, 'unencryptedDataKey has already been set') + /* Precondition: dataKey must be Binary Data */ + needs(dataKey instanceof Uint8Array, 'dataKey must be a Uint8Array') + /* Precondition: dataKey should have an ArrayBuffer that *only* stores the key. + * This is a simple check to make sure that the key is not stored on + * a large potentially shared ArrayBuffer. + * If this was the case, it may be possible to find or manipulate. + */ + needs(dataKey.byteOffset === 0, 'Unencrypted Master Key must be an isolated buffer.') + /* Precondition: The data key length must agree with algorithm specification. + * If this is not the case, it either means ciphertext was tampered + * with or the keyring implementation is not setting the length properly. + */ + needs(dataKey.byteLength === material.suite.keyLengthBytes, 'Key length does not agree with the algorithm specification.') + + /* Precondition: Trace must be set, and the flag must indicate that the data key was generated. */ + needs(trace && trace.keyName && trace.keyNamespace, 'Malformed KeyringTrace') + /* Precondition: On set the required KeyringTraceFlag must be set. */ + needs(trace.flags & setFlags, 'Required KeyringTraceFlag not set') + } } export function decorateEncryptionMaterial> (material: T) { @@ -595,3 +640,25 @@ export function subtleFunctionForMaterial> (mater throw new Error('Unsupported material') } + +export function unwrapDataKey (dataKey: Uint8Array|AwsEsdkKeyObject): Uint8Array { + if (dataKey instanceof Uint8Array) return dataKey + if (supportsKeyObject && dataKey instanceof supportsKeyObject.KeyObject) return dataKey.export() + + throw new Error('Unsupported dataKey type') +} + +export function wrapWithKeyObjectIfSupported (dataKey: Uint8Array|AwsEsdkKeyObject): Uint8Array|AwsEsdkKeyObject { + if (supportsKeyObject) { + if (dataKey instanceof Uint8Array) { + const ko = supportsKeyObject.createSecretKey(dataKey) + /* Postcondition: Zero the secret. It is now inside the KeyObject. */ + dataKey.fill(0) + return ko + } + if (dataKey instanceof supportsKeyObject.KeyObject) return dataKey + } else if (dataKey instanceof Uint8Array) { + return dataKey + } + throw new Error('Unsupported dataKey type') +} diff --git a/modules/material-management/src/index.ts b/modules/material-management/src/index.ts index 89447cfb0..4b8b477ca 100644 --- a/modules/material-management/src/index.ts +++ b/modules/material-management/src/index.ts @@ -31,6 +31,7 @@ export { NodeEncryptionMaterial, NodeDecryptionMaterial } from './cryptographic_ export { isValidCryptoKey, isCryptoKey, keyUsageForMaterial, subtleFunctionForMaterial } from './cryptographic_material' export { WebCryptoEncryptionMaterial, WebCryptoDecryptionMaterial } from './cryptographic_material' export { isEncryptionMaterial, isDecryptionMaterial } from './cryptographic_material' +export { unwrapDataKey, wrapWithKeyObjectIfSupported } from './cryptographic_material' export { CryptographicMaterial, decorateCryptographicMaterial, decorateWebCryptoMaterial, WebCryptoMaterial } from './cryptographic_material' export { SignatureKey, VerificationKey } from './signature_key' export { EncryptedDataKey, IEncryptedDataKey } from './encrypted_data_key' @@ -38,5 +39,6 @@ export { EncryptedDataKey, IEncryptedDataKey } from './encrypted_data_key' export { immutableBaseClass, immutableClass, frozenClass, readOnlyProperty } from './immutable_class' export { needs } from './needs' +export { cloneMaterial } from './clone_cryptographic_material' export * from './types' diff --git a/modules/material-management/src/types.ts b/modules/material-management/src/types.ts index 3f4e3be37..4528b82dc 100644 --- a/modules/material-management/src/types.ts +++ b/modules/material-management/src/types.ts @@ -65,3 +65,27 @@ export type DecryptionMaterial = Suite extends NodeAlgorithmSuite ? NodeDecryptionMaterial : Suite extends WebCryptoAlgorithmSuite ? WebCryptoDecryptionMaterial : never + +export type AwsEsdkKeyObjectType = 'secret' | 'public' | 'private' +export type AwsEsdkKeyFormat = 'pem' | 'der' +export type AwsEsdkKeyType = 'rsa' | 'dsa' | 'ec' +export interface AwsEsdkKeyExportOptions { + type: 'pkcs1' | 'spki' | 'pkcs8' | 'sec1' + format: T + cipher?: string + passphrase?: string | Buffer +} + +export interface AwsEsdkKeyObject { + asymmetricKeyType?: AwsEsdkKeyType + /** + * For asymmetric keys, this property represents the size of the embedded key in + * bytes. This property is `undefined` for symmetric keys. + */ + asymmetricKeySize?: number + export(options: AwsEsdkKeyExportOptions<'pem'>): string | Buffer + export(options?: AwsEsdkKeyExportOptions<'der'>): Buffer + symmetricSize?: number + type: AwsEsdkKeyObjectType +} +export type AwsEsdkCreateSecretKey = (key: Uint8Array) => AwsEsdkKeyObject diff --git a/modules/cache-material/test/clone_cryptographic_material.test.ts b/modules/material-management/test/clone_cryptographic_material.test.ts similarity index 89% rename from modules/cache-material/test/clone_cryptographic_material.test.ts rename to modules/material-management/test/clone_cryptographic_material.test.ts index 49e89d737..e5637d3cb 100644 --- a/modules/cache-material/test/clone_cryptographic_material.test.ts +++ b/modules/material-management/test/clone_cryptographic_material.test.ts @@ -27,8 +27,9 @@ import { WebCryptoAlgorithmSuite, AlgorithmSuiteIdentifier, KeyringTraceFlag, - EncryptedDataKey -} from '@aws-crypto/material-management' + EncryptedDataKey, + unwrapDataKey +} from '../src/index' const nodeSuite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const webCryptoSuite = new WebCryptoAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) @@ -47,13 +48,13 @@ const cryptoKey: any = { type: 'secret', algorithm: { name: webCryptoSuite.encry describe('cloneMaterial', () => { it('clone NodeEncryptionMaterial', () => { const material = new NodeEncryptionMaterial(nodeSuite, { some: 'context' }) - .setUnencryptedDataKey(udk128, trace) + .setUnencryptedDataKey(new Uint8Array(udk128), trace) .addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) .addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) const test = cloneMaterial(material) expect(test).to.be.instanceOf(NodeEncryptionMaterial) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk128) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk128) expect(test.keyringTrace).to.deep.equal(material.keyringTrace) expect(test.encryptedDataKeys).to.deep.equal(material.encryptedDataKeys) expect(test.encryptionContext).to.deep.equal(material.encryptionContext) @@ -61,25 +62,25 @@ describe('cloneMaterial', () => { it('clone NodeDecryptionMaterial', () => { const material = new NodeDecryptionMaterial(nodeSuite, { some: 'context' }) - .setUnencryptedDataKey(udk128, trace) + .setUnencryptedDataKey(new Uint8Array(udk128), trace) const test = cloneMaterial(material) expect(test).to.be.instanceOf(NodeDecryptionMaterial) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk128) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk128) expect(test.keyringTrace).to.deep.equal(material.keyringTrace) expect(test.encryptionContext).to.deep.equal(material.encryptionContext) }) it('clone WebCryptoEncryptionMaterial', () => { const material = new WebCryptoEncryptionMaterial(webCryptoSuite, { some: 'context' }) - .setUnencryptedDataKey(udk128, trace) + .setUnencryptedDataKey(new Uint8Array(udk128), trace) .setCryptoKey(cryptoKey, trace) .addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) .addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) const test = cloneMaterial(material) expect(test).to.be.instanceOf(WebCryptoEncryptionMaterial) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk128) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk128) expect(test.getCryptoKey()).to.deep.equal(cryptoKey) expect(test.keyringTrace).to.deep.equal(material.keyringTrace) expect(test.encryptedDataKeys).to.deep.equal(material.encryptedDataKeys) diff --git a/modules/material-management/test/cryptographic_material.test.ts b/modules/material-management/test/cryptographic_material.test.ts index 669ca728c..37572f80f 100644 --- a/modules/material-management/test/cryptographic_material.test.ts +++ b/modules/material-management/test/cryptographic_material.test.ts @@ -38,9 +38,14 @@ import { subtleFunctionForMaterial, keyUsageForMaterial, isValidCryptoKey, - isCryptoKey + isCryptoKey, + unwrapDataKey, + wrapWithKeyObjectIfSupported, + supportsKeyObject } from '../src/cryptographic_material' +import { createSecretKey } from 'crypto' + describe('decorateCryptographicMaterial', () => { it('will decorate', () => { const test = decorateCryptographicMaterial(({}), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) @@ -54,34 +59,46 @@ describe('decorateCryptographicMaterial', () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) - test.setUnencryptedDataKey(dataKey, { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) + test.setUnencryptedDataKey(new Uint8Array(dataKey), { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) expect(test.hasUnencryptedDataKey).to.equal(true) expect(test.unencryptedDataKeyLength).to.equal(dataKey.byteLength) - expect(test.getUnencryptedDataKey()).to.deep.equal(dataKey) + const udk = unwrapDataKey(test.getUnencryptedDataKey()) + expect(udk).to.deep.equal(dataKey) }) it('zeroing out the unencrypted data key', () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) + /* This is complicated. + * Now that I support KeyObjects it is good to pass a copy, + * i.e. new Uint8Array(dataKey). + * But in this case, if this is a version of Node.js that does not support KeyObjects + * passing the dataKey lets me verify that the value memory is really zeroed. + */ test.setUnencryptedDataKey(dataKey, { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) test.zeroUnencryptedDataKey() expect(test.hasUnencryptedDataKey).to.equal(false) - expect(dataKey).to.deep.equal(new Uint8Array(suite.keyLengthBytes).fill(0)) + if (!supportsKeyObject) { + expect(dataKey).to.deep.equal(new Uint8Array(suite.keyLengthBytes).fill(0)) + } else { + // If the environment supports KeyObjects then the udk was wrapped. + // There is no way to confirm that + } }) it('Precondition: The data key length must agree with algorithm specification.', () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes - 1).fill(1) - expect(() => test.setUnencryptedDataKey(dataKey)).to.throw() + expect(() => test.setUnencryptedDataKey(new Uint8Array(dataKey))).to.throw() }) it('Precondition: unencryptedDataKey must not be Zeroed out.', () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) - test.setUnencryptedDataKey(dataKey, { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) + test.setUnencryptedDataKey(new Uint8Array(dataKey), { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) test.zeroUnencryptedDataKey() expect(() => test.getUnencryptedDataKey()).to.throw() expect(() => test.unencryptedDataKeyLength).to.throw() @@ -115,7 +132,7 @@ describe('decorateCryptographicMaterial', () => { const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } // It is very hard to test this perfectly. However, this tests the spirit. - expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw() + expect(() => test.setUnencryptedDataKey(new Uint8Array(dataKey), trace)).to.throw() }) it('Precondition: dataKey must be Binary Data', () => { @@ -129,8 +146,8 @@ describe('decorateCryptographicMaterial', () => { const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - test.setUnencryptedDataKey(dataKey, trace) - expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('unencryptedDataKey has already been set') + test.setUnencryptedDataKey(new Uint8Array(dataKey), trace) + expect(() => test.setUnencryptedDataKey(new Uint8Array(dataKey), trace)).to.throw('unencryptedDataKey has already been set') }) it('Precondition: dataKey should have an ArrayBuffer that *only* stores the key.', () => { @@ -145,7 +162,7 @@ describe('decorateCryptographicMaterial', () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) - expect(() => test.setUnencryptedDataKey(dataKey, {} as any)).to.throw('Malformed KeyringTrace') + expect(() => test.setUnencryptedDataKey(new Uint8Array(dataKey), {} as any)).to.throw('Malformed KeyringTrace') }) it('Precondition: On set the required KeyringTraceFlag must be set.', () => { @@ -153,7 +170,7 @@ describe('decorateCryptographicMaterial', () => { const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1) const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX } - expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('Required KeyringTraceFlag not set') + expect(() => test.setUnencryptedDataKey(new Uint8Array(dataKey), trace)).to.throw('Required KeyringTraceFlag not set') }) it('Precondition: The unencryptedDataKey must not have been modified.', () => { @@ -163,7 +180,14 @@ describe('decorateCryptographicMaterial', () => { material.setUnencryptedDataKey(dataKey, { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) const test = material.getUnencryptedDataKey() test[0] = 12 - expect(() => material.getUnencryptedDataKey()).to.throw() + expect(() => { + const udk = unwrapDataKey(material.getUnencryptedDataKey()) + /* This should NOT be true. + * If the udk is a KeyObject then the change above was on independent memory. + * This check follows the code, and is *intended* to fail. + */ + expect(udk[0]).to.equal(12) + }).to.throw() }) }) @@ -551,3 +575,79 @@ describe('WebCryptoDecryptionMaterial', () => { expect(() => new WebCryptoDecryptionMaterial(suite, true as any)).to.throw() }) }) + +describe('KeyObject support', () => { + it('supportsKeyObject tracks values of crypto module', () => { + // supportsKeyObject should track the createSecretKey value... + expect(!!supportsKeyObject === !!createSecretKey).to.equal(true) + }) + + if (supportsKeyObject) { + const { KeyObject, createSecretKey } = supportsKeyObject + describe('wrapWithKeyObjectIfSupported', () => { + it('Uint8Array are wrapped in a KeyObject if supported', () => { + const test = wrapWithKeyObjectIfSupported(new Uint8Array(16)) + expect(test).to.be.instanceOf(KeyObject) + }) + + it('KeyObject are return unchanged', () => { + const dataKey = createSecretKey(new Uint8Array(16)) + expect(dataKey === wrapWithKeyObjectIfSupported(dataKey)).to.equal(true) + }) + + it('throws for unsupported types', () => { + expect(() => wrapWithKeyObjectIfSupported({} as any)).to.throw('Unsupported dataKey type') + }) + + it('Postcondition: Zero the secret. It is now inside the KeyObject.', () => { + const dataKey = new Uint8Array(16).fill(1) + wrapWithKeyObjectIfSupported(dataKey) + expect(dataKey).to.deep.equal(new Uint8Array(16).fill(0)) + }) + }) + + describe('unwrapDataKey', () => { + it('returns Uint8Array unmodified', () => { + const dataKey = new Uint8Array(16).fill(1) + const test = unwrapDataKey(dataKey) + expect(test === dataKey).to.equal(true) + }) + + it('exports the secret key', () => { + const rawKey = new Uint8Array(16).fill(1) + const dataKey = createSecretKey(rawKey) + const test = unwrapDataKey(dataKey) + expect(test).to.deep.equal(rawKey) + }) + + it('throws for unsupported types', () => { + expect(() => unwrapDataKey({} as any)).to.throw('Unsupported dataKey type') + }) + }) + } else { + describe('wrapWithKeyObjectIfSupported', () => { + it('Uint8Array are returned unchanged', () => { + const dataKey = new Uint8Array(16) + const test = wrapWithKeyObjectIfSupported(dataKey) + expect(test).to.be.instanceOf(Uint8Array) + expect(test === dataKey).to.equal(true) + }) + + it('throws for unsupported types', () => { + expect(() => wrapWithKeyObjectIfSupported({} as any)).to.throw('Unsupported dataKey type') + }) + }) + + describe('unwrapDataKey', () => { + it('returns Uint8Array unmodified', () => { + const dataKey = new Uint8Array(16).fill(1) + const test = unwrapDataKey(dataKey) + expect(test === dataKey).to.equal(true) + }) + + it('throws for unsupported types', () => { + expect(() => unwrapDataKey({} as any)).to.throw('Unsupported dataKey type') + }) + }) + } +}) diff --git a/modules/material-management/test/multi_keyring.test.ts b/modules/material-management/test/multi_keyring.test.ts index 1b15ad3dd..ce0841c16 100644 --- a/modules/material-management/test/multi_keyring.test.ts +++ b/modules/material-management/test/multi_keyring.test.ts @@ -20,7 +20,7 @@ import chaiAsPromised from 'chai-as-promised' import 'mocha' import { MultiKeyringNode } from '../src/multi_keyring' import { KeyringNode, KeyringWebCrypto } from '../src/keyring' -import { NodeEncryptionMaterial, NodeDecryptionMaterial } from '../src/cryptographic_material' +import { NodeEncryptionMaterial, NodeDecryptionMaterial, unwrapDataKey } from '../src/cryptographic_material' import { NodeAlgorithmSuite } from '../src/node_algorithms' import { AlgorithmSuiteIdentifier } from '../src/algorithm_suites' import { EncryptedDataKey } from '../src/encrypted_data_key' @@ -117,7 +117,7 @@ describe('MultiKeyring: onEncrypt', () => { const generator = keyRingFactory({ async onEncrypt (material: NodeEncryptionMaterial) { return material - .setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + .setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) .addEncryptedDataKey(edk0, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) }, onDecrypt: never @@ -135,7 +135,7 @@ describe('MultiKeyring: onEncrypt', () => { expect(test.keyringTrace[0] === keyringTrace0).to.equal(true) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) }) it('calls generator then child keyrings', async () => { @@ -146,7 +146,7 @@ describe('MultiKeyring: onEncrypt', () => { const generator = keyRingFactory({ async onEncrypt (material: NodeEncryptionMaterial) { return material - .setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + .setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) .addEncryptedDataKey(edk0, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) }, onDecrypt: never @@ -172,7 +172,7 @@ describe('MultiKeyring: onEncrypt', () => { expect(test.keyringTrace).to.be.an('Array').with.lengthOf(2) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) }) it('Precondition: A Generator Keyring *must* ensure generated material.', async () => { @@ -218,7 +218,7 @@ describe('MultiKeyring: onEncrypt', () => { }) const mkeyring = new MultiKeyringNode({ generator }) - const material = new NodeEncryptionMaterial(suite, {}).setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + const material = new NodeEncryptionMaterial(suite, {}).setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) await mkeyring.onEncrypt(material) }) @@ -235,7 +235,7 @@ describe('MultiKeyring: onEncrypt', () => { }) const mkeyring = new MultiKeyringNode({ children: [ child ] }) - const material = new NodeEncryptionMaterial(suite, {}).setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + const material = new NodeEncryptionMaterial(suite, {}).setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) await mkeyring.onEncrypt(material) }) @@ -250,7 +250,7 @@ describe('MultiKeyring: onDecrypt', () => { const generator = keyRingFactory({ async onDecrypt (material: NodeDecryptionMaterial /*, encryptedDataKeys: EncryptedDataKey[] */) { - return material.setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + return material.setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) }, onEncrypt: never }) @@ -263,7 +263,7 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.keyringTrace[0] === keyringTrace0).to.equal(true) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) }) it('calls children.onDecrypt', async () => { @@ -274,7 +274,7 @@ describe('MultiKeyring: onDecrypt', () => { const child = keyRingFactory({ async onDecrypt (material: NodeDecryptionMaterial /*, encryptedDataKeys: EncryptedDataKey[] */) { - return material.setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + return material.setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) }, onEncrypt: never }) @@ -287,7 +287,7 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.keyringTrace[0] === keyringTrace0).to.equal(true) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) }) it('Check for early return (Postcondition): Do not attempt to decrypt once I have a valid key.', async () => { @@ -298,7 +298,7 @@ describe('MultiKeyring: onDecrypt', () => { const child = keyRingFactory({ async onDecrypt (material: NodeDecryptionMaterial /*, encryptedDataKeys: EncryptedDataKey[] */) { - return material.setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + return material.setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) }, onEncrypt: never }) @@ -320,7 +320,7 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.keyringTrace[0] === keyringTrace0).to.equal(true) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) expect(notCalled).to.equal(true) }) @@ -332,7 +332,7 @@ describe('MultiKeyring: onDecrypt', () => { const child = keyRingFactory({ async onDecrypt (material: NodeDecryptionMaterial /*, encryptedDataKeys: EncryptedDataKey[] */) { - return material.setUnencryptedDataKey(unencryptedDataKey, keyringTrace0) + return material.setUnencryptedDataKey(new Uint8Array(unencryptedDataKey), keyringTrace0) }, onEncrypt: never }) @@ -355,7 +355,7 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.keyringTrace[0] === keyringTrace0).to.equal(true) // Make sure the unencrypted data key match - expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey) expect(called).to.equal(true) }) }) diff --git a/modules/raw-aes-keyring-browser/src/raw_aes_keyring_browser.ts b/modules/raw-aes-keyring-browser/src/raw_aes_keyring_browser.ts index 777bc1227..4a6b85494 100644 --- a/modules/raw-aes-keyring-browser/src/raw_aes_keyring_browser.ts +++ b/modules/raw-aes-keyring-browser/src/raw_aes_keyring_browser.ts @@ -25,6 +25,7 @@ import { WebCryptoAlgorithmSuite, // eslint-disable-line no-unused-vars getSubtleFunction, _importCryptoKey, + unwrapDataKey, importForWebCryptoEncryptionMaterial, importForWebCryptoDecryptionMaterial } from '@aws-crypto/material-management-browser' @@ -161,7 +162,7 @@ async function aesGcmWrapKey ( const kdfGetSubtleEncrypt = getSubtleFunction(wrappingMaterial, backend, 'encrypt') const info = new Uint8Array() - const dataKey = material.getUnencryptedDataKey() + const dataKey = unwrapDataKey(material.getUnencryptedDataKey()) const buffer = await kdfGetSubtleEncrypt(info)(iv, aad)(dataKey) const ciphertext = new Uint8Array(buffer, 0, buffer.byteLength - material.suite.tagLength / 8) const authTag = new Uint8Array(buffer, buffer.byteLength - material.suite.tagLength / 8) 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 af46996f6..3b3eb1ad0 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 @@ -22,6 +22,7 @@ import { KeyringTraceFlag, immutableClass, readOnlyProperty, + unwrapDataKey, NodeAlgorithmSuite // eslint-disable-line no-unused-vars } from '@aws-crypto/material-management-node' import { randomBytes, createCipheriv, createDecipheriv } from 'crypto' @@ -141,7 +142,7 @@ function aesGcmWrapKey ( const iv = randomBytes(ivLength) const wrappingDataKey = wrappingMaterial.getUnencryptedDataKey() - const dataKey = material.getUnencryptedDataKey() + const dataKey = unwrapDataKey(material.getUnencryptedDataKey()) const cipher = createCipheriv(encryption, wrappingDataKey, iv) .setAAD(aad) @@ -182,7 +183,8 @@ function aesGcmUnwrapKey ( const { authTag, ciphertext, iv } = rawAesEncryptedParts(material.suite, keyName, edk) const { encryption } = wrappingMaterial.suite - const decipher = createDecipheriv(encryption, wrappingMaterial.getUnencryptedDataKey(), iv) + // createDecipheriv is incorrectly typed in @types/node. It should take key: CipherKey, not key: BinaryLike + const decipher = createDecipheriv(encryption, wrappingMaterial.getUnencryptedDataKey() as any, iv) .setAAD(aad) .setAuthTag(authTag) // Buffer.concat will use the shared buffer space, and the resultant buffer will have a byteOffset... 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 0851e1798..52652750b 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 @@ -24,7 +24,8 @@ import { NodeAlgorithmSuite, AlgorithmSuiteIdentifier, EncryptedDataKey, // eslint-disable-line no-unused-vars - NodeDecryptionMaterial + NodeDecryptionMaterial, + unwrapDataKey } from '@aws-crypto/material-management-node' chai.use(chaiAsPromised) @@ -119,7 +120,7 @@ describe('RawAesKeyringNode encrypt/decrypt', () => { const material = new NodeEncryptionMaterial(suite, {}) const test = await keyring.onEncrypt(material) expect(test.hasValidKey()).to.equal(true) - const udk = test.getUnencryptedDataKey() + const udk = unwrapDataKey(test.getUnencryptedDataKey()) expect(udk).to.have.lengthOf(suite.keyLengthBytes) expect(test.encryptedDataKeys).to.have.lengthOf(1) const [edk] = test.encryptedDataKeys diff --git a/modules/raw-keyring/src/raw_aes_material.ts b/modules/raw-keyring/src/raw_aes_material.ts index 08e70435b..710fe48ef 100644 --- a/modules/raw-keyring/src/raw_aes_material.ts +++ b/modules/raw-keyring/src/raw_aes_material.ts @@ -31,6 +31,7 @@ import { WebCryptoAlgorithmSuite, AwsEsdkJsCryptoKey, // eslint-disable-line no-unused-vars AwsEsdkJsKeyUsage, // eslint-disable-line no-unused-vars + AwsEsdkKeyObject, // eslint-disable-line no-unused-vars KeyringTrace, // eslint-disable-line no-unused-vars KeyringTraceFlag, needs, @@ -45,8 +46,8 @@ import { export class NodeRawAesMaterial implements Readonly> { suite: NodeAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => NodeRawAesMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => NodeRawAesMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => NodeRawAesMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number @@ -75,8 +76,8 @@ export class WebCryptoRawAesMaterial implements Readonly>, Readonly> { suite: WebCryptoAlgorithmSuite - setUnencryptedDataKey!: (dataKey: Uint8Array, trace: KeyringTrace) => WebCryptoRawAesMaterial - getUnencryptedDataKey!: () => Uint8Array + setUnencryptedDataKey!: (dataKey: Uint8Array|AwsEsdkKeyObject, trace: KeyringTrace) => WebCryptoRawAesMaterial + getUnencryptedDataKey!: () => Uint8Array|AwsEsdkKeyObject zeroUnencryptedDataKey!: () => WebCryptoRawAesMaterial hasUnencryptedDataKey!: boolean unencryptedDataKeyLength!: number diff --git a/modules/raw-keyring/test/raw_keyring_decorators.test.ts b/modules/raw-keyring/test/raw_keyring_decorators.test.ts index b1d81de45..964c94a9b 100644 --- a/modules/raw-keyring/test/raw_keyring_decorators.test.ts +++ b/modules/raw-keyring/test/raw_keyring_decorators.test.ts @@ -18,7 +18,7 @@ import { expect } from 'chai' import 'mocha' import { _onEncrypt, _onDecrypt } from '../src/raw_keyring_decorators' -import { AlgorithmSuiteIdentifier, NodeEncryptionMaterial, NodeAlgorithmSuite, KeyringTraceFlag, NodeDecryptionMaterial, EncryptedDataKey } from '@aws-crypto/material-management' +import { AlgorithmSuiteIdentifier, NodeEncryptionMaterial, NodeAlgorithmSuite, KeyringTraceFlag, NodeDecryptionMaterial, EncryptedDataKey, unwrapDataKey } from '@aws-crypto/material-management' describe('_onEncrypt', () => { it('will create UnencryptedDataKey and call _wrapKey', async () => { @@ -41,7 +41,7 @@ describe('_onEncrypt', () => { } as any const test = await testKeyring._onEncrypt(material) - expect(test.getUnencryptedDataKey()).to.deep.equal(new Uint8Array(Array(suite.keyLengthBytes).fill(1))) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(new Uint8Array(Array(suite.keyLengthBytes).fill(1))) expect(test.keyringTrace).to.have.lengthOf(1) expect(test.keyringTrace[0].keyName).to.equal(keyName) expect(test.keyringTrace[0].keyNamespace).to.equal(keyNamespace) @@ -55,7 +55,7 @@ describe('_onEncrypt', () => { const keyName = 'keyName' const keyNamespace = 'keyNamespace' const material = new NodeEncryptionMaterial(suite, {}) - .setUnencryptedDataKey(udk, { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) + .setUnencryptedDataKey(new Uint8Array(udk), { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }) let wrapCalled = 0 const notRandomBytes = async () => { throw new Error('never') } const _wrapKey = (material: any) => { @@ -71,7 +71,7 @@ describe('_onEncrypt', () => { } as any const test = await testKeyring._onEncrypt(material) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk) expect(wrapCalled).to.equal(1) }) }) @@ -99,7 +99,7 @@ describe('_onDecrypt', () => { const _unwrapKey = (material: NodeDecryptionMaterial) => { unwrapCalled += 1 return material - .setUnencryptedDataKey(udk, { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY }) + .setUnencryptedDataKey(new Uint8Array(udk), { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY }) } const testKeyring = { @@ -111,7 +111,7 @@ describe('_onDecrypt', () => { } as any const test = await testKeyring._onDecrypt(material, [edk]) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk) expect(test.keyringTrace).to.have.lengthOf(1) expect(test.keyringTrace[0].keyName).to.equal(keyName) expect(test.keyringTrace[0].keyNamespace).to.equal(keyNamespace) @@ -126,7 +126,7 @@ describe('_onDecrypt', () => { const keyName = 'keyName' const keyNamespace = 'keyNamespace' const material = new NodeDecryptionMaterial(suite, {}) - .setUnencryptedDataKey(udk, { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY }) + .setUnencryptedDataKey(new Uint8Array(udk), { keyName, keyNamespace, flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY }) const edk = new EncryptedDataKey({ providerId: keyName, @@ -154,7 +154,7 @@ describe('_onDecrypt', () => { } as any const test = await testKeyring._onDecrypt(material, [edk]) - expect(test.getUnencryptedDataKey()).to.deep.equal(udk) + expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(udk) expect(unwrapCalled).to.equal(0) expect(filterCalled).to.equal(0) }) diff --git a/modules/raw-rsa-keyring-browser/src/raw_rsa_keyring_web_crypto.ts b/modules/raw-rsa-keyring-browser/src/raw_rsa_keyring_web_crypto.ts index 943e97789..dc26e4ccd 100644 --- a/modules/raw-rsa-keyring-browser/src/raw_rsa_keyring_web_crypto.ts +++ b/modules/raw-rsa-keyring-browser/src/raw_rsa_keyring_web_crypto.ts @@ -26,6 +26,7 @@ import { bytes2JWK, keyUsageForMaterial, importForWebCryptoEncryptionMaterial, + unwrapDataKey, MixedBackendCryptoKey, // eslint-disable-line no-unused-vars WebCryptoAlgorithmSuite // eslint-disable-line no-unused-vars } from '@aws-crypto/material-management-browser' @@ -78,7 +79,7 @@ export class RawRsaKeyringWebCrypto extends KeyringWebCrypto { const { encryption } = material.suite const importFormat = 'jwk' const keyUsages: KeyUsage[] = ['wrapKey'] // limit the use of this key (*not* decrypt, encrypt, deriveKey) - const jwk = bytes2JWK(material.getUnencryptedDataKey()) + const jwk = bytes2JWK(unwrapDataKey(material.getUnencryptedDataKey())) const cryptoKey = await subtle.importKey(importFormat, jwk, encryption, extractable, keyUsages) const wrapFormat = 'raw' diff --git a/modules/raw-rsa-keyring-node/src/raw_rsa_keyring_node.ts b/modules/raw-rsa-keyring-node/src/raw_rsa_keyring_node.ts index c5e3736a8..475285cc4 100644 --- a/modules/raw-rsa-keyring-node/src/raw_rsa_keyring_node.ts +++ b/modules/raw-rsa-keyring-node/src/raw_rsa_keyring_node.ts @@ -23,6 +23,8 @@ import { KeyringTraceFlag, immutableClass, readOnlyProperty, + unwrapDataKey, + AwsEsdkKeyObject, // eslint-disable-line no-unused-vars NodeAlgorithmSuite // eslint-disable-line no-unused-vars } from '@aws-crypto/material-management-node' @@ -51,8 +53,8 @@ import { * or more complicated options... Thoughts? */ interface RsaKey { - publicKey?: string | Buffer - privateKey?: string | Buffer + publicKey?: string | Buffer | AwsEsdkKeyObject + privateKey?: string | Buffer | AwsEsdkKeyObject } export type RawRsaKeyringNodeInput = { @@ -86,7 +88,7 @@ export class RawRsaKeyringNode extends KeyringNode { const _wrapKey = async (material: NodeEncryptionMaterial) => { /* Precondition: Public key must be defined to support encrypt. */ if (!publicKey) throw new Error('No public key defined in constructor. Encrypt disabled.') - const { buffer, byteOffset, byteLength } = material.getUnencryptedDataKey() + const { buffer, byteOffset, byteLength } = unwrapDataKey(material.getUnencryptedDataKey()) const encryptedDataKey = publicEncrypt( { key: publicKey, padding }, Buffer.from(buffer, byteOffset, byteLength)) diff --git a/modules/raw-rsa-keyring-node/test/raw_rsa_keyring_node.test.ts b/modules/raw-rsa-keyring-node/test/raw_rsa_keyring_node.test.ts index 1e4bce61c..58376d99c 100644 --- a/modules/raw-rsa-keyring-node/test/raw_rsa_keyring_node.test.ts +++ b/modules/raw-rsa-keyring-node/test/raw_rsa_keyring_node.test.ts @@ -27,7 +27,8 @@ import { NodeAlgorithmSuite, AlgorithmSuiteIdentifier, EncryptedDataKey, // eslint-disable-line no-unused-vars - NodeDecryptionMaterial + NodeDecryptionMaterial, + unwrapDataKey } from '@aws-crypto/material-management-node' chai.use(chaiAsPromised) @@ -140,7 +141,7 @@ describe('RawRsaKeyringWebCrypto encrypt/decrypt', () => { const material = new NodeEncryptionMaterial(suite, {}) const test = await keyring.onEncrypt(material) expect(test.hasValidKey()).to.equal(true) - const udk = test.getUnencryptedDataKey() + const udk = unwrapDataKey(test.getUnencryptedDataKey()) expect(udk).to.have.lengthOf(suite.keyLengthBytes) expect(test.encryptedDataKeys).to.have.lengthOf(1) const [edk] = test.encryptedDataKeys From 025a68dbe55f3e199878d4fbcb57c0a99cfd008f Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Fri, 30 Aug 2019 14:18:55 -0700 Subject: [PATCH 2/4] comments --- modules/material-management-node/src/material_helpers.ts | 2 +- modules/material-management/src/types.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/material-management-node/src/material_helpers.ts b/modules/material-management-node/src/material_helpers.ts index 81108111f..2be3fce8a 100644 --- a/modules/material-management-node/src/material_helpers.ts +++ b/modules/material-management-node/src/material_helpers.ts @@ -216,7 +216,7 @@ export function nodeKdf (material: NodeEncryptionMaterial|NodeDecryptionMaterial info instanceof Uint8Array, 'Invalid HKDF values.' ) - /* The unwrap is done once we *know* that a KDF in required. + /* The unwrap is done once we *know* that a KDF is required. * If we unwrapped before everything will work, * but we may be creating new copies of the unencrypted data key (export). */ diff --git a/modules/material-management/src/types.ts b/modules/material-management/src/types.ts index 4528b82dc..3056edd69 100644 --- a/modules/material-management/src/types.ts +++ b/modules/material-management/src/types.ts @@ -66,6 +66,10 @@ export type DecryptionMaterial = Suite extends WebCryptoAlgorithmSuite ? WebCryptoDecryptionMaterial : never +/* These are copies of the v12 Node.js types. + * I copied them here to avoid exporting v12 types + * and forcing consumers to install/use v12 in their projects. + */ export type AwsEsdkKeyObjectType = 'secret' | 'public' | 'private' export type AwsEsdkKeyFormat = 'pem' | 'der' export type AwsEsdkKeyType = 'rsa' | 'dsa' | 'ec' From 0c0f596ba49ade3c825d141598b8e614792e6af0 Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Fri, 30 Aug 2019 15:00:15 -0700 Subject: [PATCH 3/4] better test --- .../test/cryptographic_material.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/material-management/test/cryptographic_material.test.ts b/modules/material-management/test/cryptographic_material.test.ts index 37572f80f..6be93d1a3 100644 --- a/modules/material-management/test/cryptographic_material.test.ts +++ b/modules/material-management/test/cryptographic_material.test.ts @@ -182,11 +182,13 @@ describe('decorateCryptographicMaterial', () => { test[0] = 12 expect(() => { const udk = unwrapDataKey(material.getUnencryptedDataKey()) - /* This should NOT be true. - * If the udk is a KeyObject then the change above was on independent memory. - * This check follows the code, and is *intended* to fail. - */ - expect(udk[0]).to.equal(12) + if (supportsKeyObject) { + /* This should NOT be true. + * If the udk is a KeyObject then the change above was on independent memory. + * This check follows the code, and is *intended* to fail. + */ + expect(udk[0]).to.equal(12) + } }).to.throw() }) }) From 4cb2d8bcf92e41a4e39bb62f7fc754209bdcd3da Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Fri, 30 Aug 2019 15:19:08 -0700 Subject: [PATCH 4/4] lint --- modules/material-management/test/cryptographic_material.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/test/cryptographic_material.test.ts b/modules/material-management/test/cryptographic_material.test.ts index 6be93d1a3..03b297817 100644 --- a/modules/material-management/test/cryptographic_material.test.ts +++ b/modules/material-management/test/cryptographic_material.test.ts @@ -183,7 +183,7 @@ describe('decorateCryptographicMaterial', () => { expect(() => { const udk = unwrapDataKey(material.getUnencryptedDataKey()) if (supportsKeyObject) { - /* This should NOT be true. + /* This should NOT be true. * If the udk is a KeyObject then the change above was on independent memory. * This check follows the code, and is *intended* to fail. */