From c85ad2b86732a848db2f67feac84df8ad2503632 Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Mon, 8 Sep 2025 14:00:18 -0400 Subject: [PATCH] Improve handling of backup CDN read credentials --- ts/services/backups/api.ts | 6 ++-- ts/services/backups/credentials.ts | 46 ++++++++++++++++++++++------- ts/services/backups/index.ts | 33 ++++++++++++--------- ts/textsecure/WebAPI.ts | 6 ++-- ts/textsecure/downloadAttachment.ts | 32 +++++++++++++------- ts/util/backupSubscriptionData.ts | 6 +++- 6 files changed, 87 insertions(+), 42 deletions(-) diff --git a/ts/services/backups/api.ts b/ts/services/backups/api.ts index 741f81ba06..06268e0ee7 100644 --- a/ts/services/backups/api.ts +++ b/ts/services/backups/api.ts @@ -144,7 +144,9 @@ export class BackupAPI { }); return { backupExists: true, size, createdAt }; } catch (error) { - if (error instanceof HTTPError && error.code === 404) { + if (error instanceof HTTPError && error.code === 401) { + this.credentials.onCdnCredentialError(); + } else if (error instanceof HTTPError && error.code === 404) { return { backupExists: false }; } throw error; @@ -218,7 +220,7 @@ export class BackupAPI { try { subscriptionResponse = await this.#server.getSubscription(subscriberId); } catch (e) { - log.error( + log.warn( 'Backups.getSubscriptionInfo: error fetching subscription', toLogFormat(e) ); diff --git a/ts/services/backups/credentials.ts b/ts/services/backups/credentials.ts index 4a58493e23..71304af2b2 100644 --- a/ts/services/backups/credentials.ts +++ b/ts/services/backups/credentials.ts @@ -10,13 +10,14 @@ import { GenericServerPublicParams, } from '@signalapp/libsignal-client/zkgroup'; import { type BackupKey } from '@signalapp/libsignal-client/dist/AccountKeys'; +import { throttle } from 'lodash/fp'; import * as Bytes from '../../Bytes'; import { createLogger } from '../../logging/log'; import { strictAssert } from '../../util/assert'; import { drop } from '../../util/drop'; import { isMoreRecentThan, toDayMillis } from '../../util/timestamp'; -import { DAY, DurationInSeconds, HOUR } from '../../util/durations'; +import { DAY, DurationInSeconds, HOUR, MINUTE } from '../../util/durations'; import { BackOff, FIBONACCI_TIMEOUTS } from '../../util/BackOff'; import { missingCaseError } from '../../util/missingCaseError'; import { @@ -54,10 +55,22 @@ const BACKUP_CDN_READ_CREDENTIALS_VALID_DURATION = 12 * HOUR; export class BackupCredentials { #activeFetch: Promise> | undefined; - #cachedCdnReadCredentials: Record = {}; + #cachedCdnReadCredentials: Record< + BackupCredentialType, + Record + > = { + [BackupCredentialType.Media]: {}, + [BackupCredentialType.Messages]: {}, + }; readonly #fetchBackoff = new BackOff(FIBONACCI_TIMEOUTS); + // Throttle credential clearing to avoid loops + public readonly onCdnCredentialError = throttle(5 * MINUTE, () => { + log.warn('onCdnCredentialError: clearing cache'); + this.#clearCdnReadCredentials(); + }); + public start(): void { this.#scheduleFetch(); } @@ -140,7 +153,7 @@ export class BackupCredentials { } public async getCDNReadCredentials( - cdn: number, + cdnNumber: number, credentialType: BackupCredentialType ): Promise { const { server } = window.textsecure; @@ -148,16 +161,19 @@ export class BackupCredentials { // Backup CDN read credentials are short-lived; we'll just cache them in memory so // that they get invalidated for any reason, we'll fetch new ones on app restart - const cachedCredentialsForThisCdn = this.#cachedCdnReadCredentials[cdn]; + const cachedCredentialsForThisCredentialType = + this.#cachedCdnReadCredentials[credentialType]; + + const cachedCredentials = cachedCredentialsForThisCredentialType[cdnNumber]; if ( - cachedCredentialsForThisCdn && + cachedCredentials && isMoreRecentThan( - cachedCredentialsForThisCdn.retrievedAtMs, + cachedCredentials.retrievedAtMs, BACKUP_CDN_READ_CREDENTIALS_VALID_DURATION ) ) { - return cachedCredentialsForThisCdn.credentials; + return cachedCredentials.credentials; } const headers = await this.getHeadersForToday(credentialType); @@ -165,12 +181,12 @@ export class BackupCredentials { const retrievedAtMs = Date.now(); const newCredentials = await server.getBackupCDNCredentials({ headers, - cdn, + cdnNumber, }); - this.#cachedCdnReadCredentials[cdn] = { + cachedCredentialsForThisCredentialType[cdnNumber] = { credentials: newCredentials, - cdnNumber: cdn, + cdnNumber, retrievedAtMs, }; @@ -410,7 +426,15 @@ export class BackupCredentials { // Called when backup tier changes or when userChanged event public async clearCache(): Promise { - this.#cachedCdnReadCredentials = {}; + log.info('Clearing cache'); + this.#clearCdnReadCredentials(); await this.#updateCache([]); } + + #clearCdnReadCredentials(): void { + this.#cachedCdnReadCredentials = { + [BackupCredentialType.Media]: {}, + [BackupCredentialType.Messages]: {}, + }; + } } diff --git a/ts/services/backups/index.ts b/ts/services/backups/index.ts index 48819709c1..d7606f4525 100644 --- a/ts/services/backups/index.ts +++ b/ts/services/backups/index.ts @@ -153,13 +153,11 @@ export class BackupsService { public readonly credentials = new BackupCredentials(); public readonly api = new BackupAPI(this.credentials); - public readonly throttledFetchCloudBackupStatus = throttle( - 30 * SECOND, - this.fetchCloudBackupStatus.bind(this) + public readonly throttledFetchCloudBackupStatus = throttle(30 * SECOND, () => + this.#fetchCloudBackupStatus() ); - public readonly throttledFetchSubscriptionStatus = throttle( - 30 * SECOND, - this.fetchSubscriptionStatus.bind(this) + public readonly throttledFetchSubscriptionStatus = throttle(30 * SECOND, () => + this.#fetchSubscriptionStatus() ); public start(): void { @@ -183,9 +181,8 @@ export class BackupsService { drop(this.#runPeriodicRefresh()); this.credentials.start(); - window.Whisper.events.on('userChanged', () => { - drop(this.credentials.clearCache()); - this.api.clearCache(); + window.Whisper.events.on('userChanged', async () => { + await this.resetCachedData(); }); } public async downloadAndImport( @@ -1070,7 +1067,7 @@ export class BackupsService { } } - async fetchCloudBackupStatus(): Promise { + async #fetchCloudBackupStatus(): Promise { let result: BackupStatusType | undefined; const backupProtoInfo = await this.api.getBackupProtoInfo(); @@ -1086,7 +1083,7 @@ export class BackupsService { return result; } - async fetchSubscriptionStatus(): Promise< + async #fetchSubscriptionStatus(): Promise< BackupsSubscriptionType | undefined > { const backupTier = this.#getBackupTierFromStorage(); @@ -1111,17 +1108,25 @@ export class BackupsService { throw missingCaseError(backupTier); } - drop(window.storage.put('backupSubscriptionStatus', result)); + await window.storage.put('backupSubscriptionStatus', result); return result; } async refreshBackupAndSubscriptionStatus(): Promise { await Promise.all([ - this.fetchSubscriptionStatus(), - this.fetchCloudBackupStatus(), + this.#fetchSubscriptionStatus(), + this.#fetchCloudBackupStatus(), ]); } + async resetCachedData(): Promise { + this.api.clearCache(); + await this.credentials.clearCache(); + await window.storage.remove('backupSubscriptionStatus'); + await window.storage.remove('cloudBackupStatus'); + await this.refreshBackupAndSubscriptionStatus(); + } + hasMediaBackups(): boolean { return window.storage.get('backupTier') === BackupLevel.Paid; } diff --git a/ts/textsecure/WebAPI.ts b/ts/textsecure/WebAPI.ts index ce080b7295..c3ddfff4c2 100644 --- a/ts/textsecure/WebAPI.ts +++ b/ts/textsecure/WebAPI.ts @@ -1350,7 +1350,7 @@ export type GetBackupCredentialsResponseType = z.infer< export type GetBackupCDNCredentialsOptionsType = Readonly<{ headers: BackupPresentationHeadersType; - cdn: number; + cdnNumber: number; }>; export const getBackupCDNCredentialsResponseSchema = z.object({ @@ -3645,7 +3645,7 @@ export function initialize({ async function getBackupCDNCredentials({ headers, - cdn, + cdnNumber, }: GetBackupCDNCredentialsOptionsType) { return _ajax({ host: 'chatService', @@ -3655,7 +3655,7 @@ export function initialize({ accessKey: undefined, groupSendToken: undefined, headers, - urlParameters: `?cdn=${cdn}`, + urlParameters: `?cdn=${cdnNumber}`, responseType: 'json', zodSchema: getBackupCDNCredentialsResponseSchema, }); diff --git a/ts/textsecure/downloadAttachment.ts b/ts/textsecure/downloadAttachment.ts index c5b08ec580..83f960cba2 100644 --- a/ts/textsecure/downloadAttachment.ts +++ b/ts/textsecure/downloadAttachment.ts @@ -51,6 +51,7 @@ import { IV_LENGTH, MAC_LENGTH } from '../types/Crypto'; import { BackupCredentialType } from '../types/backups'; import { getValue } from '../RemoteConfig'; import { parseIntOrThrow } from '../util/parseIntOrThrow'; +import { HTTPError } from './Errors'; const log = createLogger('downloadAttachment'); @@ -223,17 +224,26 @@ export async function downloadAttachment( const backupDir = await backupsService.api.getBackupDir(); const mediaDir = await backupsService.api.getMediaDir(); - const downloadStream = await server.getAttachmentFromBackupTier({ - mediaId: mediaId.string, - backupDir, - mediaDir, - headers: cdnCredentials.headers, - cdnNumber, - options: { - ...options, - downloadOffset, - }, - }); + let downloadStream: Readable; + try { + downloadStream = await server.getAttachmentFromBackupTier({ + mediaId: mediaId.string, + backupDir, + mediaDir, + headers: cdnCredentials.headers, + cdnNumber, + options: { + ...options, + downloadOffset, + }, + }); + } catch (error) { + if (error instanceof HTTPError && error.code === 401) { + window.Signal.Services.backups.credentials.onCdnCredentialError(); + } + throw error; + } + downloadResult = await downloadToDisk({ downloadStream, downloadPath, diff --git a/ts/util/backupSubscriptionData.ts b/ts/util/backupSubscriptionData.ts index 9f61624388..7e7b07fe39 100644 --- a/ts/util/backupSubscriptionData.ts +++ b/ts/util/backupSubscriptionData.ts @@ -5,6 +5,9 @@ import Long from 'long'; import type { Backups, SignalService } from '../protobuf'; import * as Bytes from '../Bytes'; import { drop } from './drop'; +import { createLogger } from '../logging/log'; + +const log = createLogger('BackupSubscriptionData'); // These two proto messages (Backups.AccountData.IIAPSubscriberData && // SignalService.AccountRecord.IIAPSubscriberData) should remain in sync. If they drift, @@ -60,7 +63,8 @@ export async function saveBackupTier( const previousBackupTier = window.storage.get('backupTier'); await window.storage.put('backupTier', backupTier); if (backupTier !== previousBackupTier) { - drop(window.Signal.Services.backups.refreshBackupAndSubscriptionStatus()); + log.info('backup tier has changed', { previousBackupTier, backupTier }); + drop(window.Signal.Services.backups.resetCachedData()); } }