Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: try establishing mls group for mixed conversation #5578

Merged
merged 6 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@
*/

import {ClientClassification, ClientType} from '@wireapp/api-client/lib/client';
import {Conversation, ConversationProtocol, MLSConversation} from '@wireapp/api-client/lib/conversation';
import {
Conversation,
ConversationProtocol,
MLSConversation,
PostMlsMessageResponse,
} from '@wireapp/api-client/lib/conversation';
import {CONVERSATION_EVENT, ConversationMLSMessageAddEvent} from '@wireapp/api-client/lib/event';
import {BackendError, BackendErrorLabel} from '@wireapp/api-client/lib/http';
import {QualifiedId} from '@wireapp/api-client/lib/user';
Expand Down Expand Up @@ -108,6 +113,10 @@ describe('ConversationService', () => {
handleMLSMessageAddEvent: jest.fn(),
conversationExists: jest.fn(),
isConversationEstablished: jest.fn(),
tryEstablishingMLSGroup: jest.fn(),
getKeyPackagesPayload: jest.fn(),
addUsersToExistingConversation: jest.fn(),
resetKeyMaterialRenewal: jest.fn(),
} as unknown as MLSService;

const conversationService = new ConversationService(client, mockedProteusService, mockedMLSService);
Expand Down Expand Up @@ -499,6 +508,100 @@ describe('ConversationService', () => {
expect(fetchedMembers).toEqual(members);
});
});

describe('addUsersToMLSConversation', () => {
it('should claim key packages for all the users and add them to the group', async () => {
const [conversationService, {apiClient, mlsService}] = buildConversationService();

const mockGroupId = 'groupId';
const mockConversationId = {id: PayloadHelper.getUUID(), domain: 'local.wire.com'};

const otherUsersToAdd = Array(3)
.fill(0)
.map(() => ({id: PayloadHelper.getUUID(), domain: 'local.wire.com'}));

const selfUserToAdd = {id: 'self-user-id', domain: 'local.wire.com', skipOwnClientId: apiClient.clientId};

const qualifiedUsers = [...otherUsersToAdd, selfUserToAdd];

jest
.spyOn(mlsService, 'getKeyPackagesPayload')
.mockResolvedValueOnce({coreCryptoKeyPackagesPayload: [], failedToFetchKeyPackages: []});

jest.spyOn(apiClient.api.conversation, 'getConversation').mockResolvedValueOnce({
qualified_id: mockConversationId,
protocol: ConversationProtocol.MLS,
epoch: 1,
group_id: mockGroupId,
} as unknown as Conversation);

const mlsMessage: PostMlsMessageResponse = {events: [], time: ''};
jest.spyOn(mlsService, 'addUsersToExistingConversation').mockResolvedValueOnce(mlsMessage);

await conversationService.addUsersToMLSConversation({
qualifiedUsers,
groupId: mockGroupId,
conversationId: mockConversationId,
});

expect(mlsService.getKeyPackagesPayload).toHaveBeenCalledWith(qualifiedUsers);
expect(mlsService.resetKeyMaterialRenewal).toHaveBeenCalledWith(mockGroupId);
});
});

describe('tryEstablishingMLSGroup', () => {
it('should add all the users to a MLS group after group was established by the self client', async () => {
const [conversationService, {apiClient, mlsService}] = buildConversationService();
const selfUserId = {id: 'self-user-id', domain: 'local.wire.com'};

const mockConversationId = {id: PayloadHelper.getUUID(), domain: 'local.wire.com'};
const mockGroupId = 'groupId';
const otherUsersToAdd = Array(3)
.fill(0)
.map(() => ({id: PayloadHelper.getUUID(), domain: 'local.wire.com'}));

jest.spyOn(mlsService, 'tryEstablishingMLSGroup').mockResolvedValueOnce(true);
jest
.spyOn(conversationService, 'addUsersToMLSConversation')
.mockResolvedValueOnce({conversation: {members: {others: []}}} as any);

await conversationService.tryEstablishingMLSGroup({
conversationId: mockConversationId,
groupId: mockGroupId,
qualifiedUsers: otherUsersToAdd,
selfUserId,
});

expect(conversationService.addUsersToMLSConversation).toHaveBeenCalledWith({
conversationId: mockConversationId,
groupId: mockGroupId,
qualifiedUsers: [...otherUsersToAdd, {...selfUserId, skipOwnClientId: apiClient.clientId}],
});
});

it('should not add any users if MLS group was not established by the self client', async () => {
const [conversationService, {mlsService}] = buildConversationService();
const selfUserId = {id: 'self-user-id', domain: 'local.wire.com'};

const mockConversationId = {id: PayloadHelper.getUUID(), domain: 'local.wire.com'};
const mockGroupId = 'groupId';
const otherUsersToAdd = Array(3)
.fill(0)
.map(() => ({id: PayloadHelper.getUUID(), domain: 'local.wire.com'}));

jest.spyOn(mlsService, 'tryEstablishingMLSGroup').mockResolvedValueOnce(false);
jest.spyOn(conversationService, 'addUsersToMLSConversation');

await conversationService.tryEstablishingMLSGroup({
conversationId: mockConversationId,
groupId: mockGroupId,
qualifiedUsers: otherUsersToAdd,
selfUserId,
});

expect(conversationService.addUsersToMLSConversation).not.toHaveBeenCalled();
});
});
});

