From 37c9c6b5c38980d9e1b346fd9b384e9458effc1e Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 9 Jun 2021 11:49:19 +1000 Subject: [PATCH 1/2] new closed group send and handle expire timer already set --- protos/SignalService.proto | 3 +- ts/receiver/closedGroups.ts | 51 +++++++++++-------- ts/session/group/index.ts | 17 +------ .../group/ClosedGroupAddedMembersMessage.ts | 1 - .../group/ClosedGroupEncryptionPairMessage.ts | 1 - .../group/ClosedGroupMessage.ts | 4 -- .../group/ClosedGroupNameChangeMessage.ts | 1 - .../group/ClosedGroupNewMessage.ts | 7 +-- .../group/ClosedGroupRemovedMembersMessage.ts | 2 - .../ClosedGroupVisibleMessage.ts | 21 ++++---- ts/test/session/unit/utils/Messages_test.ts | 5 -- ts/test/test-utils/utils/envelope.ts | 16 ------ 12 files changed, 50 insertions(+), 79 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 9cd970f19..72459280b 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -169,7 +169,7 @@ message DataMessage { message ClosedGroupControlMessage { enum Type { - NEW = 1; // publicKey, name, encryptionKeyPair, members, admins + NEW = 1; // publicKey, name, encryptionKeyPair, members, admins, expireTimer UPDATE = 2; // name, members ENCRYPTION_KEY_PAIR = 3; // publicKey, wrappers NAME_CHANGE = 4; // name @@ -196,6 +196,7 @@ message DataMessage { repeated bytes members = 5; repeated bytes admins = 6; repeated KeyPairWrapper wrappers = 7; + optional uint32 expireTimer = 8; } message GroupInvitation { diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 3dc5045d8..9c942a979 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -181,6 +181,9 @@ export async function handleNewClosedGroup( const groupId = toHex(publicKey); const members = membersAsData.map(toHex); const admins = adminsAsData.map(toHex); + const envelopeTimestamp = _.toNumber(envelope.timestamp); + // a type new is sent and received on one to one so do not use envelope.senderIdentity here + const sender = envelope.source; if (!members.includes(ourNumber.key)) { window?.log?.info( @@ -192,19 +195,11 @@ export async function handleNewClosedGroup( // FIXME maybe we should handle an expiretimer here too? And on ClosedGroup updates? const maybeConvo = ConversationController.getInstance().get(groupId); + const expireTimer = groupUpdate.expireTimer; if (maybeConvo) { - if (maybeConvo.get('isKickedFromGroup') || maybeConvo.get('left')) { - // TODO: indicate that we've been re-invited - // to the group if that is the case - - // Enable typing: - maybeConvo.set('isKickedFromGroup', false); - maybeConvo.set('left', false); - maybeConvo.set('lastJoinedTimestamp', _.toNumber(envelope.timestamp)); - // we just got readded. Consider the zombie list to have been cleared - maybeConvo.set('zombies', []); - } else { + // if we did not left this group, just add the keypair we got if not already there + if (!maybeConvo.get('isKickedFromGroup') && !maybeConvo.get('left')) { const ecKeyPairAlreadyExistingConvo = new ECKeyPair( // tslint:disable: no-non-null-assertion encryptionKeyPair!.publicKey, @@ -215,6 +210,8 @@ export async function handleNewClosedGroup( ecKeyPairAlreadyExistingConvo.toHexKeyPair() ); + await maybeConvo.updateExpirationTimer(expireTimer, sender, Date.now()); + if (isKeyPairAlreadyHere) { await getAllEncryptionKeyPairsForGroup(groupId); window.log.info('Dropping already saved keypair for group', groupId); @@ -231,6 +228,13 @@ export async function handleNewClosedGroup( ); return; } + // convo exists and we left or got kicked, enable typing and continue processing + // Enable typing: + maybeConvo.set('isKickedFromGroup', false); + maybeConvo.set('left', false); + maybeConvo.set('lastJoinedTimestamp', _.toNumber(envelope.timestamp)); + // we just got readded. Consider the zombie list to have been cleared + maybeConvo.set('zombies', []); } const convo = @@ -246,7 +250,7 @@ export async function handleNewClosedGroup( convo, { newName: name, joiningMembers: members }, 'incoming', - _.toNumber(envelope.timestamp) + envelopeTimestamp ); // We only set group admins on group creation @@ -255,7 +259,7 @@ export async function handleNewClosedGroup( name: name, members: members, admins, - activeAt: Date.now(), + activeAt: envelopeTimestamp, weWereJustAdded: true, }; @@ -267,7 +271,8 @@ export async function handleNewClosedGroup( // But we need to override this value with the sent timestamp of the message creating this group for us. // Having that timestamp set will allow us to pickup incoming group update which were sent between // envelope.timestamp and Date.now(). And we need to listen to those (some might even remove us) - convo.set('lastJoinedTimestamp', _.toNumber(envelope.timestamp)); + convo.set('lastJoinedTimestamp', envelopeTimestamp); + await convo.updateExpirationTimer(expireTimer, sender, envelopeTimestamp); convo.updateLastMessage(); await convo.commit(); @@ -834,7 +839,6 @@ async function sendLatestKeyPairToUsers( groupId: groupPubKey, timestamp: Date.now(), encryptedKeyPairs: wrappers, - expireTimer, }); // the encryption keypair is sent using established channels @@ -887,13 +891,15 @@ export async function createClosedGroup(groupName: string, members: Array, encryptionKeyPair: ECKeyPair, dbMessage: MessageModel, + existingExpireTimer: number, isRetry: boolean = false ): Promise { const promises = createInvitePromises( @@ -961,7 +969,8 @@ async function sendToGroupMembers( groupName, admins, encryptionKeyPair, - dbMessage + dbMessage, + existingExpireTimer ); window?.log?.info(`Creating a new group and an encryptionKeyPair for group ${groupPublicKey}`); // evaluating if all invites sent, if failed give the option to retry failed invites via modal dialog @@ -1010,6 +1019,7 @@ async function sendToGroupMembers( admins, encryptionKeyPair, dbMessage, + existingExpireTimer, isRetrySend ); } @@ -1025,7 +1035,8 @@ function createInvitePromises( groupName: string, admins: Array, encryptionKeyPair: ECKeyPair, - dbMessage: MessageModel + dbMessage: MessageModel, + existingExpireTimer: number ) { return listOfMembers.map(async m => { const messageParams: ClosedGroupNewMessageParams = { @@ -1036,7 +1047,7 @@ function createInvitePromises( keypair: encryptionKeyPair, timestamp: Date.now(), identifier: dbMessage.id, - expireTimer: 0, + expireTimer: existingExpireTimer, }; const message = new ClosedGroupNewMessage(messageParams); return getMessageQueue().sendToPubKeyNonDurably(PubKey.cast(m), message); diff --git a/ts/session/group/index.ts b/ts/session/group/index.ts index 6f89423d0..7367016ad 100644 --- a/ts/session/group/index.ts +++ b/ts/session/group/index.ts @@ -339,13 +339,11 @@ export async function leaveClosedGroup(groupId: string) { expireTimer: 0, }); MessageController.getInstance().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: existingExpireTimer, }); window?.log?.info(`We are leaving the group ${groupId}. Sending our leaving message.`); @@ -372,7 +370,6 @@ async function sendNewName(convo: ConversationModel, name: string, messageId: st timestamp: Date.now(), groupId, identifier: messageId, - expireTimer: 0, name, }); await getMessageQueue().sendToGroup(nameChangeMessage); @@ -399,15 +396,13 @@ async function sendAddedMembers( } const encryptionKeyPair = ECKeyPair.fromHexKeyPair(hexEncryptionKeyPair); - const expireTimer = convo.get('expireTimer') || 0; - + const existingExpireTimer = convo.get('expireTimer') || 0; // Send the Added Members message to the group (only members already in the group will get it) const closedGroupControlMessage = new ClosedGroupAddedMembersMessage({ timestamp: Date.now(), groupId, addedMembers, identifier: messageId, - expireTimer, }); await getMessageQueue().sendToGroup(closedGroupControlMessage); @@ -420,7 +415,7 @@ async function sendAddedMembers( members, keypair: encryptionKeyPair, identifier: messageId || uuid(), - expireTimer, + expireTimer: existingExpireTimer, }); const promises = addedMembers.map(async m => { @@ -453,15 +448,12 @@ export async function sendRemovedMembers( if (removedMembers.includes(admins[0]) && stillMembers.length !== 0) { throw new Error("Can't remove admin from closed group without removing everyone."); } - const expireTimer = convo.get('expireTimer') || 0; - // Send the update to the group and generate + distribute a new encryption key pair if needed const mainClosedGroupControlMessage = new ClosedGroupRemovedMembersMessage({ timestamp: Date.now(), groupId, removedMembers, identifier: messageId, - expireTimer, }); // Send the group update, and only once sent, generate and distribute a new encryption key pair if needed await getMessageQueue().sendToGroup(mainClosedGroupControlMessage, async () => { @@ -514,13 +506,10 @@ async function generateAndSendNewEncryptionKeyPair( // 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, }); distributingClosedGroupEncryptionKeyPairs.set(toHex(groupId), newKeyPair); @@ -587,10 +576,8 @@ export async function requestEncryptionKeyPair(groupPublicKey: string | PubKey) window?.log?.info('requestEncryptionKeyPair: We are not a member of this group.'); return; } - const expireTimer = groupConvo.get('expireTimer') || 0; const ecRequestMessage = new ClosedGroupEncryptionPairRequestMessage({ - expireTimer, groupId: groupPublicKey, timestamp: Date.now(), }); diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupAddedMembersMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupAddedMembersMessage.ts index 003e9a0fb..422b8058e 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupAddedMembersMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupAddedMembersMessage.ts @@ -16,7 +16,6 @@ export class ClosedGroupAddedMembersMessage extends ClosedGroupMessage { timestamp: params.timestamp, identifier: params.identifier, groupId: params.groupId, - expireTimer: params.expireTimer, }); this.addedMembers = params.addedMembers; if (!this.addedMembers?.length) { diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairMessage.ts index 008218cf4..807033ed6 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairMessage.ts @@ -16,7 +16,6 @@ export class ClosedGroupEncryptionPairMessage extends ClosedGroupMessage { timestamp: params.timestamp, identifier: params.identifier, groupId: params.groupId, - expireTimer: params.expireTimer, }); this.encryptedKeyPairs = params.encryptedKeyPairs; if (this.encryptedKeyPairs.length === 0) { diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts index 955d42286..4edbdbe8d 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts @@ -5,12 +5,10 @@ import { MessageParams } from '../../Message'; export interface ClosedGroupMessageParams extends MessageParams { groupId: string | PubKey; - expireTimer: number; } export abstract class ClosedGroupMessage extends DataMessage { public readonly groupId: PubKey; - public readonly expireTimer: number; constructor(params: ClosedGroupMessageParams) { super({ @@ -19,7 +17,6 @@ export abstract class ClosedGroupMessage extends DataMessage { }); this.groupId = PubKey.cast(params.groupId); - this.expireTimer = params.expireTimer; if (!this.groupId || this.groupId.key.length === 0) { throw new Error('groupId must be set'); } @@ -33,7 +30,6 @@ export abstract class ClosedGroupMessage extends DataMessage { const dataMessage = new SignalService.DataMessage(); dataMessage.closedGroupControlMessage = new SignalService.DataMessage.ClosedGroupControlMessage(); - dataMessage.expireTimer = this.expireTimer; return dataMessage; } diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNameChangeMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNameChangeMessage.ts index 278e8392f..ee4e9a783 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNameChangeMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNameChangeMessage.ts @@ -14,7 +14,6 @@ export class ClosedGroupNameChangeMessage extends ClosedGroupMessage { timestamp: params.timestamp, identifier: params.identifier, groupId: params.groupId, - expireTimer: params.expireTimer, }); this.name = params.name; if (this.name.length === 0) { diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNewMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNewMessage.ts index f0a28adda..9c73ba086 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNewMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupNewMessage.ts @@ -8,6 +8,7 @@ export interface ClosedGroupNewMessageParams extends ClosedGroupMessageParams { members: Array; admins: Array; keypair: ECKeyPair; + expireTimer: number; } export class ClosedGroupNewMessage extends ClosedGroupMessage { @@ -15,18 +16,19 @@ export class ClosedGroupNewMessage extends ClosedGroupMessage { private readonly members: Array; private readonly admins: Array; private readonly keypair: ECKeyPair; + private readonly expireTimer: number; constructor(params: ClosedGroupNewMessageParams) { super({ timestamp: params.timestamp, identifier: params.identifier, groupId: params.groupId, - expireTimer: params.expireTimer, }); this.name = params.name; this.members = params.members; this.admins = params.admins; this.keypair = params.keypair; + this.expireTimer = params.expireTimer; if (!params.admins || params.admins.length === 0) { throw new Error('Admins must be set'); @@ -52,8 +54,6 @@ export class ClosedGroupNewMessage extends ClosedGroupMessage { public dataProto(): SignalService.DataMessage { const dataMessage = new SignalService.DataMessage(); - dataMessage.expireTimer = this.expireTimer; - dataMessage.closedGroupControlMessage = new SignalService.DataMessage.ClosedGroupControlMessage(); dataMessage.closedGroupControlMessage.type = @@ -63,6 +63,7 @@ export class ClosedGroupNewMessage extends ClosedGroupMessage { dataMessage.closedGroupControlMessage.admins = this.admins.map(fromHexToArray); dataMessage.closedGroupControlMessage.members = this.members.map(fromHexToArray); + dataMessage.closedGroupControlMessage.expireTimer = this.expireTimer; try { dataMessage.closedGroupControlMessage.encryptionKeyPair = new SignalService.KeyPair(); dataMessage.closedGroupControlMessage.encryptionKeyPair.privateKey = new Uint8Array( diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupRemovedMembersMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupRemovedMembersMessage.ts index b06e2ae56..70d6c8461 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupRemovedMembersMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupRemovedMembersMessage.ts @@ -1,4 +1,3 @@ -import { Constants } from '../../../..'; import { SignalService } from '../../../../../protobuf'; import { fromHexToArray } from '../../../../utils/String'; import { ClosedGroupMessage, ClosedGroupMessageParams } from './ClosedGroupMessage'; @@ -15,7 +14,6 @@ export class ClosedGroupRemovedMembersMessage extends ClosedGroupMessage { timestamp: params.timestamp, identifier: params.identifier, groupId: params.groupId, - expireTimer: params.expireTimer, }); this.removedMembers = params.removedMembers; if (!this.removedMembers?.length) { diff --git a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts index df56e73da..a5f4bc7f1 100644 --- a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts @@ -19,23 +19,24 @@ export class ClosedGroupVisibleMessage extends ClosedGroupMessage { timestamp: params.chatMessage.timestamp, identifier: params.identifier ?? params.chatMessage.identifier, groupId: params.groupId, - expireTimer: params.chatMessage.expireTimer || 0, }); this.chatMessage = params.chatMessage; + if (!params.groupId) { + throw new Error('ClosedGroupVisibleMessage: groupId must be set'); + } } public dataProto(): SignalService.DataMessage { + //expireTimer is set in the dataProto in this call directly const dataProto = this.chatMessage.dataProto(); - if (this.groupId) { - const groupMessage = new SignalService.GroupContext(); - const groupIdWithPrefix = PubKey.addTextSecurePrefixIfNeeded(this.groupId.key); - const encoded = StringUtils.encode(groupIdWithPrefix, 'utf8'); - const id = new Uint8Array(encoded); - groupMessage.id = id; - groupMessage.type = SignalService.GroupContext.Type.DELIVER; + const groupMessage = new SignalService.GroupContext(); + const groupIdWithPrefix = PubKey.addTextSecurePrefixIfNeeded(this.groupId.key); + const encoded = StringUtils.encode(groupIdWithPrefix, 'utf8'); + const id = new Uint8Array(encoded); + groupMessage.id = id; + groupMessage.type = SignalService.GroupContext.Type.DELIVER; - dataProto.group = groupMessage; - } + dataProto.group = groupMessage; return dataProto; } diff --git a/ts/test/session/unit/utils/Messages_test.ts b/ts/test/session/unit/utils/Messages_test.ts index ab4058e77..9d5f3295c 100644 --- a/ts/test/session/unit/utils/Messages_test.ts +++ b/ts/test/session/unit/utils/Messages_test.ts @@ -130,7 +130,6 @@ describe('Message Utils', () => { timestamp: Date.now(), name: 'df', groupId: TestUtils.generateFakePubKey().key, - expireTimer: 0, }); const rawMessage = await MessageUtils.toRawMessage(device, msg); expect(rawMessage.encryption).to.equal(EncryptionType.ClosedGroup); @@ -143,7 +142,6 @@ describe('Message Utils', () => { timestamp: Date.now(), addedMembers: [TestUtils.generateFakePubKey().key], groupId: TestUtils.generateFakePubKey().key, - expireTimer: 0, }); const rawMessage = await MessageUtils.toRawMessage(device, msg); expect(rawMessage.encryption).to.equal(EncryptionType.ClosedGroup); @@ -156,7 +154,6 @@ describe('Message Utils', () => { timestamp: Date.now(), removedMembers: [TestUtils.generateFakePubKey().key], groupId: TestUtils.generateFakePubKey().key, - expireTimer: 0, }); const rawMessage = await MessageUtils.toRawMessage(device, msg); expect(rawMessage.encryption).to.equal(EncryptionType.ClosedGroup); @@ -178,7 +175,6 @@ describe('Message Utils', () => { 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); @@ -200,7 +196,6 @@ describe('Message Utils', () => { timestamp: Date.now(), groupId: TestUtils.generateFakePubKey().key, encryptedKeyPairs: fakeWrappers, - expireTimer: 0, }); const rawMessage = await MessageUtils.toRawMessage(device, msg); expect(rawMessage.encryption).to.equal(EncryptionType.Fallback); diff --git a/ts/test/test-utils/utils/envelope.ts b/ts/test/test-utils/utils/envelope.ts index 3f21cb9c8..393729e47 100644 --- a/ts/test/test-utils/utils/envelope.ts +++ b/ts/test/test-utils/utils/envelope.ts @@ -33,19 +33,3 @@ export function generateEnvelopePlus(sender: string): EnvelopePlus { return envelope; } - -export function generateGroupUpdateNameChange( - 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: [], - }; - - return update; -} From 60a6d5c55e2828e6d83988d285021d4d8a48071a Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 9 Jun 2021 11:53:28 +1000 Subject: [PATCH 2/2] disable draft release on push to clearnet --- .github/workflows/release.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b78f0106c..e7fdfd45b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,7 +5,6 @@ on: push: branches: - master - - clearnet jobs: build: