From 375c5ba1a8829320b631abf6403c806a95bddbf2 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 10 Feb 2021 16:42:35 +1100 Subject: [PATCH 1/6] add the request and reply of an encryptionKeyPair if needed --- libloki/crypto.d.ts | 1 - libloki/crypto.js | 9 -- protos/SignalService.proto | 15 +-- ts/receiver/cache.ts | 1 + ts/receiver/closedGroups.ts | 116 ++++++++++++++++-- ts/receiver/contentMessage.ts | 22 +++- ts/receiver/keyPairRequestManager.ts | 47 +++++++ ts/session/group/index.ts | 90 +++++++++++--- .../group/ClosedGroupEncryptionPairMessage.ts | 3 +- .../ClosedGroupEncryptionPairReplyMessage.ts | 25 ++++ ...ClosedGroupEncryptionPairRequestMessage.ts | 19 +++ .../outgoing/content/data/group/index.ts | 2 + ts/session/sending/MessageQueueInterface.ts | 21 +++- ts/session/utils/Messages.ts | 8 +- .../receiving/KeyPairRequestManager_test.ts | 81 ++++++++++++ ts/test/session/unit/utils/Messages_test.ts | 7 +- 16 files changed, 412 insertions(+), 55 deletions(-) create mode 100644 ts/receiver/keyPairRequestManager.ts create mode 100644 ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage.ts create mode 100644 ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairRequestMessage.ts create mode 100644 ts/test/session/unit/receiving/KeyPairRequestManager_test.ts diff --git a/libloki/crypto.d.ts b/libloki/crypto.d.ts index c87aeed7f..703ce7c40 100644 --- a/libloki/crypto.d.ts +++ b/libloki/crypto.d.ts @@ -10,7 +10,6 @@ export interface CryptoInterface { EncryptGCM: any; // AES-GCM PairingType: PairingTypeEnum; _decodeSnodeAddressToPubKey: any; - decryptForPubkey: any; decryptToken: any; encryptForPubkey: any; generateEphemeralKeyPair: any; diff --git a/libloki/crypto.js b/libloki/crypto.js index da2d68f1e..438464196 100644 --- a/libloki/crypto.js +++ b/libloki/crypto.js @@ -69,14 +69,6 @@ return { ciphertext, symmetricKey, ephemeralKey: ephemeral.pubKey }; } - async function decryptForPubkey(seckeyX25519, ephemKey, ciphertext) { - const symmetricKey = await deriveSymmetricKey(ephemKey, seckeyX25519); - - const plaintext = await DecryptGCM(symmetricKey, ciphertext); - - return plaintext; - } - async function EncryptGCM(symmetricKey, plaintext) { const nonce = crypto.getRandomValues(new Uint8Array(NONCE_LENGTH)); @@ -187,7 +179,6 @@ PairingType, generateEphemeralKeyPair, encryptForPubkey, - decryptForPubkey, _decodeSnodeAddressToPubKey: decodeSnodeAddressToPubKey, sha512, }; diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 3329f2b21..4ad25d0a9 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -156,13 +156,14 @@ message DataMessage { message ClosedGroupControlMessage { enum Type { - NEW = 1; // publicKey, name, encryptionKeyPair, members, admins - UPDATE = 2; // name, members - ENCRYPTION_KEY_PAIR = 3; // wrappers - NAME_CHANGE = 4; // name - MEMBERS_ADDED = 5; // members - MEMBERS_REMOVED = 6; // members - MEMBER_LEFT = 7; + NEW = 1; // publicKey, name, encryptionKeyPair, members, admins + UPDATE = 2; // name, members + ENCRYPTION_KEY_PAIR = 3; // publicKey, wrappers + NAME_CHANGE = 4; // name + MEMBERS_ADDED = 5; // members + MEMBERS_REMOVED = 6; // members + MEMBER_LEFT = 7; + ENCRYPTION_KEY_PAIR_REQUEST = 8; } diff --git a/ts/receiver/cache.ts b/ts/receiver/cache.ts index adbfc77da..d4ecb174f 100644 --- a/ts/receiver/cache.ts +++ b/ts/receiver/cache.ts @@ -4,6 +4,7 @@ import _ from 'lodash'; export async function removeFromCache(envelope: EnvelopePlus) { const { id } = envelope; + window.log.info(`removing from cache envelope: ${id}`); return window.textsecure.storage.unprocessed.remove(id); } diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index a43dd2761..56708b2ef 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -14,6 +14,8 @@ import { getMessageQueue } from '../session'; import { decryptWithSessionProtocol } from './contentMessage'; import { addClosedGroupEncryptionKeyPair, + getAllEncryptionKeyPairsForGroup, + getLatestClosedGroupEncryptionKeyPair, removeAllClosedGroupEncryptionKeyPairs, } from '../../js/modules/data'; import { @@ -27,6 +29,8 @@ import { ConversationModel } from '../../js/models/conversations'; import _ from 'lodash'; import { forceSyncConfigurationNowIfNeeded } from '../session/utils/syncUtils'; import { MessageController } from '../session/messages'; +import { ClosedGroupEncryptionPairMessage } from '../session/messages/outgoing'; +import { ClosedGroupEncryptionPairReplyMessage } from '../session/messages/outgoing/content/data/group'; export async function handleClosedGroupControlMessage( envelope: EnvelopePlus, @@ -45,7 +49,13 @@ export async function handleClosedGroupControlMessage( } // We drop New closed group message from our other devices, as they will come as ConfigurationMessage instead if (type === Type.ENCRYPTION_KEY_PAIR) { - await handleClosedGroupEncryptionKeyPair(envelope, groupUpdate); + const isComingFromGroupPubkey = + envelope.type === SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT; + await handleClosedGroupEncryptionKeyPair( + envelope, + groupUpdate, + isComingFromGroupPubkey + ); } else if (type === Type.NEW) { await handleNewClosedGroup(envelope, groupUpdate); } else if ( @@ -53,6 +63,7 @@ export async function handleClosedGroupControlMessage( type === Type.MEMBERS_REMOVED || type === Type.MEMBERS_ADDED || type === Type.MEMBER_LEFT || + type === Type.ENCRYPTION_KEY_PAIR_REQUEST || type === Type.UPDATE ) { await performIfValid(envelope, groupUpdate); @@ -324,7 +335,8 @@ async function handleUpdateClosedGroup( */ async function handleClosedGroupEncryptionKeyPair( envelope: EnvelopePlus, - groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage + groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage, + isComingFromGroupPubkey: boolean ) { if ( groupUpdate.type !== @@ -332,12 +344,18 @@ async function handleClosedGroupEncryptionKeyPair( ) { return; } + // groupUpdate.publicKey might be set. This is used to give an explicitGroupPublicKey for this update. + const groupPublicKey = toHex(groupUpdate.publicKey) || envelope.source; + // in the case of an encryption key pair coming as a reply to a request we made + // senderIdentity will be unset as the message is not encoded for medium groups + const sender = isComingFromGroupPubkey + ? envelope.senderIdentity + : envelope.source; window.log.info( - `Got a group update for group ${envelope.source}, type: ENCRYPTION_KEY_PAIR` + `Got a group update for group ${groupPublicKey}, type: ENCRYPTION_KEY_PAIR` ); const ourNumber = await UserUtils.getOurNumber(); - const groupPublicKey = envelope.source; const ourKeyPair = await UserUtils.getIdentityKeyPair(); if (!ourKeyPair) { @@ -361,9 +379,9 @@ async function handleClosedGroupEncryptionKeyPair( await removeFromCache(envelope); return; } - if (!groupConvo.get('groupAdmins')?.includes(envelope.senderIdentity)) { + if (!groupConvo.get('members')?.includes(sender)) { window.log.warn( - `Ignoring closed group encryption key pair from non-admin. ${groupPublicKey}: ${envelope.senderIdentity}` + `Ignoring closed group encryption key pair from non-member. ${groupPublicKey}: ${envelope.senderIdentity}` ); await removeFromCache(envelope); return; @@ -426,7 +444,25 @@ async function handleClosedGroupEncryptionKeyPair( `Received a new encryptionKeyPair for group ${groupPublicKey}` ); - // Store it + // Store it if needed + const newKeyPairInHex = keyPair.toHexKeyPair(); + + const keyPairsAlreadySaved = await getAllEncryptionKeyPairsForGroup( + groupPublicKey + ); + const isKeyPairAlreadySaved = (keyPairsAlreadySaved || []).some( + k => + newKeyPairInHex.publicHex === k.publicHex && + newKeyPairInHex.privateHex === k.privateHex + ); + + if (isKeyPairAlreadySaved) { + window.log.info('Dropping already saved keypair for group', groupPublicKey); + await removeFromCache(envelope); + return; + } + window.log.info('Got a new encryption keypair for group', groupPublicKey); + await addClosedGroupEncryptionKeyPair(groupPublicKey, keyPair.toHexKeyPair()); await removeFromCache(envelope); } @@ -438,6 +474,7 @@ async function performIfValid( const { Type } = SignalService.DataMessage.ClosedGroupControlMessage; const groupPublicKey = envelope.source; + const sender = envelope.senderIdentity; const convo = ConversationController.getInstance().get(groupPublicKey); if (!convo) { @@ -472,9 +509,9 @@ async function performIfValid( // Check that the sender is a member of the group (before the update) const oldMembers = convo.get('members') || []; - if (!oldMembers.includes(envelope.senderIdentity)) { + if (!oldMembers.includes(sender)) { window.log.error( - `Error: closed group: ignoring closed group update message from non-member. ${envelope.senderIdentity} is not a current member.` + `Error: closed group: ignoring closed group update message from non-member. ${sender} is not a current member.` ); await removeFromCache(envelope); return; @@ -493,6 +530,13 @@ async function performIfValid( await handleClosedGroupMembersRemoved(envelope, groupUpdate, convo); } else if (groupUpdate.type === Type.MEMBER_LEFT) { await handleClosedGroupMemberLeft(envelope, groupUpdate, convo); + } else if (groupUpdate.type === Type.ENCRYPTION_KEY_PAIR_REQUEST) { + await handleClosedGroupEncryptionKeyPairRequest( + envelope, + groupUpdate, + convo + ); + // if you add a case here, remember to add it where performIfValid is called too. } return true; @@ -690,6 +734,60 @@ async function handleClosedGroupMemberLeft( await removeFromCache(envelope); } +async function handleClosedGroupEncryptionKeyPairRequest( + envelope: EnvelopePlus, + groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage, + convo: ConversationModel +) { + const sender = envelope.senderIdentity; + const groupPublicKey = envelope.source; + // Guard against self-sends + if (await UserUtils.isUs(sender)) { + window.log.info( + 'Dropping self send message of type ENCRYPTION_KEYPAIR_REQUEST' + ); + await removeFromCache(envelope); + return; + } + // Get the latest encryption key pair + const latestKeyPair = await getLatestClosedGroupEncryptionKeyPair( + groupPublicKey + ); + if (!latestKeyPair) { + window.log.info( + 'We do not have the keypair ourself, so dropping this message.' + ); + await removeFromCache(envelope); + return; + } + + window.log.info( + `Responding to closed group encryption key pair request from: ${sender}` + ); + await ConversationController.getInstance().getOrCreateAndWait( + sender, + 'private' + ); + + const wrappers = await ClosedGroup.buildEncryptionKeyPairWrappers( + [sender], + ECKeyPair.fromHexKeyPair(latestKeyPair) + ); + const expireTimer = convo.get('expireTimer') || 0; + + const keypairsMessage = new ClosedGroupEncryptionPairReplyMessage({ + groupId: groupPublicKey, + timestamp: Date.now(), + encryptedKeyPairs: wrappers, + expireTimer, + }); + + // the encryption keypair is sent using established channels + await getMessageQueue().sendToPubKey(PubKey.cast(sender), keypairsMessage); + + await removeFromCache(envelope); +} + export async function createClosedGroup( groupName: string, members: Array diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index a5667f9f1..7735ff130 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -15,6 +15,8 @@ import { ConversationController } from '../session/conversations'; import { getAllEncryptionKeyPairsForGroup } from '../../js/modules/data'; import { ECKeyPair } from './keypairs'; import { handleNewClosedGroup } from './closedGroups'; +import { KeyPairRequestManager } from './keyPairRequestManager'; +import { requestEncryptionKeyPair } from '../session/group'; export async function handleContentMessage(envelope: EnvelopePlus) { try { @@ -59,6 +61,10 @@ async function decryptForClosedGroup( // likely be the one we want) but try older ones in case that didn't work) let decryptedContent: ArrayBuffer | undefined; let keyIndex = 0; + + // If an error happens in here, we catch it in the inner try-catch + // When the loop is done, we check if the decryption is a success; + // If not, we trigger a new Error which will trigger in the outer try-catch do { try { const hexEncryptionKeyPair = encryptionKeyPairs.pop(); @@ -88,7 +94,6 @@ async function decryptForClosedGroup( } while (encryptionKeyPairs.length > 0); if (!decryptedContent?.byteLength) { - await removeFromCache(envelope); throw new Error( `Could not decrypt message for closed group with any of the ${encryptionKeyPairsCount} keypairs.` ); @@ -105,10 +110,23 @@ async function decryptForClosedGroup( return unpad(decryptedContent); } catch (e) { + /** + * If an error happened during the decoding, + * we trigger a request to get the latest EncryptionKeyPair for this medium group. + * Indeed, we might not have the latest one used by someone else, or not have any keypairs for this group. + * + */ + window.log.warn( 'decryptWithSessionProtocol for medium group message throw:', e ); + const keypairRequestManager = KeyPairRequestManager.getInstance(); + const groupPubKey = PubKey.cast(envelope.source); + if (keypairRequestManager.canTriggerRequestWith(groupPubKey)) { + keypairRequestManager.markRequestSendFor(groupPubKey, Date.now()); + await requestEncryptionKeyPair(groupPubKey); + } await removeFromCache(envelope); return null; } @@ -269,8 +287,6 @@ async function decrypt( envelope: EnvelopePlus, ciphertext: ArrayBuffer ): Promise { - const { textsecure } = window; - try { const plaintext = await doDecrypt(envelope, ciphertext); diff --git a/ts/receiver/keyPairRequestManager.ts b/ts/receiver/keyPairRequestManager.ts new file mode 100644 index 000000000..0bb8c4079 --- /dev/null +++ b/ts/receiver/keyPairRequestManager.ts @@ -0,0 +1,47 @@ +import { PubKey } from '../session/types'; +import { SECONDS } from '../session/utils/Number'; + +/** + * Singleton handling the logic behing requesting EncryptionKeypair for a closed group we need. + * + * Nothing is read/written to the db, it's all on memory for now. + */ +export class KeyPairRequestManager { + public static DELAY_BETWEEN_TWO_REQUEST_MS = SECONDS * 30; + private static instance: KeyPairRequestManager | null; + private readonly requestTimestamps: Map; + + private constructor() { + this.requestTimestamps = new Map(); + } + + public static getInstance() { + if (KeyPairRequestManager.instance) { + return KeyPairRequestManager.instance; + } + KeyPairRequestManager.instance = new KeyPairRequestManager(); + return KeyPairRequestManager.instance; + } + + public reset() { + this.requestTimestamps.clear(); + } + + public markRequestSendFor(pubkey: PubKey, timestamp: number) { + this.requestTimestamps.set(pubkey.key, timestamp); + } + + public get(pubkey: PubKey) { + return this.requestTimestamps.get(pubkey.key); + } + + public canTriggerRequestWith(pubkey: PubKey) { + const record = this.requestTimestamps.get(pubkey.key); + if (!record) { + return true; + } + + const now = Date.now(); + return now - record >= KeyPairRequestManager.DELAY_BETWEEN_TWO_REQUEST_MS; + } +} diff --git a/ts/session/group/index.ts b/ts/session/group/index.ts index 5a2c45b60..acd980883 100644 --- a/ts/session/group/index.ts +++ b/ts/session/group/index.ts @@ -29,6 +29,7 @@ import { UserUtils } from '../utils'; import { ClosedGroupMemberLeftMessage } from '../messages/outgoing/content/data/group/ClosedGroupMemberLeftMessage'; import { ClosedGroupAddedMembersMessage, + ClosedGroupEncryptionPairRequestMessage, ClosedGroupNameChangeMessage, ClosedGroupRemovedMembersMessage, ClosedGroupUpdateMessage, @@ -556,13 +557,55 @@ export async function generateAndSendNewEncryptionKeyPair( ); return; } + // Distribute it + const wrappers = await buildEncryptionKeyPairWrappers( + targetMembers, + newKeyPair + ); + + const expireTimer = groupConvo.get('expireTimer') || 0; + + const keypairsMessage = new ClosedGroupEncryptionPairMessage({ + groupId: toHex(groupId), + timestamp: Date.now(), + encryptedKeyPairs: wrappers, + expireTimer, + }); + + const messageSentCallback = async () => { + window.log.info( + `KeyPairMessage for ClosedGroup ${groupPublicKey} is sent. Saving the new encryptionKeyPair.` + ); + + await addClosedGroupEncryptionKeyPair( + toHex(groupId), + newKeyPair.toHexKeyPair() + ); + }; + // this is to be sent to the group pubkey adress + await getMessageQueue().sendToGroup(keypairsMessage, messageSentCallback); +} + +export async function buildEncryptionKeyPairWrappers( + targetMembers: Array, + encryptionKeyPair: ECKeyPair +) { + if ( + !encryptionKeyPair || + !encryptionKeyPair.publicKeyData.length || + !encryptionKeyPair.privateKeyData.length + ) { + throw new Error( + 'buildEncryptionKeyPairWrappers() needs a valid encryptionKeyPair set' + ); + } + const proto = new SignalService.KeyPair({ - privateKey: newKeyPair?.privateKeyData, - publicKey: newKeyPair?.publicKeyData, + privateKey: encryptionKeyPair?.privateKeyData, + publicKey: encryptionKeyPair?.publicKeyData, }); const plaintext = SignalService.KeyPair.encode(proto).finish(); - // Distribute it const wrappers = await Promise.all( targetMembers.map(async pubkey => { const ciphertext = await encryptUsingSessionProtocol( @@ -577,26 +620,37 @@ export async function generateAndSendNewEncryptionKeyPair( ); }) ); + return wrappers; +} - const expireTimer = groupConvo.get('expireTimer') || 0; +export async function requestEncryptionKeyPair( + groupPublicKey: string | PubKey +) { + const groupConvo = ConversationController.getInstance().get( + PubKey.cast(groupPublicKey).key + ); - const keypairsMessage = new ClosedGroupEncryptionPairMessage({ - groupId: toHex(groupId), - timestamp: Date.now(), - encryptedKeyPairs: wrappers, - expireTimer, - }); + if (!groupConvo) { + window.log.warn( + 'requestEncryptionKeyPair: Trying to request encryption key pair from unknown group' + ); + return; + } - const messageSentCallback = async () => { + const ourNumber = await UserUtils.getOurNumber(); + if (!groupConvo.get('members').includes(ourNumber.key)) { window.log.info( - `KeyPairMessage for ClosedGroup ${groupPublicKey} is sent. Saving the new encryptionKeyPair.` + 'requestEncryptionKeyPair: We are not a member of this group.' ); + return; + } + const expireTimer = groupConvo.get('expireTimer') || 0; - await addClosedGroupEncryptionKeyPair( - toHex(groupId), - newKeyPair.toHexKeyPair() - ); - }; + const ecRequestMessage = new ClosedGroupEncryptionPairRequestMessage({ + expireTimer, + groupId: groupPublicKey, + timestamp: Date.now(), + }); - await getMessageQueue().sendToGroup(keypairsMessage, messageSentCallback); + await getMessageQueue().sendToGroup(ecRequestMessage); } diff --git a/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairMessage.ts b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairMessage.ts index 3df2f0b3c..56c29a0f9 100644 --- a/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairMessage.ts +++ b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairMessage.ts @@ -1,11 +1,12 @@ import { Constants } from '../../../../..'; import { SignalService } from '../../../../../../protobuf'; +import { fromHexToArray } from '../../../../../utils/String'; import { ClosedGroupMessage, ClosedGroupMessageParams, } from './ClosedGroupMessage'; -interface ClosedGroupEncryptionPairMessageParams +export interface ClosedGroupEncryptionPairMessageParams extends ClosedGroupMessageParams { encryptedKeyPairs: Array< SignalService.DataMessage.ClosedGroupControlMessage.KeyPairWrapper diff --git a/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage.ts b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage.ts new file mode 100644 index 000000000..9aa014e15 --- /dev/null +++ b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage.ts @@ -0,0 +1,25 @@ +import { SignalService } from '../../../../../../protobuf'; +import { fromHexToArray } from '../../../../../utils/String'; +import { ClosedGroupEncryptionPairMessage } from './ClosedGroupEncryptionPairMessage'; + +/** + * On Desktop, we need separate class for message being sent to a closed group or a private chat. + * + * This is because we use the class of the message to know what encryption to use. + * See toRawMessage(); + * + * This class is just used to let us send the encryption key par after we receivied a ENCRYPTION_KEYPAIR_REQUEST + * from a member of a group. + * This reply must be sent to this user's pubkey, and so be encoded using sessionProtocol. + */ +export class ClosedGroupEncryptionPairReplyMessage extends ClosedGroupEncryptionPairMessage { + public dataProto(): SignalService.DataMessage { + const dataMessage = super.dataProto(); + // tslint:disable: no-non-null-assertion + dataMessage.closedGroupControlMessage!.publicKey = fromHexToArray( + this.groupId.key + ); + + return dataMessage; + } +} diff --git a/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairRequestMessage.ts b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairRequestMessage.ts new file mode 100644 index 000000000..2b0052cf0 --- /dev/null +++ b/ts/session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairRequestMessage.ts @@ -0,0 +1,19 @@ +import { Constants } from '../../../../..'; +import { SignalService } from '../../../../../../protobuf'; +import { ClosedGroupMessage } from './ClosedGroupMessage'; + +export class ClosedGroupEncryptionPairRequestMessage extends ClosedGroupMessage { + public dataProto(): SignalService.DataMessage { + const dataMessage = super.dataProto(); + + // tslint:disable: no-non-null-assertion + dataMessage.closedGroupControlMessage!.type = + SignalService.DataMessage.ClosedGroupControlMessage.Type.ENCRYPTION_KEY_PAIR_REQUEST; + + return dataMessage; + } + + public ttl(): number { + return Constants.TTL_DEFAULT.ENCRYPTION_PAIR_GROUP; + } +} diff --git a/ts/session/messages/outgoing/content/data/group/index.ts b/ts/session/messages/outgoing/content/data/group/index.ts index c16a052c1..495c34a7b 100644 --- a/ts/session/messages/outgoing/content/data/group/index.ts +++ b/ts/session/messages/outgoing/content/data/group/index.ts @@ -1,5 +1,7 @@ export * from './ClosedGroupChatMessage'; export * from './ClosedGroupEncryptionPairMessage'; +export * from './ClosedGroupEncryptionPairRequestMessage'; +export * from './ClosedGroupEncryptionPairReplyMessage'; export * from './ClosedGroupNewMessage'; export * from './ClosedGroupAddedMembersMessage'; export * from './ClosedGroupNameChangeMessage'; diff --git a/ts/session/sending/MessageQueueInterface.ts b/ts/session/sending/MessageQueueInterface.ts index 4fe5ac837..0306f5b34 100644 --- a/ts/session/sending/MessageQueueInterface.ts +++ b/ts/session/sending/MessageQueueInterface.ts @@ -2,13 +2,30 @@ import { ContentMessage, OpenGroupMessage } from '../messages/outgoing'; import { RawMessage } from '../types/RawMessage'; import { TypedEventEmitter } from '../utils'; import { PubKey } from '../types'; -import { ClosedGroupMessage } from '../messages/outgoing/content/data/group/ClosedGroupMessage'; import { ClosedGroupChatMessage } from '../messages/outgoing/content/data/group/ClosedGroupChatMessage'; +import { + ClosedGroupAddedMembersMessage, + ClosedGroupEncryptionPairMessage, + ClosedGroupNameChangeMessage, + ClosedGroupRemovedMembersMessage, + ClosedGroupUpdateMessage, +} from '../messages/outgoing/content/data/group'; +import { ClosedGroupMemberLeftMessage } from '../messages/outgoing/content/data/group/ClosedGroupMemberLeftMessage'; +import { ClosedGroupEncryptionPairRequestMessage } from '../messages/outgoing/content/data/group/ClosedGroupEncryptionPairRequestMessage'; export type GroupMessageType = | OpenGroupMessage | ClosedGroupChatMessage - | ClosedGroupMessage; + | ClosedGroupAddedMembersMessage + | ClosedGroupRemovedMembersMessage + | ClosedGroupNameChangeMessage + | ClosedGroupMemberLeftMessage + | ClosedGroupUpdateMessage + | ClosedGroupEncryptionPairMessage + | ClosedGroupEncryptionPairRequestMessage; + +// ClosedGroupEncryptionPairReplyMessage must be sent to a user pubkey. Not a group. + export interface MessageQueueInterfaceEvents { sendSuccess: ( message: RawMessage | OpenGroupMessage, diff --git a/ts/session/utils/Messages.ts b/ts/session/utils/Messages.ts index 623875d4c..97d7787c0 100644 --- a/ts/session/utils/Messages.ts +++ b/ts/session/utils/Messages.ts @@ -16,12 +16,16 @@ import { getLatestClosedGroupEncryptionKeyPair } from '../../../js/modules/data' import { UserUtils } from '.'; import { ECKeyPair } from '../../receiver/keypairs'; import _ from 'lodash'; +import { ClosedGroupEncryptionPairReplyMessage } from '../messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage'; -export function getEncryptionTypeFromMessageType( +function getEncryptionTypeFromMessageType( message: ContentMessage ): EncryptionType { // ClosedGroupNewMessage is sent using established channels, so using fallback - if (message instanceof ClosedGroupNewMessage) { + if ( + message instanceof ClosedGroupNewMessage || + message instanceof ClosedGroupEncryptionPairReplyMessage + ) { return EncryptionType.Fallback; } diff --git a/ts/test/session/unit/receiving/KeyPairRequestManager_test.ts b/ts/test/session/unit/receiving/KeyPairRequestManager_test.ts new file mode 100644 index 000000000..bab9f4dc8 --- /dev/null +++ b/ts/test/session/unit/receiving/KeyPairRequestManager_test.ts @@ -0,0 +1,81 @@ +import chai from 'chai'; +// tslint:disable: no-require-imports no-var-requires no-implicit-dependencies + +import _ from 'lodash'; +import { describe } from 'mocha'; +import { KeyPairRequestManager } from '../../../../receiver/keyPairRequestManager'; +import { TestUtils } from '../../../test-utils'; + +const chaiAsPromised = require('chai-as-promised'); +chai.use(chaiAsPromised); + +chai.should(); +const { expect } = chai; + +// tslint:disable-next-line: max-func-body-length +describe('KeyPairRequestManager', () => { + let inst: KeyPairRequestManager; + beforeEach(() => { + KeyPairRequestManager.getInstance().reset(); + inst = KeyPairRequestManager.getInstance(); + }); + + it('getInstance() should return an instance', () => { + expect(inst).to.exist; + }); + + describe('markRequestSendFor', () => { + it('should be able to set a timestamp for a pubkey', () => { + const groupPubkey = TestUtils.generateFakePubKey(); + const now = Date.now(); + inst.markRequestSendFor(groupPubkey, now); + expect(inst.get(groupPubkey)).to.be.equal(now); + }); + + it('should be able to override a timestamp for a pubkey', () => { + const groupPubkey = TestUtils.generateFakePubKey(); + const timestamp1 = Date.now(); + inst.markRequestSendFor(groupPubkey, timestamp1); + expect(inst.get(groupPubkey)).to.be.equal(timestamp1); + const timestamp2 = Date.now() + 1000; + inst.markRequestSendFor(groupPubkey, timestamp2); + expect(inst.get(groupPubkey)).to.be.equal(timestamp2); + }); + }); + + describe('canTriggerRequestWith', () => { + it('should return true if there is no timestamp set for this pubkey', () => { + const groupPubkey = TestUtils.generateFakePubKey(); + const can = inst.canTriggerRequestWith(groupPubkey); + expect(can).to.be.equal( + true, + 'should return true if we there is no timestamp set for this pubkey' + ); + }); + + it('should return false if there is a timestamp set for this pubkey and it is less than DELAY_BETWEEN_TWO_REQUEST_MS', () => { + const groupPubkey = TestUtils.generateFakePubKey(); + const timestamp1 = Date.now(); + + inst.markRequestSendFor(groupPubkey, timestamp1); + const can = inst.canTriggerRequestWith(groupPubkey); + expect(can).to.be.equal( + false, + 'return false if there is a timestamp set for this pubkey and it is less than DELAY_BETWEEN_TWO_REQUEST_MS' + ); + }); + + it('should return true if there is a timestamp set for this pubkey and it is more than DELAY_BETWEEN_TWO_REQUEST_MS', () => { + const groupPubkey = TestUtils.generateFakePubKey(); + const timestamp1 = + Date.now() - KeyPairRequestManager.DELAY_BETWEEN_TWO_REQUEST_MS; + + inst.markRequestSendFor(groupPubkey, timestamp1); + const can = inst.canTriggerRequestWith(groupPubkey); + expect(can).to.be.equal( + true, + 'true if there is a timestamp set for this pubkey and it is more than DELAY_BETWEEN_TWO_REQUEST_MS' + ); + }); + }); +}); diff --git a/ts/test/session/unit/utils/Messages_test.ts b/ts/test/session/unit/utils/Messages_test.ts index b9b4d91e7..8ca9837cb 100644 --- a/ts/test/session/unit/utils/Messages_test.ts +++ b/ts/test/session/unit/utils/Messages_test.ts @@ -17,6 +17,7 @@ import { import { ConversationModel } from '../../../../../js/models/conversations'; import { MockConversation } from '../../../test-utils/utils'; import { ConfigurationMessage } from '../../../../session/messages/outgoing/content/ConfigurationMessage'; +import { ClosedGroupEncryptionPairReplyMessage } from '../../../../session/messages/outgoing/content/data/group/ClosedGroupEncryptionPairReplyMessage'; // tslint:disable-next-line: no-require-imports no-var-requires const chaiAsPromised = require('chai-as-promised'); chai.use(chaiAsPromised); @@ -185,7 +186,7 @@ describe('Message Utils', () => { expect(rawMessage.encryption).to.equal(EncryptionType.ClosedGroup); }); - it('passing ClosedGroupEncryptionPairMessage returns ClosedGroup', async () => { + it('passing ClosedGroupEncryptionKeyPairReply returns Fallback', async () => { const device = TestUtils.generateFakePubKey(); const fakeWrappers = new Array< @@ -197,14 +198,14 @@ describe('Message Utils', () => { encryptedKeyPair: new Uint8Array(8), }) ); - const msg = new ClosedGroupEncryptionPairMessage({ + const msg = new ClosedGroupEncryptionPairReplyMessage({ timestamp: Date.now(), groupId: TestUtils.generateFakePubKey().key, encryptedKeyPairs: fakeWrappers, expireTimer: 0, }); const rawMessage = await MessageUtils.toRawMessage(device, msg); - expect(rawMessage.encryption).to.equal(EncryptionType.ClosedGroup); + expect(rawMessage.encryption).to.equal(EncryptionType.Fallback); }); it('passing a ConfigurationMessage returns Fallback', async () => { From ad06b947086d45dd612ca12677c9da5255abd705 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 10 Feb 2021 18:08:31 +1100 Subject: [PATCH 2/6] do not drop a message which was not decrypted for a medium group Instead, trigger a request to the group to get the encryption keypair. We will try to process those messages on an app restart --- ts/receiver/cache.ts | 2 +- ts/receiver/contentMessage.ts | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ts/receiver/cache.ts b/ts/receiver/cache.ts index d4ecb174f..45264d404 100644 --- a/ts/receiver/cache.ts +++ b/ts/receiver/cache.ts @@ -54,7 +54,7 @@ export async function getAllFromCache() { const attempts = _.toNumber(item.attempts || 0) + 1; try { - if (attempts >= 3) { + if (attempts >= 10) { window.log.warn( 'getAllFromCache final attempt for envelope', item.id diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 7735ff130..1b87af67d 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -127,7 +127,12 @@ async function decryptForClosedGroup( keypairRequestManager.markRequestSendFor(groupPubKey, Date.now()); await requestEncryptionKeyPair(groupPubKey); } - await removeFromCache(envelope); + throw new Error( + `Waiting for an encryption keypair to be received for group ${groupPubKey.key}` + ); + // do not remove it from the cache yet. We will try to decrypt it once we get the encryption keypair + // TODO drop it if after some time we still don't get to decrypt it + // await removeFromCache(envelope); return null; } } From 6d28f343c945686f12252dcee69f7d0d66916656 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 11 Feb 2021 10:23:21 +1100 Subject: [PATCH 3/6] try to decrypt unprocessed message when we get a new encryptionkeypair --- ts/receiver/cache.ts | 56 +++++++++++++++++++++++++++++++++++-- ts/receiver/closedGroups.ts | 6 +++- ts/receiver/receiver.ts | 14 +++++++++- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/ts/receiver/cache.ts b/ts/receiver/cache.ts index 45264d404..02a0c2be9 100644 --- a/ts/receiver/cache.ts +++ b/ts/receiver/cache.ts @@ -31,9 +31,7 @@ export async function addToCache( return window.textsecure.storage.unprocessed.add(data); } -export async function getAllFromCache() { - window.log.info('getAllFromCache'); - +async function fetchAllFromCache(): Promise> { const { textsecure } = window; const count = await textsecure.storage.unprocessed.getCount(); @@ -47,7 +45,59 @@ export async function getAllFromCache() { } const items = await textsecure.storage.unprocessed.getAll(); + return items; +} + +export async function getAllFromCache() { + window.log.info('getAllFromCache'); + const items = await fetchAllFromCache(); + window.log.info('getAllFromCache loaded', items.length, 'saved envelopes'); + const { textsecure } = window; + + return Promise.all( + _.map(items, async (item: any) => { + const attempts = _.toNumber(item.attempts || 0) + 1; + + try { + if (attempts >= 10) { + window.log.warn( + 'getAllFromCache final attempt for envelope', + item.id + ); + await textsecure.storage.unprocessed.remove(item.id); + } else { + await textsecure.storage.unprocessed.updateAttempts( + item.id, + attempts + ); + } + } catch (error) { + window.log.error( + 'getAllFromCache error updating item after load:', + error && error.stack ? error.stack : error + ); + } + + return item; + }) + ); +} + +export async function getAllFromCacheForSource(source: string) { + const items = await fetchAllFromCache(); + + // keep items without source too (for old message already added to the cache) + const itemsFromSource = items.filter( + item => !!item.senderIdentity || item.senderIdentity === source + ); + + window.log.info( + 'getAllFromCacheForSource loaded', + itemsFromSource.length, + 'saved envelopes' + ); + const { textsecure } = window; return Promise.all( _.map(items, async (item: any) => { diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 56708b2ef..43a43e063 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -29,8 +29,8 @@ import { ConversationModel } from '../../js/models/conversations'; import _ from 'lodash'; import { forceSyncConfigurationNowIfNeeded } from '../session/utils/syncUtils'; import { MessageController } from '../session/messages'; -import { ClosedGroupEncryptionPairMessage } from '../session/messages/outgoing'; import { ClosedGroupEncryptionPairReplyMessage } from '../session/messages/outgoing/content/data/group'; +import { queueAllCachedFromSource } from './receiver'; export async function handleClosedGroupControlMessage( envelope: EnvelopePlus, @@ -250,6 +250,8 @@ export async function handleNewClosedGroup( window.SwarmPolling.addGroupId(PubKey.cast(groupId)); await removeFromCache(envelope); + // trigger decrypting of all this group messages we did not decrypt successfully yet. + await queueAllCachedFromSource(groupId); } async function handleUpdateClosedGroup( @@ -465,6 +467,8 @@ async function handleClosedGroupEncryptionKeyPair( await addClosedGroupEncryptionKeyPair(groupPublicKey, keyPair.toHexKeyPair()); await removeFromCache(envelope); + // trigger decrypting of all this group messages we did not decrypt successfully yet. + await queueAllCachedFromSource(groupPublicKey); } async function performIfValid( diff --git a/ts/receiver/receiver.ts b/ts/receiver/receiver.ts index 02140be0e..84f414ac7 100644 --- a/ts/receiver/receiver.ts +++ b/ts/receiver/receiver.ts @@ -3,7 +3,12 @@ import { EnvelopePlus } from './types'; export { downloadAttachment } from './attachments'; -import { addToCache, getAllFromCache, removeFromCache } from './cache'; +import { + addToCache, + getAllFromCache, + getAllFromCacheForSource, + removeFromCache, +} from './cache'; import { processMessage } from '../session/snode_api/swarmPolling'; import { onError } from './errors'; @@ -189,6 +194,13 @@ export async function queueAllCached() { }); } +export async function queueAllCachedFromSource(source: string) { + const items = await getAllFromCacheForSource(source); + items.forEach(async item => { + await queueCached(item); + }); +} + async function queueCached(item: any) { const { textsecure } = window; From 01f834ae986fbb3aa5cad16f9739e5c8bab28673 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 11 Feb 2021 11:25:03 +1100 Subject: [PATCH 4/6] only handle the first ever configuration message incoming --- ts/receiver/contentMessage.ts | 44 +++++++-- .../messages/ConfigurationMessage_test.ts | 1 - .../receiving/ConfigurationMessage_test.ts | 93 +++++++++++++++++++ ts/test/test-utils/utils/envelope.ts | 15 +++ 4 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 ts/test/session/unit/receiving/ConfigurationMessage_test.ts diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 1b87af67d..845b60b34 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -12,7 +12,11 @@ import { GroupUtils, UserUtils } from '../session/utils'; import { fromHexToArray, toHex } from '../session/utils/String'; import { concatUInt8Array, getSodium } from '../session/crypto'; import { ConversationController } from '../session/conversations'; -import { getAllEncryptionKeyPairsForGroup } from '../../js/modules/data'; +import { + createOrUpdateItem, + getAllEncryptionKeyPairsForGroup, + getItemById, +} from '../../js/modules/data'; import { ECKeyPair } from './keypairs'; import { handleNewClosedGroup } from './closedGroups'; import { KeyPairRequestManager } from './keyPairRequestManager'; @@ -519,7 +523,7 @@ async function handleTypingMessage( } } -async function handleConfigurationMessage( +export async function handleConfigurationMessage( envelope: EnvelopePlus, configurationMessage: SignalService.ConfigurationMessage ): Promise { @@ -527,15 +531,35 @@ async function handleConfigurationMessage( if (!ourPubkey) { return; } - + console.warn('ourPubkey', ourPubkey); + console.warn('envelope.source', envelope.source); if (envelope.source !== ourPubkey) { - window.log.info('dropping configuration change from someone else than us.'); + window?.log?.info( + 'Dropping configuration change from someone else than us.' + ); return removeFromCache(envelope); } + const ITEM_ID_PROCESSED_CONFIGURATION_MESSAGE = + 'ITEM_ID_PROCESSED_CONFIGURATION_MESSAGE'; + const didWeHandleAConfigurationMessageAlready = + (await getItemById(ITEM_ID_PROCESSED_CONFIGURATION_MESSAGE))?.value || + false; + if (didWeHandleAConfigurationMessageAlready) { + window?.log?.warn( + 'Dropping configuration change as we already handled one... ' + ); + await removeFromCache(envelope); + return; + } + await createOrUpdateItem({ + id: ITEM_ID_PROCESSED_CONFIGURATION_MESSAGE, + value: true, + }); + const numberClosedGroup = configurationMessage.closedGroups?.length || 0; - window.log.warn( + window?.log?.warn( `Received ${numberClosedGroup} closed group on configuration. Creating them... ` ); @@ -551,7 +575,13 @@ async function handleConfigurationMessage( publicKey: c.publicKey, } ); - await handleNewClosedGroup(envelope, groupUpdate); + try { + await handleNewClosedGroup(envelope, groupUpdate); + } catch (e) { + window?.log?.warn( + 'failed to handle a new closed group from configuration message' + ); + } }) ); @@ -563,7 +593,7 @@ async function handleConfigurationMessage( for (let i = 0; i < numberOpenGroup; i++) { const current = configurationMessage.openGroups[i]; if (!allOpenGroups.includes(current)) { - window.log.info( + window?.log?.info( `triggering join of public chat '${current}' from ConfigurationMessage` ); void OpenGroup.join(current); diff --git a/ts/test/session/unit/messages/ConfigurationMessage_test.ts b/ts/test/session/unit/messages/ConfigurationMessage_test.ts index 8ceb988fb..04555f9c6 100644 --- a/ts/test/session/unit/messages/ConfigurationMessage_test.ts +++ b/ts/test/session/unit/messages/ConfigurationMessage_test.ts @@ -5,7 +5,6 @@ import { ConfigurationMessage, ConfigurationMessageClosedGroup, } from '../../../../session/messages/outgoing/content/ConfigurationMessage'; -import { PubKey } from '../../../../session/types'; import { TestUtils } from '../../../test-utils'; describe('ConfigurationMessage', () => { diff --git a/ts/test/session/unit/receiving/ConfigurationMessage_test.ts b/ts/test/session/unit/receiving/ConfigurationMessage_test.ts new file mode 100644 index 000000000..0589ff1fc --- /dev/null +++ b/ts/test/session/unit/receiving/ConfigurationMessage_test.ts @@ -0,0 +1,93 @@ +import { SignalService } from '../../../../protobuf'; +import { handleConfigurationMessage } from '../../../../receiver/contentMessage'; +import chai from 'chai'; + +import { ConfigurationMessage } from '../../../../session/messages/outgoing/content/ConfigurationMessage'; +import { UserUtils } from '../../../../session/utils'; +import { TestUtils } from '../../../test-utils'; + +import Sinon, * as sinon from 'sinon'; +import * as cache from '../../../../receiver/cache'; +import * as data from '../../../../../js/modules/data'; +import { EnvelopePlus } from '../../../../receiver/types'; + +// tslint:disable-next-line: no-require-imports no-var-requires +const chaiAsPromised = require('chai-as-promised'); +chai.use(chaiAsPromised); +chai.should(); + +const { expect } = chai; + +describe('ConfigurationMessage_receiving', () => { + const sandbox = sinon.createSandbox(); + let createOrUpdateStub: Sinon.SinonStub; + let getItemByIdStub: Sinon.SinonStub; + let sender: string; + + let envelope: EnvelopePlus; + let config: ConfigurationMessage; + + beforeEach(() => { + sandbox.stub(cache, 'removeFromCache').resolves(); + sender = TestUtils.generateFakePubKey().key; + config = new ConfigurationMessage({ + activeOpenGroups: [], + activeClosedGroups: [], + timestamp: Date.now(), + identifier: 'whatever', + }); + }); + + afterEach(() => { + TestUtils.restoreStubs(); + sandbox.restore(); + }); + + it('should not be processed if we do not have a pubkey', async () => { + sandbox.stub(UserUtils, 'getCurrentDevicePubKey').resolves(undefined); + envelope = TestUtils.generateEnvelopePlus(sender); + + const proto = config.contentProto(); + createOrUpdateStub = sandbox.stub(data, 'createOrUpdateItem').resolves(); + getItemByIdStub = sandbox.stub(data, 'getItemById').resolves(); + await handleConfigurationMessage( + envelope, + proto.configurationMessage as SignalService.ConfigurationMessage + ); + expect(createOrUpdateStub.callCount).to.equal(0); + expect(getItemByIdStub.callCount).to.equal(0); + }); + + describe('with ourNumber set', () => { + const ourNumber = TestUtils.generateFakePubKey().key; + + beforeEach(() => { + sandbox.stub(UserUtils, 'getCurrentDevicePubKey').resolves(ourNumber); + }); + + it('should not be processed if the message is not coming from our number', async () => { + const proto = config.contentProto(); + // sender !== ourNumber + envelope = TestUtils.generateEnvelopePlus(sender); + + createOrUpdateStub = sandbox.stub(data, 'createOrUpdateItem').resolves(); + getItemByIdStub = sandbox.stub(data, 'getItemById').resolves(); + await handleConfigurationMessage( + envelope, + proto.configurationMessage as SignalService.ConfigurationMessage + ); + expect(createOrUpdateStub.callCount).to.equal(0); + expect(getItemByIdStub.callCount).to.equal(0); + }); + + // it('should be processed if the message is coming from our number', async () => { + // const proto = config.contentProto(); + // envelope = TestUtils.generateEnvelopePlus(ourNumber); + + // createOrUpdateStub = sandbox.stub(data, 'createOrUpdateItem').resolves(); + // getItemByIdStub = sandbox.stub(data, 'getItemById').resolves(); + // await handleConfigurationMessage(envelope, proto.configurationMessage as SignalService.ConfigurationMessage); + // expect(getItemByIdStub.callCount).to.equal(1); + // }); + }); +}); diff --git a/ts/test/test-utils/utils/envelope.ts b/ts/test/test-utils/utils/envelope.ts index 32690304f..73b59ad4c 100644 --- a/ts/test/test-utils/utils/envelope.ts +++ b/ts/test/test-utils/utils/envelope.ts @@ -22,6 +22,21 @@ export function generateEnvelopePlusClosedGroup( return envelope; } +export function generateEnvelopePlus(sender: string): EnvelopePlus { + const envelope: EnvelopePlus = { + receivedAt: Date.now(), + timestamp: Date.now() - 2000, + id: uuid(), + type: SignalService.Envelope.Type.UNIDENTIFIED_SENDER, + source: sender, + senderIdentity: sender, + content: new Uint8Array(), + toJSON: () => ['fake'], + }; + + return envelope; +} + export function generateGroupUpdateNameChange( groupId: string ): SignalService.DataMessage.ClosedGroupControlMessage { From a31c457c086e3b570e1d7ebba61a9b0667a18dfa Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 11 Feb 2021 14:04:08 +1100 Subject: [PATCH 5/6] allow closedgroup new message to be sent to our other devices also, do not drop it on the receiving side --- ts/receiver/closedGroups.ts | 14 +++++----- ts/receiver/contentMessage.ts | 3 +-- ts/session/sending/MessageQueue.ts | 27 +++++++------------ .../session/unit/sending/MessageQueue_test.ts | 20 +++----------- 4 files changed, 21 insertions(+), 43 deletions(-) diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 43a43e063..d6375dfd4 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -152,9 +152,9 @@ export async function handleNewClosedGroup( await removeFromCache(envelope); return; } - const ourPrimary = await UserUtils.getOurNumber(); + const ourNumber = await UserUtils.getOurNumber(); - if (envelope.senderIdentity === ourPrimary.key) { + if (envelope.senderIdentity === ourNumber.key) { window.log.warn( 'Dropping new closed group updatemessage from our other device.' ); @@ -173,7 +173,7 @@ export async function handleNewClosedGroup( const members = membersAsData.map(toHex); const admins = adminsAsData.map(toHex); - if (!members.includes(ourPrimary.key)) { + if (!members.includes(ourNumber.key)) { log.info( 'Got a new group message but apparently we are not a member of it. Dropping it.' ); @@ -694,15 +694,13 @@ async function handleClosedGroupMemberLeft( const oldMembers = convo.get('members') || []; const leftMemberWasPresent = oldMembers.includes(sender); const members = didAdminLeave ? [] : oldMembers.filter(s => s !== sender); - // Guard against self-sends + // Show log if we sent this message ourself (from another device or not) const ourPubkey = await UserUtils.getCurrentDevicePubKey(); if (!ourPubkey) { throw new Error('Could not get user pubkey'); } - if (sender === ourPubkey) { - window.log.info('self send group update ignored'); - await removeFromCache(envelope); - return; + if (await UserUtils.isUs(sender)) { + window.log.info('Got self-sent group update member left...'); } // Generate and distribute a new encryption key pair if needed diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 845b60b34..9175c8cd6 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -531,8 +531,7 @@ export async function handleConfigurationMessage( if (!ourPubkey) { return; } - console.warn('ourPubkey', ourPubkey); - console.warn('envelope.source', envelope.source); + if (envelope.source !== ourPubkey) { window?.log?.info( 'Dropping configuration change from someone else than us.' diff --git a/ts/session/sending/MessageQueue.ts b/ts/session/sending/MessageQueue.ts index 0beb1884a..a2c6cc7d5 100644 --- a/ts/session/sending/MessageQueue.ts +++ b/ts/session/sending/MessageQueue.ts @@ -6,6 +6,7 @@ import { } from './MessageQueueInterface'; import { ChatMessage, + ClosedGroupNewMessage, ContentMessage, DataMessage, ExpirationTimerUpdateMessage, @@ -40,8 +41,7 @@ export class MessageQueue implements MessageQueueInterface { ) { throw new Error('SyncMessage needs to be sent with sendSyncMessage'); } - - await this.sendMessageToDevices([user], message); + await this.process(user, message, sentCb); } public async send( @@ -55,7 +55,7 @@ export class MessageQueue implements MessageQueueInterface { ) { throw new Error('SyncMessage needs to be sent with sendSyncMessage'); } - await this.sendMessageToDevices([device], message, sentCb); + await this.process(device, message, sentCb); } /** @@ -136,7 +136,7 @@ export class MessageQueue implements MessageQueueInterface { throw new Error('ourNumber is not set'); } - await this.sendMessageToDevices([PubKey.cast(ourPubKey)], message, sentCb); + await this.process(PubKey.cast(ourPubKey), message, sentCb); } public async processPending(device: PubKey) { @@ -172,18 +172,6 @@ export class MessageQueue implements MessageQueueInterface { }); } - public async sendMessageToDevices( - devices: Array, - message: ContentMessage, - sentCb?: (message: RawMessage) => Promise - ) { - const promises = devices.map(async device => { - await this.process(device, message, sentCb); - }); - - return Promise.all(promises); - } - private async processAllPending() { const devices = await this.pendingMessageCache.getDevices(); const promises = devices.map(async device => this.processPending(device)); @@ -191,6 +179,9 @@ export class MessageQueue implements MessageQueueInterface { return Promise.all(promises); } + /** + * This method should not be called directly. Only through sendToPubKey. + */ private async process( device: PubKey, message: ContentMessage, @@ -199,9 +190,11 @@ export class MessageQueue implements MessageQueueInterface { // Don't send to ourselves const currentDevice = await UserUtils.getCurrentDevicePubKey(); if (currentDevice && device.isEqual(currentDevice)) { - // We allow a message for ourselve only if it's a ConfigurationMessage or a message with a syncTarget set + // We allow a message for ourselve only if it's a ConfigurationMessage, a ClosedGroupNewMessage, + // or a message with a syncTarget set. if ( message instanceof ConfigurationMessage || + message instanceof ClosedGroupNewMessage || (message as any).syncTarget?.length > 0 ) { window.log.warn('Processing sync message'); diff --git a/ts/test/session/unit/sending/MessageQueue_test.ts b/ts/test/session/unit/sending/MessageQueue_test.ts index c53974056..c85ca7fd4 100644 --- a/ts/test/session/unit/sending/MessageQueue_test.ts +++ b/ts/test/session/unit/sending/MessageQueue_test.ts @@ -141,30 +141,18 @@ describe('MessageQueue', () => { describe('sendToPubKey', () => { it('should send the message to the device', async () => { - const devices = TestUtils.generateFakePubKeys(1); - const stub = sandbox - .stub(messageQueueStub, 'sendMessageToDevices') - .resolves(); + const device = TestUtils.generateFakePubKey(); + const stub = sandbox.stub(messageQueueStub as any, 'process').resolves(); const message = TestUtils.generateChatMessage(); - await messageQueueStub.sendToPubKey(devices[0], message); + await messageQueueStub.sendToPubKey(device, message); const args = stub.lastCall.args as [Array, ContentMessage]; - expect(args[0]).to.have.same.members(devices); + expect(args[0]).to.be.equal(device); expect(args[1]).to.equal(message); }); }); - describe('sendMessageToDevices', () => { - it('can send to many devices', async () => { - const devices = TestUtils.generateFakePubKeys(5); - const message = TestUtils.generateChatMessage(); - - await messageQueueStub.sendMessageToDevices(devices, message); - expect(pendingMessageCache.getCache()).to.have.length(devices.length); - }); - }); - describe('sendToGroup', () => { it('should throw an error if invalid non-group message was passed', async () => { // const chatMessage = TestUtils.generateChatMessage(); From df3ca5d38a2ab59c667c0aebdc84701444c5b18f Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 11 Feb 2021 14:33:55 +1100 Subject: [PATCH 6/6] add a sql function to check if a keypair is already saved in db --- app/sql.js | 13 +++++++++++++ js/modules/data.d.ts | 5 +++++ js/modules/data.js | 5 +++++ ts/receiver/closedGroups.ts | 13 +++++-------- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/app/sql.js b/app/sql.js index b9a1128ce..34aede29d 100644 --- a/app/sql.js +++ b/app/sql.js @@ -171,6 +171,7 @@ module.exports = { getAllEncryptionKeyPairsForGroup, getLatestClosedGroupEncryptionKeyPair, addClosedGroupEncryptionKeyPair, + isKeyPairAlreadySaved, removeAllClosedGroupEncryptionKeyPairs, }; @@ -3267,6 +3268,18 @@ async function addClosedGroupEncryptionKeyPair( ); } +async function isKeyPairAlreadySaved( + groupPublicKey, + newKeyPairInHex // : HexKeyPair +) { + const allKeyPairs = await getAllEncryptionKeyPairsForGroup(groupPublicKey); + return (allKeyPairs || []).some( + k => + newKeyPairInHex.publicHex === k.publicHex && + newKeyPairInHex.privateHex === k.privateHex + ); +} + async function removeAllClosedGroupEncryptionKeyPairs(groupPublicKey) { await db.run( `DELETE FROM ${CLOSED_GROUP_V2_KEY_PAIRS_TABLE} WHERE groupPublicKey = $groupPublicKey`, diff --git a/js/modules/data.d.ts b/js/modules/data.d.ts index 479f94611..87c242603 100644 --- a/js/modules/data.d.ts +++ b/js/modules/data.d.ts @@ -1,5 +1,6 @@ import { KeyPair } from '../../libtextsecure/libsignal-protocol'; import { HexKeyPair } from '../../ts/receiver/closedGroups'; +import { ECKeyPair } from '../../ts/receiver/keypairs'; import { PubKey } from '../../ts/session/types'; import { ConversationType } from '../../ts/state/ducks/conversations'; import { Message } from '../../ts/types/Message'; @@ -401,6 +402,10 @@ export function removeAllClosedGroupRatchets(groupId: string): Promise; export function getAllEncryptionKeyPairsForGroup( groupPublicKey: string | PubKey ): Promise | undefined>; +export function isKeyPairAlreadySaved( + groupPublicKey: string, + keypair: HexKeyPair +): Promise; export function getLatestClosedGroupEncryptionKeyPair( groupPublicKey: string ): Promise; diff --git a/js/modules/data.js b/js/modules/data.js index f3661b5fd..4f3690c4f 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -196,6 +196,7 @@ module.exports = { getAllEncryptionKeyPairsForGroup, getLatestClosedGroupEncryptionKeyPair, addClosedGroupEncryptionKeyPair, + isKeyPairAlreadySaved, removeAllClosedGroupEncryptionKeyPairs, }; @@ -723,6 +724,10 @@ async function addClosedGroupEncryptionKeyPair(groupPublicKey, keypair) { return channels.addClosedGroupEncryptionKeyPair(groupPublicKey, keypair); } +async function isKeyPairAlreadySaved(groupPublicKey, keypair) { + return channels.isKeyPairAlreadySaved(groupPublicKey, keypair); +} + async function removeAllClosedGroupEncryptionKeyPairs(groupPublicKey) { return channels.removeAllClosedGroupEncryptionKeyPairs(groupPublicKey); } diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index d6375dfd4..33325a563 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -16,6 +16,7 @@ import { addClosedGroupEncryptionKeyPair, getAllEncryptionKeyPairsForGroup, getLatestClosedGroupEncryptionKeyPair, + isKeyPairAlreadySaved, removeAllClosedGroupEncryptionKeyPairs, } from '../../js/modules/data'; import { @@ -449,16 +450,12 @@ async function handleClosedGroupEncryptionKeyPair( // Store it if needed const newKeyPairInHex = keyPair.toHexKeyPair(); - const keyPairsAlreadySaved = await getAllEncryptionKeyPairsForGroup( - groupPublicKey - ); - const isKeyPairAlreadySaved = (keyPairsAlreadySaved || []).some( - k => - newKeyPairInHex.publicHex === k.publicHex && - newKeyPairInHex.privateHex === k.privateHex + const isKeyPairAlreadyHere = await isKeyPairAlreadySaved( + groupPublicKey, + newKeyPairInHex ); - if (isKeyPairAlreadySaved) { + if (isKeyPairAlreadyHere) { window.log.info('Dropping already saved keypair for group', groupPublicKey); await removeFromCache(envelope); return;