Merge branch 'master' into verify_version_and_type

Author: seebees

modules/encrypt-node/src/encrypt.ts

@@ -26,9 +26,15 @@ export async function encrypt (
   plaintext: Buffer|Uint8Array|Readable|string|NodeJS.ReadableStream,
   op: EncryptInput = {}
 ): Promise<EncryptOutput> {
-  const stream = encryptStream(cmm, op)
   const { encoding } = op
+  if (plaintext instanceof Uint8Array) {
+    op.plaintextLength = plaintext.byteLength
+  } else if (typeof plaintext === 'string') {
+    plaintext = Buffer.from(plaintext, encoding)
+    op.plaintextLength = plaintext.byteLength
+  }
 
+  const stream = encryptStream(cmm, op)
   const result: Buffer[] = []
   let messageHeader: MessageHeader|false = false
   stream
@@ -38,8 +44,6 @@ export async function encrypt (
   // This will check both Uint8Array|Buffer
   if (plaintext instanceof Uint8Array) {
     stream.end(plaintext)
-  } else if (typeof plaintext === 'string') {
-    stream.end(Buffer.from(plaintext, encoding))
   } else if (plaintext.readable) {
     plaintext.pipe(stream)
   } else {

modules/encrypt-node/src/encrypt_stream.ts

@@ -78,7 +78,7 @@ export function encryptStream (
 
       wrappingStream.emit('MessageHeader', messageHeader)
 
-      const encryptStream = getFramedEncryptStream(getCipher, messageHeader, dispose)
+      const encryptStream = getFramedEncryptStream(getCipher, messageHeader, dispose, plaintextLength)
       const signatureStream = new SignatureStream(getSigner)
 
       pipeline(encryptStream, signatureStream)

modules/encrypt-node/src/framed_encrypt_stream.ts

@@ -15,7 +15,8 @@
 
 import {
   serializeFactory, aadFactory,
-  MessageHeader // eslint-disable-line no-unused-vars
+  MessageHeader, // eslint-disable-line no-unused-vars
+  Maximum
 } from '@aws-crypto/serialize'
 // @ts-ignore
 import { Transform as PortableTransform } from 'readable-stream'
@@ -49,11 +50,17 @@ const ioTick = () => new Promise(resolve => setImmediate(resolve))
 const noop = () => {}
 type ErrBack = (err?: Error) => void
 
-export function getFramedEncryptStream (getCipher: GetCipher, messageHeader: MessageHeader, dispose: Function) {
+export function getFramedEncryptStream (getCipher: GetCipher, messageHeader: MessageHeader, dispose: Function, plaintextLength?: number) {
   let accumulatingFrame: AccumulatingFrame = { contentLength: 0, content: [], sequenceNumber: 1 }
   let pathologicalDrain: Function = noop
   const { frameLength } = messageHeader
 
+  /* Precondition: plaintextLength must be within bounds.
+   * The Maximum.BYTES_PER_MESSAGE is set to be within Number.MAX_SAFE_INTEGER
+   * See serialize/identifiers.ts enum Maximum for more details.
+   */
+  needs(!plaintextLength || (plaintextLength >= 0 && Maximum.BYTES_PER_MESSAGE >= plaintextLength), 'plaintextLength out of bounds.')
+
   /* Keeping the messageHeader, accumulatingFrame and pathologicalDrain private is the intention here.
    * It is already unlikely that these values could be touched in the current composition of streams,
    * but a different composition may change this.
@@ -63,6 +70,11 @@ export function getFramedEncryptStream (getCipher: GetCipher, messageHeader: Mes
     _transform (chunk: Buffer, encoding: string, callback: ErrBack) {
       const contentLeft = frameLength - accumulatingFrame.contentLength
 
+      /* Precondition: Must not process more than plaintextLength.
+       * The plaintextLength is the MAXIMUM value that can be encrypted.
+       */
+      needs(!plaintextLength || (plaintextLength -= chunk.length) >= 0, 'Encrypted data exceeded plaintextLength.')
+
       /* Check for early return (Postcondition): Have not accumulated a frame. */
       if (contentLeft > chunk.length) {
         // eat more

modules/encrypt-node/test/framed_encrypt_stream.test.ts

@@ -30,6 +30,24 @@ describe('getFramedEncryptStream', () => {
     expect(test._transform).is.a('function')
   })
 
+  it('Precondition: plaintextLength must be within bounds.', () => {
+    const getCipher: any = () => {}
+    expect(() => getFramedEncryptStream(getCipher, {} as any, () => {}, -1)).to.throw(Error, 'plaintextLength out of bounds.')
+    expect(() => getFramedEncryptStream(getCipher, {} as any, () => {}, Number.MAX_SAFE_INTEGER + 1)).to.throw(Error, 'plaintextLength out of bounds.')
+
+    /* Math is hard.
+     * I want to make sure that I don't have an errant off by 1 error.
+     */
+    expect(() => getFramedEncryptStream(getCipher, {} as any, () => {}, Number.MAX_SAFE_INTEGER)).to.not.throw(Error)
+  })
+
+  it('Precondition: Must not process more than plaintextLength.', () => {
+    const getCipher: any = () => {}
+    const test = getFramedEncryptStream(getCipher, { } as any, () => {}, 8)
+
+    expect(() => test._transform(Buffer.from(Array(9)), 'binary', () => {})).to.throw(Error, 'Encrypted data exceeded plaintextLength.')
+  })
+
   it('Check for early return (Postcondition): Have not accumulated a frame.', () => {
     const getCipher: any = () => {}
     const frameLength = 10

modules/kms-keyring-browser/test/kms_keyring_browser.test.ts

@@ -38,7 +38,7 @@ const { expect } = chai
 /* Injected from @aws-sdk/karma-credential-loader. */
 declare const credentials: any
 
-describe('KmsKeyringNode::constructor', () => {
+describe('KmsKeyringBrowser::constructor', () => {
   it('constructor decorates', async () => {
     const generatorKeyId = 'arn:aws:kms:us-west-2:658956600833:alias/EncryptDecrypt'
     const keyArn = 'arn:aws:kms:us-west-2:658956600833:key/b3537ef1-d8dc-4780-9f5a-55776cbb2f7f'
@@ -60,7 +60,7 @@ describe('KmsKeyringNode::constructor', () => {
   })
 })
 
-describe('RawAesKeyringWebCrypto encrypt/decrypt', () => {
+describe('KmsKeyringBrowser encrypt/decrypt', () => {
   const generatorKeyId = 'arn:aws:kms:us-west-2:658956600833:alias/EncryptDecrypt'
   const keyArn = 'arn:aws:kms:us-west-2:658956600833:key/b3537ef1-d8dc-4780-9f5a-55776cbb2f7f'
   const keyIds = [keyArn]

modules/kms-keyring-node/test/kms_keyring_node.test.ts

@@ -49,7 +49,7 @@ describe('KmsKeyringNode::constructor', () => {
   })
 })
 
-describe('RawAesKeyringWebCrypto encrypt/decrypt', () => {
+describe('KmsKeyringNode encrypt/decrypt', () => {
   const generatorKeyId = 'arn:aws:kms:us-west-2:658956600833:alias/EncryptDecrypt'
   const keyArn = 'arn:aws:kms:us-west-2:658956600833:key/b3537ef1-d8dc-4780-9f5a-55776cbb2f7f'
   const keyIds = [keyArn]

modules/kms-keyring/src/kms_keyring.ts

@@ -172,6 +172,8 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
           return this.isDiscovery || keyIds.includes(providerInfo)
         })
 
+      let cmkErrors: Error[] = []
+
       for (const edk of decryptableEDKs) {
         let dataKey: RequiredDecryptResponse|false = false
         try {
@@ -182,10 +184,11 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
             grantTokens
           )
         } catch (e) {
-          // there should be some debug here?  or wrap?
-          // Failures decrypt should not short-circuit the process
-          // If the caller does not have access they may have access
-          // through another Keyring.
+          /* Failures onDecrypt should not short-circuit the process
+           * If the caller does not have access they may have access
+           * through another Keyring.
+           */
+          cmkErrors.push(e)
         }
 
         /* Check for early return (Postcondition): clientProvider may not return a client. */
@@ -204,6 +207,20 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
         return material
       }
 
+      /* Postcondition: A CMK must provide a valid data key or KMS must not have raised any errors.
+       * If I have a data key,
+       * decrypt errors can be ignored.
+       * However, if I was unable to decrypt a data key AND I have errors,
+       * these errors should bubble up.
+       * Otherwise, the only error customers will see is that
+       * the material does not have an unencrypted data key.
+       * So I return a concatenated Error message
+       */
+      needs(material.hasValidKey() || (!material.hasValidKey() && !cmkErrors.length)
+        , cmkErrors
+          .reduce((m, e, i) => `${m} Error #${i + 1} \n ${e.stack} \n`,
+            'Unable to decrypt data key and one or more KMS CMKs had an error. \n '))
+
       return material
     }
   }

modules/kms-keyring/test/kms_keyring.ondecrypt.test.ts

@@ -138,10 +138,20 @@ describe('KmsKeyring: _onDecrypt',
       const discovery = true
       const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
 
+      let edkCount = 0
       const clientProvider: any = () => {
         return { decrypt }
-        function decrypt () {
-          throw new Error('failed to decrypt')
+        function decrypt ({ CiphertextBlob, EncryptionContext, GrantTokens }: any) {
+          if (edkCount === 0) {
+            edkCount += 1
+            throw new Error('failed to decrypt')
+          }
+          expect(EncryptionContext).to.deep.equal(context)
+          expect(GrantTokens).to.equal(grantTokens)
+          return {
+            Plaintext: new Uint8Array(suite.keyLengthBytes),
+            KeyId: Buffer.from(<Uint8Array>CiphertextBlob).toString('utf8')
+          }
         }
       }
       class TestKmsKeyring extends KmsKeyringClass(Keyring as KeyRingConstructible<NodeAlgorithmSuite>) {}
@@ -160,11 +170,11 @@ describe('KmsKeyring: _onDecrypt',
 
       const material = await testKeyring.onDecrypt(
         new NodeDecryptionMaterial(suite, context),
-        [edk]
+        [edk, edk]
       )
 
-      expect(material.hasUnencryptedDataKey).to.equal(false)
-      expect(material.keyringTrace).to.have.lengthOf(0)
+      expect(material.hasUnencryptedDataKey).to.equal(true)
+      expect(material.keyringTrace).to.have.lengthOf(1)
     })
 
     it('Check for early return (Postcondition): clientProvider may not return a client.', async () => {
@@ -279,4 +289,49 @@ describe('KmsKeyring: _onDecrypt',
         [edk]
       )).to.rejectedWith(Error, 'Key length does not agree with the algorithm specification.')
     })
+
+    it('Postcondition: A CMK must provide a valid data key or KMS must not have raised any errors.', async () => {
+      const generatorKeyId = 'arn:aws:kms:us-east-1:123456789012:alias/example-alias'
+      const context = { some: 'context' }
+      const grantTokens = ['grant']
+      const discovery = true
+      const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
+
+      const clientProviderError: any = () => {
+        return { decrypt }
+        function decrypt () {
+          throw new Error('failed to decrypt')
+        }
+      }
+      class TestKmsKeyring extends KmsKeyringClass(Keyring as KeyRingConstructible<NodeAlgorithmSuite>) {}
+
+      const testKeyring = new TestKmsKeyring({
+        clientProvider: clientProviderError,
+        grantTokens,
+        discovery
+      })
+
+      const edk = new EncryptedDataKey({
+        providerId: 'aws-kms',
+        providerInfo: generatorKeyId,
+        encryptedDataKey: Buffer.from(generatorKeyId)
+      })
+
+      await expect(testKeyring.onDecrypt(
+        new NodeDecryptionMaterial(suite, context),
+        [edk, edk]
+      )).to.rejectedWith(Error, 'Unable to decrypt data key and one or more KMS CMKs had an error.')
+
+      /* This will make the decrypt loop not have an error.
+       * This will exercise the `(!material.hasValidKey() && !cmkErrors.length)` `needs` condition.
+       */
+      const clientProviderNoError: any = () => false
+      await expect(new TestKmsKeyring({
+        clientProvider: clientProviderNoError,
+        grantTokens,
+        discovery
+      }).onDecrypt(new NodeDecryptionMaterial(suite, context),
+        [edk, edk]
+      )).to.not.rejectedWith(Error)
+    })
   })

modules/material-management/src/multi_keyring.ts

@@ -113,19 +113,37 @@ function buildPrivateOnDecrypt<S extends SupportedAlgorithmSuites> () {
     const children = this.children.slice()
     if (this.generator) children.unshift(this.generator)
 
+    let childKeyringErrors: Error[] = []
+
     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
 
       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.
+      /* Failures onDecrypt should not short-circuit the process
+       * If the caller does not have access they may have access
+       * through another Keyring.
+       */
+        childKeyringErrors.push(e)
       }
     }
+
+    /* Postcondition: A child keyring must provide a valid data key or no child keyring must have raised an error.
+     * If I have a data key,
+     * decrypt errors can be ignored.
+     * However, if I was unable to decrypt a data key AND I have errors,
+     * these errors should bubble up.
+     * Otherwise, the only error customers will see is that
+     * the material does not have an unencrypted data key.
+     * So I return a concatenated Error message
+     */
+    needs(material.hasValidKey() || (!material.hasValidKey() && !childKeyringErrors.length)
+      , childKeyringErrors
+        .reduce((m, e, i) => `${m} Error #${i + 1} \n ${e.stack} \n`,
+          'Unable to decrypt data key and one or more child keyrings had an error. \n '))
+
     return material
   }
 }

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

@@ -358,6 +358,37 @@ describe('MultiKeyring: onDecrypt', () => {
     expect(unwrapDataKey(test.getUnencryptedDataKey())).to.deep.equal(unencryptedDataKey)
     expect(called).to.equal(true)
   })
+
+  it('Postcondition: A child keyring must provide a valid data key or no child keyring must have raised an error.', async () => {
+    const suite = new NodeAlgorithmSuite(AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16)
+    const [edk0] = makeEDKandTraceForDecrypt(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, 'Unable to decrypt data key and one or more child keyrings had an error.')
+
+    /* This will make the decrypt loop not have an error.
+     * This will exercise the `(!material.hasValidKey() && !childKeyringErrors.length)` `needs` condition.
+     */
+    const childNoDataKey = keyRingFactory({
+      async onDecrypt (material: NodeDecryptionMaterial /*, encryptedDataKeys: EncryptedDataKey[] */) {
+        return material
+      },
+      onEncrypt: never
+    })
+
+    const mkeyringNoErrors = new MultiKeyringNode({ children: [ childNoDataKey ] })
+
+    await expect(mkeyringNoErrors.onDecrypt(material, [edk0])).to.not.rejectedWith(Error)
+  })
 })
 
 function makeEDKandTraceForEncrypt (num: number): [EncryptedDataKey, KeyringTrace] {

modules/serialize/src/identifiers.ts

@@ -80,6 +80,12 @@ export enum Maximum {
    * or some value larger 2 ** 63.
    */
   BYTES_PER_CACHED_KEY_LIMIT = 2 ** 53 - 1, // eslint-disable-line no-unused-vars
+  /* This value should be Maximum.FRAME_COUNT * Maximum.FRAME_SIZE.
+   * However this would be ~ 2 ** 64, much larger than Number.MAX_SAFE_INTEGER.
+   * For the same reasons outlined above in BYTES_PER_CACHED_KEY_LIMIT
+   * this value is set to 2 ** 53 - 1.
+   */
+  BYTES_PER_MESSAGE = 2 ** 53 - 1, // eslint-disable-line no-unused-vars
   // Maximum number of frames allowed in one message as defined in specification
   FRAME_COUNT = 2 ** 32 - 1, // eslint-disable-line no-unused-vars
   // Maximum bytes allowed in a single frame as defined in specification