fix: KeyringTraceFlag requirements and data key caching (#210)

seebees · web-flow · commit 7dfa1ae1a469 · 2019-09-19T10:27:22.000-07:00
cloneMaterial incorectly required WRAPPING_KEY_GENERATED_DATA_KEY.
in the case of DecryptionMaterials, this should never be the case.
When I wrote the tests, I did not take this case into consideration
and simply set the generate and decrypt flags on the same keyringTrace.

The organization of the traces has also changed.
There is now a single trace for each “operation”.
This mean that a trace will exists for the set operation
as well as the encrypted data key.
This lets me require only valid trace flags for each operation.
I simplified the cloneMaterial function to rely on the set methods
`setUnencryptedDataKey` and `addEncryptedDataKey`
to deny invalid fields.
I also, organize the KeyringTraceFlags to determin which flags are valid.

The KMS Keyring needed to be updated
to make sure that the trace contains the WRAPPING_KEY_SIGNED_ENC_CTX
flag.

Tests are updated, and added.
diff --git a/modules/cache-material/test/caching_cryptographic_materials_decorators.test.ts b/modules/cache-material/test/caching_cryptographic_materials_decorators.test.ts
@@ -198,22 +198,27 @@ describe('Cryptographic Material Functions', () => {
 
   const nodeSuite = new NodeAlgorithmSuite(suiteId)
   const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
-  const trace = {
+  const encryptTrace = {
     keyNamespace: 'keyNamespace',
     keyName: 'keyName',
-    flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
+    flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
+  }
+  const decryptTrace = {
+    keyNamespace: 'keyNamespace',
+    keyName: 'keyName',
+    flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
   }
 
   const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
   const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })
 
   const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
-    .setUnencryptedDataKey(udk128, trace)
+    .setUnencryptedDataKey(udk128, encryptTrace)
     .addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
     .addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
 
   const decryptionMaterial = new NodeDecryptionMaterial(nodeSuite, {})
-    .setUnencryptedDataKey(udk128, trace)
+    .setUnencryptedDataKey(udk128, decryptTrace)
 
   const context = {}
 
@@ -343,17 +348,17 @@ describe('Cryptographic Material Functions', () => {
 
       const nodeSuite = new NodeAlgorithmSuite(suiteId)
       const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
-      const trace = {
+      const encryptTrace = {
         keyNamespace: 'keyNamespace',
         keyName: 'keyName',
-        flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
+        flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
       }
 
       const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
       const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })
 
       const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
-        .setUnencryptedDataKey(udk128, trace)
+        .setUnencryptedDataKey(udk128, encryptTrace)
         .addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
         .addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
 
@@ -401,17 +406,17 @@ describe('Cryptographic Material Functions', () => {
 
       const nodeSuite = new NodeAlgorithmSuite(suiteId)
       const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
-      const trace = {
+      const encryptTrace = {
         keyNamespace: 'keyNamespace',
         keyName: 'keyName',
-        flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
+        flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
       }
 
       const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
       const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })
 
       const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
-        .setUnencryptedDataKey(udk128, trace)
+        .setUnencryptedDataKey(udk128, encryptTrace)
         .addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
         .addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
 
diff --git a/modules/kms-keyring/src/kms_keyring.ts b/modules/kms-keyring/src/kms_keyring.ts
@@ -123,7 +123,8 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
           * See cryptographic_materials as setUnencryptedDataKey will throw in this case.
           */
           .setUnencryptedDataKey(dataKey.Plaintext, trace)
-          .addEncryptedDataKey(kmsResponseToEncryptedDataKey(dataKey), KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
+          .addEncryptedDataKey(kmsResponseToEncryptedDataKey(dataKey),
+            KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
       } else if (generatorKeyId) {
         keyIds.unshift(generatorKeyId)
       }
diff --git a/modules/kms-keyring/test/kms_keyring.onencrypt.test.ts b/modules/kms-keyring/test/kms_keyring.onencrypt.test.ts
@@ -81,17 +81,22 @@ describe('KmsKeyring: _onEncrypt', () => {
     expect(edkEncrypt.providerId).to.equal('aws-kms')
     expect(edkEncrypt.providerInfo).to.equal(encryptKmsKey)
 
-    expect(material.keyringTrace).to.have.lengthOf(2)
-    const [traceGenerate, traceEncrypt] = material.keyringTrace
+    expect(material.keyringTrace).to.have.lengthOf(3)
+    const [traceGenerate, traceEncrypt1, traceEncrypt2] = material.keyringTrace
     expect(traceGenerate.keyNamespace).to.equal('aws-kms')
     expect(traceGenerate.keyName).to.equal(generatorKeyId)
     expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
     expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
     expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
-    expect(traceEncrypt.keyNamespace).to.equal('aws-kms')
-    expect(traceEncrypt.keyName).to.equal(encryptKmsKey)
-    expect(traceEncrypt.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
-    expect(traceEncrypt.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
+
+    expect(traceEncrypt1.keyNamespace).to.equal('aws-kms')
+    expect(traceEncrypt1.keyName).to.equal(generatorKeyId)
+    expect(traceEncrypt1.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
+    expect(traceEncrypt1.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
+    expect(traceEncrypt2.keyNamespace).to.equal('aws-kms')
+    expect(traceEncrypt2.keyName).to.equal(encryptKmsKey)
+    expect(traceEncrypt2.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
+    expect(traceEncrypt2.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
   })
 
   it('Check for early return (Postcondition): Discovery Keyrings do not encrypt.', async () => {
diff --git a/modules/material-management/src/clone_cryptographic_material.ts b/modules/material-management/src/clone_cryptographic_material.ts
@@ -27,7 +27,7 @@ import {
 import {
   AwsEsdkKeyObject // eslint-disable-line no-unused-vars
 } from './types'
-import { KeyringTraceFlag } from './keyring_trace'
+import { needs } from './needs'
 
 type Material = NodeEncryptionMaterial|NodeDecryptionMaterial|WebCryptoEncryptionMaterial|WebCryptoDecryptionMaterial
 
@@ -42,36 +42,49 @@ export function cloneMaterial<M extends Material> (source: M): M {
       ? new WebCryptoEncryptionMaterial(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.
+  /* The setTrace _must_ be the first trace,
+   * If the material is an EncryptionMaterial
+   * then the data key *must* have been generated.
+   * If the material is DecryptionMaterial
+   * then the data key *must* have been decrypted.
+   * i.e. the required flags are:
+   * WRAPPING_KEY_GENERATED_DATA_KEY, WRAPPING_KEY_DECRYPTED_DATA_KEY
+   * These are controlled by the material itself.
+   * Furthermore, subsequent trace entries,
+   * *must* be in the same order as the added encrypted data keys.
+   * See cryptographic_materials.ts `decorateCryptographicMaterial`, `decorateWebCryptoMaterial`.
    */
-  const trace = source.keyringTrace.find(({ flags }) => flags & KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
+  const [setTrace, ...traces] = source.keyringTrace.slice()
 
   if (source.hasUnencryptedDataKey) {
     const udk = cloneUnencryptedDataKey(source.getUnencryptedDataKey())
-    if (!trace) throw new Error('Malformed source material.')
-    clone.setUnencryptedDataKey(udk, trace)
+    clone.setUnencryptedDataKey(udk, setTrace)
   }
 
   if ((<WebCryptoDecryptionMaterial>source).hasCryptoKey) {
     const cryptoKey = (<WebCryptoDecryptionMaterial>source).getCryptoKey()
-    if (!trace) throw new Error('Malformed source material.')
     ;(<WebCryptoDecryptionMaterial>clone)
-      .setCryptoKey(cryptoKey, trace)
+      .setCryptoKey(cryptoKey, setTrace)
   }
 
   if (isEncryptionMaterial(source) && isEncryptionMaterial(clone)) {
-    source.encryptedDataKeys.forEach(edk => {
+    const encryptedDataKeys = source.encryptedDataKeys
+    /* Precondition: For each encrypted data key, there must be a trace. */
+    needs(encryptedDataKeys.length === traces.length, 'KeyringTrace length does not match encrypted data keys.')
+    encryptedDataKeys.forEach((edk, i) => {
       const { providerInfo, providerId } = edk
-      const trace = source.keyringTrace.find(({ keyNamespace, keyName }) => keyName === providerInfo && keyNamespace === providerId)
-      if (!trace) throw new Error('Malformed Encrypted Data Key')
-      clone.addEncryptedDataKey(edk, trace.flags)
+      const { keyNamespace, keyName, flags } = traces[i]
+      /* Precondition: The traces must be in the same order as the encrypted data keys. */
+      needs(keyName === providerInfo && keyNamespace === providerId, 'Keyring trace does not match encrypted data key.')
+      clone.addEncryptedDataKey(edk, flags)
     })
 
     if (source.suite.signatureCurve && source.signatureKey) {
       clone.setSignatureKey(source.signatureKey)
     }
   } else if (isDecryptionMaterial(source) && isDecryptionMaterial(clone)) {
+    /* Precondition: On Decrypt there must not be any additional traces other than the setTrace. */
+    needs(!traces.length, 'Only 1 trace is valid on DecryptionMaterials.')
     if (source.suite.signatureCurve && source.verificationKey) {
       clone.setVerificationKey(source.verificationKey)
     }
diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts
@@ -287,7 +287,20 @@ export function isDecryptionMaterial (obj: any): obj is WebCryptoDecryptionMater
   return (obj instanceof WebCryptoDecryptionMaterial) || (obj instanceof NodeDecryptionMaterial)
 }
 
-export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>> (material: T, setFlags: KeyringTraceFlag) {
+export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>> (material: T, setFlag: KeyringTraceFlag) {
+  /* Precondition: setFlag must be in the set of KeyringTraceFlag.SET_FLAGS. */
+  needs(setFlag & KeyringTraceFlag.SET_FLAGS, 'Invalid setFlag')
+  /* When a KeyringTraceFlag is passed to setUnencryptedDataKey,
+   * it must be valid for the type of material.
+   * It is invalid to claim that EncryptionMaterial were decrypted.
+   */
+  const deniedSetFlags = (KeyringTraceFlag.SET_FLAGS ^ setFlag) | (
+    setFlag === KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
+      ? KeyringTraceFlag.DECRYPT_FLAGS
+      : setFlag === KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
+        ? KeyringTraceFlag.ENCRYPT_FLAGS
+        : 0)
+
   let unencryptedDataKeyZeroed = false
   let unencryptedDataKey: AwsEsdkKeyObject | Uint8Array
   // This copy of the unencryptedDataKey is stored to insure that the
@@ -390,11 +403,16 @@ export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>
     /* 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')
+    needs(trace.flags & setFlag, 'Required KeyringTraceFlag not set')
+    /* Precondition: Only valid flags are allowed.
+     * An unencrypted data key can not be both generated and decrypted.
+     */
+    needs(!(trace.flags & deniedSetFlags), 'Invalid KeyringTraceFlags set.')
   }
 }
 
 export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (material: T) {
+  const deniedEncryptFlags = KeyringTraceFlag.SET_FLAGS | KeyringTraceFlag.DECRYPT_FLAGS
   const encryptedDataKeys: EncryptedDataKey[] = []
   let signatureKey: Readonly<SignatureKey>|undefined
 
@@ -412,16 +430,18 @@ export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (mat
 
     /* Precondition: flags must indicate that the key was encrypted. */
     needs(flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY, 'Encrypted data key flag must be set.')
-    /* When the unencrypted data key is first set, a given wrapping key may or may not also encrypt that key.
-     * This means that the first EDK that is added may already have a trace.
-     * The flags for the EDK and the existing trace should be merged iif this is the first EDK
-     * and the only existing trace corresponds to this EDK.
+
+    /* Precondition: flags must not include a setFlag or a decrypt flag.
+     * The setFlag is reserved for setting the unencrypted data key
+     * and must only occur once in the set of KeyringTrace flags.
+     * The two setFlags in use are:
+     * KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
+     * KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
+     *
+     * KeyringTraceFlag.WRAPPING_KEY_VERIFIED_ENC_CTX is reserved for the decrypt path
      */
-    if (firstEdkAndTraceMatch(encryptedDataKeys, material.keyringTrace, edk)) {
-      material.keyringTrace[0].flags |= flags
-    } else {
-      material.keyringTrace.push({ keyName: edk.providerInfo, keyNamespace: edk.providerId, flags })
-    }
+    needs(!(flags & deniedEncryptFlags), 'Invalid flag for EncryptedDataKey.')
+    material.keyringTrace.push({ keyName: edk.providerInfo, keyNamespace: edk.providerId, flags })
 
     encryptedDataKeys.push(edk)
     return material
@@ -463,14 +483,6 @@ export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (mat
   return material
 }
 
-/* Verify that the this is the first EDK and that it matches the 1 and only 1 trace. */
-function firstEdkAndTraceMatch (edks: EncryptedDataKey[], traces: KeyringTrace[], edk: EncryptedDataKey) {
-  return edks.length === 0 &&
-  traces.length === 1 &&
-  edk.providerId === traces[0].keyNamespace &&
-  edk.providerInfo === traces[0].keyName
-}
-
 export function decorateDecryptionMaterial<T extends DecryptionMaterial<T>> (material: T) {
   // Verification Key
   let verificationKey: Readonly<VerificationKey>|undefined
diff --git a/modules/material-management/src/keyring_trace.ts b/modules/material-management/src/keyring_trace.ts
@@ -72,4 +72,26 @@ export enum KeyringTraceFlag {
  * Bit flag indicating this wrapping key verified the signature of the encryption context.
  */
   WRAPPING_KEY_VERIFIED_ENC_CTX = (1 << 4), // eslint-disable-line no-unused-vars
+
+  /* KeyringTraceFlags are organized here.
+   * The three groupings are set, encrypt, and decrypt.
+   * An unencrypted data key is set and is required to have a SET_FLAG.
+   * For the encrypt path, the unencrypted data key must be generated.
+   * For the decrypt path, the unencrypted data key must be decrypted.
+   *
+   * A encrypted data key must be encrypted
+   * and the encryption context may be signed.
+   *
+   * When an encrypted data key is decrypted,
+   * the encryption context may be verified.
+   *
+   * This organization is to keep a KeyringTrace for an encrypted data key
+   * for listing the WRAPPING_KEY_VERIFIED_ENC_CTX flag.
+   */
+
+  ENCRYPT_FLAGS = WRAPPING_KEY_ENCRYPTED_DATA_KEY | WRAPPING_KEY_SIGNED_ENC_CTX, // eslint-disable-line no-unused-vars
+
+  SET_FLAGS = WRAPPING_KEY_GENERATED_DATA_KEY | WRAPPING_KEY_DECRYPTED_DATA_KEY, // eslint-disable-line no-unused-vars
+
+  DECRYPT_FLAGS = WRAPPING_KEY_DECRYPTED_DATA_KEY | WRAPPING_KEY_VERIFIED_ENC_CTX, // eslint-disable-line no-unused-vars
 }
diff --git a/modules/material-management/test/clone_cryptographic_material.test.ts b/modules/material-management/test/clone_cryptographic_material.test.ts
diff --git a/modules/material-management/test/cryptographic_material.test.ts b/modules/material-management/test/cryptographic_material.test.ts
diff --git a/modules/material-management/test/multi_keyring.test.ts b/modules/material-management/test/multi_keyring.test.ts
