Skip to content

fix: browser-encrypt can encrypt 0 bytes #866

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

Merged
merged 5 commits into from
Mar 14, 2022
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
5 changes: 3 additions & 2 deletions modules/encrypt-browser/src/encrypt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,15 @@ export async function _encrypt(
await getSubtleEncrypt(headerIv, header)(new Uint8Array(0))
)

const numberOfFrames = Math.ceil(plaintextLength / frameLength)
// In the case of plaintextLength == 0 there still needs to be 1 frame.
const numberOfFrames = Math.max(Math.ceil(plaintextLength / frameLength), 1)
/* The final frame has a variable length.
* The value needs to be known, but should only be calculated once.
* So I calculate how much of a frame I should have at the end.
* This value will NEVER be larger than the frameLength.
*/
const finalFrameLength =
frameLength - (numberOfFrames * frameLength - plaintextLength)
plaintextLength - (numberOfFrames - 1) * frameLength
const bodyContent = []

for (
Expand Down
41 changes: 41 additions & 0 deletions modules/encrypt-browser/test/encrypt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,45 @@ describe('encrypt structural testing', () => {
// This will throw if it does not deserialize correctly
deserializeSignature(footerSection)
})

it('can encrypt empty message', async () => {
const encryptionContext = { simple: 'context' }

const plaintext = new Uint8Array()
const { result, messageHeader } = await encrypt(keyRing, plaintext, {
encryptionContext,
})

/* 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 headerInfo = deserializeMessageHeader(result)
if (!headerInfo) throw new Error('I should never see this error')

expect(messageHeader).to.deep.equal(headerInfo.messageHeader)

const readPos =
headerInfo.headerLength + headerInfo.headerAuth.headerAuthLength
const bodyHeader = decodeBodyHeader(result, headerInfo, readPos)

if (!bodyHeader) throw new Error('I should never see this error')

expect(bodyHeader.isFinalFrame).to.equal(true)
expect(bodyHeader.contentLength).to.equal(0)

const sigPos =
bodyHeader.readPos + bodyHeader.contentLength + bodyHeader.tagLength / 8

// This implicitly tests that I have consumed all the data,
// because otherwise the footer section will be too large
const footerSection = result.slice(sigPos)
// This will throw if it does not deserialize correctly
deserializeSignature(footerSection)
})
})
37 changes: 37 additions & 0 deletions modules/encrypt-node/test/encrypt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,43 @@ describe('encrypt structural testing', () => {
// This will throw if it does not deserialize correctly
deserializeSignature(footerSection)
})

it('can encrypt empty message', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: any reason to check the encryption context and EDKs in the browser version of this test but not the Node version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a miss, inc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to elaborate more, when encrypting 0 bytes, the only part of the message that is relevant is the encryption context :)

const plaintext = new Uint8Array()
const { result, messageHeader } = await encrypt(keyRing, plaintext)

/* 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 headerInfo = deserializeMessageHeader(result)
if (!headerInfo) throw new Error('this should never happen')

expect(messageHeader).to.deep.equal(headerInfo.messageHeader)

const readPos =
headerInfo.headerLength + headerInfo.headerAuth.headerAuthLength
const bodyHeader = decodeBodyHeader(result, headerInfo, readPos)

if (!bodyHeader) throw new Error('I should never see this error')

expect(bodyHeader.isFinalFrame).to.equal(true)
expect(bodyHeader.contentLength).to.equal(0)

const sigPos =
bodyHeader.readPos + bodyHeader.contentLength + bodyHeader.tagLength / 8

// This implicitly tests that I have consumed all the data,
// because otherwise the footer section will be too large
const footerSection = result.slice(sigPos)
// This will throw if it does not deserialize correctly
deserializeSignature(footerSection)
})
})

async function finishedAsync(stream: any) {
Expand Down