From ca4aad6bad929d20d556b387357b46c30916054e Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Wed, 3 May 2023 13:53:28 -0700 Subject: [PATCH] Extra toast for Message Receiver errors --- ts/background.ts | 15 ++++++- ts/textsecure/MessageReceiver.ts | 62 +++++++++++++++++++++----- ts/textsecure/messageReceiverEvents.ts | 12 +++++ ts/util/handleRetry.ts | 18 ++++++++ 4 files changed, 96 insertions(+), 11 deletions(-) diff --git a/ts/background.ts b/ts/background.ts index f06babe467..b22ebdbb77 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -88,6 +88,7 @@ import type { ErrorEvent, FetchLatestEvent, GroupEvent, + InvalidPlaintextEvent, KeysEvent, MessageEvent, MessageEventData, @@ -140,7 +141,11 @@ import * as Conversation from './types/Conversation'; import * as Stickers from './types/Stickers'; import * as Errors from './types/errors'; import { SignalService as Proto } from './protobuf'; -import { onRetryRequest, onDecryptionError } from './util/handleRetry'; +import { + onRetryRequest, + onDecryptionError, + onInvalidPlaintextMessage, +} from './util/handleRetry'; import { themeChanged } from './shims/themeChanged'; import { createIPCEvents } from './util/createIPCEvents'; import { RemoveAllConfiguration } from './types/RemoveAllConfiguration'; @@ -390,6 +395,14 @@ export async function startApp(): Promise { drop(onDecryptionErrorQueue.add(() => onDecryptionError(event))); }) ); + messageReceiver.addEventListener( + 'invalid-plaintext', + queuedEventListener((event: InvalidPlaintextEvent): void => { + drop( + onDecryptionErrorQueue.add(() => onInvalidPlaintextMessage(event)) + ); + }) + ); messageReceiver.addEventListener( 'retry-request', queuedEventListener((event: RetryRequestEvent): void => { diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 995999b01e..0ad2cd60b7 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -96,6 +96,7 @@ import { DecryptionErrorEvent, SentEvent, ProfileKeyUpdateEvent, + InvalidPlaintextEvent, MessageEvent, RetryRequestEvent, ReadEvent, @@ -160,6 +161,11 @@ type DecryptSealedSenderResult = Readonly<{ unsealedPlaintext?: SealedSenderDecryptionResult; }>; +type InnerDecryptResultType = Readonly<{ + plaintext: Uint8Array; + wasEncrypted: boolean; +}>; + type CacheAddItemType = { envelope: ProcessedEnvelope; data: UnprocessedType; @@ -533,6 +539,11 @@ export default class MessageReceiver handler: (ev: DecryptionErrorEvent) => void ): void; + public override addEventListener( + name: 'invalid-plaintext', + handler: (ev: InvalidPlaintextEvent) => void + ): void; + public override addEventListener( name: 'sent', handler: (ev: SentEvent) => void @@ -1395,18 +1406,20 @@ export default class MessageReceiver } log.info(logId); - const plaintext = await this.decrypt( + const decryptResult = await this.decrypt( stores, envelope, ciphertext, uuidKind ); - if (!plaintext) { + if (!decryptResult) { log.warn(`${logId}: plaintext was falsey`); - return { plaintext, envelope }; + return { plaintext: undefined, envelope }; } + const { plaintext, wasEncrypted } = decryptResult; + // Note: we need to process this as part of decryption, because we might need this // sender key to decrypt the next message in the queue! let isGroupV2 = false; @@ -1414,6 +1427,32 @@ export default class MessageReceiver let inProgressMessageType = ''; try { const content = Proto.Content.decode(plaintext); + if (!wasEncrypted && Bytes.isEmpty(content.decryptionErrorMessage)) { + log.warn( + `${logId}: dropping plaintext envelope without decryption error message` + ); + + const event = new InvalidPlaintextEvent({ + senderDevice: envelope.sourceDevice ?? 1, + senderUuid: envelope.sourceUuid, + timestamp: envelope.timestamp, + }); + + this.removeFromCache(envelope); + + const envelopeId = getEnvelopeId(envelope); + + // Avoid deadlocks by scheduling processing on decrypted queue + drop( + this.addToQueue( + async () => this.dispatchEvent(event), + `decrypted/dispatchEvent/InvalidPlaintextEvent(${envelopeId})`, + TaskType.Decrypted + ) + ); + + return { plaintext: undefined, envelope }; + } isGroupV2 = Boolean(content.dataMessage?.groupV2) || @@ -1742,7 +1781,7 @@ export default class MessageReceiver envelope: UnsealedEnvelope, ciphertext: Uint8Array, uuidKind: UUIDKind - ): Promise { + ): Promise { const { sessionStore, identityKeyStore, zone } = stores; const logId = getEnvelopeId(envelope); @@ -1784,7 +1823,10 @@ export default class MessageReceiver const buffer = Buffer.from(ciphertext); const plaintextContent = PlaintextContent.deserialize(buffer); - return this.unpad(plaintextContent.body()); + return { + plaintext: this.unpad(plaintextContent.body()), + wasEncrypted: false, + }; } if (envelope.type === envelopeTypeEnum.CIPHERTEXT) { log.info(`decrypt/${logId}: ciphertext message`); @@ -1813,7 +1855,7 @@ export default class MessageReceiver ), zone ); - return plaintext; + return { plaintext, wasEncrypted: true }; } if (envelope.type === envelopeTypeEnum.PREKEY_BUNDLE) { log.info(`decrypt/${logId}: prekey message`); @@ -1846,7 +1888,7 @@ export default class MessageReceiver ), zone ); - return plaintext; + return { plaintext, wasEncrypted: true }; } if (envelope.type === envelopeTypeEnum.UNIDENTIFIED_SENDER) { log.info(`decrypt/${logId}: unidentified message`); @@ -1857,7 +1899,7 @@ export default class MessageReceiver ); if (plaintext) { - return this.unpad(plaintext); + return { plaintext: this.unpad(plaintext), wasEncrypted: false }; } if (unsealedPlaintext) { @@ -1871,7 +1913,7 @@ export default class MessageReceiver // Return just the content because that matches the signature of the other // decrypt methods used above. - return this.unpad(content); + return { plaintext: this.unpad(content), wasEncrypted: true }; } throw new Error('Unexpected lack of plaintext from unidentified sender'); @@ -1884,7 +1926,7 @@ export default class MessageReceiver envelope: UnsealedEnvelope, ciphertext: Uint8Array, uuidKind: UUIDKind - ): Promise { + ): Promise { try { return await this.innerDecrypt(stores, envelope, ciphertext, uuidKind); } catch (error) { diff --git a/ts/textsecure/messageReceiverEvents.ts b/ts/textsecure/messageReceiverEvents.ts index 32eda7845f..5df3991e2c 100644 --- a/ts/textsecure/messageReceiverEvents.ts +++ b/ts/textsecure/messageReceiverEvents.ts @@ -161,6 +161,18 @@ export class DecryptionErrorEvent extends ConfirmableEvent { } } +export type InvalidPlaintextEventData = Readonly<{ + senderDevice: number; + senderUuid: string; + timestamp: number; +}>; + +export class InvalidPlaintextEvent extends Event { + constructor(public readonly data: InvalidPlaintextEventData) { + super('invalid-plaintext'); + } +} + export type RetryRequestEventData = Readonly<{ groupId?: string; ratchetKey?: PublicKey; diff --git a/ts/util/handleRetry.ts b/ts/util/handleRetry.ts index d5f931d424..088bdd729d 100644 --- a/ts/util/handleRetry.ts +++ b/ts/util/handleRetry.ts @@ -29,6 +29,7 @@ import type { ConversationModel } from '../models/conversations'; import type { DecryptionErrorEvent, DecryptionErrorEventData, + InvalidPlaintextEvent, RetryRequestEvent, RetryRequestEventData, } from '../textsecure/messageReceiverEvents'; @@ -205,6 +206,23 @@ function maybeShowDecryptionToast( }); } +export function onInvalidPlaintextMessage({ + data, +}: InvalidPlaintextEvent): void { + const { senderUuid, senderDevice, timestamp } = data; + const logId = `${senderUuid}.${senderDevice} ${timestamp}`; + + log.info(`onInvalidPlaintextMessage/${logId}: Starting...`); + + const conversation = window.ConversationController.getOrCreate( + senderUuid, + 'private' + ); + + const name = conversation.getTitle(); + maybeShowDecryptionToast(logId, name, senderDevice); +} + export async function onDecryptionError( event: DecryptionErrorEvent ): Promise {