From 39e0c27b41a3a61cfc4874b3917f4ea96f606fab Mon Sep 17 00:00:00 2001 From: Emery Date: Sat, 3 Aug 2019 21:33:55 -0700 Subject: [PATCH 1/4] fix: Conditions for materials-management Every condition needs a test. This also uncovered some missing conditions and tests. * If a multi keyring has errors AND fails to decrypt the EDK it should throw and error. * Lower the test_conditions threshold --- .../decrypt-node/test/decipher_stream.test.ts | 11 --- .../src/cryptographic_material.ts | 7 +- modules/material-management/src/keyring.ts | 2 +- .../material-management/src/multi_keyring.ts | 12 +++- .../test/algorithm_suites.test.ts | 14 +++- .../test/cryptographic_material.test.ts | 70 ++++++++++++++++++- .../test/multi_keyring.test.ts | 24 ++++++- .../test/signature_key.test.ts | 22 ++++++ .../test/get_import_options.test.ts | 2 +- package.json | 2 +- 10 files changed, 141 insertions(+), 25 deletions(-) diff --git a/modules/decrypt-node/test/decipher_stream.test.ts b/modules/decrypt-node/test/decipher_stream.test.ts index a0e05455e..9a00eee9f 100644 --- a/modules/decrypt-node/test/decipher_stream.test.ts +++ b/modules/decrypt-node/test/decipher_stream.test.ts @@ -118,15 +118,4 @@ describe('getDecipherStream', () => { await expect(test._onAuthTag(Buffer.from([]), () => {})).to.rejectedWith(Error, 'AuthTag before frame.') }) - - it('Precondition: I must have received all content for this frame.', async () => { - /* The fact that I can not figure out how to test this, - * makes me want to remove the condition. - * However, it is very important to make sure that the entire frame has been accumulated. - * I suspect that the fact that frameComplete is a closure, - * means that this is impossible. - * This kind of non-test is also included in the cryptographic materials - * for a similar kind of closure around the udkForVerification. - */ - }) }) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 3ed6faaa5..7c7e4729a 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -58,7 +58,7 @@ function portableTimingSafeEqual (a: Uint8Array, b: Uint8Array) { * Side channel attacks are pernicious and subtle. */ eval('') // eslint-disable-line no-eval - /* Check for early return (Postcondition): Size is well-know information + /* Check for early return (Postcondition) UNTESTED: Size is well-know information. * and does not leak information about contents. */ if (a.byteLength !== b.byteLength) return false @@ -338,8 +338,9 @@ export function decorateCryptographicMaterial udkForVerification.fill(0) unencryptedDataKeyZeroed = true - /* Postcondition: Both unencryptedDataKey and udkForVerification must be either set or unset. + /* Postcondition UNTESTED: Both unencryptedDataKey and udkForVerification must be either set or unset. * If it is ever the case that only one was unset, then something is wrong in a profound way. + * It is not clear how this could ever happen, unless someone is manipulating the OS... */ needs(unsetCount === 0 || unsetCount === 2, 'Either unencryptedDataKey or udkForVerification was not set.') return material @@ -493,7 +494,7 @@ export function decorateWebCryptoMaterial> (mater if (!material.hasUnencryptedDataKey) { /* Precondition: If the CryptoKey is the only version, the trace information must be set here. */ needs(trace && trace.keyName && trace.keyNamespace, 'Malformed KeyringTrace') - /* Precondition: On set the required KeyringTraceFlag must be set. */ + /* Precondition: On setting the CryptoKey the required KeyringTraceFlag must be set. */ needs(trace.flags & setFlags, 'Required KeyringTraceFlag not set') /* If I a setting a cryptoKey without an unencrypted data key, * an unencrypted data should never be set. diff --git a/modules/material-management/src/keyring.ts b/modules/material-management/src/keyring.ts index 5e0c13fc0..0c8631373 100644 --- a/modules/material-management/src/keyring.ts +++ b/modules/material-management/src/keyring.ts @@ -50,7 +50,7 @@ export abstract class Keyring { */ needs(material === _material, 'New EncryptionMaterial instances can not be created.') - /* Postcondition: If this keyring generated data key, it must be the right length. + /* Postcondition UNTESTED: If this keyring generated data key, it must be the right length. * See cryptographic_materials.ts This is handled in setUnencryptedDataKey * this condition is listed here to keep help keep track of important conditions */ diff --git a/modules/material-management/src/multi_keyring.ts b/modules/material-management/src/multi_keyring.ts index 581995f99..7ad237804 100644 --- a/modules/material-management/src/multi_keyring.ts +++ b/modules/material-management/src/multi_keyring.ts @@ -113,6 +113,8 @@ function buildPrivateOnDecrypt () { const children = this.children.slice() if (this.generator) children.unshift(this.generator) + let keyringError = false + for (const keyring of children) { /* Check for early return (Postcondition): Do not attempt to decrypt once I have a valid key. */ if (material.hasValidKey()) return material @@ -120,12 +122,20 @@ function buildPrivateOnDecrypt () { try { await keyring.onDecrypt(material, encryptedDataKeys) } catch (e) { - // there should be some debug here? or wrap? // Failures onDecrypt should not short-circuit the process // If the caller does not have access they may have access // through another Keyring. + keyringError = true } } + + /* Postcondition: Need either a valid data key or no errors. + * If I have a key, decrypt errors should be ignored. + * However, if I was unable to decrypt a key AND I have errors, + * these errors should bubble up. + */ + needs(material.hasValidKey() || !keyringError, 'Keyring errors and no keyring was successfully able to decrypt the encrypted data key.') + return material } } diff --git a/modules/material-management/test/algorithm_suites.test.ts b/modules/material-management/test/algorithm_suites.test.ts index 592751725..fe7435971 100644 --- a/modules/material-management/test/algorithm_suites.test.ts +++ b/modules/material-management/test/algorithm_suites.test.ts @@ -28,10 +28,22 @@ describe('AlgorithmSuiteIdentifier', () => { describe('AlgorithmSuite', () => { it('should not allow an instance', () => { // @ts-ignore Trying to test something that Typescript should deny... - expect(() => new AlgorithmSuite()).to.throw() + expect(() => new AlgorithmSuite()).to.throw('new AlgorithmSuite is not allowed') }) it('prototype should be immutable', () => { expect(Object.isFrozen(AlgorithmSuite.prototype)) }) + + it('Precondition: A algorithm suite specification must be passed.', () => { + class Test extends AlgorithmSuite {} + + expect(() => new Test(undefined as any)).to.throw('Algorithm specification not set.') + }) + + it('Precondition: The Algorithm Suite Identifier must exist.', () => { + class Test extends AlgorithmSuite {} + + expect(() => new Test({ id: 'does not exist' } as any)).to.throw('No suite by that identifier exists.') + }) }) diff --git a/modules/material-management/test/cryptographic_material.test.ts b/modules/material-management/test/cryptographic_material.test.ts index b2c7ee205..669ca728c 100644 --- a/modules/material-management/test/cryptographic_material.test.ts +++ b/modules/material-management/test/cryptographic_material.test.ts @@ -97,12 +97,24 @@ describe('decorateCryptographicMaterial', () => { expect(() => test.unencryptedDataKeyLength).to.throw() }) - it('Precondition: If the unencryptedDataKey has not been set, it should not be settable later.', () => { + it('Precondition: the 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) + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } + test.setUnencryptedDataKey(dataKey, trace) + test.zeroUnencryptedDataKey() + expect(() => test.unencryptedDataKeyLength).to.throw('unencryptedDataKey has been zeroed.') + }) + + it(`Precondition: If the unencryptedDataKey has not been set, it should not be settable later. + Precondition: If the udkForVerification has not been set, it should not be settable later.`, () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const test = decorateCryptographicMaterial(({ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY) test.zeroUnencryptedDataKey() 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() }) @@ -116,8 +128,32 @@ 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 }) - expect(() => test.setUnencryptedDataKey(dataKey)).to.throw() + 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') + }) + + it('Precondition: dataKey should have an ArrayBuffer that *only* stores the 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(new ArrayBuffer(suite.keyLengthBytes + 10), 5, suite.keyLengthBytes).fill(1) + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } + expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('Unencrypted Master Key must be an isolated buffer.') + }) + + it('Precondition: Trace must be set, and the flag must indicate that the data key was generated.', () => { + 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') + }) + + it('Precondition: On set the required KeyringTraceFlag must be set.', () => { + 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) + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX } + expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('Required KeyringTraceFlag not set') }) it('Precondition: The unencryptedDataKey must not have been modified.', () => { @@ -172,6 +208,13 @@ describe('decorateEncryptionMaterial', () => { expect(() => test.addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)).to.throw() }) + it('Precondition: flags must indicate that the key was encrypted.', () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + const test: any = decorateEncryptionMaterial(({ suite, keyringTrace: [], hasUnencryptedDataKey: true })) + const edk = new EncryptedDataKey({ providerId: 'p', providerInfo: 'p', encryptedDataKey: new Uint8Array(3) }) + expect(() => test.addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_VERIFIED_ENC_CTX)).to.throw('Encrypted data key flag must be set.') + }) + it('Precondition: The SignatureKey stored must agree with the algorithm specification.', () => { const suiteWithSig = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA384_ECDSA_P384) const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) @@ -309,6 +352,27 @@ describe('decorateWebCryptoMaterial', () => { expect(() => test.setCryptoKey(key2, trace)).to.throw() }) + it('Precondition: If the CryptoKey is the only version, the trace information must be set here.', () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16_HKDF_SHA256_ECDSA_P256) + const test: any = decorateWebCryptoMaterial(({ suite, validUsages: ['deriveKey'], keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY) + decorateCryptographicMaterial(test, KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY) + + const key: any = { type: 'secret', algorithm: { name: 'HKDF' }, usages: ['deriveKey'], extractable: false } + expect(() => test.setCryptoKey(key, { keyNamespace: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY })).to.throw('Malformed KeyringTrace') + expect(() => test.setCryptoKey(key, { keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY })).to.throw('Malformed KeyringTrace') + expect(() => test.setCryptoKey(key)).to.throw('Malformed KeyringTrace') + }) + + it('Precondition: On setting the CryptoKey the required KeyringTraceFlag must be set.', () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16_HKDF_SHA256_ECDSA_P256) + const test: any = decorateWebCryptoMaterial(({ suite, validUsages: ['deriveKey'], keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY) + decorateCryptographicMaterial(test, KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY) + + const key: any = { type: 'secret', algorithm: { name: 'HKDF' }, usages: ['deriveKey'], extractable: false } + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX } + expect(() => test.setCryptoKey(key, trace)).to.throw('Required KeyringTraceFlag not set') + }) + it('Precondition: dataKey must be a supported type.', () => { const test: any = decorateWebCryptoMaterial(({}), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY) const key: any = {} diff --git a/modules/material-management/test/multi_keyring.test.ts b/modules/material-management/test/multi_keyring.test.ts index b1dc7dacc..79f033e12 100644 --- a/modules/material-management/test/multi_keyring.test.ts +++ b/modules/material-management/test/multi_keyring.test.ts @@ -290,7 +290,7 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) }) - it('does not call subsequent Keyrings after receiving DecryptionMaterial', async () => { + it('Check for early return (Postcondition): Do not attempt to decrypt once I have a valid key.', async () => { const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) const unencryptedDataKey = new Uint8Array(suite.keyLengthBytes) const [edk0, keyringTrace0] = makeEDKandTrace(0) @@ -338,13 +338,14 @@ describe('MultiKeyring: onDecrypt', () => { }) let called = false - const childNotSucceded = keyRingFactory({ + const childNotSucceeded = keyRingFactory({ async onDecrypt () { + // Because this keyring does not return a value, it will result in an error called = true }, onEncrypt: never }) - const children = [childNotSucceded, child] + const children = [childNotSucceeded, child] const mkeyring = new MultiKeyringNode({ children }) @@ -357,6 +358,23 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) expect(called).to.equal(true) }) + + it('Postcondition: Need either a valid data key or no errors.', async () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + const [edk0] = makeEDKandTrace(0) + const material = new NodeDecryptionMaterial(suite, {}) + const childNotSucceeded = keyRingFactory({ + async onDecrypt () { + // Because this keyring does not return a value, it will result in an error + }, + onEncrypt: never + }) + const children = [childNotSucceeded] + + const mkeyring = new MultiKeyringNode({ children }) + + await expect(mkeyring.onDecrypt(material, [edk0])).to.rejectedWith(Error, 'Keyring errors and no keyring was successfully able to decrypt the encrypted data key.') + }) }) function makeEDKandTrace (num: number): [EncryptedDataKey, KeyringTrace] { diff --git a/modules/material-management/test/signature_key.test.ts b/modules/material-management/test/signature_key.test.ts index 8f9c4e31a..0c85d3e26 100644 --- a/modules/material-management/test/signature_key.test.ts +++ b/modules/material-management/test/signature_key.test.ts @@ -49,6 +49,17 @@ describe('SignatureKey', () => { const compressPoint = SignatureKey.encodeCompressPoint(publicKey, suite) expect(compressPoint).to.deep.equal(new Uint8Array(prime256v1CompressedFixture)) }) + + it('Precondition: Do not create a SignatureKey for an algorithm suite that does not have an EC named curve.', () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + expect(() => new SignatureKey(new Uint8Array(3), new Uint8Array(3), suite)).to.throw('Unsupported Algorithm') + }) + + it('Precondition: Do not return a compress point for an algorithm suite that does not have an EC named curve.', () => { + const publicKey = new Uint8Array(prime256v1PublicFixture) + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + expect(() => SignatureKey.encodeCompressPoint(publicKey, suite)).to.throw('Unsupported Algorithm') + }) }) describe('VerificationKey', () => { @@ -65,4 +76,15 @@ describe('VerificationKey', () => { const publicKey = VerificationKey.decodeCompressPoint(compressPoint, suite) expect(publicKey).to.deep.equal(new Uint8Array(prime256v1PublicFixture)) }) + + it('Precondition: Do not create a VerificationKey for an algorithm suite that does not have an EC named curve.', () => { + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + expect(() => new VerificationKey(new Uint8Array(3), suite)).to.throw('Unsupported Algorithm') + }) + + it('Precondition: Do not decode a public key for an algorithm suite that does not have an EC named curve.', () => { + const compressPoint = new Uint8Array(prime256v1CompressedFixture) + const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) + expect(() => VerificationKey.decodeCompressPoint(compressPoint, suite)).to.throw('Unsupported Algorithm') + }) }) diff --git a/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts b/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts index c4d5ef6b5..1ad5cfe11 100644 --- a/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts +++ b/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts @@ -27,7 +27,7 @@ chai.use(chaiAsPromised) const { expect } = chai describe('flattenMixedCryptoKey', () => { - it('Check for early return (Postcondition): empty inputs should return an empty array.', () => { + it('Check for early return (Postcondition): nothing is an empty array.', () => { const test = flattenMixedCryptoKey() expect(test).to.be.an('array').with.lengthOf(0) }) diff --git a/package.json b/package.json index 3c79bf71f..aa840f6ec 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "integration-node-encrypt": "npm run build; lerna run integration_node --stream --no-prefix -- -- encrypt -m $npm_package_config_encryptManifestList -k $npm_package_config_encryptKeyManifest -o $npm_package_config_decryptOracle", "integration-node": "run-s integration-node-*", "integration": "run-s integration-*", - "test_conditions": "./util/bootstrap_tsconfig 23" + "test_conditions": "./util/bootstrap_tsconfig 2" }, "config": { "localTestVectors": "../../aws-encryption-sdk-test-vectors/vectors/awses-decrypt/python-1.3.8.zip", From 958aa662259e1c1fec69d1c0a5fcb8b3423ec3a8 Mon Sep 17 00:00:00 2001 From: Emery Date: Tue, 6 Aug 2019 15:27:36 -0700 Subject: [PATCH 2/4] remove, they all should be done now --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index aa840f6ec..1f4aecf4d 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "integration-node-encrypt": "npm run build; lerna run integration_node --stream --no-prefix -- -- encrypt -m $npm_package_config_encryptManifestList -k $npm_package_config_encryptKeyManifest -o $npm_package_config_decryptOracle", "integration-node": "run-s integration-node-*", "integration": "run-s integration-*", - "test_conditions": "./util/bootstrap_tsconfig 2" + "test_conditions": "./util/bootstrap_tsconfig" }, "config": { "localTestVectors": "../../aws-encryption-sdk-test-vectors/vectors/awses-decrypt/python-1.3.8.zip", From 310dc45fefe693f4ed45de143f12741218e59255 Mon Sep 17 00:00:00 2001 From: Emery Date: Tue, 6 Aug 2019 15:41:07 -0700 Subject: [PATCH 3/4] Remove and move to seperate PR --- .../material-management/src/multi_keyring.ts | 12 +----------- .../test/multi_keyring.test.ts | 17 ----------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/modules/material-management/src/multi_keyring.ts b/modules/material-management/src/multi_keyring.ts index 7ad237804..581995f99 100644 --- a/modules/material-management/src/multi_keyring.ts +++ b/modules/material-management/src/multi_keyring.ts @@ -113,8 +113,6 @@ function buildPrivateOnDecrypt () { const children = this.children.slice() if (this.generator) children.unshift(this.generator) - let keyringError = false - for (const keyring of children) { /* Check for early return (Postcondition): Do not attempt to decrypt once I have a valid key. */ if (material.hasValidKey()) return material @@ -122,20 +120,12 @@ function buildPrivateOnDecrypt () { try { await keyring.onDecrypt(material, encryptedDataKeys) } catch (e) { + // there should be some debug here? or wrap? // Failures onDecrypt should not short-circuit the process // If the caller does not have access they may have access // through another Keyring. - keyringError = true } } - - /* Postcondition: Need either a valid data key or no errors. - * If I have a key, decrypt errors should be ignored. - * However, if I was unable to decrypt a key AND I have errors, - * these errors should bubble up. - */ - needs(material.hasValidKey() || !keyringError, 'Keyring errors and no keyring was successfully able to decrypt the encrypted data key.') - return material } } diff --git a/modules/material-management/test/multi_keyring.test.ts b/modules/material-management/test/multi_keyring.test.ts index 79f033e12..1b15ad3dd 100644 --- a/modules/material-management/test/multi_keyring.test.ts +++ b/modules/material-management/test/multi_keyring.test.ts @@ -358,23 +358,6 @@ describe('MultiKeyring: onDecrypt', () => { expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey) expect(called).to.equal(true) }) - - it('Postcondition: Need either a valid data key or no errors.', async () => { - const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16) - const [edk0] = makeEDKandTrace(0) - const material = new NodeDecryptionMaterial(suite, {}) - const childNotSucceeded = keyRingFactory({ - async onDecrypt () { - // Because this keyring does not return a value, it will result in an error - }, - onEncrypt: never - }) - const children = [childNotSucceeded] - - const mkeyring = new MultiKeyringNode({ children }) - - await expect(mkeyring.onDecrypt(material, [edk0])).to.rejectedWith(Error, 'Keyring errors and no keyring was successfully able to decrypt the encrypted data key.') - }) }) function makeEDKandTrace (num: number): [EncryptedDataKey, KeyringTrace] { From 3a930a64a24d51f1262b4413bd271e7dbebd3e0f Mon Sep 17 00:00:00 2001 From: Emery Date: Tue, 6 Aug 2019 15:56:50 -0700 Subject: [PATCH 4/4] better words --- modules/raw-rsa-keyring-browser/src/get_import_options.ts | 2 +- modules/raw-rsa-keyring-browser/test/get_import_options.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/raw-rsa-keyring-browser/src/get_import_options.ts b/modules/raw-rsa-keyring-browser/src/get_import_options.ts index 6771e71eb..a960ab3a9 100644 --- a/modules/raw-rsa-keyring-browser/src/get_import_options.ts +++ b/modules/raw-rsa-keyring-browser/src/get_import_options.ts @@ -123,7 +123,7 @@ export function verify (...args: RsaWrappingKeyAlgorithm[]) { } export function flattenMixedCryptoKey (key?: CryptoKey|MixedBackendCryptoKey): CryptoKey[] { - /* Check for early return (Postcondition): nothing is an empty array. */ + /* Check for early return (Postcondition): empty inputs should return an empty array. */ if (!key) return [] if (isCryptoKey(key)) return [key] const { nonZeroByteCryptoKey, zeroByteCryptoKey } = key diff --git a/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts b/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts index 1ad5cfe11..c4d5ef6b5 100644 --- a/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts +++ b/modules/raw-rsa-keyring-browser/test/get_import_options.test.ts @@ -27,7 +27,7 @@ chai.use(chaiAsPromised) const { expect } = chai describe('flattenMixedCryptoKey', () => { - it('Check for early return (Postcondition): nothing is an empty array.', () => { + it('Check for early return (Postcondition): empty inputs should return an empty array.', () => { const test = flattenMixedCryptoKey() expect(test).to.be.an('array').with.lengthOf(0) })