Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: plaintextLength must be enforced #213

Merged
merged 4 commits into from
Sep 19, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions modules/encrypt-node/src/encrypt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion modules/encrypt-node/src/encrypt_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions modules/encrypt-node/src/framed_encrypt_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions modules/encrypt-node/test/framed_encrypt_stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions modules/serialize/src/identifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down