diff --git a/app/attachment_channel.ts b/app/attachment_channel.ts index 0694eac3dd..50ac161c0b 100644 --- a/app/attachment_channel.ts +++ b/app/attachment_channel.ts @@ -134,7 +134,7 @@ async function safeDecryptToSink( }); file.on('error', (error: Error) => { log.warn( - 'safeDecryptToSync/incremental: growing-file emitted an error:', + 'safeDecryptToSink/incremental: growing-file emitted an error:', Errors.toLogFormat(error) ); }); diff --git a/package.json b/package.json index 8bf4ef0627..26c01f7a2f 100644 --- a/package.json +++ b/package.json @@ -130,7 +130,7 @@ "@react-aria/utils": "3.25.3", "@react-spring/web": "9.7.5", "@react-types/shared": "3.27.0", - "@signalapp/libsignal-client": "0.79.1", + "@signalapp/libsignal-client": "0.80.0", "@signalapp/minimask": "1.0.1", "@signalapp/quill-cjs": "2.1.2", "@signalapp/ringrtc": "2.57.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 583ea37bbc..aa7e325171 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -126,8 +126,8 @@ importers: specifier: 3.27.0 version: 3.27.0(react@18.3.1) '@signalapp/libsignal-client': - specifier: 0.79.1 - version: 0.79.1 + specifier: 0.80.0 + version: 0.80.0 '@signalapp/minimask': specifier: 1.0.1 version: 1.0.1 @@ -3296,8 +3296,8 @@ packages: '@signalapp/libsignal-client@0.76.7': resolution: {integrity: sha512-iGWTlFkko7IKlm96Iy91Wz5sIN089nj02ifOk6BWtLzeVi0kFaNj+jK26Sl1JRXy/VfXevcYtiOivOg43BPqpg==} - '@signalapp/libsignal-client@0.79.1': - resolution: {integrity: sha512-mL+SCJEbDqLYpd5JtA53JgjuYinrWF7jSonA2bD3HJMdHyAJ1jP+ThPD3Vh71yR+EhTCflyr5Tm43sQ0KoLtlg==} + '@signalapp/libsignal-client@0.80.0': + resolution: {integrity: sha512-cOFORDUUSdQFjQ8RDA8niC1UIoFRi7+Dd8ZNizGcJ2Q16D9RhKU9yZ0/VIef+mWu/iJcPovoC3GCvw0nE881vw==} '@signalapp/minimask@1.0.1': resolution: {integrity: sha512-QAwo0joA60urTNbW9RIz6vLKQjy+jdVtH7cvY0wD9PVooD46MAjE40MLssp4xUJrph91n2XvtJ3pbEUDrmT2AA==} @@ -13943,7 +13943,7 @@ snapshots: type-fest: 4.26.1 uuid: 11.0.2 - '@signalapp/libsignal-client@0.79.1': + '@signalapp/libsignal-client@0.80.0': dependencies: node-gyp-build: 4.8.4 type-fest: 4.26.1 diff --git a/ts/AttachmentCrypto.ts b/ts/AttachmentCrypto.ts index f61a73d26c..426295a331 100644 --- a/ts/AttachmentCrypto.ts +++ b/ts/AttachmentCrypto.ts @@ -582,9 +582,10 @@ export async function decryptAndReencryptLocally( }; } catch (error) { log.error( - `${logId}: Failed to decrypt attachment`, + `${logId}: Failed to decrypt and reencrypt attachment`, Errors.toLogFormat(error) ); + await safeUnlink(absoluteTargetPath); throw error; } finally { diff --git a/ts/jobs/AttachmentDownloadManager.ts b/ts/jobs/AttachmentDownloadManager.ts index 941c5abe13..9227d1d155 100644 --- a/ts/jobs/AttachmentDownloadManager.ts +++ b/ts/jobs/AttachmentDownloadManager.ts @@ -13,7 +13,10 @@ import { AttachmentDownloadUrgency, coreAttachmentDownloadJobSchema, } from '../types/AttachmentDownload'; -import { downloadAttachment as downloadAttachmentUtil } from '../util/downloadAttachment'; +import { + downloadAttachment as downloadAttachmentUtil, + isIncrementalMacVerificationError, +} from '../util/downloadAttachment'; import { DataReader, DataWriter } from '../sql/Client'; import { getValue } from '../RemoteConfig'; @@ -27,6 +30,7 @@ import { canAttachmentHaveThumbnail, shouldAttachmentEndUpInRemoteBackup, getUndownloadedAttachmentSignature, + isIncremental, } from '../types/Attachment'; import { type ReadonlyMessageAttributesType } from '../model-types.d'; import { getMessageById } from '../messages/getMessageById'; @@ -44,7 +48,7 @@ import { type JobManagerJobResultType, type JobManagerJobType, } from './JobManager'; -import { IMAGE_JPEG } from '../types/MIME'; +import { IMAGE_WEBP } from '../types/MIME'; import { AttachmentDownloadSource } from '../sql/Interface'; import { drop } from '../util/drop'; import { @@ -506,8 +510,8 @@ async function runDownloadAttachmentJob({ if (result.downloadedVariant === AttachmentVariant.ThumbnailFromBackup) { return { - status: 'finished', - newJob: { ...job, attachment: result.attachmentWithThumbnail }, + status: 'retry', + updatedJob: { ...job, attachment: result.attachmentWithThumbnail }, }; } @@ -543,6 +547,38 @@ async function runDownloadAttachmentJob({ return { status: 'finished' }; } + if (isIncrementalMacVerificationError(error)) { + log.warn( + 'Attachment decryption failed with incrementalMac verification error; dropping incrementalMac' + ); + // If incrementalMac fails verification, we will delete it and retry (without + // streaming support) + strictAssert(isIncremental(job.attachment), 'must have incrementalMac'); + const attachmentWithoutIncrementalMac: AttachmentType = { + ...job.attachment, + pending: false, + incrementalMac: undefined, + chunkSize: undefined, + }; + // Double-check it no longer supports incremental playback just to make sure we + // avoid any loops + strictAssert( + !isIncremental(attachmentWithoutIncrementalMac), + 'no longer has incrementalMac' + ); + + await addAttachmentToMessage( + message.id, + attachmentWithoutIncrementalMac, + logId, + { type: job.attachmentType } + ); + return { + status: 'retry', + updatedJob: { ...job, attachment: attachmentWithoutIncrementalMac }, + }; + } + if (error instanceof AttachmentPermanentlyUndownloadableError) { const canBackfill = job.isManualDownload && @@ -672,27 +708,28 @@ export async function runDownloadAttachmentJobInner({ canAttachmentHaveThumbnail(attachment) && !wasAttachmentImportedFromLocalBackup; - const preferBackupThumbnail = + const attemptBackupThumbnailFirst = isForCurrentlyVisibleMessage && mightHaveBackupThumbnailToDownload; - if (preferBackupThumbnail) { - logId += '.preferringBackupThumbnail'; - } + let attachmentWithBackupThumbnail: AttachmentType | undefined; + + if (attemptBackupThumbnailFirst) { + logId += '.preferringBackupThumbnail'; - if (preferBackupThumbnail) { try { - const attachmentWithThumbnail = await downloadBackupThumbnail({ + attachmentWithBackupThumbnail = await downloadBackupThumbnail({ attachment, abortSignal, dependencies, }); - await addAttachmentToMessage(messageId, attachmentWithThumbnail, logId, { - type: attachmentType, - }); - return { - downloadedVariant: AttachmentVariant.ThumbnailFromBackup, - attachmentWithThumbnail, - }; + await addAttachmentToMessage( + messageId, + attachmentWithBackupThumbnail, + logId, + { + type: attachmentType, + } + ); } catch (e) { log.warn( `${logId}: error when trying to download thumbnail`, @@ -801,32 +838,25 @@ export async function runDownloadAttachmentJobInner({ ); return { downloadedVariant: AttachmentVariant.Default }; } catch (error) { - if (mightHaveBackupThumbnailToDownload && !preferBackupThumbnail) { + if (mightHaveBackupThumbnailToDownload && !attemptBackupThumbnailFirst) { log.error( `${logId}: failed to download fullsize attachment, falling back to backup thumbnail`, Errors.toLogFormat(error) ); try { - const attachmentWithThumbnail = omit( - await downloadBackupThumbnail({ - attachment, - abortSignal, - dependencies, - }), - 'pending' - ); + attachmentWithBackupThumbnail = await downloadBackupThumbnail({ + attachment, + abortSignal, + dependencies, + }); await addAttachmentToMessage( messageId, - attachmentWithThumbnail, + { ...attachment, pending: false }, logId, { type: attachmentType, } ); - return { - downloadedVariant: AttachmentVariant.ThumbnailFromBackup, - attachmentWithThumbnail, - }; } catch (thumbnailError) { log.error( `${logId}: fallback attempt to download thumbnail failed`, @@ -835,6 +865,13 @@ export async function runDownloadAttachmentJobInner({ } } + if (attachmentWithBackupThumbnail) { + return { + downloadedVariant: AttachmentVariant.ThumbnailFromBackup, + attachmentWithThumbnail: attachmentWithBackupThumbnail, + }; + } + let showToast = false; // Show toast if manual download failed @@ -889,7 +926,7 @@ async function downloadBackupThumbnail({ const attachmentWithThumbnail = { ...attachment, thumbnailFromBackup: { - contentType: IMAGE_JPEG, + contentType: IMAGE_WEBP, ...downloadedThumbnail, size: calculatedSize, }, diff --git a/ts/jobs/JobManager.ts b/ts/jobs/JobManager.ts index a0ba176b6a..01bc7b64fc 100644 --- a/ts/jobs/JobManager.ts +++ b/ts/jobs/JobManager.ts @@ -66,10 +66,8 @@ export type JobManagerParamsType< const DEFAULT_TICK_INTERVAL = MINUTE; export type JobManagerJobResultType = - | { - status: 'retry'; - } - | { status: 'finished'; newJob?: CoreJobType } + | { status: 'retry'; updatedJob?: CoreJobType & JobManagerJobType } + | { status: 'finished' } | { status: 'rate-limited'; pauseDurationMs: number }; export type ActiveJobData = { @@ -287,6 +285,7 @@ export abstract class JobManager { return; } + const isFirstAttempt = job.attempts === 0; const isLastAttempt = job.attempts + 1 >= (this.params.getRetryConfig(job).maxAttempts ?? Infinity); @@ -303,7 +302,9 @@ export abstract class JobManager { this.#handleJobStartPromises(job); jobRunResult = await runJobPromise; const { status } = jobRunResult; - log.info(`${logId}: job completed with status: ${status}`); + log.info( + `${logId}: job completed with status: ${status}${status === 'retry' && jobRunResult.updatedJob ? ' with updated job' : ''}` + ); switch (status) { case 'finished': @@ -313,7 +314,13 @@ export abstract class JobManager { if (isLastAttempt) { throw new Error('Cannot retry on last attempt'); } - await this.#retryJobLater(job); + // If we get an updated job, retry it without delay only if it's the first + // attempt (to avoid loops) + if (isFirstAttempt && jobRunResult.updatedJob) { + await this.#retryJobWithoutDelay(jobRunResult.updatedJob); + } else { + await this.#retryJobLater(jobRunResult.updatedJob ?? job); + } return; case 'rate-limited': log.info( @@ -334,18 +341,21 @@ export abstract class JobManager { } } finally { this.#removeRunningJob(job); - if (jobRunResult?.status === 'finished') { - if (jobRunResult.newJob) { - log.info( - `${logId}: adding new job as a result of this one completing` - ); - await this.addJob(jobRunResult.newJob); - } - } drop(this.maybeStartJobs()); } } + async #retryJobWithoutDelay(job: CoreJobType & JobManagerJobType) { + const now = Date.now(); + await this.params.saveJob({ + ...job, + active: false, + attempts: job.attempts + 1, + retryAfter: now, + lastAttemptTimestamp: now, + }); + } + async #retryJobLater(job: CoreJobType & JobManagerJobType) { const now = Date.now(); await this.params.saveJob({ diff --git a/ts/test-electron/Crypto_test.ts b/ts/test-electron/Crypto_test.ts index 532a320850..e6defe9626 100644 --- a/ts/test-electron/Crypto_test.ts +++ b/ts/test-electron/Crypto_test.ts @@ -856,7 +856,7 @@ describe('Crypto', () => { plaintextHash, modifyIncrementalMac: true, }), - /Corrupted/ + /^Corrupted input data/ ); } finally { unlinkSync(sourcePath); diff --git a/ts/test-electron/services/AttachmentDownloadManager_test.ts b/ts/test-electron/services/AttachmentDownloadManager_test.ts index 2ff7bd5ec6..a3a77c4621 100644 --- a/ts/test-electron/services/AttachmentDownloadManager_test.ts +++ b/ts/test-electron/services/AttachmentDownloadManager_test.ts @@ -527,6 +527,44 @@ describe('AttachmentDownloadManager/JobManager', () => { await jobAttempts[1].completed; }); + + it('retries job with updated job if provided', async () => { + strictAssert(downloadManager, 'must exist'); + const job = ( + await addJobs(1, { + source: AttachmentDownloadSource.BACKUP_IMPORT, + }) + )[0]; + const jobAttempts = getPromisesForAttempts(job, 3); + + runJob.callsFake(async args => { + return new Promise(resolve => { + Promise.resolve().then(() => { + resolve({ + status: 'retry', + updatedJob: { + ...args.job, + attachment: { ...job.attachment, caption: 'retried' }, + }, + }); + }); + }); + }); + + await downloadManager?.start(); + await jobAttempts[0].completed; + assertRunJobCalledWith([job]); + await jobAttempts[1].completed; + assert.deepStrictEqual( + runJob.getCall(0).args[0].job.attachment, + job.attachment + ); + assert.deepStrictEqual(runJob.getCall(1).args[0].job.attachment, { + ...job.attachment, + caption: 'retried', + }); + }); + describe('will drop jobs from non-media backup imports that are old', () => { it('will not queue attachments older than 90 days (2 * message queue time)', async () => { hasMediaBackups.returns(false); @@ -660,12 +698,20 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => { AttachmentVariant.Default ); }); - it('will download thumbnail if attachment is from backup', async () => { + + it('will download thumbnail first if attachment is from backup', async () => { const job = composeJob({ messageId: '1', receivedAt: 1, }); + downloadAttachment = sandbox.stub().callsFake(({ options }) => { + if (options.variant === AttachmentVariant.ThumbnailFromBackup) { + return Promise.resolve(downloadedAttachment); + } + throw new Error('error while downloading'); + }); + const result = await runDownloadAttachmentJobInner({ job, isForCurrentlyVisibleMessage: true, @@ -692,16 +738,23 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => { result.attachmentWithThumbnail.thumbnailFromBackup?.path, '/path/to/file' ); - assert.strictEqual(downloadAttachment.callCount, 1); + assert.strictEqual(downloadAttachment.callCount, 2); - const downloadCallArgs = downloadAttachment.getCall(0).args[0]; - assert.deepStrictEqual(downloadCallArgs.attachment, job.attachment); + const firstDownloadCallArgs = downloadAttachment.getCall(0).args[0]; + assert.deepStrictEqual(firstDownloadCallArgs.attachment, job.attachment); assert.deepStrictEqual( - downloadCallArgs.options.variant, + firstDownloadCallArgs.options.variant, AttachmentVariant.ThumbnailFromBackup ); + + const secondDownloadCallArgs = downloadAttachment.getCall(1).args[0]; + assert.deepStrictEqual( + secondDownloadCallArgs.options.variant, + AttachmentVariant.Default + ); }); - it('will download full size if thumbnail already backed up', async () => { + + it('will download full size if backup thumbnail already downloaded', async () => { const job = composeJob({ messageId: '1', receivedAt: 1, @@ -739,7 +792,10 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => { }); it('will attempt to download full size if thumbnail fails', async () => { - downloadAttachment = sandbox.stub().callsFake(() => { + downloadAttachment = sandbox.stub().callsFake(({ options }) => { + if (options.variant === AttachmentVariant.Default) { + return Promise.resolve(downloadedAttachment); + } throw new Error('error while downloading'); }); @@ -748,22 +804,20 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => { receivedAt: 1, }); - await assert.isRejected( - runDownloadAttachmentJobInner({ - job, - isForCurrentlyVisibleMessage: true, - hasMediaBackups: true, - abortSignal: abortController.signal, - maxAttachmentSizeInKib: 100 * MEBIBYTE, - maxTextAttachmentSizeInKib: 2 * MEBIBYTE, - dependencies: { - deleteDownloadData, - downloadAttachment, - processNewAttachment, - }, - }) - ); - + const result = await runDownloadAttachmentJobInner({ + job, + isForCurrentlyVisibleMessage: true, + hasMediaBackups: true, + abortSignal: abortController.signal, + maxAttachmentSizeInKib: 100 * MEBIBYTE, + maxTextAttachmentSizeInKib: 2 * MEBIBYTE, + dependencies: { + deleteDownloadData, + downloadAttachment, + processNewAttachment, + }, + }); + assert.strictEqual(result.downloadedVariant, AttachmentVariant.Default); assert.strictEqual(downloadAttachment.callCount, 2); const downloadCallArgs0 = downloadAttachment.getCall(0).args[0]; diff --git a/ts/test-mock/backups/backups_test.ts b/ts/test-mock/backups/backups_test.ts index 01f81a206a..defc11c54e 100644 --- a/ts/test-mock/backups/backups_test.ts +++ b/ts/test-mock/backups/backups_test.ts @@ -219,7 +219,7 @@ describe('backups', function (this: Mocha.Suite) { const catTimestamp = bootstrap.getTimestamp(); const plaintextCat = await readFile(CAT_PATH); - const ciphertextCat = await bootstrap.storeAttachmentOnCDN( + const ciphertextCat = await bootstrap.encryptAndStoreAttachmentOnCDN( plaintextCat, IMAGE_JPEG ); diff --git a/ts/test-mock/bootstrap.ts b/ts/test-mock/bootstrap.ts index 61fdcaef23..84c8b65ba8 100644 --- a/ts/test-mock/bootstrap.ts +++ b/ts/test-mock/bootstrap.ts @@ -674,7 +674,7 @@ export class Bootstrap { return join(this.#storagePath, 'attachments.noindex', relativePath); } - public async storeAttachmentOnCDN( + public async encryptAndStoreAttachmentOnCDN( data: Buffer, contentType: MIMEType ): Promise { @@ -684,13 +684,13 @@ export class Bootstrap { const passthrough = new PassThrough(); - const [{ digest }] = await Promise.all([ + const [{ digest, chunkSize, incrementalMac }] = await Promise.all([ encryptAttachmentV2({ keys, plaintext: { data, }, - needIncrementalMac: false, + needIncrementalMac: true, sink: passthrough, }), this.server.storeAttachmentOnCdn(cdnNumber, cdnKey, passthrough), @@ -703,6 +703,8 @@ export class Bootstrap { cdnNumber, key: keys, digest, + chunkSize, + incrementalMac, }; } diff --git a/ts/test-mock/messaging/attachments_test.ts b/ts/test-mock/messaging/attachments_test.ts index bbc222ee86..35c2563823 100644 --- a/ts/test-mock/messaging/attachments_test.ts +++ b/ts/test-mock/messaging/attachments_test.ts @@ -6,6 +6,8 @@ import { assert } from 'chai'; import { expect } from 'playwright/test'; import { type PrimaryDevice, StorageState } from '@signalapp/mock-server'; import * as path from 'path'; +import { readFile } from 'fs/promises'; + import type { App } from '../playwright'; import { Bootstrap } from '../bootstrap'; import { @@ -16,6 +18,8 @@ import { } from '../helpers'; import * as durations from '../../util/durations'; import { strictAssert } from '../../util/assert'; +import { VIDEO_MP4 } from '../../types/MIME'; +import { toBase64 } from '../../Bytes'; export const debug = createDebug('mock:test:attachments'); @@ -27,6 +31,14 @@ const CAT_PATH = path.join( 'fixtures', 'cat-screenshot.png' ); +const VIDEO_PATH = path.join( + __dirname, + '..', + '..', + '..', + 'fixtures', + 'ghost-kitty.mp4' +); describe('attachments', function (this: Mocha.Suite) { this.timeout(durations.MINUTE); @@ -121,4 +133,83 @@ describe('attachments', function (this: Mocha.Suite) { assert.strictEqual(incomingAttachment?.key, sentAttachment?.key); assert.strictEqual(incomingAttachment?.digest, sentAttachment?.digest); }); + + it('can download videos with incrementalMac and is resilient to bad incrementalMacs', async () => { + const { desktop } = bootstrap; + const page = await app.getWindow(); + + await page.getByTestId(pinned.device.aci).click(); + + const plaintextVideo = await readFile(VIDEO_PATH); + const videoPointer1 = await bootstrap.encryptAndStoreAttachmentOnCDN( + plaintextVideo, + VIDEO_MP4 + ); + const videoPointer2 = await bootstrap.encryptAndStoreAttachmentOnCDN( + plaintextVideo, + VIDEO_MP4 + ); + + const incrementalTimestamp = Date.now(); + const badIncrementalTimestamp = incrementalTimestamp + 1; + + await sendTextMessage({ + from: pinned, + to: desktop, + desktop, + text: 'video with good incrementalMac', + attachments: [videoPointer1], + timestamp: incrementalTimestamp, + }); + await sendTextMessage({ + from: pinned, + to: desktop, + desktop, + text: 'video with bad incrementalMac', + attachments: [ + { ...videoPointer2, chunkSize: (videoPointer2.chunkSize ?? 42) + 1 }, + ], + timestamp: badIncrementalTimestamp, + }); + + await expect( + getMessageInTimelineByTimestamp(page, incrementalTimestamp).locator( + 'img.module-image__image' + ) + ).toBeVisible(); + await expect( + getMessageInTimelineByTimestamp(page, badIncrementalTimestamp).locator( + 'img.module-image__image' + ) + ).toBeVisible(); + + // goodIncrementalMac preserved + { + const messageInDB = ( + await app.getMessagesBySentAt(incrementalTimestamp) + )[0]; + strictAssert(messageInDB, 'message exists in DB'); + const attachmentInDB = messageInDB.attachments?.[0]; + strictAssert(videoPointer1.incrementalMac, 'must exist'); + strictAssert(videoPointer1.chunkSize, 'must exist'); + assert.strictEqual( + attachmentInDB?.incrementalMac, + toBase64(videoPointer1.incrementalMac) + ); + assert.strictEqual(attachmentInDB?.chunkSize, videoPointer1.chunkSize); + } + + // badIncrementalMac removed + { + const messageInDB = ( + await app.getMessagesBySentAt(badIncrementalTimestamp) + )[0]; + strictAssert(messageInDB, 'message exists in DB'); + const attachmentInDB = messageInDB.attachments?.[0]; + strictAssert(videoPointer2.incrementalMac, 'must exist'); + strictAssert(videoPointer2.chunkSize, 'must exist'); + assert.strictEqual(attachmentInDB?.incrementalMac, undefined); + assert.strictEqual(attachmentInDB?.chunkSize, undefined); + } + }); }); diff --git a/ts/test-mock/messaging/backfill_test.ts b/ts/test-mock/messaging/backfill_test.ts index a6f6df45a0..00f6d516fd 100644 --- a/ts/test-mock/messaging/backfill_test.ts +++ b/ts/test-mock/messaging/backfill_test.ts @@ -51,19 +51,19 @@ describe('attachment backfill', function (this: Mocha.Suite) { const { unknownContacts } = bootstrap; [unknownContact] = unknownContacts; - textAttachment = await bootstrap.storeAttachmentOnCDN( + textAttachment = await bootstrap.encryptAndStoreAttachmentOnCDN( Buffer.from('look at this pic, it is gorgeous!'), LONG_MESSAGE ); const plaintextCat = await readFile(CAT_PATH); - catAttachment = await bootstrap.storeAttachmentOnCDN( + catAttachment = await bootstrap.encryptAndStoreAttachmentOnCDN( plaintextCat, IMAGE_JPEG ); const plaintextSnow = await readFile(SNOW_PATH); - snowAttachment = await bootstrap.storeAttachmentOnCDN( + snowAttachment = await bootstrap.encryptAndStoreAttachmentOnCDN( plaintextSnow, IMAGE_JPEG ); diff --git a/ts/textsecure/downloadAttachment.ts b/ts/textsecure/downloadAttachment.ts index 83f960cba2..2fe8ecba0b 100644 --- a/ts/textsecure/downloadAttachment.ts +++ b/ts/textsecure/downloadAttachment.ts @@ -164,7 +164,10 @@ export async function downloadAttachment( } // Start over if we go over the size - if (downloadOffset >= size && absoluteDownloadPath) { + if ( + downloadOffset >= getAttachmentCiphertextLength(size) && + absoluteDownloadPath + ) { log.warn('went over, retrying'); await safeUnlink(absoluteDownloadPath); downloadOffset = 0; @@ -310,7 +313,7 @@ export async function downloadAttachment( // backup thumbnails don't get trimmed, so we just calculate the size as the // ciphertextSize, less IV and MAC const calculatedSize = downloadSize - IV_LENGTH - MAC_LENGTH; - return decryptAndReencryptLocally({ + return await decryptAndReencryptLocally({ type: 'backupThumbnail', ciphertextPath: cipherTextAbsolutePath, idForLogging: logId, diff --git a/ts/util/downloadAttachment.ts b/ts/util/downloadAttachment.ts index fd020edc63..febf4495f2 100644 --- a/ts/util/downloadAttachment.ts +++ b/ts/util/downloadAttachment.ts @@ -1,6 +1,6 @@ // Copyright 2020 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only - +import { ErrorCode, LibSignalErrorBase } from '@signalapp/libsignal-client'; import { type AttachmentType, AttachmentVariant, @@ -68,6 +68,9 @@ export async function downloadAttachment({ onSizeUpdate(attachment.size); return result; } catch (error) { + if (isIncrementalMacVerificationError(error)) { + throw error; + } // We also just log this error instead of throwing, since we want to still try to // find it on the backup then transit tiers. log.error( @@ -90,6 +93,10 @@ export async function downloadAttachment({ } ); } catch (error) { + if (isIncrementalMacVerificationError(error)) { + throw error; + } + const shouldFallbackToTransitTier = variant !== AttachmentVariant.ThumbnailFromBackup; @@ -128,6 +135,10 @@ export async function downloadAttachment({ } ); } catch (error) { + if (isIncrementalMacVerificationError(error)) { + throw error; + } + if (mightBeOnBackupTierInTheFuture) { // We don't want to throw the AttachmentPermanentlyUndownloadableError because we // may just need to wait for this attachment to end up on the backup tier @@ -150,3 +161,10 @@ export async function downloadAttachment({ } } } + +export function isIncrementalMacVerificationError(error: unknown): boolean { + return ( + error instanceof LibSignalErrorBase && + error.code === ErrorCode.IncrementalMacVerificationFailed + ); +}