function generateImage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {GenericMessage} from '@wireapp/protocol-messaging';
import {
AddUsersFailureReasons,
AddUsersParams,
KeyPackageClaimUser,
MLSCreateConversationResponse,
SendMlsMessageParams,
SendResult,
Expand Down Expand Up @@ -566,6 +567,53 @@ export class ConversationService extends TypedEventEmitter<Events> {
}
};

/**
* Will try to register mls group by sending an empty commit to establish it.
* After group was successfully established, it will try to add other users to the group.
*
* @param groupId - id of the MLS group
* @param conversationId - id of the conversation
* @param selfUserId - id of the self user
* @param qualifiedUsers - list of qualified users to add to the group (should not include the self user)
*/
public async tryEstablishingMLSGroup({
groupId,
conversationId,
selfUserId,
qualifiedUsers,
}: {
groupId: string;
conversationId: QualifiedId;
selfUserId: QualifiedId;
qualifiedUsers: QualifiedId[];
}): Promise<void> {
const wasGroupEstablishedBySelfClient = await this.mlsService.tryEstablishingMLSGroup(groupId);

if (!wasGroupEstablishedBySelfClient) {
this.logger.info('Group was not established by self client, skipping adding users to the group.');
return;
}

this.logger.info('Group was established by self client, adding other users to the group...');
const usersToAdd: KeyPackageClaimUser[] = [
...qualifiedUsers,
{...selfUserId, skipOwnClientId: this.apiClient.validatedClientId},
];

const {conversation} = await this.addUsersToMLSConversation({
conversationId,
groupId,
qualifiedUsers: usersToAdd,
});

const addedUsers = conversation.members.others;
if (addedUsers.length > 0) {
this.logger.info(`Successfully added ${addedUsers} users to the group.`);
} else {
this.logger.info('No other users were added to the group.');
}
}

