From 8ef14e6f3904b58416d8f43f50d68b37c798a2f9 Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Wed, 5 May 2021 11:39:16 -0500 Subject: [PATCH] When incoming message should've been sealed sender, reply with profile key --- ts/background.ts | 36 +++- ts/groups.ts | 18 +- ts/models/conversations.ts | 47 +++--- ts/models/messages.ts | 5 +- ts/services/ourProfileKey.ts | 87 ++++++++++ ts/services/storage.ts | 9 +- ts/services/storageRecordOps.ts | 3 +- ts/test-both/services/ourProfileKey_test.ts | 178 ++++++++++++++++++++ ts/textsecure/AccountManager.ts | 3 +- ts/util/shouldRespondWithProfileKey.ts | 36 ++++ 10 files changed, 384 insertions(+), 38 deletions(-) create mode 100644 ts/services/ourProfileKey.ts create mode 100644 ts/test-both/services/ourProfileKey_test.ts create mode 100644 ts/util/shouldRespondWithProfileKey.ts diff --git a/ts/background.ts b/ts/background.ts index 3890bae88529..b67b85003afa 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -16,6 +16,8 @@ import { createBatcher } from './util/batcher'; import { updateConversationsWithUuidLookup } from './updateConversationsWithUuidLookup'; import { initializeAllJobQueues } from './jobs/initializeAllJobQueues'; import { removeStorageKeyJobQueue } from './jobs/removeStorageKeyJobQueue'; +import { ourProfileKeyService } from './services/ourProfileKey'; +import { shouldRespondWithProfileKey } from './util/shouldRespondWithProfileKey'; const MAX_ATTACHMENT_DOWNLOAD_AGE = 3600 * 72 * 1000; @@ -35,6 +37,8 @@ export async function startApp(): Promise { initializeAllJobQueues(); + ourProfileKeyService.initialize(window.storage); + let resolveOnAppView: (() => void) | undefined; const onAppView = new Promise(resolve => { resolveOnAppView = resolve; @@ -56,6 +60,10 @@ export async function startApp(): Promise { concurrency: 1, timeout: 1000 * 60 * 2, }); + + const profileKeyResponseQueue = new window.PQueue(); + profileKeyResponseQueue.pause(); + window.Whisper.deliveryReceiptQueue = new window.PQueue({ concurrency: 1, timeout: 1000 * 60 * 2, @@ -1790,8 +1798,10 @@ export async function startApp(): Promise { connectCount += 1; - window.Whisper.deliveryReceiptQueue.pause(); // avoid flood of delivery receipts until we catch up - window.Whisper.Notifications.disable(); // avoid notification flood until empty + // To avoid a flood of operations before we catch up, we pause some queues. + profileKeyResponseQueue.pause(); + window.Whisper.deliveryReceiptQueue.pause(); + window.Whisper.Notifications.disable(); // initialize the socket and start listening for messages window.log.info('Initializing socket and listening for messages'); @@ -2112,6 +2122,7 @@ export async function startApp(): Promise { newVersion ); + profileKeyResponseQueue.start(); window.Whisper.deliveryReceiptQueue.start(); window.Whisper.Notifications.enable(); @@ -2183,6 +2194,7 @@ export async function startApp(): Promise { // scenarios where we're coming back from sleep, we can get offline/online events // very fast, and it looks like a network blip. But we need to suppress // notifications in these scenarios too. So we listen for 'reconnect' events. + profileKeyResponseQueue.pause(); window.Whisper.deliveryReceiptQueue.pause(); window.Whisper.Notifications.disable(); } @@ -2366,7 +2378,7 @@ export async function startApp(): Promise { // special case for syncing details about ourselves if (details.profileKey) { window.log.info('Got sync message with our own profile key'); - window.storage.put('profileKey', details.profileKey); + ourProfileKeyService.set(details.profileKey); } } @@ -2619,6 +2631,24 @@ export async function startApp(): Promise { const message = initIncomingMessage(data, messageDescriptor); + // We don't need this to interrupt our processing of the message, so we "fire and + // forget". + (async () => { + if (await shouldRespondWithProfileKey(message)) { + const contact = message.getContact(); + if (!contact) { + assert(false, 'Expected message to have a contact'); + return; + } + + profileKeyResponseQueue.add(() => { + contact.queueJob(() => contact.sendProfileKeyUpdate()); + }); + } + })().catch(err => { + window.log.error(err); + }); + if (data.message.reaction) { window.normalizeUuids( data.message.reaction, diff --git a/ts/groups.ts b/ts/groups.ts index f19982507497..f621c254992a 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -75,6 +75,7 @@ import MessageSender, { CallbackResultType } from './textsecure/SendMessage'; import { CURRENT_SCHEMA_VERSION as MAX_MESSAGE_SCHEMA } from '../js/modules/types/message'; import { ConversationModel } from './models/conversations'; import { getGroupSizeHardLimit } from './groups/limits'; +import { ourProfileKeyService } from './services/ourProfileKey'; export { joinViaLink } from './groups/joinViaLink'; @@ -1251,7 +1252,7 @@ export async function modifyGroupV2({ // Send message to notify group members (including pending members) of change const profileKey = conversation.get('profileSharing') - ? window.storage.get('profileKey') + ? await ourProfileKeyService.get() : undefined; const sendOptions = await conversation.getSendOptions(); @@ -1620,7 +1621,7 @@ export async function createGroupV2({ }); const timestamp = Date.now(); - const profileKey = ourConversation.get('profileKey'); + const profileKey = await ourProfileKeyService.get(); const groupV2Info = conversation.getGroupV2Info({ includePendingMembers: true, @@ -1633,7 +1634,7 @@ export async function createGroupV2({ sender.sendMessageToGroup({ groupV2: groupV2Info, timestamp, - profileKey: profileKey ? base64ToArrayBuffer(profileKey) : undefined, + profileKey, }), timestamp, }); @@ -1953,8 +1954,6 @@ export async function initiateMigrationToGroupV2( // Ensure we have the credentials we need before attempting GroupsV2 operations await maybeFetchNewCredentials(); - let ourProfileKey: undefined | string; - try { await conversation.queueJob(async () => { const ACCESS_ENUM = @@ -1997,7 +1996,6 @@ export async function initiateMigrationToGroupV2( `initiateMigrationToGroupV2/${logId}: cannot get our own conversation. Cannot migrate` ); } - ourProfileKey = ourConversation.get('profileKey'); const { membersV2, @@ -2137,6 +2135,10 @@ export async function initiateMigrationToGroupV2( const logId = conversation.idForLogging(); const timestamp = Date.now(); + const ourProfileKey: + | ArrayBuffer + | undefined = await ourProfileKeyService.get(); + await wrapWithSyncMessageSend({ conversation, logId: `sendMessageToGroup/${logId}`, @@ -2147,9 +2149,7 @@ export async function initiateMigrationToGroupV2( includePendingMembers: true, }), timestamp, - profileKey: ourProfileKey - ? base64ToArrayBuffer(ourProfileKey) - : undefined, + profileKey: ourProfileKey, }), timestamp, }); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index e6549e3305c3..dbecb929137f 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -54,6 +54,7 @@ import { SerializedCertificateType, } from '../textsecure/OutgoingMessage'; import { senderCertificateService } from '../services/senderCertificate'; +import { ourProfileKeyService } from '../services/ourProfileKey'; /* eslint-disable more/no-then */ window.Whisper = window.Whisper || {}; @@ -3033,11 +3034,6 @@ export class ConversationModel extends window.Backbone const destination = this.getSendTarget()!; const recipients = this.getRecipients(); - let profileKey: ArrayBuffer | undefined; - if (this.get('profileSharing')) { - profileKey = window.storage.get('profileKey'); - } - return this.queueJob(async () => { window.log.info( 'Sending deleteForEveryone to conversation', @@ -3073,7 +3069,12 @@ export class ConversationModel extends window.Backbone const options = await this.getSendOptions(); - const promise = (() => { + const promise = (async () => { + let profileKey: ArrayBuffer | undefined; + if (this.get('profileSharing')) { + profileKey = await ourProfileKeyService.get(); + } + if (this.isPrivate()) { return window.textsecure.messaging.sendMessageToIdentifier( destination, @@ -3143,11 +3144,6 @@ export class ConversationModel extends window.Backbone const destination = this.getSendTarget()!; const recipients = this.getRecipients(); - let profileKey: ArrayBuffer | undefined; - if (this.get('profileSharing')) { - profileKey = window.storage.get('profileKey'); - } - return this.queueJob(async () => { window.log.info( 'Sending reaction to conversation', @@ -3185,6 +3181,11 @@ export class ConversationModel extends window.Backbone throw new Error('Cannot send reaction while offline!'); } + let profileKey: ArrayBuffer | undefined; + if (this.get('profileSharing')) { + profileKey = await ourProfileKeyService.get(); + } + // Special-case the self-send case - we send only a sync message if (this.isMe()) { const dataMessage = await window.textsecure.messaging.getMessageProto( @@ -3262,7 +3263,13 @@ export class ConversationModel extends window.Backbone return; } window.log.info('Sending profileKeyUpdate to conversation', id, recipients); - const profileKey = window.storage.get('profileKey'); + const profileKey = await ourProfileKeyService.get(); + if (!profileKey) { + window.log.error( + 'Attempted to send profileKeyUpdate but our profile key was not found' + ); + return; + } await window.textsecure.messaging.sendProfileKeyUpdate( profileKey, recipients, @@ -3301,11 +3308,6 @@ export class ConversationModel extends window.Backbone const expireTimer = this.get('expireTimer'); const recipients = this.getRecipients(); - let profileKey: ArrayBuffer | undefined; - if (this.get('profileSharing')) { - profileKey = window.storage.get('profileKey'); - } - this.queueJob(async () => { const now = Date.now(); @@ -3399,6 +3401,11 @@ export class ConversationModel extends window.Backbone now, }); + let profileKey: ArrayBuffer | undefined; + if (this.get('profileSharing')) { + profileKey = await ourProfileKeyService.get(); + } + // Special-case the self-send case - we send only a sync message if (this.isMe()) { const dataMessage = await window.textsecure.messaging.getMessageProto( @@ -4035,11 +4042,13 @@ export class ConversationModel extends window.Backbone return message; } + const sendOptions = await this.getSendOptions(); + let profileKey; if (this.get('profileSharing')) { - profileKey = window.storage.get('profileKey'); + profileKey = await ourProfileKeyService.get(); } - const sendOptions = await this.getSendOptions(); + let promise; if (this.isMe()) { diff --git a/ts/models/messages.ts b/ts/models/messages.ts index b19e3bfa710b..98d50895018b 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -47,6 +47,7 @@ import { PropsType as ProfileChangeNotificationPropsType } from '../components/c import { AttachmentType, isImage, isVideo } from '../types/Attachment'; import { MIMEType } from '../types/MIME'; import { LinkPreviewType } from '../types/message/LinkPreviews'; +import { ourProfileKeyService } from '../services/ourProfileKey'; /* eslint-disable camelcase */ /* eslint-disable more/no-then */ @@ -2131,8 +2132,8 @@ export class MessageModel extends window.Backbone.Model { .filter(exists); const profileKey = conversation.get('profileSharing') - ? window.storage.get('profileKey') - : null; + ? await ourProfileKeyService.get() + : undefined; // Determine retry recipients and get their most up-to-date addressing information let recipients = _.intersection(intendedRecipients, currentRecipients); diff --git a/ts/services/ourProfileKey.ts b/ts/services/ourProfileKey.ts new file mode 100644 index 000000000000..f388dc5b6baa --- /dev/null +++ b/ts/services/ourProfileKey.ts @@ -0,0 +1,87 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from '../util/assert'; +import * as log from '../logging/log'; + +// We define a stricter storage here that returns `unknown` instead of `any`. +type Storage = { + get(key: string): unknown; + put(key: string, value: unknown): Promise; + remove(key: string): Promise; + onready: (callback: () => unknown) => void; +}; + +export class OurProfileKeyService { + private getPromise: undefined | Promise; + + private promisesBlockingGet: Array> = []; + + private storage?: Storage; + + initialize(storage: Storage): void { + log.info('Our profile key service: initializing'); + + const storageReadyPromise = new Promise(resolve => { + storage.onready(() => { + resolve(); + }); + }); + this.promisesBlockingGet = [storageReadyPromise]; + + this.storage = storage; + } + + get(): Promise { + if (this.getPromise) { + log.info( + 'Our profile key service: was already fetching. Piggybacking off of that' + ); + } else { + log.info('Our profile key service: kicking off a new fetch'); + this.getPromise = this.doGet(); + } + return this.getPromise; + } + + async set(newValue: undefined | ArrayBuffer): Promise { + log.info('Our profile key service: updating profile key'); + assert(this.storage, 'OurProfileKeyService was not initialized'); + if (newValue) { + await this.storage.put('profileKey', newValue); + } else { + await this.storage.remove('profileKey'); + } + } + + blockGetWithPromise(promise: Promise): void { + this.promisesBlockingGet.push(promise); + } + + private async doGet(): Promise { + log.info( + `Our profile key service: waiting for ${this.promisesBlockingGet.length} promises before fetching` + ); + + await Promise.allSettled(this.promisesBlockingGet); + this.promisesBlockingGet = []; + + delete this.getPromise; + + assert(this.storage, 'OurProfileKeyService was not initialized'); + + log.info('Our profile key service: fetching profile key from storage'); + const result = this.storage.get('profileKey'); + if (result === undefined || result instanceof ArrayBuffer) { + return result; + } + + assert( + false, + 'Profile key in storage was defined, but not an ArrayBuffer. Returning undefined' + ); + return undefined; + } +} + +export const ourProfileKeyService = new OurProfileKeyService(); diff --git a/ts/services/storage.ts b/ts/services/storage.ts index 41d9b4d678ed..646eec935256 100644 --- a/ts/services/storage.ts +++ b/ts/services/storage.ts @@ -34,6 +34,7 @@ import { storageJobQueue } from '../util/JobQueue'; import { sleep } from '../util/sleep'; import { isMoreRecentThan } from '../util/timestamp'; import { isStorageWriteFeatureEnabled } from '../storage/isFeatureEnabled'; +import { ourProfileKeyService } from './ourProfileKey'; const { eraseStorageServiceStateFromConversations, @@ -1156,7 +1157,9 @@ export const runStorageServiceSyncJob = debounce(() => { return; } - storageJobQueue(async () => { - await sync(); - }, `sync v${window.storage.get('manifestVersion')}`); + ourProfileKeyService.blockGetWithPromise( + storageJobQueue(async () => { + await sync(); + }, `sync v${window.storage.get('manifestVersion')}`) + ); }, 500); diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 93ad46e43ee1..38e4e8308913 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -38,6 +38,7 @@ import { getSafeLongFromTimestamp, getTimestampFromLong, } from '../util/timestampLongUtils'; +import { ourProfileKeyService } from './ourProfileKey'; const { updateConversation } = dataInterface; @@ -851,7 +852,7 @@ export async function mergeAccountRecord( window.storage.put('phoneNumberDiscoverability', discoverability); if (profileKey) { - window.storage.put('profileKey', profileKey.toArrayBuffer()); + ourProfileKeyService.set(profileKey.toArrayBuffer()); } if (pinnedConversations) { diff --git a/ts/test-both/services/ourProfileKey_test.ts b/ts/test-both/services/ourProfileKey_test.ts new file mode 100644 index 000000000000..5d460a6189c8 --- /dev/null +++ b/ts/test-both/services/ourProfileKey_test.ts @@ -0,0 +1,178 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { noop } from 'lodash'; +import { sleep } from '../../util/sleep'; + +import { OurProfileKeyService } from '../../services/ourProfileKey'; + +describe('"our profile key" service', () => { + const createFakeStorage = () => ({ + get: sinon.stub(), + put: sinon.stub().resolves(), + remove: sinon.stub().resolves(), + onready: sinon.stub().callsArg(0), + }); + + describe('get', () => { + it("fetches the key from storage if it's there", async () => { + const fakeProfileKey = new ArrayBuffer(2); + const fakeStorage = createFakeStorage(); + fakeStorage.get.withArgs('profileKey').returns(fakeProfileKey); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + + assert.strictEqual(await service.get(), fakeProfileKey); + }); + + it('resolves with undefined if the key is not in storage', async () => { + const service = new OurProfileKeyService(); + service.initialize(createFakeStorage()); + + assert.isUndefined(await service.get()); + }); + + it("doesn't grab the profile key from storage until storage is ready", async () => { + let onReadyCallback = noop; + const fakeStorage = { + ...createFakeStorage(), + get: sinon.stub().returns(new ArrayBuffer(2)), + onready: sinon.stub().callsFake(callback => { + onReadyCallback = callback; + }), + }; + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + + const getPromise = service.get(); + + // We want to make sure this isn't called even after a tick of the event loop. + await sleep(1); + sinon.assert.notCalled(fakeStorage.get); + + onReadyCallback(); + + await getPromise; + sinon.assert.calledOnce(fakeStorage.get); + }); + + it("doesn't grab the profile key until all blocking promises are ready", async () => { + const fakeStorage = createFakeStorage(); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + + let resolve1 = noop; + service.blockGetWithPromise( + new Promise(resolve => { + resolve1 = resolve; + }) + ); + + let reject2 = noop; + service.blockGetWithPromise( + new Promise((_resolve, reject) => { + reject2 = reject; + }) + ); + + let reject3 = noop; + service.blockGetWithPromise( + new Promise((_resolve, reject) => { + reject3 = reject; + }) + ); + + const getPromise = service.get(); + + resolve1(); + await sleep(1); + sinon.assert.notCalled(fakeStorage.get); + + reject2(new Error('uh oh')); + await sleep(1); + sinon.assert.notCalled(fakeStorage.get); + + reject3(new Error('oh no')); + + await getPromise; + + sinon.assert.calledOnce(fakeStorage.get); + }); + + it("if there are blocking promises, doesn't grab the profile key from storage more than once (in other words, subsequent calls piggyback)", async () => { + const fakeStorage = createFakeStorage(); + fakeStorage.get.returns(new ArrayBuffer(2)); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + + let resolve = noop; + service.blockGetWithPromise( + new Promise(innerResolve => { + resolve = innerResolve; + }) + ); + + const getPromises = [service.get(), service.get(), service.get()]; + resolve(); + const results = await Promise.all(getPromises); + assert(new Set(results).size === 1, 'All results should be the same'); + + sinon.assert.calledOnce(fakeStorage.get); + }); + + it('removes all of the blocking promises after waiting for them once', async () => { + const fakeStorage = createFakeStorage(); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + + let resolve = noop; + service.blockGetWithPromise( + new Promise(innerResolve => { + resolve = innerResolve; + }) + ); + + const getPromise = service.get(); + + sinon.assert.notCalled(fakeStorage.get); + resolve(); + await getPromise; + sinon.assert.calledOnce(fakeStorage.get); + + await service.get(); + sinon.assert.calledTwice(fakeStorage.get); + }); + }); + + describe('set', () => { + it('updates the key in storage', async () => { + const fakeProfileKey = new ArrayBuffer(2); + const fakeStorage = createFakeStorage(); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + await service.set(fakeProfileKey); + + sinon.assert.calledOnce(fakeStorage.put); + sinon.assert.calledWith(fakeStorage.put, 'profileKey', fakeProfileKey); + }); + + it('clears the key in storage', async () => { + const fakeStorage = createFakeStorage(); + + const service = new OurProfileKeyService(); + service.initialize(fakeStorage); + await service.set(undefined); + + sinon.assert.calledOnce(fakeStorage.remove); + sinon.assert.calledWith(fakeStorage.remove, 'profileKey'); + }); + }); +}); diff --git a/ts/textsecure/AccountManager.ts b/ts/textsecure/AccountManager.ts index d87b6b2da677..b859f3aa1116 100644 --- a/ts/textsecure/AccountManager.ts +++ b/ts/textsecure/AccountManager.ts @@ -28,6 +28,7 @@ import { generatePreKey, } from '../Curve'; import { isMoreRecentThan, isOlderThan } from '../util/timestamp'; +import { ourProfileKeyService } from '../services/ourProfileKey'; const ARCHIVE_AGE = 30 * 24 * 60 * 60 * 1000; const PREKEY_ROTATION_AGE = 24 * 60 * 60 * 1000; @@ -624,7 +625,7 @@ export default class AccountManager extends EventTarget { await window.textsecure.storage.put('password', password); await window.textsecure.storage.put('registrationId', registrationId); if (profileKey) { - await window.textsecure.storage.put('profileKey', profileKey); + await ourProfileKeyService.set(profileKey); } if (userAgent) { await window.textsecure.storage.put('userAgent', userAgent); diff --git a/ts/util/shouldRespondWithProfileKey.ts b/ts/util/shouldRespondWithProfileKey.ts new file mode 100644 index 000000000000..118a6c21da04 --- /dev/null +++ b/ts/util/shouldRespondWithProfileKey.ts @@ -0,0 +1,36 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { MessageModel } from '../models/messages'; +import { assert } from './assert'; + +export async function shouldRespondWithProfileKey( + message: MessageModel +): Promise { + if (!message.isIncoming() || message.get('unidentifiedDeliveryReceived')) { + return false; + } + + const sender = message.getContact(); + if (!sender) { + assert( + false, + 'MessageModel#shouldRespondWithProfileKey had no sender. Returning false' + ); + return false; + } + + if (sender.isMe() || !sender.getAccepted() || sender.isBlocked()) { + return false; + } + + // We do message check in an attempt to avoid a database lookup. If someone was EVER in + // a shared group with us, we should've shared our profile key with them in the past, + // so we should respond with a profile key now. + if (sender.get('sharedGroupNames')?.length) { + return true; + } + + await sender.updateSharedGroups(); + return Boolean(sender.get('sharedGroupNames')?.length); +}