From 26b699d1cceb63885af83f267958f7afeb5d42a1 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Tue, 28 May 2024 20:53:43 -0500 Subject: [PATCH] Store IV when encrypting or decrypting attachments Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com> --- ts/AttachmentCrypto.ts | 67 ++++++++++++++++++++++++--- ts/test-electron/Crypto_test.ts | 71 ++++++++++++++++++++++++++++- ts/textsecure/downloadAttachment.ts | 3 +- ts/types/Attachment.ts | 2 + ts/util/attachments.ts | 3 +- ts/util/uploadAttachment.ts | 1 + 6 files changed, 136 insertions(+), 11 deletions(-) diff --git a/ts/AttachmentCrypto.ts b/ts/AttachmentCrypto.ts index c7bc4e8373..81962e1add 100644 --- a/ts/AttachmentCrypto.ts +++ b/ts/AttachmentCrypto.ts @@ -27,6 +27,7 @@ import type { ContextType } from './types/Message2'; import { strictAssert } from './util/assert'; import * as Errors from './types/errors'; import { isNotNil } from './util/isNotNil'; +import { missingCaseError } from './util/missingCaseError'; // This file was split from ts/Crypto.ts because it pulls things in from node, and // too many things pull in Crypto.ts, so it broke storybook. @@ -46,12 +47,14 @@ export function _generateAttachmentIv(): Uint8Array { export type EncryptedAttachmentV2 = { digest: Uint8Array; + iv: Uint8Array; plaintextHash: string; ciphertextSize: number; }; export type DecryptedAttachmentV2 = { path: string; + iv: Uint8Array; plaintextHash: string; }; @@ -59,10 +62,21 @@ export type PlaintextSourceType = | { data: Uint8Array } | { absolutePath: string }; +export type HardcodedIVForEncryptionType = + | { + reason: 'test'; + iv: Uint8Array; + } + | { + reason: 'reencrypting-for-backup'; + iv: Uint8Array; + digestToMatch: Uint8Array; + }; + type EncryptAttachmentV2PropsType = { plaintext: PlaintextSourceType; keys: Readonly; - dangerousTestOnlyIv?: Readonly; + dangerousIv?: HardcodedIVForEncryptionType; dangerousTestOnlySkipPadding?: boolean; }; @@ -96,7 +110,7 @@ export async function encryptAttachmentV2ToDisk( export async function encryptAttachmentV2({ keys, plaintext, - dangerousTestOnlyIv, + dangerousIv, dangerousTestOnlySkipPadding, sink, }: EncryptAttachmentV2PropsType & { @@ -106,9 +120,26 @@ export async function encryptAttachmentV2({ const { aesKey, macKey } = splitKeys(keys); - if (dangerousTestOnlyIv && window.getEnvironment() !== Environment.Test) { - throw new Error(`${logId}: Used dangerousTestOnlyIv outside tests!`); + if (dangerousIv) { + if (dangerousIv.reason === 'test') { + if (window.getEnvironment() !== Environment.Test) { + throw new Error( + `${logId}: Used dangerousIv with reason test outside tests!` + ); + } + } else if (dangerousIv.reason === 'reencrypting-for-backup') { + strictAssert( + dangerousIv.digestToMatch.byteLength === DIGEST_LENGTH, + `${logId}: Must provide valid digest to match if providing iv for re-encryption` + ); + log.info( + `${logId}: using hardcoded iv because we are re-encrypting for backup` + ); + } else { + throw missingCaseError(dangerousIv); + } } + if ( dangerousTestOnlySkipPadding && window.getEnvironment() !== Environment.Test @@ -117,7 +148,8 @@ export async function encryptAttachmentV2({ `${logId}: Used dangerousTestOnlySkipPadding outside tests!` ); } - const iv = dangerousTestOnlyIv || _generateAttachmentIv(); + + const iv = dangerousIv?.iv || _generateAttachmentIv(); const plaintextHash = createHash(HashType.size256); const digest = createHash(HashType.size256); @@ -167,8 +199,16 @@ export async function encryptAttachmentV2({ strictAssert(ciphertextSize != null, 'Failed to measure ciphertext size!'); + if (dangerousIv?.reason === 'reencrypting-for-backup') { + if (!constantTimeEqual(ourDigest, dangerousIv.digestToMatch)) { + throw new Error( + `${logId}: iv was hardcoded for backup re-encryption, but digest does not match` + ); + } + } return { digest: ourDigest, + iv, plaintextHash: ourPlaintextHash, ciphertextSize, }; @@ -230,6 +270,7 @@ export async function decryptAttachmentV2( let readFd; let writeFd; + let iv: Uint8Array | undefined; try { try { readFd = await open(ciphertextPath, 'r'); @@ -252,7 +293,9 @@ export async function decryptAttachmentV2( getMacAndUpdateHmac(hmac, theirMacValue => { theirMac = theirMacValue; }), - getIvAndDecipher(aesKey), + getIvAndDecipher(aesKey, theirIv => { + iv = theirIv; + }), trimPadding(options.size), peekAndUpdateHash(plaintextHash), writeFd.createWriteStream(), @@ -297,6 +340,11 @@ export async function decryptAttachmentV2( throw new Error(`${logId}: Bad digest`); } + strictAssert( + iv != null && iv.byteLength === IV_LENGTH, + `${logId}: failed to find their iv` + ); + if (outerEncryption) { strictAssert(outerHmac, 'outerHmac must exist'); @@ -318,6 +366,7 @@ export async function decryptAttachmentV2( return { path: relativeTargetPath, + iv, plaintextHash: ourPlaintextHash, }; } @@ -404,7 +453,10 @@ export function getMacAndUpdateHmac( * Gets the IV from the start of the stream and creates a decipher. * Then deciphers the rest of the stream. */ -export function getIvAndDecipher(aesKey: Uint8Array): Transform { +export function getIvAndDecipher( + aesKey: Uint8Array, + onFoundIv?: (iv: Buffer) => void +): Transform { let maybeIvBytes: Buffer | null = Buffer.alloc(0); let decipher: Decipher | null = null; return new Transform({ @@ -428,6 +480,7 @@ export function getIvAndDecipher(aesKey: Uint8Array): Transform { // remainder of the bytes through. const iv = maybeIvBytes.subarray(0, IV_LENGTH); const remainder = maybeIvBytes.subarray(IV_LENGTH); + onFoundIv?.(iv); maybeIvBytes = null; // free memory decipher = createDecipheriv(CipherType.AES256CBC, aesKey, iv); callback(null, decipher.update(remainder)); diff --git a/ts/test-electron/Crypto_test.ts b/ts/test-electron/Crypto_test.ts index b6bc947a0f..500cd5d7c8 100644 --- a/ts/test-electron/Crypto_test.ts +++ b/ts/test-electron/Crypto_test.ts @@ -37,6 +37,7 @@ import { CipherType, } from '../Crypto'; import { + type HardcodedIVForEncryptionType, KEY_SET_LENGTH, _generateAttachmentIv, decryptAttachmentV2, @@ -608,19 +609,24 @@ describe('Crypto', () => { path, data, plaintextHash, + encryptionKeys, + dangerousIv, }: { path?: string; data: Uint8Array; plaintextHash: Uint8Array; + encryptionKeys?: Uint8Array; + dangerousIv?: HardcodedIVForEncryptionType; }): Promise { let plaintextPath; let ciphertextPath; - const keys = generateAttachmentKeys(); + const keys = encryptionKeys ?? generateAttachmentKeys(); try { const encryptedAttachment = await encryptAttachmentV2ToDisk({ keys, plaintext: path ? { absolutePath: path } : { data }, + dangerousIv, }); ciphertextPath = window.Signal.Migrations.getAbsoluteAttachmentPath( @@ -639,6 +645,21 @@ describe('Crypto', () => { ); const plaintext = readFileSync(plaintextPath); + + assert.deepStrictEqual( + encryptedAttachment.iv, + decryptedAttachment.iv + ); + if (dangerousIv) { + assert.deepStrictEqual(encryptedAttachment.iv, dangerousIv.iv); + if (dangerousIv.reason === 'reencrypting-for-backup') { + assert.deepStrictEqual( + encryptedAttachment.digest, + dangerousIv.digestToMatch + ); + } + } + assert.isTrue(constantTimeEqual(data, plaintext)); assert.strictEqual( encryptedAttachment.ciphertextSize, @@ -711,6 +732,52 @@ describe('Crypto', () => { plaintextHash, }); }); + describe('dangerousIv', () => { + it('uses hardcodedIv in tests', async () => { + await testV2RoundTripData({ + data: FILE_CONTENTS, + plaintextHash: FILE_HASH, + dangerousIv: { + reason: 'test', + iv: _generateAttachmentIv(), + }, + }); + }); + + it('uses hardcodedIv when re-encrypting for backup', async () => { + const keys = generateAttachmentKeys(); + const previouslyEncrypted = await encryptAttachmentV2ToDisk({ + keys, + plaintext: { data: FILE_CONTENTS }, + }); + + await testV2RoundTripData({ + data: FILE_CONTENTS, + plaintextHash: FILE_HASH, + encryptionKeys: keys, + dangerousIv: { + reason: 'reencrypting-for-backup', + iv: previouslyEncrypted.iv, + digestToMatch: previouslyEncrypted.digest, + }, + }); + + // If the digest is wrong, it should throw + await assert.isRejected( + testV2RoundTripData({ + data: FILE_CONTENTS, + plaintextHash: FILE_HASH, + encryptionKeys: keys, + dangerousIv: { + reason: 'reencrypting-for-backup', + iv: previouslyEncrypted.iv, + digestToMatch: getRandomBytes(32), + }, + }), + 'iv was hardcoded for backup re-encryption, but digest does not match' + ); + }); + }); }); it('v2 -> v1 (disk -> memory)', async () => { @@ -774,7 +841,7 @@ describe('Crypto', () => { const encryptedAttachmentV2 = await encryptAttachmentV2ToDisk({ keys, plaintext: { absolutePath: FILE_PATH }, - dangerousTestOnlyIv, + dangerousIv: { iv: dangerousTestOnlyIv, reason: 'test' }, }); ciphertextPath = window.Signal.Migrations.getAbsoluteAttachmentPath( encryptedAttachmentV2.path diff --git a/ts/textsecure/downloadAttachment.ts b/ts/textsecure/downloadAttachment.ts index 336779a977..f6a3d4f1d7 100644 --- a/ts/textsecure/downloadAttachment.ts +++ b/ts/textsecure/downloadAttachment.ts @@ -132,7 +132,7 @@ export async function downloadAttachment( window.Signal.Migrations.getAbsoluteAttachmentPath(downloadedPath); const { aesKey, macKey } = splitKeys(Bytes.fromBase64(key)); - const { path, plaintextHash } = await decryptAttachmentV2({ + const { path, plaintextHash, iv } = await decryptAttachmentV2({ ciphertextPath: cipherTextAbsolutePath, idForLogging: logId, aesKey, @@ -155,6 +155,7 @@ export async function downloadAttachment( ? MIME.stringToMIMEType(contentType) : MIME.APPLICATION_OCTET_STREAM, plaintextHash, + iv: Bytes.toBase64(iv), }; } diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index b689c5265a..7be2ca6681 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -74,6 +74,7 @@ export type AttachmentType = { cdnId?: string; cdnKey?: string; key?: string; + iv?: string; data?: Uint8Array; textAttachment?: TextAttachmentType; wasTooBig?: boolean; @@ -97,6 +98,7 @@ export type UploadedAttachmentType = Proto.IAttachmentPointer & Readonly<{ // Required fields cdnKey: string; + iv: Uint8Array; key: Uint8Array; size: number; digest: Uint8Array; diff --git a/ts/util/attachments.ts b/ts/util/attachments.ts index 7f88417bef..38b5541dd5 100644 --- a/ts/util/attachments.ts +++ b/ts/util/attachments.ts @@ -77,7 +77,7 @@ export const downscaleOutgoingAttachment = async ( export type CdnFieldsType = Pick< AttachmentType, - 'cdnId' | 'cdnKey' | 'cdnNumber' | 'key' | 'digest' | 'plaintextHash' + 'cdnId' | 'cdnKey' | 'cdnNumber' | 'key' | 'digest' | 'iv' | 'plaintextHash' >; export function copyCdnFields( @@ -91,6 +91,7 @@ export function copyCdnFields( cdnKey: uploaded.cdnKey, cdnNumber: dropNull(uploaded.cdnNumber), key: Bytes.toBase64(uploaded.key), + iv: Bytes.toBase64(uploaded.iv), digest: Bytes.toBase64(uploaded.digest), plaintextHash: uploaded.plaintextHash, }; diff --git a/ts/util/uploadAttachment.ts b/ts/util/uploadAttachment.ts index 0f784da8e3..d0eacbf887 100644 --- a/ts/util/uploadAttachment.ts +++ b/ts/util/uploadAttachment.ts @@ -40,6 +40,7 @@ export async function uploadAttachment( cdnKey, cdnNumber, key: keys, + iv: encrypted.iv, size: attachment.data.byteLength, digest: encrypted.digest, plaintextHash: encrypted.plaintextHash,