Skip to content

Commit 7f7228b

Browse files
authored
fix: Conditions for materials-management (#185)
Every condition needs a test. This also uncovered some missing conditions and tests. * Remove the test_conditions threshold
1 parent b07a084 commit 7f7228b

File tree

9 files changed

+113
-24
lines changed

9 files changed

+113
-24
lines changed

modules/decrypt-node/test/decipher_stream.test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,4 @@ describe('getDecipherStream', () => {
118118

119119
await expect(test._onAuthTag(Buffer.from([]), () => {})).to.rejectedWith(Error, 'AuthTag before frame.')
120120
})
121-
122-
it('Precondition: I must have received all content for this frame.', async () => {
123-
/* The fact that I can not figure out how to test this,
124-
* makes me want to remove the condition.
125-
* However, it is very important to make sure that the entire frame has been accumulated.
126-
* I suspect that the fact that frameComplete is a closure,
127-
* means that this is impossible.
128-
* This kind of non-test is also included in the cryptographic materials
129-
* for a similar kind of closure around the udkForVerification.
130-
*/
131-
})
132121
})

modules/material-management/src/cryptographic_material.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ function portableTimingSafeEqual (a: Uint8Array, b: Uint8Array) {
5858
* Side channel attacks are pernicious and subtle.
5959
*/
6060
eval('') // eslint-disable-line no-eval
61-
/* Check for early return (Postcondition): Size is well-know information
61+
/* Check for early return (Postcondition) UNTESTED: Size is well-know information.
6262
* and does not leak information about contents.
6363
*/
6464
if (a.byteLength !== b.byteLength) return false
@@ -338,8 +338,9 @@ export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>
338338
udkForVerification.fill(0)
339339
unencryptedDataKeyZeroed = true
340340

341-
/* Postcondition: Both unencryptedDataKey and udkForVerification must be either set or unset.
341+
/* Postcondition UNTESTED: Both unencryptedDataKey and udkForVerification must be either set or unset.
342342
* If it is ever the case that only one was unset, then something is wrong in a profound way.
343+
* It is not clear how this could ever happen, unless someone is manipulating the OS...
343344
*/
344345
needs(unsetCount === 0 || unsetCount === 2, 'Either unencryptedDataKey or udkForVerification was not set.')
345346
return material
@@ -493,7 +494,7 @@ export function decorateWebCryptoMaterial<T extends WebCryptoMaterial<T>> (mater
493494
if (!material.hasUnencryptedDataKey) {
494495
/* Precondition: If the CryptoKey is the only version, the trace information must be set here. */
495496
needs(trace && trace.keyName && trace.keyNamespace, 'Malformed KeyringTrace')
496-
/* Precondition: On set the required KeyringTraceFlag must be set. */
497+
/* Precondition: On setting the CryptoKey the required KeyringTraceFlag must be set. */
497498
needs(trace.flags & setFlags, 'Required KeyringTraceFlag not set')
498499
/* If I a setting a cryptoKey without an unencrypted data key,
499500
* an unencrypted data should never be set.

modules/material-management/src/keyring.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export abstract class Keyring<S extends SupportedAlgorithmSuites> {
5050
*/
5151
needs(material === _material, 'New EncryptionMaterial instances can not be created.')
5252

53-
/* Postcondition: If this keyring generated data key, it must be the right length.
53+
/* Postcondition UNTESTED: If this keyring generated data key, it must be the right length.
5454
* See cryptographic_materials.ts This is handled in setUnencryptedDataKey
5555
* this condition is listed here to keep help keep track of important conditions
5656
*/

modules/material-management/test/algorithm_suites.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,22 @@ describe('AlgorithmSuiteIdentifier', () => {
2828
describe('AlgorithmSuite', () => {
2929
it('should not allow an instance', () => {
3030
// @ts-ignore Trying to test something that Typescript should deny...
31-
expect(() => new AlgorithmSuite()).to.throw()
31+
expect(() => new AlgorithmSuite()).to.throw('new AlgorithmSuite is not allowed')
3232
})
3333

3434
it('prototype should be immutable', () => {
3535
expect(Object.isFrozen(AlgorithmSuite.prototype))
3636
})
37+
38+
it('Precondition: A algorithm suite specification must be passed.', () => {
39+
class Test extends AlgorithmSuite {}
40+
41+
expect(() => new Test(undefined as any)).to.throw('Algorithm specification not set.')
42+
})
43+
44+
it('Precondition: The Algorithm Suite Identifier must exist.', () => {
45+
class Test extends AlgorithmSuite {}
46+
47+
expect(() => new Test({ id: 'does not exist' } as any)).to.throw('No suite by that identifier exists.')
48+
})
3749
})

modules/material-management/test/cryptographic_material.test.ts

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,24 @@ describe('decorateCryptographicMaterial', () => {
9797
expect(() => test.unencryptedDataKeyLength).to.throw()
9898
})
9999

100-
it('Precondition: If the unencryptedDataKey has not been set, it should not be settable later.', () => {
100+
it('Precondition: the unencryptedDataKey must not be Zeroed out.', () => {
101+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
102+
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
103+
const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1)
104+
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }
105+
test.setUnencryptedDataKey(dataKey, trace)
106+
test.zeroUnencryptedDataKey()
107+
expect(() => test.unencryptedDataKeyLength).to.throw('unencryptedDataKey has been zeroed.')
108+
})
109+
110+
it(`Precondition: If the unencryptedDataKey has not been set, it should not be settable later.
111+
Precondition: If the udkForVerification has not been set, it should not be settable later.`, () => {
101112
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
102113
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
103114
test.zeroUnencryptedDataKey()
104115
const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1)
105116
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }
117+
// It is very hard to test this perfectly. However, this tests the spirit.
106118
expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw()
107119
})
108120

@@ -116,8 +128,32 @@ describe('decorateCryptographicMaterial', () => {
116128
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
117129
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
118130
const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1)
119-
test.setUnencryptedDataKey(dataKey, { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY })
120-
expect(() => test.setUnencryptedDataKey(dataKey)).to.throw()
131+
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }
132+
test.setUnencryptedDataKey(dataKey, trace)
133+
expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('unencryptedDataKey has already been set')
134+
})
135+
136+
it('Precondition: dataKey should have an ArrayBuffer that *only* stores the key.', () => {
137+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
138+
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
139+
const dataKey = new Uint8Array(new ArrayBuffer(suite.keyLengthBytes + 10), 5, suite.keyLengthBytes).fill(1)
140+
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY }
141+
expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('Unencrypted Master Key must be an isolated buffer.')
142+
})
143+
144+
it('Precondition: Trace must be set, and the flag must indicate that the data key was generated.', () => {
145+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
146+
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
147+
const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1)
148+
expect(() => test.setUnencryptedDataKey(dataKey, {} as any)).to.throw('Malformed KeyringTrace')
149+
})
150+
151+
it('Precondition: On set the required KeyringTraceFlag must be set.', () => {
152+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
153+
const test = decorateCryptographicMaterial((<any>{ suite, keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
154+
const dataKey = new Uint8Array(suite.keyLengthBytes).fill(1)
155+
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX }
156+
expect(() => test.setUnencryptedDataKey(dataKey, trace)).to.throw('Required KeyringTraceFlag not set')
121157
})
122158

123159
it('Precondition: The unencryptedDataKey must not have been modified.', () => {
@@ -172,6 +208,13 @@ describe('decorateEncryptionMaterial', () => {
172208
expect(() => test.addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)).to.throw()
173209
})
174210

211+
it('Precondition: flags must indicate that the key was encrypted.', () => {
212+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
213+
const test: any = decorateEncryptionMaterial((<any>{ suite, keyringTrace: [], hasUnencryptedDataKey: true }))
214+
const edk = new EncryptedDataKey({ providerId: 'p', providerInfo: 'p', encryptedDataKey: new Uint8Array(3) })
215+
expect(() => test.addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_VERIFIED_ENC_CTX)).to.throw('Encrypted data key flag must be set.')
216+
})
217+
175218
it('Precondition: The SignatureKey stored must agree with the algorithm specification.', () => {
176219
const suiteWithSig = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA384_ECDSA_P384)
177220
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
@@ -309,6 +352,27 @@ describe('decorateWebCryptoMaterial', () => {
309352
expect(() => test.setCryptoKey(key2, trace)).to.throw()
310353
})
311354

355+
it('Precondition: If the CryptoKey is the only version, the trace information must be set here.', () => {
356+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16_HKDF_SHA256_ECDSA_P256)
357+
const test: any = decorateWebCryptoMaterial((<any>{ suite, validUsages: ['deriveKey'], keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY)
358+
decorateCryptographicMaterial(test, KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY)
359+
360+
const key: any = { type: 'secret', algorithm: { name: 'HKDF' }, usages: ['deriveKey'], extractable: false }
361+
expect(() => test.setCryptoKey(key, { keyNamespace: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY })).to.throw('Malformed KeyringTrace')
362+
expect(() => test.setCryptoKey(key, { keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY })).to.throw('Malformed KeyringTrace')
363+
expect(() => test.setCryptoKey(key)).to.throw('Malformed KeyringTrace')
364+
})
365+
366+
it('Precondition: On setting the CryptoKey the required KeyringTraceFlag must be set.', () => {
367+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16_HKDF_SHA256_ECDSA_P256)
368+
const test: any = decorateWebCryptoMaterial((<any>{ suite, validUsages: ['deriveKey'], keyringTrace: [] }), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY)
369+
decorateCryptographicMaterial(test, KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY)
370+
371+
const key: any = { type: 'secret', algorithm: { name: 'HKDF' }, usages: ['deriveKey'], extractable: false }
372+
const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX }
373+
expect(() => test.setCryptoKey(key, trace)).to.throw('Required KeyringTraceFlag not set')
374+
})
375+
312376
it('Precondition: dataKey must be a supported type.', () => {
313377
const test: any = decorateWebCryptoMaterial((<any>{}), KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY)
314378
const key: any = {}

modules/material-management/test/multi_keyring.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ describe('MultiKeyring: onDecrypt', () => {
290290
expect(test.getUnencryptedDataKey()).to.deep.equal(unencryptedDataKey)
291291
})
292292

293-
it('does not call subsequent Keyrings after receiving DecryptionMaterial', async () => {
293+
it('Check for early return (Postcondition): Do not attempt to decrypt once I have a valid key.', async () => {
294294
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
295295
const unencryptedDataKey = new Uint8Array(suite.keyLengthBytes)
296296
const [edk0, keyringTrace0] = makeEDKandTrace(0)
@@ -338,13 +338,14 @@ describe('MultiKeyring: onDecrypt', () => {
338338
})
339339

340340
let called = false
341-
const childNotSucceded = keyRingFactory({
341+
const childNotSucceeded = keyRingFactory({
342342
async onDecrypt () {
343+
// Because this keyring does not return a value, it will result in an error
343344
called = true
344345
},
345346
onEncrypt: never
346347
})
347-
const children = [childNotSucceded, child]
348+
const children = [childNotSucceeded, child]
348349

349350
const mkeyring = new MultiKeyringNode({ children })
350351

modules/material-management/test/signature_key.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ describe('SignatureKey', () => {
4949
const compressPoint = SignatureKey.encodeCompressPoint(publicKey, suite)
5050
expect(compressPoint).to.deep.equal(new Uint8Array(prime256v1CompressedFixture))
5151
})
52+
53+
it('Precondition: Do not create a SignatureKey for an algorithm suite that does not have an EC named curve.', () => {
54+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
55+
expect(() => new SignatureKey(new Uint8Array(3), new Uint8Array(3), suite)).to.throw('Unsupported Algorithm')
56+
})
57+
58+
it('Precondition: Do not return a compress point for an algorithm suite that does not have an EC named curve.', () => {
59+
const publicKey = new Uint8Array(prime256v1PublicFixture)
60+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
61+
expect(() => SignatureKey.encodeCompressPoint(publicKey, suite)).to.throw('Unsupported Algorithm')
62+
})
5263
})
5364

5465
describe('VerificationKey', () => {
@@ -65,4 +76,15 @@ describe('VerificationKey', () => {
6576
const publicKey = VerificationKey.decodeCompressPoint(compressPoint, suite)
6677
expect(publicKey).to.deep.equal(new Uint8Array(prime256v1PublicFixture))
6778
})
79+
80+
it('Precondition: Do not create a VerificationKey for an algorithm suite that does not have an EC named curve.', () => {
81+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
82+
expect(() => new VerificationKey(new Uint8Array(3), suite)).to.throw('Unsupported Algorithm')
83+
})
84+
85+
it('Precondition: Do not decode a public key for an algorithm suite that does not have an EC named curve.', () => {
86+
const compressPoint = new Uint8Array(prime256v1CompressedFixture)
87+
const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
88+
expect(() => VerificationKey.decodeCompressPoint(compressPoint, suite)).to.throw('Unsupported Algorithm')
89+
})
6890
})

modules/raw-rsa-keyring-browser/src/get_import_options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export function verify (...args: RsaWrappingKeyAlgorithm[]) {
123123
}
124124

125125
export function flattenMixedCryptoKey (key?: CryptoKey|MixedBackendCryptoKey): CryptoKey[] {
126-
/* Check for early return (Postcondition): nothing is an empty array. */
126+
/* Check for early return (Postcondition): empty inputs should return an empty array. */
127127
if (!key) return []
128128
if (isCryptoKey(key)) return [key]
129129
const { nonZeroByteCryptoKey, zeroByteCryptoKey } = key

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"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",
2727
"integration-node": "run-s integration-node-*",
2828
"integration": "run-s integration-*",
29-
"test_conditions": "./util/bootstrap_tsconfig 23"
29+
"test_conditions": "./util/bootstrap_tsconfig"
3030
},
3131
"config": {
3232
"localTestVectors": "../../aws-encryption-sdk-test-vectors/vectors/awses-decrypt/python-1.3.8.zip",

0 commit comments

Comments
 (0)