fix: maxBodySize can not short circuit on frameLengh (#181)

In the case of a framed message that encrypts less than the frameLenght,
maxBodySize was doing the wrong thing.

Remove the check.
Add tests vectors that cover the case.

Author: seebees

modules/decrypt-node/src/decrypt_stream.ts

@@ -40,7 +40,7 @@ export function decryptStream (
     ? new NodeDefaultCryptographicMaterialsManager(cmm)
     : cmm
 
-  const parseHeaderStream = new ParseHeaderStream(cmm, { maxBodySize })
+  const parseHeaderStream = new ParseHeaderStream(cmm)
   const verifyStream = new VerifyStream({ maxBodySize })
   const decipherStream = getDecipherStream()
 

modules/decrypt-node/src/parse_header_stream.ts

@@ -19,10 +19,9 @@ import { Transform } from 'stream' // eslint-disable-line no-unused-vars
 import {
   NodeAlgorithmSuite,
   NodeMaterialsManager, // eslint-disable-line no-unused-vars
-  getDecryptionHelper,
-  needs
+  getDecryptionHelper
 } from '@aws-crypto/material-management-node'
-import { deserializeFactory, kdfInfo, ContentType } from '@aws-crypto/serialize'
+import { deserializeFactory, kdfInfo } from '@aws-crypto/serialize'
 import { VerifyInfo } from './verify_stream' // eslint-disable-line no-unused-vars
 
 const toUtf8 = (input: Uint8Array) => Buffer
@@ -35,20 +34,12 @@ interface HeaderState {
   buffer: Buffer
 }
 
-export interface ParseHeaderOptions {
-  maxBodySize?: number
-}
-
 export class ParseHeaderStream extends PortableTransformWithType {
   private materialsManager!: NodeMaterialsManager
   private _headerState: HeaderState
-  private _maxBodySize?: number
-  constructor (cmm: NodeMaterialsManager, { maxBodySize }: ParseHeaderOptions = {}) {
-    /* Precondition: ParseHeaderStream requires maxBodySize must be falsey or a number. */
-    needs(!maxBodySize || typeof maxBodySize === 'number', 'Unsupported MaxBodySize.')
+  constructor (cmm: NodeMaterialsManager) {
     super()
     Object.defineProperty(this, 'materialsManager', { value: cmm, enumerable: true })
-    Object.defineProperty(this, '_maxBodySize', { value: maxBodySize, enumerable: true })
     this._headerState = {
       buffer: Buffer.alloc(0)
     }
@@ -67,22 +58,7 @@ export class ParseHeaderStream extends PortableTransformWithType {
     const { rawHeader, headerIv, headerAuthTag } = headerInfo
 
     const suite = new NodeAlgorithmSuite(algorithmSuite.id)
-    const { encryptionContext, encryptedDataKeys, contentType, frameLength } = messageHeader
-
-    /* Framed messages store the frame size in the header.
-     * It is easy to confirm here.
-     * For non-framed messages, the size is in the body header.
-     * The check in verify stream _should_ be adequate from a logical perspective.
-     * However, doing this check here allows framed messages to exit before the CMM is called.
-     * This means that decryption of the Encrypted Data Key is never even attempted.
-     */
-    if (contentType === ContentType.FRAMED_DATA) {
-      /* Precondition: If maxBodySize was set I can not buffer a frame more data than maxBodySize.
-       * Before returning *any* cleartext, the stream **MUST** verify the decryption.
-       * This means that I must buffer the message until the AuthTag is reached.
-       */
-      needs(!this._maxBodySize || this._maxBodySize >= frameLength, 'maxBodySize exceeded.')
-    }
+    const { encryptionContext, encryptedDataKeys } = messageHeader
 
     this.materialsManager
       .decryptMaterials({ suite, encryptionContext, encryptedDataKeys })

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

@@ -83,4 +83,21 @@ describe('decrypt', () => {
       { encoding: 'base64' }
     )).to.rejectedWith(Error, 'Invalid Signature')
   })
+
+  it('can decrypt maxBodySize message with a single final frame.', async () => {
+    const { plaintext: test } = await decrypt(
+      fixtures.decryptKeyring(),
+      fixtures.base64Ciphertext4BytesWith4KFrameLength(),
+      { encoding: 'base64', maxBodySize: 4 }
+    )
+    expect(test).to.deep.equal(Buffer.from('asdf'))
+  })
+
+  it('will not decrypt data that exceeds maxBodySize.', async () => {
+    return expect(decrypt(
+      fixtures.decryptKeyring(),
+      fixtures.base64Ciphertext4BytesWith4KFrameLength(),
+      { encoding: 'base64', maxBodySize: 3 }
+    )).to.rejectedWith(Error, 'maxBodySize exceeded.')
+  })
 })

modules/decrypt-node/test/fixtures.ts

@@ -30,6 +30,10 @@ export function invalidSignatureCiphertextAlgAes256GcmIv12Tag16HkdfSha384EcdsaP3
   return 'AYADeJgnuW8vpQmi5QoqHIZWhjkAcAACABVhd3MtY3J5cHRvLXB1YmxpYy1rZXkAREFuWXRGRWV3Wm0rMjhLaElHcHg4UmhrYVVhTGNjSnB5ZjFud0lWUUZHbXlwZ3poSDJYZFNJQko0c0tpU0gzY2t6dz09AAZzaW1wbGUAB2NvbnRleHQAAQABawABawADAAAAAgAAAAAMAAAABQAAAAAAAAAAAAAAABqRZqpijpYGNM6P1L/78AUAAAABAAAAAAAAAAAAAAABIg1k1IeKV+CPUVBnpUkgyVUUZl7wAAAAAgAAAAAAAAAAAAAAAjl6P288VtjjKYeZA7mSeeJgjIUHbAAAAAMAAAAAAAAAAAAAAAO7OY+25yJkVcFvMMXn7VztyOhuIQoAAAAEAAAAAAAAAAAAAAAEG6jOHAz3NwyxgUjm5XFNMBx+2CCvAAAABQAAAAAAAAAAAAAABYRtGxVPUKbha73ay/kYrpl8Drik2gAAAAYAAAAAAAAAAAAAAAbosyHzP31p9EdOf3+dSa5gGfRW9e0AAAAHAAAAAAAAAAAAAAAHsulmBR4FQMbTk+00j5Fa/jD73/UJAAAACAAAAAAAAAAAAAAACMKgPZWTdDKzdPhXQDenInSRW/eOLgAAAAkAAAAAAAAAAAAAAAkdfSyNpBYk9XbFhf6DUnr2acw5lC4AAAAKAAAAAAAAAAAAAAAKnJpofr1UwwPy/+aqviMTrHXgOhM8AAAACwAAAAAAAAAAAAAAC9lvtW1lzA9RGUjnIGadlEhLxRC/FAAAAAwAAAAAAAAAAAAAAAyqJBaQEdmkOUX7uCki3Gh17YlQU3MAAAANAAAAAAAAAAAAAAANEK36ZE9VLiIj2X50N73UHEUtm0BbAAAADgAAAAAAAAAAAAAADkkr1fxL3qLbbC7OSDHqDnrBonOwxQAAAA8AAAAAAAAAAAAAAA8qcNFG+ofU3sOEZd8OXB/rkz0vDa8AAAAQAAAAAAAAAAAAAAAQ3KdsWJ/P8hF8aOhQdQP3v1KBDpB5AAAAEQAAAAAAAAAAAAAAEWyQGXefoGv9ZDfXUi93q+wUQGPzVwAAABIAAAAAAAAAAAAAABIDL/v5IY/z+s28FWzVo46vKNjOEeoAAAATAAAAAAAAAAAAAAATy1uc+McQfMJD8GrAJUaKlyTbXgFgAAAAFAAAAAAAAAAAAAAAFB6Sh2Po4oetBUwm1ABP9F9e1T70GAAAABUAAAAAAAAAAAAAABWm2oOg6agE6jzm3iDZ1brMSTHCOG8AAAAWAAAAAAAAAAAAAAAWsdIbfir5Dame3Uxkri54N2P7rqn6AAAAFwAAAAAAAAAAAAAAF6iPI1YW4fZzyL/355ZHBOLG3VPf1AAAABgAAAAAAAAAAAAAABj5Kjd5Twiu6bpb4o+jas0LRRJFH64AAAAZAAAAAAAAAAAAAAAZTf4xiUOtHeZmi+80M3Oay452R/rJAAAAGgAAAAAAAAAAAAAAGp+ET0LYxOX4JEL8gJudVVPW6qIv3AAAABsAAAAAAAAAAAAAABuTreBPGwJ2bftxQ6Kjwekfth4vWtsAAAAcAAAAAAAAAAAAAAAcdLoFVjR+yx4NVo1BSxv8Llya90EFAAAAHQAAAAAAAAAAAAAAHcFqEIL2wsYK36KQHyJqvJTiF/6nlQAAAB4AAAAAAAAAAAAAAB57QTT/UVRxBucxfhQRYeEU0mUeFxcAAAAfAAAAAAAAAAAAAAAfJyKwIcAURvMfN/Gd5MchygA20EYHAAAAIAAAAAAAAAAAAAAAILXRfQjIux8TeED/TdHHdLuaUEWWZgAAACEAAAAAAAAAAAAAACEi1SsfUozCXF0mCT/tHN8zVvSyWF4AAAAiAAAAAAAAAAAAAAAiFPt44yxRbwruA1F5YkYNokeDLmdiAAAAIwAAAAAAAAAAAAAAIwqdX86PI6IZgTs2SMHo4tLExClkIgAAACQAAAAAAAAAAAAAACQJGEuD6oBPBXU8iupaaNJFzEH/zKcAAAAlAAAAAAAAAAAAAAAlyQiA+1xRREA/qe5Djux6WaPEyUzhAAAAJgAAAAAAAAAAAAAAJqsZT21o1ikdiLkExG949WuTdw1mQQAAACcAAAAAAAAAAAAAACekCgcIX2x9/3zx982dDXfKUQSqARQAAAAoAAAAAAAAAAAAAAAocSNt9kEXLUF0Mydaj4MiBo1WrmGGAAAAKQAAAAAAAAAAAAAAKRHbcJJmpG367RxDInqlcBefk34RbgAAACoAAAAAAAAAAAAAACqmDdWYD/QVD9isxpCTm4KE+j6HKdMAAAArAAAAAAAAAAAAAAAreua98WTPIWH6dSAdzfYWPM9q9hoGAAAALAAAAAAAAAAAAAAALA+DQHkvoxKqVP3dmTQoM17QR4hz1gAAAC0AAAAAAAAAAAAAAC3TCjJBU0hDgBiC/bAHZe5T9CoMfTQAAAAuAAAAAAAAAAAAAAAujkLmjR2G1at5H5QHzKg/B2zNIH+mAAAALwAAAAAAAAAAAAAAL6+0F5aK0j3xqvgrsjmkzt7rZYUQQAAAADAAAAAAAAAAAAAAADDZMoeMElExOKgTTa0/gKqBPiRAqF4AAAAxAAAAAAAAAAAAAAAxbk1Qj+CqjC+gruT6bljBsQD5YTBVAAAAMgAAAAAAAAAAAAAAMhjQQjFR5A9Kn5ot/h4nqKrDTZJsNgAAADMAAAAAAAAAAAAAADO2SB3R/RrukhQx7/jxmjWiLknnnj0AAAA0AAAAAAAAAAAAAAA0wXykERn6CEIMhDCuLhUBmVn6fCu7AAAANQAAAAAAAAAAAAAANf7M3//4JJPLi+mmkKec2QrmuprdigAAADYAAAAAAAAAAAAAADadAVLY8PSrHytIi05tgse0HdyYVikAAAA3AAAAAAAAAAAAAAA3dj606o4y/YZw7gGHrD6JrGWQULV2AAAAOAAAAAAAAAAAAAAAOPgZF/TYVQogBfVMR6P4q5YWnSozUwAAADkAAAAAAAAAAAAAADl41/2WlW/Aq+EVJSHVH8eolMg7stIAAAA6AAAAAAAAAAAAAAA6IdfaZedkARnjm0CYxQhB28ljrigJAAAAOwAAAAAAAAAAAAAAO5PRn7sBV99dQJosnpj8Dy61bUW//QAAADwAAAAAAAAAAAAAADwkmUiXJJBJ4KvATXEeY1b2cOVPDOr/////AAAAPQAAAAAAAAAAAAAAPQAAAABAZDjPrFjtf/NJrKKMK2W9AGgwZgIxAN4h4KUn2VHZhxd/PQlZSmawzL1txgo79vsZjVhV15xqyMZLLcpNuNmK3hNHA83v+AIxAP0Sga/B1gZuyGmQK2cSnDdRIL6bmAzzeTiMcjRoJ6KrYRbLwg8mzmdQLgdvSoPt00=='
 }
 
+export function base64Ciphertext4BytesWith4KFrameLength () {
+  return 'AYADeEpqVoZzS6rBmbCKSa4acg4AXwABABVhd3MtY3J5cHRvLXB1YmxpYy1rZXkAREE4blREMk5rYzRDRXhoQTNlR0E3M1kyWWY2N2c4TDQySjlwL2UzNHo5RjdUR1hlT2pJZGR4TjEyeitUemk4Wmpxdz09AAEAAWsAAWsAAwAAAAIAAAAADAAAEAAAAAAAAAAAAAAAAABggOHpalFEq+ovk6c4Ugaj/////wAAAAEAAAAAAAAAAAAAAAEAAAAEDyVA5PCXHZo4sqQuqprmzWXwXUsAZzBlAjAY3vK9flPg8WEnoxIjPuhbj4o+bKUsJzbZgtDBaqIaBZYo8yt7JQB4vFyzb/PwucoCMQCz2fzhOQBAM63n56vBdIjBVqbNPyszIy0QjE8OJ/X8mV48VNi5wCF4u7I4MmK91N0='
+}
+
 export function base64Plaintext () {
   return '3Ye0RVTIjYp9Yvi+81Dzq9h9gAUF6akM1mqTbPKMhmwgxTWuj6Wlf8UFUMG7zALPDpN77EleMS3dXUOGlr/nalmwXkBseEo+QxCJgeo6WMuB2xQHZqJT+0glM3mcl2FWwiQDZ9G84dYOW1KSDfiyISe9rTqARl0fmEnD1oB6zlP4cYg7+DDTxOOvw5RndoiOBZ+mLbZT9vHTsJkWB3HgFO06dFAtwSgjUAEaNWMjk04vIT+9SBvql6cOk+GLfUdkH33chNk22yKPF2UQ6+lvW0YqGODIfTBQXypPuuKYXJ3T583YxeiKoxuxZFpVNkg30r5cYPOYulINy+YrWQIbNFRP9Zk0CNkAJ7zsIMhQ8IXH+zG1bQmwh1RDGSAfZhmsR147Jsi6qty9Fe9O'
 }