private async handleMLSMessageAddEvent(event: ConversationMLSMessageAddEvent): Promise<HandledEventPayload | null> {
try {
return await this.mlsService.handleMLSMessageAddEvent(event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,17 @@ export const CoreCryptoMLSError = {
'You tried to join with an external commit but did not merge it yet. We will reapply this message for you when you merge your external commit',
FUTURE_EPOCH: 'Incoming message is for a future epoch. We will buffer it until the commit for that epoch arrives',
},
CONVERSATION_ALREADY_EXISTS: 'Conversation already exists',
} as const;

export const isCoreCryptoMLSWrongEpochError = (error: unknown): boolean => {
return error instanceof Error && error.message === CoreCryptoMLSError.DECRYPTION.WRONG_EPOCH;
};

export const isCoreCryptoMLSConversationAlreadyExistsError = (error: unknown): boolean => {
return error instanceof Error && error.message === CoreCryptoMLSError.CONVERSATION_ALREADY_EXISTS;
};

const mlsDecryptionErrorsToIgnore: string[] = [
CoreCryptoMLSError.DECRYPTION.ALREADY_DECRYPTED,
CoreCryptoMLSError.DECRYPTION.EXTERNAL_COMMIT_NOT_MERGED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import {
ConversationMLSMessageAddEvent,
ConversationMLSWelcomeEvent,
} from '@wireapp/api-client/lib/event';
import {BackendError, BackendErrorLabel, StatusCode} from '@wireapp/api-client/lib/http';

import {randomUUID} from 'crypto';

import {APIClient} from '@wireapp/api-client';
import {CoreCrypto, DecryptedMessage} from '@wireapp/core-crypto';

import {CoreCryptoMLSError} from './CoreCryptoMLSError';
import {MLSService} from './MLSService';

import {openDB} from '../../../storage/CoreDB';
Expand Down Expand Up @@ -416,4 +418,70 @@ describe('MLSService', () => {
expect(apiClient.api.client.uploadMLSKeyPackages).not.toHaveBeenCalled();
});
});

describe('tryEstablishingMLSGroup', () => {
it('returns false if group did already exist locally', async () => {
const [mlsService] = await createMLSService();

const mockGroupId = 'mock-group-id';

jest.spyOn(mlsService, 'conversationExists').mockResolvedValueOnce(true);
jest.spyOn(mlsService, 'registerConversation').mockImplementation(jest.fn());

const wasConversationEstablished = await mlsService.tryEstablishingMLSGroup(mockGroupId);

expect(mlsService.registerConversation).not.toHaveBeenCalled();
expect(wasConversationEstablished).toBe(false);
});

it('returns false if corecrypto has thrown an error when trying to register group locally', async () => {
const [mlsService] = await createMLSService();

const mockGroupId = 'mock-group-id';

jest.spyOn(mlsService, 'conversationExists').mockResolvedValueOnce(false);
jest
.spyOn(mlsService, 'registerConversation')
.mockRejectedValueOnce(new Error(CoreCryptoMLSError.CONVERSATION_ALREADY_EXISTS));

const wasConversationEstablished = await mlsService.tryEstablishingMLSGroup(mockGroupId);

expect(mlsService.registerConversation).toHaveBeenCalledWith(mockGroupId, []);
expect(wasConversationEstablished).toBe(false);
});

it('returns false and wipes group locally if any backend error was thrown', async () => {
const [mlsService] = await createMLSService();

const mockGroupId = 'mock-group-id2';

jest.spyOn(mlsService, 'conversationExists').mockResolvedValueOnce(false);
jest
.spyOn(mlsService, 'registerConversation')
.mockRejectedValueOnce(new BackendError('', BackendErrorLabel.MLS_STALE_MESSAGE, StatusCode.CONFLICT));
jest.spyOn(mlsService, 'wipeConversation').mockImplementation(jest.fn());

const wasConversationEstablished = await mlsService.tryEstablishingMLSGroup(mockGroupId);

expect(mlsService.registerConversation).toHaveBeenCalledWith(mockGroupId, []);
expect(mlsService.wipeConversation).toHaveBeenCalledWith(mockGroupId);
expect(wasConversationEstablished).toBe(false);
});

it('returns true after MLS group was etablished successfully', async () => {
const [mlsService] = await createMLSService();

const mockGroupId = 'mock-group-id2';

jest.spyOn(mlsService, 'conversationExists').mockResolvedValueOnce(false);
jest.spyOn(mlsService, 'registerConversation').mockResolvedValueOnce({events: [], time: ''});
jest.spyOn(mlsService, 'wipeConversation').mockImplementation(jest.fn());

const wasConversationEstablished = await mlsService.tryEstablishingMLSGroup(mockGroupId);

expect(mlsService.registerConversation).toHaveBeenCalledWith(mockGroupId, []);
expect(mlsService.wipeConversation).not.toHaveBeenCalled();
expect(wasConversationEstablished).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
RemoveProposalArgs,
} from '@wireapp/core-crypto';

import {shouldMLSDecryptionErrorBeIgnored} from './CoreCryptoMLSError';
import {isCoreCryptoMLSConversationAlreadyExistsError, shouldMLSDecryptionErrorBeIgnored} from './CoreCryptoMLSError';
import {MLSServiceConfig, UploadCommitOptions} from './MLSService.types';
import {subconversationGroupIdStore} from './stores/subconversationGroupIdStore/subconversationGroupIdStore';

Expand Down Expand Up @@ -494,6 +494,41 @@ export class MLSService extends TypedEventEmitter<Events> {
return response;
}

/**
* Will try to register mls group and send an empty commit to establish it.
*
* @param groupId - id of the MLS group
* @returns true if the client has successfully established the group, false otherwise
*/
public readonly tryEstablishingMLSGroup = async (groupId: string): Promise<boolean> => {
this.logger.info(`Trying to establish a MLS group with id ${groupId}.`);

// Before trying to register a group, check if the group is already established locally.
// We could have received a welcome message in the meantime.
const doesMLSGroupExistLocally = await this.conversationExists(groupId);
if (doesMLSGroupExistLocally) {
this.logger.info(`MLS Group with id ${groupId} already exists, skipping the initialisation.`);
return false;
}

try {
await this.registerConversation(groupId, []);
return true;
} catch (error) {
// If conversation already existed, locally, nothing more to do, we've received a welcome message.
if (isCoreCryptoMLSConversationAlreadyExistsError(error)) {
this.logger.info(`MLS Group with id ${groupId} already exists, skipping the initialisation.`);
return false;
}

this.logger.info(`MLS Group with id ${groupId} was not established succesfully, wiping the group locally...`);
// Otherwise it's a backend error. Somebody else might have created the group in the meantime.
// We should wipe the group locally, wait for the welcome message or join later via external commit.
await this.wipeConversation(groupId);
return false;
}
};

/**
* Will send a removal commit for given clients
* @param groupId groupId of the conversation
Expand Down
Loading