From 5bfb87ef033e06aae20980655078361c728cd490 Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Mon, 22 Sep 2025 18:14:20 -0400 Subject: [PATCH] Fix attachment ciphertext size calculations for backup tier downloads --- ts/AttachmentCrypto.ts | 36 +++++++++++++++---- ts/jobs/AttachmentBackupManager.ts | 23 ++++++++---- ts/jobs/AttachmentDownloadManager.ts | 13 +++++-- ts/test-electron/Crypto_test.ts | 12 ++++--- .../AttachmentDownloadManager_test.ts | 8 +++-- ts/textsecure/downloadAttachment.ts | 35 ++++++++++-------- ts/util/processAttachment.ts | 8 +++-- 7 files changed, 98 insertions(+), 37 deletions(-) diff --git a/ts/AttachmentCrypto.ts b/ts/AttachmentCrypto.ts index 5551e9e0bf..db28ad4fa8 100644 --- a/ts/AttachmentCrypto.ts +++ b/ts/AttachmentCrypto.ts @@ -52,6 +52,7 @@ import { missingCaseError } from './util/missingCaseError.js'; import { getEnvironment, Environment } from './environment.js'; import { isNotEmpty, toBase64, toHex } from './Bytes.js'; import { decipherWithAesKey } from './util/decipherWithAesKey.js'; +import { MediaTier } from './types/AttachmentDownload.js'; const { ensureFile } = fsExtra; @@ -198,7 +199,12 @@ export async function encryptAttachmentV2({ ); } chunkSizeChoice = isNumber(size) - ? inferChunkSize(getAttachmentCiphertextLength(size)) + ? inferChunkSize( + getAttachmentCiphertextSize({ + unpaddedPlaintextSize: size, + mediaTier: MediaTier.STANDARD, + }) + ) : undefined; incrementalDigestCreator = needIncrementalMac && chunkSizeChoice @@ -664,20 +670,38 @@ export function measureSize({ return passthrough; } -export function getAttachmentCiphertextLength(plaintextLength: number): number { - const paddedPlaintextSize = logPadSize(plaintextLength); +export function getAttachmentCiphertextSize({ + unpaddedPlaintextSize, + mediaTier, +}: { + unpaddedPlaintextSize: number; + mediaTier: MediaTier; +}): number { + const paddedSize = logPadSize(unpaddedPlaintextSize); + switch (mediaTier) { + case MediaTier.STANDARD: + return getCiphertextSize(paddedSize); + case MediaTier.BACKUP: + // objects on backup tier are doubly encrypted! + return getCiphertextSize(getCiphertextSize(paddedSize)); + default: + throw missingCaseError(mediaTier); + } +} + +export function getCiphertextSize(paddedPlaintextSize: number): number { return ( IV_LENGTH + - getAesCbcCiphertextLength(paddedPlaintextSize) + + getAesCbcCiphertextSize(paddedPlaintextSize) + ATTACHMENT_MAC_LENGTH ); } -export function getAesCbcCiphertextLength(plaintextLength: number): number { +export function getAesCbcCiphertextSize(plaintextSize: number): number { const AES_CBC_BLOCK_SIZE = 16; return ( - (1 + Math.floor(plaintextLength / AES_CBC_BLOCK_SIZE)) * AES_CBC_BLOCK_SIZE + (1 + Math.floor(plaintextSize / AES_CBC_BLOCK_SIZE)) * AES_CBC_BLOCK_SIZE ); } diff --git a/ts/jobs/AttachmentBackupManager.ts b/ts/jobs/AttachmentBackupManager.ts index ad982d86ec..d7b489c4b2 100644 --- a/ts/jobs/AttachmentBackupManager.ts +++ b/ts/jobs/AttachmentBackupManager.ts @@ -23,9 +23,8 @@ import { } from '../services/backups/index.js'; import { type EncryptedAttachmentV2, - getAttachmentCiphertextLength, - getAesCbcCiphertextLength, decryptAttachmentV2ToSink, + getAttachmentCiphertextSize, } from '../AttachmentCrypto.js'; import { getBackupMediaRootKey, @@ -66,6 +65,7 @@ import { findRetryAfterTimeFromError } from './helpers/findRetryAfterTimeFromErr import { BackupCredentialType } from '../types/backups.js'; import { supportsIncrementalMac } from '../types/MIME.js'; import type { MIMEType } from '../types/MIME.js'; +import { MediaTier } from '../types/AttachmentDownload.js'; const log = createLogger('AttachmentBackupManager'); @@ -582,7 +582,10 @@ async function copyToBackupTier({ dependencies.backupMediaBatch, 'backupMediaBatch must be intialized' ); - const ciphertextLength = getAttachmentCiphertextLength(size); + const ciphertextSizeOnTransitTier = getAttachmentCiphertextSize({ + unpaddedPlaintextSize: size, + mediaTier: MediaTier.STANDARD, + }); const { responses } = await dependencies.backupMediaBatch({ headers: await dependencies.backupsService.credentials.getHeadersForToday( @@ -594,7 +597,7 @@ async function copyToBackupTier({ cdn: cdnNumber, key: cdnKey, }, - objectLength: ciphertextLength, + objectLength: ciphertextSizeOnTransitTier, mediaId, hmacKey: macKey, encryptionKey: aesKey, @@ -613,9 +616,17 @@ async function copyToBackupTier({ } // Update our local understanding of what's in the backup cdn - const sizeOnBackupCdn = getAesCbcCiphertextLength(ciphertextLength); + const ciphertextSizeOnBackupTier = getAttachmentCiphertextSize({ + unpaddedPlaintextSize: size, + mediaTier: MediaTier.BACKUP, + }); + await DataWriter.saveBackupCdnObjectMetadata([ - { mediaId, cdnNumber: response.cdn, sizeOnBackupCdn }, + { + mediaId, + cdnNumber: response.cdn, + sizeOnBackupCdn: ciphertextSizeOnBackupTier, + }, ]); return { diff --git a/ts/jobs/AttachmentDownloadManager.ts b/ts/jobs/AttachmentDownloadManager.ts index 5124ed2725..e18eb27a3f 100644 --- a/ts/jobs/AttachmentDownloadManager.ts +++ b/ts/jobs/AttachmentDownloadManager.ts @@ -12,6 +12,7 @@ import { type CoreAttachmentDownloadJobType, AttachmentDownloadUrgency, coreAttachmentDownloadJobSchema, + MediaTier, } from '../types/AttachmentDownload.js'; import { downloadAttachment as downloadAttachmentUtil, @@ -52,7 +53,7 @@ import { IMAGE_WEBP } from '../types/MIME.js'; import { AttachmentDownloadSource } from '../sql/Interface.js'; import { drop } from '../util/drop.js'; import { - getAttachmentCiphertextLength, + getAttachmentCiphertextSize, type ReencryptedAttachmentV2, } from '../AttachmentCrypto.js'; import { safeParsePartial } from '../util/schemas.js'; @@ -319,7 +320,15 @@ export class AttachmentDownloadManager extends JobManager { assert.strictEqual( encryptedAttachment.ciphertextSize, - getAttachmentCiphertextLength(data.byteLength) + getAttachmentCiphertextSize({ + unpaddedPlaintextSize: data.byteLength, + mediaTier: MediaTier.STANDARD, + }) ); if (overrideSize == null) { @@ -1193,7 +1197,7 @@ describe('Crypto', () => { } it('calculates cipherTextLength correctly', () => { for (let i = 0; i < 128; i += 1) { - assert.strictEqual(getAesCbcCiphertextLength(i), encrypt(i).length); + assert.strictEqual(getAesCbcCiphertextSize(i), encrypt(i).length); } }); }); diff --git a/ts/test-electron/services/AttachmentDownloadManager_test.ts b/ts/test-electron/services/AttachmentDownloadManager_test.ts index d73d17c815..3c57f4b3fd 100644 --- a/ts/test-electron/services/AttachmentDownloadManager_test.ts +++ b/ts/test-electron/services/AttachmentDownloadManager_test.ts @@ -17,6 +17,7 @@ import { import { type AttachmentDownloadJobType, AttachmentDownloadUrgency, + MediaTier, } from '../../types/AttachmentDownload.js'; import { DataReader, DataWriter } from '../../sql/Client.js'; import { DAY, MINUTE, MONTH } from '../../util/durations/index.js'; @@ -29,7 +30,7 @@ import type { downloadAttachment as downloadAttachmentUtil } from '../../util/do import { AttachmentDownloadSource } from '../../sql/Interface.js'; import { generateAttachmentKeys, - getAttachmentCiphertextLength, + getAttachmentCiphertextSize, } from '../../AttachmentCrypto.js'; import { MEBIBYTE } from '../../types/AttachmentSize.js'; import { generateAci } from '../../types/ServiceId.js'; @@ -58,7 +59,10 @@ function composeJob({ attachmentType: 'attachment', attachmentSignature: `${digest}.${plaintextHash}`, size, - ciphertextSize: getAttachmentCiphertextLength(size), + ciphertextSize: getAttachmentCiphertextSize({ + unpaddedPlaintextSize: size, + mediaTier: MediaTier.STANDARD, + }), contentType, active: false, attempts: 0, diff --git a/ts/textsecure/downloadAttachment.ts b/ts/textsecure/downloadAttachment.ts index 64389d392a..27590986b6 100644 --- a/ts/textsecure/downloadAttachment.ts +++ b/ts/textsecure/downloadAttachment.ts @@ -22,7 +22,7 @@ import { } from '../types/Attachment.js'; import * as Bytes from '../Bytes.js'; import { - getAttachmentCiphertextLength, + getAttachmentCiphertextSize, safeUnlink, splitKeys, type ReencryptedAttachmentV2, @@ -142,6 +142,7 @@ export async function downloadAttachment( let downloadResult: Awaited>; let downloadPath = + mediaTier === MediaTier.STANDARD && options.variant === AttachmentVariant.Default ? attachment.downloadPath : undefined; @@ -170,11 +171,13 @@ export async function downloadAttachment( } } + const expectedCiphertextSize = getAttachmentCiphertextSize({ + unpaddedPlaintextSize: size, + mediaTier, + }); + // Start over if we go over the size - if ( - downloadOffset >= getAttachmentCiphertextLength(size) && - absoluteDownloadPath - ) { + if (downloadOffset >= expectedCiphertextSize && absoluteDownloadPath) { log.warn('went over, retrying'); await safeUnlink(absoluteDownloadPath); downloadOffset = 0; @@ -211,7 +214,7 @@ export async function downloadAttachment( downloadPath, downloadStream, onSizeUpdate: options.onSizeUpdate, - size, + expectedCiphertextSize, }); } else { strictAssert(mediaTier === MediaTier.BACKUP, 'backup media tier'); @@ -256,12 +259,14 @@ export async function downloadAttachment( downloadPath, downloadOffset, onSizeUpdate: options.onSizeUpdate, - size: getAttachmentCiphertextLength( + expectedCiphertextSize: options.variant === AttachmentVariant.ThumbnailFromBackup - ? // be generous, accept downloads of up to twice what we expect for thumbnail - MAX_BACKUP_THUMBNAIL_SIZE * 2 - : size - ), + ? getAttachmentCiphertextSize({ + // to be generous, we accept downloads of up to twice what we expect + unpaddedPlaintextSize: MAX_BACKUP_THUMBNAIL_SIZE * 2, + mediaTier: MediaTier.BACKUP, + }) + : expectedCiphertextSize, }); } @@ -345,13 +350,13 @@ async function downloadToDisk({ downloadPath, downloadStream, onSizeUpdate, - size, + expectedCiphertextSize, }: { downloadOffset?: number; downloadPath?: string; downloadStream: Readable; onSizeUpdate: (totalBytes: number) => void; - size: number; + expectedCiphertextSize: number; }): Promise<{ absolutePath: string; downloadSize: number }> { const absoluteTargetPath = downloadPath ? window.Signal.Migrations.getAbsoluteDownloadsPath(downloadPath) @@ -373,7 +378,7 @@ async function downloadToDisk({ writeStream = createWriteStream(absoluteTargetPath); } - const targetSize = getAttachmentCiphertextLength(size) - downloadOffset; + const targetSize = expectedCiphertextSize - downloadOffset; let downloadSize = 0; try { @@ -428,7 +433,7 @@ function checkSize(expectedBytes: number) { } if (totalBytes > expectedBytes) { - log.warn( + log.error( `checkSize: Received ${totalBytes} bytes, expected ${expectedBytes}` ); } diff --git a/ts/util/processAttachment.ts b/ts/util/processAttachment.ts index 4e2149c88f..9d599face6 100644 --- a/ts/util/processAttachment.ts +++ b/ts/util/processAttachment.ts @@ -21,7 +21,8 @@ import { handleVideoAttachment } from './handleVideoAttachment.js'; import { isHeic, stringToMIMEType } from '../types/MIME.js'; import { ToastType } from '../types/Toast.js'; import { isImageTypeSupported, isVideoTypeSupported } from './GoogleChrome.js'; -import { getAttachmentCiphertextLength } from '../AttachmentCrypto.js'; +import { getAttachmentCiphertextSize } from '../AttachmentCrypto.js'; +import { MediaTier } from '../types/AttachmentDownload.js'; const log = createLogger('processAttachment'); @@ -85,7 +86,10 @@ function isAttachmentSizeOkay(attachment: Readonly): boolean { const limitBytes = getMaximumOutgoingAttachmentSizeInKb(getRemoteConfigValue) * KIBIBYTE; - const paddedAndEncryptedSize = getAttachmentCiphertextLength(attachment.size); + const paddedAndEncryptedSize = getAttachmentCiphertextSize({ + unpaddedPlaintextSize: attachment.size, + mediaTier: MediaTier.STANDARD, + }); if (paddedAndEncryptedSize > limitBytes) { window.reduxActions.toast.showToast({ toastType: ToastType.FileSize,