Merge branch 'master' into prtemplate

Author: johnwalker

modules/decrypt-browser/src/decrypt.ts

@@ -100,8 +100,21 @@ interface FramedDecryptOptions extends BodyDecryptOptions {
 async function bodyDecrypt ({ buffer, getSubtleDecrypt, headerInfo }: BodyDecryptOptions) {
   let readPos = headerInfo.headerIv.byteLength + headerInfo.rawHeader.byteLength + headerInfo.headerAuthTag.byteLength
   const clearBuffers: ArrayBuffer[] = []
+  let sequenceNumber = 0
   while (true) {
+    /* Keeping track of the sequence number myself. */
+    sequenceNumber += 1
+
     const { clearBlob, frameInfo } = await framedDecrypt({ buffer, getSubtleDecrypt, headerInfo, readPos })
+
+    /* Precondition: The sequenceNumber is required to monotonically increase, starting from 1.
+     * This is to avoid a bad actor from abusing the sequence number on un-signed algorithm suites.
+     * If the frame size matched the data format (say NDJSON),
+     * then the data could be significantly altered just by rearranging the frames.
+     * Non-framed data returns a sequenceNumber of 1.
+     */
+    needs(frameInfo.sequenceNumber === sequenceNumber, 'Encrypted body sequence out of order.')
+
     clearBuffers.push(clearBlob)
     readPos = frameInfo.readPos
     if (frameInfo.isFinalFrame) {

modules/decrypt-browser/test/decrypt.test.ts

@@ -32,4 +32,15 @@ describe('decrypt', () => {
     expect(messageHeader.encryptionContext).to.deep.equal(fixtures.encryptionContext())
     expect(test).to.deep.equal(fixtures.plaintext())
   })
+
+  it('Precondition: The sequence number is required to monotonically increase, starting from 1.', async () => {
+    return decrypt(
+      fixtures.decryptKeyring(),
+      fixtures.frameSequenceOutOfOrder()
+    ).then(() => {
+      throw new Error('should not succeed')
+    }, err => {
+      expect(err).to.be.instanceOf(Error)
+    })
+  })
 })

modules/decrypt-browser/test/fixtures.ts

@@ -38,6 +38,21 @@ export function encryptionContext () {
   }
 }
 
+export function frameSequenceOutOfOrder () {
+  /* This is the string 'asdf' encrypted with ALG_AES128_GCM_IV12_TAG16.
+   * The data key is all zeros.
+   * The frames have been reordered in order to test decryption failure.
+   */
+  return new Uint8Array([
+    1, 128, 0, 20, 111, 198, 208, 129, 64, 41, 115, 91, 247, 27, 148, 148, 102, 70, 149, 210, 0, 0, 0, 1, 0, 1, 107, 0, 1, 107, 0, 3, 0, 0, 0, 2, 0, 0, 0, 0, 12, 0, 0, 0, 1, // header
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 178, 85, 20, 93, 96, 34, 206, 116, 216, 115, 183, 217, 192, 84, 155, 15, // header auth
+    0, 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 71, 217, 75, 144, 128, 252, 7, 157, 174, 2, 89, 248, 206, 24, 122, 69, 226, 95, // f
+    0, 0, 0, 48, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 62, 114, 251, 4, 157, 182, 94, 8, 230, 234, 185, 222, 120, 209, 235, 186, 207, 112, // d
+    0, 0, 0, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 42, 44, 171, 202, 172, 191, 0, 232, 145, 31, 77, 90, 8, 233, 45, 106, 60, 176, // s
+    0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16, 248, 112, 28, 63, 229, 61, 238, 146, 255, 228, 38, 170, 100, 42, 251, 21, 208 // a
+  ])
+}
+
 class TestKeyring extends KeyringWebCrypto {
   async _onEncrypt () : Promise<WebCryptoEncryptionMaterial> {
     throw new Error('I should never see this error')

modules/decrypt-node/src/decipher_stream.ts

@@ -16,8 +16,11 @@
 // @ts-ignore
 import { Transform as PortableTransform } from 'readable-stream'
 import { Transform } from 'stream' // eslint-disable-line no-unused-vars
-import { DecipherGCM } from 'crypto' // eslint-disable-line no-unused-vars
-import { needs } from '@aws-crypto/material-management-node'
+import {
+  needs,
+  GetDecipher, // eslint-disable-line no-unused-vars
+  AwsEsdkJsDecipherGCM // eslint-disable-line no-unused-vars
+} from '@aws-crypto/material-management-node'
 import {
   aadFactory,
   ContentType // eslint-disable-line no-unused-vars
@@ -31,12 +34,12 @@ const PortableTransformWithType = (<new (...args: any[]) => Transform>PortableTr
 export interface DecipherInfo {
   messageId: Buffer
   contentType: ContentType
-  getDecipher: (iv: Uint8Array) => DecipherGCM
+  getDecipher: GetDecipher
   dispose: () => void
 }
 
 interface DecipherState {
-  decipher: DecipherGCM
+  decipher: AwsEsdkJsDecipherGCM
   content: Buffer[]
   contentLength: number
 }

modules/decrypt-node/src/decrypt_stream.ts

@@ -25,7 +25,7 @@ import Duplexify from 'duplexify'
 import { Duplex } from 'stream' // eslint-disable-line no-unused-vars
 
 // @ts-ignore
-import { pipeline } from 'readable-stream'
+import { pipeline, PassThrough } from 'readable-stream'
 
 export interface DecryptStreamOptions {
   maxBodySize?: number
@@ -44,7 +44,13 @@ export function decryptStream (
   const verifyStream = new VerifyStream({ maxBodySize })
   const decipherStream = getDecipherStream()
 
-  pipeline(parseHeaderStream, verifyStream, decipherStream)
+  /* pipeline will _either_ stream.destroy or the callback.
+   * decipherStream uses destroy to dispose the material.
+   * So I tack a pass though stream onto the end.
+   */
+  pipeline(parseHeaderStream, verifyStream, decipherStream, new PassThrough(), (err: Error) => {
+    if (err) stream.emit('error', err)
+  })
 
   const stream = new Duplexify(parseHeaderStream, decipherStream)
 

modules/decrypt-node/src/parse_header_stream.ts

@@ -108,7 +108,17 @@ export class ParseHeaderStream extends PortableTransformWithType {
         // The header is parsed, pass control
         const readPos = rawHeader.byteLength + headerIv.byteLength + headerAuthTag.byteLength
         const tail = headerBuffer.slice(readPos)
-        this._transform = (chunk: any, _enc: string, cb: Function) => cb(null, chunk)
+        /* needs calls in downstream _transform streams will throw.
+         * But streams are async.
+         * So this error should be turned into an `.emit('error', ex)`.
+         */
+        this._transform = (chunk: any, _enc: string, cb: Function) => {
+          try {
+            cb(null, chunk)
+          } catch (ex) {
+            this.emit('error', ex)
+          }
+        }
         // flush the tail.  Stream control is now in the verify and decrypt streams
         return setImmediate(() => this._transform(tail, encoding, callback))
       })

modules/decrypt-node/src/verify_stream.ts

@@ -16,10 +16,10 @@
 // @ts-ignore
 import { Transform as PortableTransform } from 'readable-stream'
 import { Transform } from 'stream' // eslint-disable-line no-unused-vars
-import { DecipherGCM } from 'crypto' // eslint-disable-line no-unused-vars
 import {
   needs,
-  GetVerify // eslint-disable-line no-unused-vars
+  GetVerify, // eslint-disable-line no-unused-vars
+  GetDecipher // eslint-disable-line no-unused-vars
 } from '@aws-crypto/material-management-node'
 import {
   deserializeSignature,
@@ -35,7 +35,7 @@ const PortableTransformWithType = (<new (...args: any[]) => Transform>PortableTr
 
 export interface VerifyInfo {
   headerInfo: HeaderInfo
-  getDecipher: (iv: Uint8Array) => DecipherGCM
+  getDecipher: GetDecipher
   dispose: () => void
   verify?: AWSVerify
 }
@@ -49,14 +49,16 @@ interface VerifyState {
   authTagBuffer: Buffer
   currentFrame?: BodyHeader
   signatureInfo: Buffer
+  sequenceNumber: number
 }
 
 export class VerifyStream extends PortableTransformWithType {
   private _headerInfo!: HeaderInfo
   private _verifyState: VerifyState = {
     buffer: Buffer.alloc(0),
     authTagBuffer: Buffer.alloc(0),
-    signatureInfo: Buffer.alloc(0)
+    signatureInfo: Buffer.alloc(0),
+    sequenceNumber: 0
   }
   private _verify?: AWSVerify
   private _maxBodySize?: number
@@ -118,6 +120,17 @@ export class VerifyStream extends PortableTransformWithType {
        */
       needs(!this._maxBodySize || this._maxBodySize >= frameHeader.contentLength, 'maxBodySize exceeded.')
 
+      /* Keeping track of the sequence number myself. */
+      state.sequenceNumber += 1
+
+      /* Precondition: The sequence number is required to monotonically increase, starting from 1.
+       * This is to avoid a bad actor from abusing the sequence number on un-signed algorithm suites.
+       * If the frame size matched the data format (say NDJSON),
+       * then the data could be significantly altered just by rearranging the frames.
+       * Non-framed data returns a sequenceNumber of 1.
+       */
+      needs(frameHeader.sequenceNumber === state.sequenceNumber, 'Encrypted body sequence out of order.')
+
       if (this._verify) {
         this._verify.update(frameBuffer.slice(0, frameHeader.readPos))
       }

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

@@ -67,4 +67,12 @@ describe('decrypt', () => {
     expect(messageHeader.encryptionContext).to.deep.equal(fixtures.encryptionContext())
     expect(test.toString('base64')).to.equal(fixtures.base64Plaintext())
   })
+
+  it('Precondition: The sequence number is required to monotonically increase, starting from 1.', async () => {
+    return expect(decrypt(
+      fixtures.decryptKeyring(),
+      fixtures.frameSequenceOutOfOrder(),
+      { encoding: 'base64' }
+    )).to.rejectedWith(Error, 'Encrypted body sequence out of order.')
+  })
 })

modules/decrypt-node/test/fixtures.ts

@@ -37,6 +37,19 @@ export function encryptionContext () {
   }
 }
 
+export function frameSequenceOutOfOrder () {
+  /* This is the string 'asdf' encrypted with ALG_AES128_GCM_IV12_TAG16.
+   * The data key is all zeros.
+   * The frames have been reordered in order to test decryption failure.
+   */
+  return 'AYAAFG/G0IFAKXNb9xuUlGZGldIAAAABAAFrAAFrAAMAAAACAAAAAAwAAAAB' + // header
+  'AAAAAAAAAAAAAAAAslUUXWAiznTYc7fZwFSbDw' + // header auth
+  'AAAAQAAAAAAAAAAAAAAAR9lLkID8B52uAln4zhh6ReJf' + // f
+  'AAAAMAAAAAAAAAAAAAAAPnL7BJ22Xgjm6rneeNHrus9w' + // d
+  'AAAAIAAAAAAAAAAAAAAAKiyryqy/AOiRH01aCOktajyw' + // s
+  'AAAAEAAAAAAAAAAAAAAAEPhwHD/lPe6S/+QmqmQq+xXQ' // a
+}
+
 export function decryptKeyring () {
   class TestKeyring extends KeyringNode {
     async _onEncrypt () : Promise<NodeEncryptionMaterial> {

modules/encrypt-browser/src/encrypt.ts

@@ -21,6 +21,7 @@ import {
   AlgorithmSuiteIdentifier,
   getEncryptHelper,
   KeyringWebCrypto,
+  needs,
   WebCryptoMaterialsManager // eslint-disable-line no-unused-vars
 } from '@aws-crypto/material-management-browser'
 import {
@@ -35,7 +36,8 @@ import {
   serializeSignatureInfo,
   FRAME_LENGTH,
   MESSAGE_ID_LENGTH,
-  raw2der
+  raw2der,
+  Maximum
 } from '@aws-crypto/serialize'
 import { fromUtf8 } from '@aws-sdk/util-utf8-browser'
 import { getWebCryptoBackend } from '@aws-crypto/web-crypto-backend'
@@ -60,6 +62,9 @@ export async function encrypt (
   plaintext: Uint8Array,
   { suiteId, encryptionContext, frameLength = FRAME_LENGTH }: EncryptInput = {}
 ): Promise<EncryptResult> {
+  /* Precondition: The frameLength must be less than the maximum frame size for browser encryption. */
+  needs(frameLength > 0 && Maximum.FRAME_SIZE >= frameLength, `frameLength out of bounds: 0 > frameLength >= ${Maximum.FRAME_SIZE}`)
+
   const backend = await getWebCryptoBackend()
   if (!backend) throw new Error('No supported crypto backend')
 
@@ -119,14 +124,24 @@ export async function encrypt (
     const frameHeader = isFinalFrame
       ? serialize.finalFrameHeader(sequenceNumber, frameIv, finalFrameLength)
       : serialize.frameHeader(sequenceNumber, frameIv)
+    const contentString = messageAADContentString({ contentType: messageHeader.contentType, isFinalFrame })
     const messageAdditionalData = messageAAD(
       messageId,
-      messageAADContentString({ contentType: messageHeader.contentType, isFinalFrame }),
+      contentString,
       sequenceNumber,
-      plaintextLength
+      isFinalFrame ? finalFrameLength : frameLength
     )
 
-    const cipherBufferAndAuthTag = await getSubtleEncrypt(frameIv, messageAdditionalData)(plaintext)
+    /* Slicing an ArrayBuffer in a browser is suboptimal.
+     * It makes a copy.s
+     * So I just make a new view for the length of the frame.
+     */
+    const framePlaintext = new Uint8Array(
+      plaintext.buffer,
+      (sequenceNumber - 1) * frameLength,
+      isFinalFrame ? finalFrameLength : frameLength
+    )
+    const cipherBufferAndAuthTag = await getSubtleEncrypt(frameIv, messageAdditionalData)(framePlaintext)
 
     bodyContent.push(frameHeader, cipherBufferAndAuthTag)
   }

modules/encrypt-browser/test/encrypt.test.ts

@@ -26,7 +26,9 @@ import {
   importForWebCryptoEncryptionMaterial
 } from '@aws-crypto/material-management-browser'
 import {
-  deserializeFactory
+  deserializeFactory,
+  decodeBodyHeader,
+  deserializeSignature
 } from '@aws-crypto/serialize'
 import { encrypt } from '../src/index'
 import { toUtf8, fromUtf8 } from '@aws-sdk/util-utf8-browser'
@@ -85,4 +87,38 @@ describe('encrypt structural testing', () => {
 
     expect(messageHeader).to.deep.equal(messageInfo.messageHeader)
   })
+
+  it('Precondition: The frameLength must be less than the maximum frame size for browser encryption.', async () => {
+    const frameLength = 0
+    expect(encrypt(keyRing, fromUtf8('asdf'), { frameLength })).to.rejectedWith(Error)
+  })
+
+  it('can fully parse a framed message', async () => {
+    const plaintext = fromUtf8('asdf')
+    const frameLength = 1
+    const { cipherMessage } = await encrypt(keyRing, plaintext, { frameLength })
+
+    const headerInfo = deserializeMessageHeader(cipherMessage)
+    if (!headerInfo) throw new Error('this should never happen')
+
+    const tagLength = headerInfo.algorithmSuite.tagLength / 8
+    let readPos = headerInfo.headerLength + headerInfo.algorithmSuite.ivLength + tagLength
+    let i = 0
+    let bodyHeader: any
+    // for every frame...
+    for (; i < 4; i++) {
+      bodyHeader = decodeBodyHeader(cipherMessage, headerInfo, readPos)
+      if (!bodyHeader) throw new Error('this should never happen')
+      readPos = bodyHeader.readPos + bodyHeader.contentLength + tagLength
+    }
+
+    expect(i).to.equal(4) // 4 frames
+    expect(bodyHeader.isFinalFrame).to.equal(true) // we got to the end
+
+    // This implicitly tests that I have consumed all the data,
+    // because otherwise the footer section will be too large
+    const footerSection = cipherMessage.slice(readPos)
+    // This will throw if it does not deserialize correctly
+    deserializeSignature(footerSection)
+  })
 })

modules/encrypt-node/src/encrypt_stream.ts

@@ -16,7 +16,8 @@
 import {
   NodeDefaultCryptographicMaterialsManager, NodeAlgorithmSuite, AlgorithmSuiteIdentifier, // eslint-disable-line no-unused-vars
   KeyringNode, NodeEncryptionMaterial, getEncryptHelper, EncryptionContext, // eslint-disable-line no-unused-vars
-  NodeMaterialsManager // eslint-disable-line no-unused-vars
+  NodeMaterialsManager, // eslint-disable-line no-unused-vars
+  needs
 } from '@aws-crypto/material-management-node'
 import { getFramedEncryptStream } from './framed_encrypt_stream'
 import { SignatureStream } from './signature_stream'
@@ -26,7 +27,8 @@ import {
   MessageHeader, // eslint-disable-line no-unused-vars
   serializeFactory, kdfInfo, ContentType, SerializationVersion, ObjectType,
   FRAME_LENGTH,
-  MESSAGE_ID_LENGTH
+  MESSAGE_ID_LENGTH,
+  Maximum
 } from '@aws-crypto/serialize'
 
 // @ts-ignore
@@ -56,6 +58,9 @@ export function encryptStream (
 ): Duplex {
   const { suiteId, context, frameLength = FRAME_LENGTH } = op
 
+  /* Precondition: The frameLength must be less than the maximum frame size Node.js stream. */
+  needs(frameLength > 0 && Maximum.FRAME_SIZE >= frameLength, `frameLength out of bounds: 0 > frameLength >= ${Maximum.FRAME_SIZE}`)
+
   /* If the cmm is a Keyring, wrap it with NodeDefaultCryptographicMaterialsManager. */
   cmm = cmm instanceof KeyringNode
     ? new NodeDefaultCryptographicMaterialsManager(cmm)

modules/encrypt-node/src/framed_encrypt_stream.ts

@@ -19,9 +19,12 @@ import {
 } from '@aws-crypto/serialize'
 // @ts-ignore
 import { Transform as PortableTransform } from 'readable-stream'
-import { CipherGCM } from 'crypto' // eslint-disable-line no-unused-vars
 import { Transform } from 'stream' // eslint-disable-line no-unused-vars
-import { needs } from '@aws-crypto/material-management-node'
+import {
+  GetCipher, // eslint-disable-line no-unused-vars
+  AwsEsdkJsCipherGCM, // eslint-disable-line no-unused-vars
+  needs
+} from '@aws-crypto/material-management-node'
 
 const fromUtf8 = (input: string) => Buffer.from(input, 'utf8')
 const serialize = serializeFactory(fromUtf8)
@@ -38,7 +41,7 @@ interface EncryptFrame {
   content: Buffer[]
   bodyHeader: Buffer
   headerSent?: boolean
-  cipher: CipherGCM,
+  cipher: AwsEsdkJsCipherGCM,
   isFinalFrame: boolean
 }
 
@@ -165,8 +168,6 @@ export function getFramedEncryptStream (getCipher: GetCipher, messageHeader: Mes
   })()
 }
 
-type GetCipher = (iv: Uint8Array) => CipherGCM
-
 type EncryptFrameInput = {
   pendingFrame: AccumulatingFrame,
   messageHeader: MessageHeader,