Revert "Store IV when encrypting or decrypting attachments"

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

ts/AttachmentCrypto.ts

@@ -27,7 +27,6 @@ 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.
@@ -47,14 +46,12 @@ export function _generateAttachmentIv(): Uint8Array {
 
 export type EncryptedAttachmentV2 = {
   digest: Uint8Array;
-  iv: Uint8Array;
   plaintextHash: string;
   ciphertextSize: number;
 };
 
 export type DecryptedAttachmentV2 = {
   path: string;
-  iv: Uint8Array;
   plaintextHash: string;
 };
 
@@ -62,21 +59,10 @@ 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<Uint8Array>;
-  dangerousIv?: HardcodedIVForEncryptionType;
+  dangerousTestOnlyIv?: Readonly<Uint8Array>;
   dangerousTestOnlySkipPadding?: boolean;
 };
 
@@ -110,7 +96,7 @@ export async function encryptAttachmentV2ToDisk(
 export async function encryptAttachmentV2({
   keys,
   plaintext,
-  dangerousIv,
+  dangerousTestOnlyIv,
   dangerousTestOnlySkipPadding,
   sink,
 }: EncryptAttachmentV2PropsType & {
@@ -120,26 +106,9 @@ export async function encryptAttachmentV2({
 
   const { aesKey, macKey } = splitKeys(keys);
 
-  if (dangerousIv) {
+  if (dangerousTestOnlyIv && window.getEnvironment() !== Environment.Test) {
-    if (dangerousIv.reason === 'test') {
+    throw new Error(`${logId}: Used dangerousTestOnlyIv outside tests!`);
-      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
@@ -148,8 +117,7 @@ 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);
 
@@ -199,16 +167,8 @@ 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,
   };
@@ -270,7 +230,6 @@ export async function decryptAttachmentV2(
 
   let readFd;
   let writeFd;
-  let iv: Uint8Array | undefined;
   try {
     try {
       readFd = await open(ciphertextPath, 'r');
@@ -293,9 +252,7 @@ export async function decryptAttachmentV2(
         getMacAndUpdateHmac(hmac, theirMacValue => {
           theirMac = theirMacValue;
         }),
-        getIvAndDecipher(aesKey, theirIv => {
+        getIvAndDecipher(aesKey),
-          iv = theirIv;
-        }),
         trimPadding(options.size),
         peekAndUpdateHash(plaintextHash),
         writeFd.createWriteStream(),
@@ -340,11 +297,6 @@ 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');
 
@@ -366,7 +318,6 @@ export async function decryptAttachmentV2(
 
   return {
     path: relativeTargetPath,
-    iv,
     plaintextHash: ourPlaintextHash,
   };
 }
@@ -453,10 +404,7 @@ 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(
+export function getIvAndDecipher(aesKey: Uint8Array): Transform {
-  aesKey: Uint8Array,
-  onFoundIv?: (iv: Buffer) => void
-): Transform {
   let maybeIvBytes: Buffer | null = Buffer.alloc(0);
   let decipher: Decipher | null = null;
   return new Transform({
@@ -480,7 +428,6 @@ export function getIvAndDecipher(
         // 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));

ts/test-electron/Crypto_test.ts

@@ -37,7 +37,6 @@ import {
   CipherType,
 } from '../Crypto';
 import {
-  type HardcodedIVForEncryptionType,
   KEY_SET_LENGTH,
   _generateAttachmentIv,
   decryptAttachmentV2,
@@ -609,24 +608,19 @@ describe('Crypto', () => {
         path,
         data,
         plaintextHash,
-        encryptionKeys,
-        dangerousIv,
       }: {
         path?: string;
         data: Uint8Array;
         plaintextHash: Uint8Array;
-        encryptionKeys?: Uint8Array;
-        dangerousIv?: HardcodedIVForEncryptionType;
       }): Promise<void> {
         let plaintextPath;
         let ciphertextPath;
-        const keys = encryptionKeys ?? generateAttachmentKeys();
+        const keys = generateAttachmentKeys();
 
         try {
           const encryptedAttachment = await encryptAttachmentV2ToDisk({
             keys,
             plaintext: path ? { absolutePath: path } : { data },
-            dangerousIv,
           });
 
           ciphertextPath = window.Signal.Migrations.getAbsoluteAttachmentPath(
@@ -645,21 +639,6 @@ 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,
@@ -732,52 +711,6 @@ 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 () => {
@@ -841,7 +774,7 @@ describe('Crypto', () => {
         const encryptedAttachmentV2 = await encryptAttachmentV2ToDisk({
           keys,
           plaintext: { absolutePath: FILE_PATH },
-          dangerousIv: { iv: dangerousTestOnlyIv, reason: 'test' },
+          dangerousTestOnlyIv,
         });
         ciphertextPath = window.Signal.Migrations.getAbsoluteAttachmentPath(
           encryptedAttachmentV2.path

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, iv } = await decryptAttachmentV2({
+  const { path, plaintextHash } = await decryptAttachmentV2({
     ciphertextPath: cipherTextAbsolutePath,
     idForLogging: logId,
     aesKey,
@@ -155,7 +155,6 @@ export async function downloadAttachment(
       ? MIME.stringToMIMEType(contentType)
       : MIME.APPLICATION_OCTET_STREAM,
     plaintextHash,
-    iv: Bytes.toBase64(iv),
   };
 }
 

ts/types/Attachment.ts

@@ -74,7 +74,6 @@ export type AttachmentType = {
   cdnId?: string;
   cdnKey?: string;
   key?: string;
-  iv?: string;
   data?: Uint8Array;
   textAttachment?: TextAttachmentType;
   wasTooBig?: boolean;
@@ -98,7 +97,6 @@ export type UploadedAttachmentType = Proto.IAttachmentPointer &
   Readonly<{
     // Required fields
     cdnKey: string;
-    iv: Uint8Array;
     key: Uint8Array;
     size: number;
     digest: Uint8Array;

ts/util/attachments.ts

@@ -77,7 +77,7 @@ export const downscaleOutgoingAttachment = async (
 
 export type CdnFieldsType = Pick<
   AttachmentType,
-  'cdnId' | 'cdnKey' | 'cdnNumber' | 'key' | 'digest' | 'iv' | 'plaintextHash'
+  'cdnId' | 'cdnKey' | 'cdnNumber' | 'key' | 'digest' | 'plaintextHash'
 >;
 
 export function copyCdnFields(
@@ -91,7 +91,6 @@ 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,
   };

ts/util/uploadAttachment.ts

@@ -40,7 +40,6 @@ export async function uploadAttachment(
     cdnKey,
     cdnNumber,
     key: keys,
-    iv: encrypted.iv,
     size: attachment.data.byteLength,
     digest: encrypted.digest,
     plaintextHash: encrypted.plaintextHash,