From cc0aced08dd0e66ce13079b1e6f2ee0d64af5160 Mon Sep 17 00:00:00 2001 From: Emery Date: Fri, 2 Aug 2019 17:03:29 -0700 Subject: [PATCH] fix: maxBodySize can not short circuit on frameLengh 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. --- modules/decrypt-node/src/decrypt_stream.ts | 2 +- .../decrypt-node/src/parse_header_stream.ts | 32 +++---------------- modules/decrypt-node/test/decrypt.test.ts | 17 ++++++++++ modules/decrypt-node/test/fixtures.ts | 4 +++ 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/modules/decrypt-node/src/decrypt_stream.ts b/modules/decrypt-node/src/decrypt_stream.ts index bc7647bfd..fe6d0f1d0 100644 --- a/modules/decrypt-node/src/decrypt_stream.ts +++ b/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() diff --git a/modules/decrypt-node/src/parse_header_stream.ts b/modules/decrypt-node/src/parse_header_stream.ts index f9cfdcef1..83bf743a3 100644 --- a/modules/decrypt-node/src/parse_header_stream.ts +++ b/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 }) diff --git a/modules/decrypt-node/test/decrypt.test.ts b/modules/decrypt-node/test/decrypt.test.ts index 3101ed46c..e0b100240 100644 --- a/modules/decrypt-node/test/decrypt.test.ts +++ b/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.') + }) }) diff --git a/modules/decrypt-node/test/fixtures.ts b/modules/decrypt-node/test/fixtures.ts index 224f97690..60f020cc1 100644 --- a/modules/decrypt-node/test/fixtures.ts +++ b/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' }