From 4c18c3ce50b197cee80029b86d92315f9d9edc9b Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 8 Jul 2019 12:03:19 -0700 Subject: [PATCH 1/3] fix: encrypt/decrypt node Small frames exacerbate a race between VerifyStream and DecipherStream. Once an `AuthTag` event is sent to DecipherStream the current frame is decrypted and verified before being flushed. More frame data will not be processed by DecipherStream because DecipherStream will not execute the _transform callback until the frame is flushed. However nothing stops VerifyStream from continuing to process data. VerifyStream will push the next frame's data, which will be cached by DecipherStream. But once VerifyStream reaches the end of the frame it will emit `AuthTag`. This means the DecipherStream will receive the AuthTag before the frame data and it will correctly fail on precondition checks. To resolve this a simple callback is passed from VerifyStream to DecipherStream in the `AuthTag` event. Tests for both encrypt and decrypt are included here, because that is how this issue was found. --- modules/decrypt-node/package.json | 3 +- modules/decrypt-node/src/decipher_stream.ts | 17 +- modules/decrypt-node/src/decrypt.ts | 5 +- modules/decrypt-node/src/decrypt_stream.ts | 4 + modules/decrypt-node/src/verify_stream.ts | 54 ++++-- modules/decrypt-node/test/decrypt.test.ts | 89 +++++----- modules/decrypt-node/test/fixtures.ts | 53 ++++++ modules/encrypt-node/package.json | 2 + modules/encrypt-node/src/encrypt.ts | 2 +- modules/encrypt-node/test/encrypt.test.ts | 185 ++++++++++++++++++-- 10 files changed, 340 insertions(+), 74 deletions(-) create mode 100644 modules/decrypt-node/test/fixtures.ts diff --git a/modules/decrypt-node/package.json b/modules/decrypt-node/package.json index 4477bd8ec..f36f8687e 100644 --- a/modules/decrypt-node/package.json +++ b/modules/decrypt-node/package.json @@ -24,16 +24,17 @@ "tslib": "^1.9.3" }, "devDependencies": { - "@aws-crypto/encrypt-node": "^0.1.0-preview.1", "@types/chai": "^4.1.4", "@types/mocha": "^5.2.5", "@types/node": "^11.11.4", "@typescript-eslint/eslint-plugin": "^1.9.0", "@typescript-eslint/parser": "^1.9.0", + "@types/from2": "^2.3.0", "chai": "^4.1.2", "mocha": "^5.2.0", "nyc": "^14.0.0", "standard": "^12.0.1", + "from2": "^2.3.0", "ts-node": "^7.0.1", "typescript": "^3.5.0" }, diff --git a/modules/decrypt-node/src/decipher_stream.ts b/modules/decrypt-node/src/decipher_stream.ts index f687db582..6624d643d 100644 --- a/modules/decrypt-node/src/decipher_stream.ts +++ b/modules/decrypt-node/src/decipher_stream.ts @@ -68,9 +68,9 @@ export function getDecipherStream () { decipherInfo = info }) .on('BodyInfo', this._onBodyHeader) - .on('AuthTag', async (authTag: Buffer) => { + .on('AuthTag', async (authTag: Buffer, next: Function) => { try { - await this._onAuthTag(authTag) + await this._onAuthTag(authTag, next) } catch (e) { this.emit('error', e) } @@ -134,7 +134,7 @@ export function getDecipherStream () { super._read(size) } - _onAuthTag = async (authTag: Buffer) => { + _onAuthTag = async (authTag: Buffer, next:Function) => { const { decipher, content, contentLength } = decipherState /* Precondition: _onAuthTag must be called only after a frame has been accumulated. * However there is an edge case. The final frame _can_ be zero length. @@ -169,6 +169,17 @@ export function getDecipherStream () { } } + /* This frame is complete. + * Need to notify the VerifyStream continue. + * See the note in `AuthTag` for details. + * The short answer is that for small frame sizes, + * the "next" frame associated auth tag may be + * parsed and send before the "current" is processed. + * This will cause the auth tag event to fire before + * any _transform events fire and a 'Lengths do not match' precondition to fail. + */ + next() + // This frame is complete. Notify _transform to continue, see needs above for more details if (frameComplete) frameComplete() // reset for next frame. diff --git a/modules/decrypt-node/src/decrypt.ts b/modules/decrypt-node/src/decrypt.ts index c903a4211..995d766c3 100644 --- a/modules/decrypt-node/src/decrypt.ts +++ b/modules/decrypt-node/src/decrypt.ts @@ -36,7 +36,7 @@ export interface DecryptOptions { export async function decrypt ( cmm: NodeMaterialsManager|KeyringNode, - ciphertext: Buffer|Uint8Array|Readable|string, + ciphertext: Buffer|Uint8Array|Readable|string|NodeJS.ReadableStream, { encoding, maxBodySize } : DecryptOptions = {} ): Promise { const stream = decryptStream(cmm, { maxBodySize }) @@ -46,6 +46,9 @@ export async function decrypt ( stream .once('MessageHeader', (header: MessageHeader) => { messageHeader = header }) .on('data', (chunk: Buffer) => plaintext.push(chunk)) + .on('error', () => { + console.log('wtf???') + }) // This will check both Uint8Array|Buffer if (ciphertext instanceof Uint8Array) { diff --git a/modules/decrypt-node/src/decrypt_stream.ts b/modules/decrypt-node/src/decrypt_stream.ts index 0a64981b1..9a07d1a19 100644 --- a/modules/decrypt-node/src/decrypt_stream.ts +++ b/modules/decrypt-node/src/decrypt_stream.ts @@ -46,6 +46,10 @@ export function decryptStream ( pipeline(parseHeaderStream, verifyStream, decipherStream) + decipherStream.on('error', (e) => { + console.log('?????', e) + }) + const stream = new Duplexify(parseHeaderStream, decipherStream) // Forward header events diff --git a/modules/decrypt-node/src/verify_stream.ts b/modules/decrypt-node/src/verify_stream.ts index b3bdb5775..d32cff9a8 100644 --- a/modules/decrypt-node/src/verify_stream.ts +++ b/modules/decrypt-node/src/verify_stream.ts @@ -121,8 +121,6 @@ export class VerifyStream extends PortableTransformWithType { if (this._verify) { this._verify.update(frameBuffer.slice(0, frameHeader.readPos)) } - // clear the buffer. It _could_ have cipher text... - state.buffer = Buffer.alloc(0) const tail = chunk.slice(frameHeader.readPos) this.emit('BodyInfo', frameHeader) state.currentFrame = frameHeader @@ -155,23 +153,49 @@ export class VerifyStream extends PortableTransformWithType { state.authTagBuffer = Buffer.concat([authTagBuffer, chunk]) return callback() } else { - state.authTagBuffer = Buffer.concat([authTagBuffer, chunk], tagLengthBytes) + const finalAuthTagBuffer = Buffer.concat([authTagBuffer, chunk], tagLengthBytes) if (this._verify) { - this._verify.update(state.authTagBuffer) + this._verify.update(finalAuthTagBuffer) } - this.emit('AuthTag', state.authTagBuffer) - const tail = chunk.slice(left) - if (!currentFrame.isFinalFrame) { - state.buffer = Buffer.alloc(0) - state.currentFrame = undefined - state.authTagBuffer = Buffer.alloc(0) + /* Reset state. + * Ciphertext buffers and authTag buffers need to be cleared. + */ + state.buffer = Buffer.alloc(0) + state.currentFrame = undefined + state.authTagBuffer = Buffer.alloc(0) + /* After the final frame the file format is _much_ simpler. + * Making sure the cascading if blocks fall to the signature can be tricky and brittle. + * After the final frame, just moving on to concatenate the signature is much simpler. + */ + if (currentFrame.isFinalFrame) { + /* Overwriting the _transform function. + * Data flow control is not handled here. + */ + this._transform = (chunk: Buffer, _enc: string, callback: Function) => { + if (chunk.length) { + state.signatureInfo = Buffer.concat([state.signatureInfo, chunk]) + } + + callback() + } } - return setImmediate(() => this._transform(tail, enc, callback)) - } - } - if (chunk.length) { - state.signatureInfo = Buffer.concat([state.signatureInfo, chunk]) + const tail = chunk.slice(left) + /* The decipher_stream uses the `AuthTag` event to flush the accumulated frame. + * This is because ciphertext should never be returned until it is verified. + * i.e. the auth tag checked. + * This can create an issue if the chucks and frame size are small. + * If the verify stream continues processing and sends the next auth tag, + * before the current auth tag has been completed. + * This is basically a back pressure issue. + * Since the frame size, and consequently the high water mark, + * can not be know when the stream is created, + * the internal stream state would need to be modified. + * I assert that a simple callback is a simpler way to handle this. + */ + const next = () => this._transform(tail, enc, callback) + return this.emit('AuthTag', finalAuthTagBuffer, next) + } } callback() diff --git a/modules/decrypt-node/test/decrypt.test.ts b/modules/decrypt-node/test/decrypt.test.ts index 61000f28d..41ccce584 100644 --- a/modules/decrypt-node/test/decrypt.test.ts +++ b/modules/decrypt-node/test/decrypt.test.ts @@ -17,45 +17,54 @@ import { expect } from 'chai' import 'mocha' -import { - NodeDecryptionMaterial, // eslint-disable-line no-unused-vars - NodeEncryptionMaterial, // eslint-disable-line no-unused-vars - KeyringNode, EncryptedDataKey, - KeyringTraceFlag, AlgorithmSuiteIdentifier -} from '@aws-crypto/material-management-node' - -// import * as fs from 'fs' - -import { encrypt } from '@aws-crypto/encrypt-node' -import { decrypt } from '../src/decrypt' - -describe('simple', () => { - it('decrypt what I encrypt', async () => { - class TestKeyring extends KeyringNode { - async _onEncrypt (material: NodeEncryptionMaterial) { - const unencryptedDataKey = new Uint8Array(material.suite.keyLengthBytes).fill(1) - const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } - const edk = new EncryptedDataKey({ providerId: 'k', providerInfo: 'k', encryptedDataKey: new Uint8Array(3) }) - return material - .setUnencryptedDataKey(unencryptedDataKey, trace) - .addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) - } - async _onDecrypt (material: NodeDecryptionMaterial) { - const unencryptedDataKey = new Uint8Array(material.suite.keyLengthBytes).fill(1) - const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY } - return material.setUnencryptedDataKey(unencryptedDataKey, trace) - } - } - - const keyRing = new TestKeyring() - const suiteId = AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16 - - const plaintext = 'asdf' - const { ciphertext } = await encrypt(keyRing, plaintext, { suiteId }) - - const { plaintext: test, messageHeader } = await decrypt(keyRing, ciphertext) - - expect(messageHeader.suiteId).to.equal(suiteId) - expect(test.toString()).to.equal(plaintext) +import { AlgorithmSuiteIdentifier } from '@aws-crypto/material-management-node' +import { decrypt } from '../src/index' +import * as fixtures from './fixtures' +import from from 'from2' + +describe('decrypt', () => { + it('string with encoding', async () => { + const { plaintext: test, messageHeader } = await decrypt( + fixtures.decryptKeyring(), + fixtures.base64CiphertextAlgAes256GcmIv12Tag16HkdfSha384EcdsaP384(), + { encoding: 'base64' } + ) + + expect(messageHeader.suiteId).to.equal(AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA384_ECDSA_P384) + expect(messageHeader.encryptionContext).to.deep.equal(fixtures.encryptionContext()) + expect(test.toString('base64')).to.equal(fixtures.base64Plaintext()) + }) + + it('buffer', async () => { + const { plaintext: test, messageHeader } = await decrypt( + fixtures.decryptKeyring(), + Buffer.from(fixtures.base64CiphertextAlgAes256GcmIv12Tag16HkdfSha384EcdsaP384(), 'base64') + ) + + expect(messageHeader.suiteId).to.equal(AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA384_ECDSA_P384) + expect(messageHeader.encryptionContext).to.deep.equal(fixtures.encryptionContext()) + expect(test.toString('base64')).to.equal(fixtures.base64Plaintext()) + }) + + it('stream', async () => { + const ciphertext = Buffer.from(fixtures.base64CiphertextAlgAes256GcmIv12Tag16HkdfSha384EcdsaP384(), 'base64') + const i = ciphertext.values() + const ciphertextStream = from((_: number, next: Function) => { + /* Pushing 1 byte at time is the most annoying thing. + * This is done intentionally to hit _every_ boundary condition. + */ + const { value, done } = i.next() + if (done) return next(null, null) + next(null, new Uint8Array([value])) + }) + + const { plaintext: test, messageHeader } = await decrypt( + fixtures.decryptKeyring(), + ciphertextStream + ) + + expect(messageHeader.suiteId).to.equal(AlgorithmSuiteIdentifier.ALG_AES256_GCM_IV12_TAG16_HKDF_SHA384_ECDSA_P384) + expect(messageHeader.encryptionContext).to.deep.equal(fixtures.encryptionContext()) + expect(test.toString('base64')).to.equal(fixtures.base64Plaintext()) }) }) diff --git a/modules/decrypt-node/test/fixtures.ts b/modules/decrypt-node/test/fixtures.ts new file mode 100644 index 000000000..8829d6aa9 --- /dev/null +++ b/modules/decrypt-node/test/fixtures.ts @@ -0,0 +1,53 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use + * this file except in compliance with the License. A copy of the License is + * located at + * + * http://aws.amazon.com/apache2.0/ + * + * or in the "license" file accompanying this file. This file is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* eslint-env mocha */ + +import { + NodeDecryptionMaterial, // eslint-disable-line no-unused-vars + NodeEncryptionMaterial, // eslint-disable-line no-unused-vars + KeyringNode, + KeyringTraceFlag +} from '@aws-crypto/material-management-node' + +export function base64CiphertextAlgAes256GcmIv12Tag16HkdfSha384EcdsaP384 () { + 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/B1gZuyGmQK2cSnDdRIL6bmAzzeTiMcjRoJ6KrYRbLwg8mzmdQLgdvSoPtFg==' +} + +export function base64Plaintext () { + return '3Ye0RVTIjYp9Yvi+81Dzq9h9gAUF6akM1mqTbPKMhmwgxTWuj6Wlf8UFUMG7zALPDpN77EleMS3dXUOGlr/nalmwXkBseEo+QxCJgeo6WMuB2xQHZqJT+0glM3mcl2FWwiQDZ9G84dYOW1KSDfiyISe9rTqARl0fmEnD1oB6zlP4cYg7+DDTxOOvw5RndoiOBZ+mLbZT9vHTsJkWB3HgFO06dFAtwSgjUAEaNWMjk04vIT+9SBvql6cOk+GLfUdkH33chNk22yKPF2UQ6+lvW0YqGODIfTBQXypPuuKYXJ3T583YxeiKoxuxZFpVNkg30r5cYPOYulINy+YrWQIbNFRP9Zk0CNkAJ7zsIMhQ8IXH+zG1bQmwh1RDGSAfZhmsR147Jsi6qty9Fe9O' +} + +export function encryptionContext () { + return { + 'aws-crypto-public-key': 'AnYtFEewZm+28KhIGpx8RhkaUaLccJpyf1nwIVQFGmypgzhH2XdSIBJ4sKiSH3ckzw==', + simple: 'context' + } +} + +export function decryptKeyring () { + class TestKeyring extends KeyringNode { + async _onEncrypt () : Promise { + throw new Error('I should never see this error') + } + async _onDecrypt (material: NodeDecryptionMaterial) { + const unencryptedDataKey = new Uint8Array(material.suite.keyLengthBytes).fill(0) + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY } + return material.setUnencryptedDataKey(unencryptedDataKey, trace) + } + } + + return new TestKeyring() +} diff --git a/modules/encrypt-node/package.json b/modules/encrypt-node/package.json index 319d11ef5..32b92dcbf 100644 --- a/modules/encrypt-node/package.json +++ b/modules/encrypt-node/package.json @@ -24,11 +24,13 @@ }, "devDependencies": { "@types/chai": "^4.1.4", + "@types/from2": "^2.3.0", "@types/mocha": "^5.2.5", "@types/node": "^11.11.4", "@typescript-eslint/eslint-plugin": "^1.9.0", "@typescript-eslint/parser": "^1.9.0", "chai": "^4.1.2", + "from2": "^2.3.0", "mocha": "^5.2.0", "nyc": "^14.0.0", "standard": "^12.0.1", diff --git a/modules/encrypt-node/src/encrypt.ts b/modules/encrypt-node/src/encrypt.ts index 545256c7c..321ee467f 100644 --- a/modules/encrypt-node/src/encrypt.ts +++ b/modules/encrypt-node/src/encrypt.ts @@ -23,7 +23,7 @@ export interface EncryptOutput { export async function encrypt ( cmm: KeyringNode|NodeMaterialsManager, - plaintext: Buffer|Uint8Array|Readable|string, + plaintext: Buffer|Uint8Array|Readable|string|NodeJS.ReadableStream, op: EncryptInput = {} ): Promise { const stream = encryptStream(cmm, op) diff --git a/modules/encrypt-node/test/encrypt.test.ts b/modules/encrypt-node/test/encrypt.test.ts index 17eea3377..f7d3c9318 100644 --- a/modules/encrypt-node/test/encrypt.test.ts +++ b/modules/encrypt-node/test/encrypt.test.ts @@ -15,20 +15,179 @@ /* eslint-env mocha */ -// import { expect } from 'chai' -// import 'mocha' -// import { -// NodeDecryptionMaterial, // eslint-disable-line no-unused-vars -// NodeAlgorithmSuite, NodeEncryptionMaterial, NodeCryptographicMaterialsManager, KeyringNode, EncryptedDataKey, -// KeyringTraceFlag, AlgorithmSuiteIdentifier -// } from '@aws-crypto/material-management-node' +import * as chai from 'chai' +import chaiAsPromised from 'chai-as-promised' +import 'mocha' +import { + NodeDecryptionMaterial, // eslint-disable-line no-unused-vars + NodeEncryptionMaterial, // eslint-disable-line no-unused-vars + KeyringNode, EncryptedDataKey, + KeyringTraceFlag, AlgorithmSuiteIdentifier, NodeAlgorithmSuite +} from '@aws-crypto/material-management-node' +import { + deserializeFactory, + MessageHeader // eslint-disable-line no-unused-vars +} from '@aws-crypto/serialize' +import { encrypt, encryptStream } from '../src/index' +import from from 'from2' +// @ts-ignore +import { finished } from 'readable-stream' +import { randomBytes } from 'crypto' -// import * as fs from 'fs' +chai.use(chaiAsPromised) +const { expect } = chai -// import { encryptStream, getEncryptionInfo } from '../src/encrypt_stream' +const toUtf8 = (input: Uint8Array) => Buffer + .from(input.buffer, input.byteOffset, input.byteLength) + .toString('utf8') +const { deserializeMessageHeader } = deserializeFactory(toUtf8, NodeAlgorithmSuite) -// import { getFramedEncryptStream } from '../src/framed_encrypt_stream' -// import { SignatureStream } from '../src/signature_stream' -// import { encrypt } from '../src/encrypt' +/* These tests only check structure. + * see decrypt-node for actual cryptographic tests + * see integration-node for exhaustive compatibility tests + */ +describe('encrypt structural testing', () => { + const edk = new EncryptedDataKey({ + providerId: 'k', + providerInfo: 'k', + encryptedDataKey: new Uint8Array(3), + /* rawInfo added because it will always be there when deserialized. + * This way deep equal will pass nicely. + * 107 is 'k' in ASCII + */ + rawInfo: new Uint8Array([107]) + }) + class TestKeyring extends KeyringNode { + async _onEncrypt (material: NodeEncryptionMaterial) { + const unencryptedDataKey = new Uint8Array(material.suite.keyLengthBytes).fill(0) + const trace = { keyNamespace: 'k', keyName: 'k', flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY } + return material + .setUnencryptedDataKey(unencryptedDataKey, trace) + .addEncryptedDataKey(edk, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY) + } + async _onDecrypt (): Promise { + throw new Error('I should never see this error') + } + } + + const keyRing = new TestKeyring() + + it('encrypt a string', async () => { + const suiteId = AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16 + + const plaintext = 'asdf' + const { ciphertext, messageHeader } = await encrypt(keyRing, plaintext, { suiteId }) + + expect(messageHeader.suiteId).to.equal(suiteId) + expect(messageHeader.encryptionContext).to.deep.equal({}) + expect(messageHeader.encryptedDataKeys).lengthOf(1) + expect(messageHeader.encryptedDataKeys[0]).to.deep.equal(edk) + + const messageInfo = deserializeMessageHeader(ciphertext) + if (!messageInfo) throw new Error('I should never see this error') + + expect(messageHeader).to.deep.equal(messageInfo.messageHeader) + }) + + it('encrypt a buffer', async () => { + const context = { simple: 'context' } + + const plaintext = Buffer.from('asdf') + const { ciphertext, messageHeader } = await encrypt(keyRing, plaintext, { context }) + + /* The default algorithm suite will add a signature key to the context. + * So I only check that the passed context elements exist. + */ + expect(messageHeader.encryptionContext).to.haveOwnProperty('simple').and.to.equal('context') + expect(messageHeader.encryptedDataKeys).lengthOf(1) + expect(messageHeader.encryptedDataKeys[0]).to.deep.equal(edk) + + const messageInfo = deserializeMessageHeader(ciphertext) + if (!messageInfo) throw new Error('I should never see this error') + + expect(messageHeader).to.deep.equal(messageInfo.messageHeader) + }) + + it('encrypt a stream', async () => { + const context = { simple: 'context' } + + let pushed = false + const plaintext = from((_: number, next: Function) => { + if (pushed) return next(null, null) + pushed = true + next(null, 'asdf') + }) + + const { ciphertext, messageHeader } = await encrypt(keyRing, plaintext, { context }) + + /* The default algorithm suite will add a signature key to the context. + * So I only check that the passed context elements exist. + */ + expect(messageHeader.encryptionContext).to.haveOwnProperty('simple').and.to.equal('context') + expect(messageHeader.encryptedDataKeys).lengthOf(1) + expect(messageHeader.encryptedDataKeys[0]).to.deep.equal(edk) + + const messageInfo = deserializeMessageHeader(ciphertext) + if (!messageInfo) throw new Error('I should never see this error') + + expect(messageHeader).to.deep.equal(messageInfo.messageHeader) + }) + + it('Unsupported plaintext', async () => { + const plaintext = {} as any + expect(encrypt(keyRing, plaintext)).to.rejectedWith(Error) + }) + + it('encryptStream', async () => { + const context = { simple: 'context' } + + const data = randomBytes(300) + const i = data.values() + const plaintext = from((_: number, next: Function) => { + /* Pushing 1 byte at time is the most annoying thing. + * This is done intentionally to hit _every_ boundary condition. + */ + const { value, done } = i.next() + if (done) return next(null, null) + next(null, new Uint8Array([value])) + }) + + let messageHeader: any + const buffer: Buffer[] = [] + const stream = plaintext + .pipe(encryptStream(keyRing, { context, frameLength: 5 })) + .on('MessageHeader', (header: MessageHeader) => { + // MessageHeader should only be called once + if (messageHeader) throw new Error('I should never see this error') + messageHeader = header + }) + // data event to drain the stream + .on('data', (chunk: Buffer) => { + buffer.push(chunk) + }) + + await finishedAsync(stream) + + if (!messageHeader) throw new Error('I should never see this error') + + const ciphertext = Buffer.concat(buffer) + + /* The default algorithm suite will add a signature key to the context. + * So I only check that the passed context elements exist. + */ + expect(messageHeader.encryptionContext).to.haveOwnProperty('simple').and.to.equal('context') + expect(messageHeader.encryptedDataKeys).lengthOf(1) + expect(messageHeader.encryptedDataKeys[0]).to.deep.equal(edk) + + const messageInfo = deserializeMessageHeader(ciphertext) + if (!messageInfo) throw new Error('I should never see this error') + + expect(messageHeader).to.deep.equal(messageInfo.messageHeader) + }) +}) -// const never = () => { throw new Error('never') } +function finishedAsync (stream: any) { + return new Promise((resolve, reject) => { + finished(stream, (err: Error) => err ? reject(err) : resolve()) + }) +} From 6a2dd9afa790de9454b325e80a8584f5f9b5dfcf Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 8 Jul 2019 14:28:41 -0700 Subject: [PATCH 2/3] lint --- modules/decrypt-node/src/verify_stream.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/decrypt-node/src/verify_stream.ts b/modules/decrypt-node/src/verify_stream.ts index d32cff9a8..784b91e56 100644 --- a/modules/decrypt-node/src/verify_stream.ts +++ b/modules/decrypt-node/src/verify_stream.ts @@ -175,7 +175,7 @@ export class VerifyStream extends PortableTransformWithType { if (chunk.length) { state.signatureInfo = Buffer.concat([state.signatureInfo, chunk]) } - + callback() } } @@ -188,7 +188,7 @@ export class VerifyStream extends PortableTransformWithType { * If the verify stream continues processing and sends the next auth tag, * before the current auth tag has been completed. * This is basically a back pressure issue. - * Since the frame size, and consequently the high water mark, + * Since the frame size, and consequently the high water mark, * can not be know when the stream is created, * the internal stream state would need to be modified. * I assert that a simple callback is a simpler way to handle this. From 5ae5c031881ee7b80e4deeeb2a1d5bf057df3110 Mon Sep 17 00:00:00 2001 From: seebees Date: Tue, 9 Jul 2019 12:34:49 -0700 Subject: [PATCH 3/3] remove debuging messages --- modules/decrypt-node/src/decrypt.ts | 3 --- modules/decrypt-node/src/decrypt_stream.ts | 4 ---- 2 files changed, 7 deletions(-) diff --git a/modules/decrypt-node/src/decrypt.ts b/modules/decrypt-node/src/decrypt.ts index 995d766c3..5019a46a8 100644 --- a/modules/decrypt-node/src/decrypt.ts +++ b/modules/decrypt-node/src/decrypt.ts @@ -46,9 +46,6 @@ export async function decrypt ( stream .once('MessageHeader', (header: MessageHeader) => { messageHeader = header }) .on('data', (chunk: Buffer) => plaintext.push(chunk)) - .on('error', () => { - console.log('wtf???') - }) // This will check both Uint8Array|Buffer if (ciphertext instanceof Uint8Array) { diff --git a/modules/decrypt-node/src/decrypt_stream.ts b/modules/decrypt-node/src/decrypt_stream.ts index 9a07d1a19..0a64981b1 100644 --- a/modules/decrypt-node/src/decrypt_stream.ts +++ b/modules/decrypt-node/src/decrypt_stream.ts @@ -46,10 +46,6 @@ export function decryptStream ( pipeline(parseHeaderStream, verifyStream, decipherStream) - decipherStream.on('error', (e) => { - console.log('?????', e) - }) - const stream = new Duplexify(parseHeaderStream, decipherStream) // Forward header events