From 334da0a1693bb6140e4b5defb89fa596ded18ae7 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 1 Feb 2021 11:35:06 +1100 Subject: [PATCH] finish explicit group updates --- app/sql.js | 8 +- ts/receiver/closedGroups.ts | 134 +++++++++++++++--- ts/receiver/contentMessage.ts | 15 +- ts/receiver/dataMessage.ts | 5 +- ts/session/crypto/MessageEncrypter.ts | 2 +- ts/session/group/index.ts | 4 +- .../unit/receiving/ClosedGroupUpdates_test.ts | 76 +++++----- ts/test/test-utils/utils/envelope.ts | 50 ++++--- 8 files changed, 203 insertions(+), 91 deletions(-) diff --git a/app/sql.js b/app/sql.js index cfbabe14a..b9a1128ce 100644 --- a/app/sql.js +++ b/app/sql.js @@ -3215,6 +3215,12 @@ async function updateExistingClosedGroupToClosedGroup(instance) { * @param {*} groupPublicKey string | PubKey */ async function getAllEncryptionKeyPairsForGroup(groupPublicKey) { + const rows = await getAllEncryptionKeyPairsForGroupRaw(groupPublicKey); + + return map(rows, row => jsonToObject(row.json)); +} + +async function getAllEncryptionKeyPairsForGroupRaw(groupPublicKey) { const pubkeyAsString = groupPublicKey.key ? groupPublicKey.key : groupPublicKey; @@ -3225,7 +3231,7 @@ async function getAllEncryptionKeyPairsForGroup(groupPublicKey) { } ); - return map(rows, row => jsonToObject(row.json)); + return rows; } async function getLatestClosedGroupEncryptionKeyPair(groupPublicKey) { diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 7e8875afb..483e25189 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -22,6 +22,7 @@ import { ECKeyPair } from './keypairs'; import { getOurNumber } from '../session/utils/User'; import { UserUtils } from '../session/utils'; import { ConversationModel } from '../../js/models/conversations'; +import _ from 'lodash'; export async function handleClosedGroupControlMessage( envelope: EnvelopePlus, @@ -40,7 +41,13 @@ export async function handleClosedGroupControlMessage( await handleClosedGroupEncryptionKeyPair(envelope, groupUpdate); } else if (type === Type.NEW) { await handleNewClosedGroup(envelope, groupUpdate); - } else if (type === Type.NAME_CHANGE || type === Type.MEMBERS_REMOVED || type === Type.MEMBERS_ADDED || type === Type.MEMBER_LEFT || type === Type.UPDATE) { + } else if ( + type === Type.NAME_CHANGE || + type === Type.MEMBERS_REMOVED || + type === Type.MEMBERS_ADDED || + type === Type.MEMBER_LEFT || + type === Type.UPDATE + ) { await performIfValid(envelope, groupUpdate); } else { window.log.error('Unknown group update type: ', type); @@ -230,7 +237,6 @@ async function handleUpdateClosedGroup( const curAdmins = convo.get('groupAdmins'); - // NOTE: admins cannot change with closed groups const members = membersBinary.map(toHex); const diff = ClosedGroup.buildGroupDiff(convo, { name, members }); @@ -291,6 +297,7 @@ async function handleUpdateClosedGroup( convo.set('members', members); await convo.commit(); + convo.updateLastMessage(); await removeFromCache(envelope); } @@ -310,6 +317,9 @@ async function handleClosedGroupEncryptionKeyPair( ) { return; } + window.log.info( + `Got a group update for group ${envelope.source}, type: ENCRYPTION_KEY_PAIR` + ); const ourNumber = await UserUtils.getOurNumber(); const groupPublicKey = envelope.source; const ourKeyPair = await UserUtils.getIdentityKeyPair(); @@ -410,9 +420,10 @@ async function handleClosedGroupEncryptionKeyPair( await removeFromCache(envelope); } - -async function performIfValid(envelope: EnvelopePlus, - groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage) { +async function performIfValid( + envelope: EnvelopePlus, + groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage +) { const { Type } = SignalService.DataMessage.ClosedGroupControlMessage; const groupPublicKey = envelope.source; @@ -470,7 +481,6 @@ async function performIfValid(envelope: EnvelopePlus, await handleClosedGroupMemberLeft(envelope, groupUpdate, convo); } - return true; } @@ -481,18 +491,20 @@ async function handleClosedGroupNameChanged( ) { // Only add update message if we have something to show const newName = groupUpdate.name; - if ( - newName !== convo.get(('name')) - ) { + window.log.info( + `Got a group update for group ${envelope.source}, type: NAME_CHANGED` + ); + + if (newName !== convo.get('name')) { const groupDiff: ClosedGroup.GroupDiff = { newName, }; await ClosedGroup.addUpdateMessage(convo, groupDiff, 'incoming'); convo.set({ name: newName }); + convo.updateLastMessage(); await convo.commit(); } - await removeFromCache(envelope); } @@ -504,11 +516,18 @@ async function handleClosedGroupMembersAdded( const { members: addedMembersBinary } = groupUpdate; const addedMembers = (addedMembersBinary || []).map(toHex); const oldMembers = convo.get('members') || []; - const membersNotAlreadyPresent = addedMembers.filter(m => !oldMembers.includes(m)); + const membersNotAlreadyPresent = addedMembers.filter( + m => !oldMembers.includes(m) + ); console.warn('membersNotAlreadyPresent', membersNotAlreadyPresent); + window.log.info( + `Got a group update for group ${envelope.source}, type: MEMBERS_ADDED` + ); if (membersNotAlreadyPresent.length === 0) { - window.log.info('no new members in this group update compared to what we have already. Skipping update'); + window.log.info( + 'no new members in this group update compared to what we have already. Skipping update' + ); await removeFromCache(envelope); return; } @@ -522,6 +541,7 @@ async function handleClosedGroupMembersAdded( await ClosedGroup.addUpdateMessage(convo, groupDiff, 'incoming'); convo.set({ members }); + convo.updateLastMessage(); await convo.commit(); await removeFromCache(envelope); } @@ -529,8 +549,77 @@ async function handleClosedGroupMembersAdded( async function handleClosedGroupMembersRemoved( envelope: EnvelopePlus, groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage, - convo: ConversationModel) { + convo: ConversationModel +) { + // Check that the admin wasn't removed + const currentMembers = convo.get('members'); + // removedMembers are all members in the diff + const removedMembers = groupUpdate.members.map(toHex); + // effectivelyRemovedMembers are the members which where effectively on this group before the update + // and is used for the group update message only + const effectivelyRemovedMembers = removedMembers.filter(m => + currentMembers.includes(m) + ); + const groupPubKey = envelope.source; + window.log.info( + `Got a group update for group ${envelope.source}, type: MEMBERS_REMOVED` + ); + + const membersAfterUpdate = _.difference(currentMembers, removedMembers); + const groupAdmins = convo.get('groupAdmins'); + if (!groupAdmins?.length) { + throw new Error('No admins found for closed group member removed update.'); + } + const firstAdmin = groupAdmins[0]; + if (removedMembers.includes(firstAdmin)) { + window.log.warn( + 'Ignoring invalid closed group update: trying to remove the admin.' + ); + await removeFromCache(envelope); + return; + } + + // If the current user was removed: + // • Stop polling for the group + // • Remove the key pairs associated with the group + const ourPubKey = await UserUtils.getOurNumber(); + const wasCurrentUserRemoved = !membersAfterUpdate.includes(ourPubKey.key); + if (wasCurrentUserRemoved) { + await window.Signal.Data.removeAllClosedGroupEncryptionKeyPairs( + groupPubKey + ); + // Disable typing: + convo.set('isKickedFromGroup', true); + window.SwarmPolling.removePubkey(groupPubKey); + } + // Generate and distribute a new encryption key pair if needed + const isCurrentUserAdmin = firstAdmin === ourPubKey.key; + if (isCurrentUserAdmin) { + try { + await ClosedGroup.generateAndSendNewEncryptionKeyPair( + groupPubKey, + membersAfterUpdate + ); + } catch (e) { + window.log.warn('Could not distribute new encryption keypair.'); + } + } + + // Only add update message if we have something to show + if (membersAfterUpdate.length !== currentMembers.length) { + const groupDiff: ClosedGroup.GroupDiff = { + leavingMembers: effectivelyRemovedMembers, + }; + await ClosedGroup.addUpdateMessage(convo, groupDiff, 'incoming'); + convo.updateLastMessage(); + } + + // Update the group + convo.set({ members: membersAfterUpdate }); + + await convo.commit(); + await removeFromCache(envelope); } async function handleClosedGroupMemberLeft( @@ -545,7 +634,7 @@ async function handleClosedGroupMemberLeft( // otherwise, we remove the sender from the list of current members in this group const oldMembers = convo.get('members') || []; const leftMemberWasPresent = oldMembers.includes(sender); - const members = didAdminLeave ? [] : (oldMembers).filter(s => s !== sender); + const members = didAdminLeave ? [] : oldMembers.filter(s => s !== sender); // Guard against self-sends const ourPubkey = await UserUtils.getCurrentDevicePubKey(); if (!ourPubkey) { @@ -558,9 +647,13 @@ async function handleClosedGroupMemberLeft( } // Generate and distribute a new encryption key pair if needed - const isCurrentUserAdmin = convo.get('groupAdmins')?.includes(ourPubkey) || false; + const isCurrentUserAdmin = + convo.get('groupAdmins')?.includes(ourPubkey) || false; if (isCurrentUserAdmin && !!members.length) { - await ClosedGroup.generateAndSendNewEncryptionKeyPair(groupPublicKey, members); + await ClosedGroup.generateAndSendNewEncryptionKeyPair( + groupPublicKey, + members + ); } if (didAdminLeave) { @@ -571,16 +664,14 @@ async function handleClosedGroupMemberLeft( convo.set('isKickedFromGroup', true); window.SwarmPolling.removePubkey(groupPublicKey); } - // Update the group // Only add update message if we have something to show - if ( - leftMemberWasPresent - ) { + if (leftMemberWasPresent) { const groupDiff: ClosedGroup.GroupDiff = { leavingMembers: didAdminLeave ? oldMembers : [sender], }; await ClosedGroup.addUpdateMessage(convo, groupDiff, 'incoming'); + convo.updateLastMessage(); } convo.set('members', members); @@ -590,7 +681,6 @@ async function handleClosedGroupMemberLeft( await removeFromCache(envelope); } - export async function createClosedGroup( groupName: string, members: Array @@ -644,7 +734,9 @@ export async function createClosedGroup( // the sending pipeline needs to know from GroupUtils when a message is for a medium group await ClosedGroup.updateOrCreateClosedGroup(groupDetails); convo.set('lastJoinedTimestamp', Date.now()); + convo.set('active_at', Date.now()); await convo.commit(); + convo.updateLastMessage(); // Send a closed group update message to all members individually const promises = listOfMembers.map(async m => { diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 5d3be8a5c..bf2f52096 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -75,6 +75,9 @@ async function decryptForClosedGroup( encryptionKeyPair, true ); + if (decryptedContent?.byteLength) { + break; + } keyIndex++; } catch (e) { window.log.info( @@ -83,13 +86,21 @@ async function decryptForClosedGroup( } } while (encryptionKeyPairs.length > 0); - if (!decryptedContent) { + if (!decryptedContent?.byteLength) { await removeFromCache(envelope); throw new Error( `Could not decrypt message for closed group with any of the ${encryptionKeyPairsCount} keypairs.` ); } - window.log.info('ClosedGroup Message decrypted successfully.'); + if (keyIndex !== 0) { + window.log.warn( + 'Decrypted a closed group message with not the latest encryptionkeypair we have' + ); + } + window.log.info( + 'ClosedGroup Message decrypted successfully with keyIndex:', + keyIndex + ); const ourDevicePubKey = await UserUtils.getCurrentDevicePubKey(); if ( diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index ce85c8e96..cad896093 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -251,7 +251,10 @@ export async function handleDataMessage( window.log.info('data message from', getEnvelopeId(envelope)); if (dataMessage.closedGroupControlMessage) { - await handleClosedGroupControlMessage(envelope, dataMessage.closedGroupControlMessage as SignalService.DataMessage.ClosedGroupControlMessage); + await handleClosedGroupControlMessage( + envelope, + dataMessage.closedGroupControlMessage as SignalService.DataMessage.ClosedGroupControlMessage + ); return; } diff --git a/ts/session/crypto/MessageEncrypter.ts b/ts/session/crypto/MessageEncrypter.ts index 6cc04e6de..28e32ff5b 100644 --- a/ts/session/crypto/MessageEncrypter.ts +++ b/ts/session/crypto/MessageEncrypter.ts @@ -114,7 +114,7 @@ export async function encryptUsingSessionProtocol( window?.log?.info( 'encryptUsingSessionProtocol for ', - recipientHexEncodedX25519PublicKey + recipientHexEncodedX25519PublicKey.key ); const recipientX25519PublicKey = recipientHexEncodedX25519PublicKey.withoutPrefixToArray(); diff --git a/ts/session/group/index.ts b/ts/session/group/index.ts index 9eae0e79d..12262604b 100644 --- a/ts/session/group/index.ts +++ b/ts/session/group/index.ts @@ -340,13 +340,13 @@ export async function leaveClosedGroup(groupId: string) { received_at: now, }); window.getMessageController().register(dbMessage.id, dbMessage); - + const existingExpireTimer = convo.get('expireTimer') || 0; // Send the update to the group const ourLeavingMessage = new ClosedGroupMemberLeftMessage({ timestamp: Date.now(), groupId, identifier: dbMessage.id, - expireTimer: 0, + expireTimer: existingExpireTimer, }); window.log.info( diff --git a/ts/test/session/unit/receiving/ClosedGroupUpdates_test.ts b/ts/test/session/unit/receiving/ClosedGroupUpdates_test.ts index 53e950ffa..2791bfef5 100644 --- a/ts/test/session/unit/receiving/ClosedGroupUpdates_test.ts +++ b/ts/test/session/unit/receiving/ClosedGroupUpdates_test.ts @@ -5,11 +5,13 @@ import { describe } from 'mocha'; import { GroupUtils, PromiseUtils, UserUtils } from '../../../../session/utils'; import { TestUtils } from '../../../../test/test-utils'; -import { generateEnvelopePlusClosedGroup, generateGroupUpdateNameChange } from '../../../test-utils/utils/envelope'; +import { + generateEnvelopePlusClosedGroup, + generateGroupUpdateNameChange, +} from '../../../test-utils/utils/envelope'; import { handleClosedGroupControlMessage } from '../../../../receiver/closedGroups'; import { ConversationController } from '../../../../session/conversations'; - // tslint:disable-next-line: no-require-imports no-var-requires no-implicit-dependencies const chaiAsPromised = require('chai-as-promised'); chai.use(chaiAsPromised); @@ -18,41 +20,41 @@ const { expect } = chai; // tslint:disable-next-line: max-func-body-length describe('ClosedGroupUpdates', () => { - // Initialize new stubbed cache - const sandbox = sinon.createSandbox(); - const ourDevice = TestUtils.generateFakePubKey(); - const ourNumber = ourDevice.key; - const groupId = TestUtils.generateFakePubKey().key; - const members = TestUtils.generateFakePubKeys(10); - const sender = members[3].key; - const getConvo = sandbox.stub(ConversationController.getInstance(), 'get'); - - beforeEach(async () => { - // Utils Stubs - sandbox.stub(UserUtils, 'getCurrentDevicePubKey').resolves(ourNumber); - }); - - afterEach(() => { - TestUtils.restoreStubs(); - sandbox.restore(); + // Initialize new stubbed cache + const sandbox = sinon.createSandbox(); + const ourDevice = TestUtils.generateFakePubKey(); + const ourNumber = ourDevice.key; + const groupId = TestUtils.generateFakePubKey().key; + const members = TestUtils.generateFakePubKeys(10); + const sender = members[3].key; + const getConvo = sandbox.stub(ConversationController.getInstance(), 'get'); + + beforeEach(async () => { + // Utils Stubs + sandbox.stub(UserUtils, 'getCurrentDevicePubKey').resolves(ourNumber); + }); + + afterEach(() => { + TestUtils.restoreStubs(); + sandbox.restore(); + }); + + describe('handleClosedGroupControlMessage', () => { + describe('performIfValid', () => { + it('does not perform if convo does not exist', async () => { + const envelope = generateEnvelopePlusClosedGroup(groupId, sender); + const groupUpdate = generateGroupUpdateNameChange(groupId); + getConvo.returns(undefined as any); + await handleClosedGroupControlMessage(envelope, groupUpdate); + }); }); - describe('handleClosedGroupControlMessage', () => { - describe('performIfValid', () => { - it('does not perform if convo does not exist', async () => { - const envelope = generateEnvelopePlusClosedGroup(groupId, sender); - const groupUpdate = generateGroupUpdateNameChange(groupId); - getConvo.returns(undefined as any); - await handleClosedGroupControlMessage(envelope, groupUpdate); - }); - }); - - // describe('handleClosedGroupNameChanged', () => { - // it('does not trigger an update of the group if the name is the same', async () => { - // const envelope = generateEnvelopePlusClosedGroup(groupId, sender); - // const groupUpdate = generateGroupUpdateNameChange(groupId); - // await handleClosedGroupControlMessage(envelope, groupUpdate); - // }); - // }); - }); + // describe('handleClosedGroupNameChanged', () => { + // it('does not trigger an update of the group if the name is the same', async () => { + // const envelope = generateEnvelopePlusClosedGroup(groupId, sender); + // const groupUpdate = generateGroupUpdateNameChange(groupId); + // await handleClosedGroupControlMessage(envelope, groupUpdate); + // }); + // }); + }); }); diff --git a/ts/test/test-utils/utils/envelope.ts b/ts/test/test-utils/utils/envelope.ts index 00bc1e961..32690304f 100644 --- a/ts/test/test-utils/utils/envelope.ts +++ b/ts/test/test-utils/utils/envelope.ts @@ -4,38 +4,36 @@ import { SignalService } from '../../../protobuf'; import uuid from 'uuid'; import { fromHexToArray } from '../../../session/utils/String'; - export function generateEnvelopePlusClosedGroup( - groupId: string, - sender: string + groupId: string, + sender: string ): EnvelopePlus { - const envelope: EnvelopePlus = { - senderIdentity: sender, - receivedAt: Date.now(), - timestamp: Date.now() - 2000, - id: uuid(), - type: SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT, - source: groupId, - content: new Uint8Array(), - toJSON: () => ['fake'], - }; + const envelope: EnvelopePlus = { + senderIdentity: sender, + receivedAt: Date.now(), + timestamp: Date.now() - 2000, + id: uuid(), + type: SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT, + source: groupId, + content: new Uint8Array(), + toJSON: () => ['fake'], + }; - return envelope; + return envelope; } - export function generateGroupUpdateNameChange( - groupId: string + groupId: string ): SignalService.DataMessage.ClosedGroupControlMessage { - const update: SignalService.DataMessage.ClosedGroupControlMessage = { - type: SignalService.DataMessage.ClosedGroupControlMessage.Type.NAME_CHANGE, - toJSON: () => ['fake'], - publicKey: fromHexToArray(groupId), - name: 'fakeNewName', - members: [], - admins: [], - wrappers: [], - }; + const update: SignalService.DataMessage.ClosedGroupControlMessage = { + type: SignalService.DataMessage.ClosedGroupControlMessage.Type.NAME_CHANGE, + toJSON: () => ['fake'], + publicKey: fromHexToArray(groupId), + name: 'fakeNewName', + members: [], + admins: [], + wrappers: [], + }; - return update; + return update; }