Skip to content

Commit

Permalink
Encryption should not hinder verification (#2734)
Browse files Browse the repository at this point in the history 
Co-authored-by: Faye Duxovni <[email protected]>
  • Loading branch information
@BillCarsonFr @duxovni
BillCarsonFr and duxovni authored Oct 18, 2022
1 parent 0231d40 commit 1c3dd0e
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 109 deletions.
143 changes: 119 additions & 24 deletions spec/unit/crypto/algorithms/megolm.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,9 +493,9 @@ describe("MegolmDecryption", function() {
bobClient1.initCrypto(),
bobClient2.initCrypto(),
]);
const aliceDevice = aliceClient.crypto.olmDevice;
const bobDevice1 = bobClient1.crypto.olmDevice;
const bobDevice2 = bobClient2.crypto.olmDevice;
const aliceDevice = aliceClient.crypto!.olmDevice;
const bobDevice1 = bobClient1.crypto!.olmDevice;
const bobDevice2 = bobClient2.crypto!.olmDevice;

const encryptionCfg = {
"algorithm": "m.megolm.v1.aes-sha2",
Expand Down Expand Up @@ -532,10 +532,10 @@ describe("MegolmDecryption", function() {
},
};

aliceClient.crypto.deviceList.storeDevicesForUser(
aliceClient.crypto!.deviceList.storeDevicesForUser(
"@bob:example.com", BOB_DEVICES,
);
aliceClient.crypto.deviceList.downloadKeys = async function(userIds) {
aliceClient.crypto!.deviceList.downloadKeys = async function(userIds) {
return this.getDevicesFromStore(userIds);
};

Expand All @@ -551,7 +551,7 @@ describe("MegolmDecryption", function() {
body: "secret",
},
});
await aliceClient.crypto.encryptEvent(event, room);
await aliceClient.crypto!.encryptEvent(event, room);

expect(aliceClient.sendToDevice).toHaveBeenCalled();
const [msgtype, contentMap] = mocked(aliceClient.sendToDevice).mock.calls[0];
Expand Down Expand Up @@ -583,6 +583,100 @@ describe("MegolmDecryption", function() {
bobClient2.stopClient();
});

it("does not block unverified devices when sending verification events", async function() {
const aliceClient = (new TestClient(
"@alice:example.com", "alicedevice",
)).client;
const bobClient = (new TestClient(
"@bob:example.com", "bobdevice",
)).client;
await Promise.all([
aliceClient.initCrypto(),
bobClient.initCrypto(),
]);
const bobDevice = bobClient.crypto!.olmDevice;

const encryptionCfg = {
"algorithm": "m.megolm.v1.aes-sha2",
};
const roomId = "!someroom";
const room = new Room(roomId, aliceClient, "@alice:example.com", {});

const bobMember = new RoomMember(roomId, "@bob:example.com");
room.getEncryptionTargetMembers = async function() {
return [bobMember];
};
room.setBlacklistUnverifiedDevices(true);
aliceClient.store.storeRoom(room);
await aliceClient.setRoomEncryption(roomId, encryptionCfg);

const BOB_DEVICES: Record<string, IDevice> = {
bobdevice: {
algorithms: [olmlib.OLM_ALGORITHM, olmlib.MEGOLM_ALGORITHM],
keys: {
"ed25519:bobdevice": bobDevice.deviceEd25519Key,
"curve25519:bobdevice": bobDevice.deviceCurve25519Key,
},
verified: 0,
known: true,
},
};

aliceClient.crypto!.deviceList.storeDevicesForUser(
"@bob:example.com", BOB_DEVICES,
);
aliceClient.crypto!.deviceList.downloadKeys = async function(userIds) {
// @ts-ignore private
return this.getDevicesFromStore(userIds);
};

await bobDevice.generateOneTimeKeys(1);
const oneTimeKeys = await bobDevice.getOneTimeKeys();
const signedOneTimeKeys: Record<string, { key: string, signatures: object }> = {};
for (const keyId in oneTimeKeys.curve25519) {
if (oneTimeKeys.curve25519.hasOwnProperty(keyId)) {
const k = {
key: oneTimeKeys.curve25519[keyId],
signatures: {},
};
signedOneTimeKeys["signed_curve25519:" + keyId] = k;
await bobClient.crypto!.signObject(k);
break;
}
}

aliceClient.claimOneTimeKeys = jest.fn().mockResolvedValue({
one_time_keys: {
'@bob:example.com': {
bobdevice: signedOneTimeKeys,
},
},
failures: {},
});

aliceClient.sendToDevice = jest.fn().mockResolvedValue({});

const event = new MatrixEvent({
type: "m.key.verification.start",
sender: "@alice:example.com",
room_id: roomId,
event_id: "$event",
content: {
from_device: "alicedevice",
method: "m.sas.v1",
transaction_id: "transactionid",
},
});
await aliceClient.crypto!.encryptEvent(event, room);

expect(aliceClient.sendToDevice).toHaveBeenCalled();
const [msgtype] = mocked(aliceClient.sendToDevice).mock.calls[0];
expect(msgtype).toEqual("m.room.encrypted");

aliceClient.stopClient();
bobClient.stopClient();
});

it("notifies devices when unable to create olm session", async function() {
const aliceClient = (new TestClient(
"@alice:example.com", "alicedevice",
Expand All @@ -594,8 +688,8 @@ describe("MegolmDecryption", function() {
aliceClient.initCrypto(),
bobClient.initCrypto(),
]);
const aliceDevice = aliceClient.crypto.olmDevice;
const bobDevice = bobClient.crypto.olmDevice;
const aliceDevice = aliceClient.crypto!.olmDevice;
const bobDevice = bobClient.crypto!.olmDevice;

const encryptionCfg = {
"algorithm": "m.megolm.v1.aes-sha2",
Expand Down Expand Up @@ -632,10 +726,11 @@ describe("MegolmDecryption", function() {
},
};

aliceClient.crypto.deviceList.storeDevicesForUser(
aliceClient.crypto!.deviceList.storeDevicesForUser(
"@bob:example.com", BOB_DEVICES,
);
aliceClient.crypto.deviceList.downloadKeys = async function(userIds) {
aliceClient.crypto!.deviceList.downloadKeys = async function(userIds) {
// @ts-ignore private
return this.getDevicesFromStore(userIds);
};

Expand All @@ -654,7 +749,7 @@ describe("MegolmDecryption", function() {
event_id: "$event",
content: {},
});
await aliceClient.crypto.encryptEvent(event, aliceRoom);
await aliceClient.crypto!.encryptEvent(event, aliceRoom);

expect(aliceClient.sendToDevice).toHaveBeenCalled();
const [msgtype, contentMap] = mocked(aliceClient.sendToDevice).mock.calls[0];
Expand Down Expand Up @@ -685,10 +780,10 @@ describe("MegolmDecryption", function() {
aliceClient.initCrypto(),
bobClient.initCrypto(),
]);
const bobDevice = bobClient.crypto.olmDevice;
const bobDevice = bobClient.crypto!.olmDevice;

const aliceEventEmitter = new TypedEventEmitter<ClientEvent.ToDeviceEvent, any>();
aliceClient.crypto.registerEventHandlers(aliceEventEmitter);
aliceClient.crypto!.registerEventHandlers(aliceEventEmitter);

const roomId = "!someroom";

Expand All @@ -705,7 +800,7 @@ describe("MegolmDecryption", function() {
},
}));

await expect(aliceClient.crypto.decryptEvent(new MatrixEvent({
await expect(aliceClient.crypto!.decryptEvent(new MatrixEvent({
type: "m.room.encrypted",
sender: "@bob:example.com",
event_id: "$event",
Expand All @@ -732,7 +827,7 @@ describe("MegolmDecryption", function() {
},
}));

await expect(aliceClient.crypto.decryptEvent(new MatrixEvent({
await expect(aliceClient.crypto!.decryptEvent(new MatrixEvent({
type: "m.room.encrypted",
sender: "@bob:example.com",
event_id: "$event",
Expand Down Expand Up @@ -762,10 +857,10 @@ describe("MegolmDecryption", function() {
]);

const aliceEventEmitter = new TypedEventEmitter<ClientEvent.ToDeviceEvent, any>();
aliceClient.crypto.registerEventHandlers(aliceEventEmitter);
aliceClient.crypto!.registerEventHandlers(aliceEventEmitter);

aliceClient.crypto.downloadKeys = jest.fn();
const bobDevice = bobClient.crypto.olmDevice;
aliceClient.crypto!.downloadKeys = jest.fn();
const bobDevice = bobClient.crypto!.olmDevice;

const roomId = "!someroom";

Expand All @@ -788,7 +883,7 @@ describe("MegolmDecryption", function() {
setTimeout(resolve, 100);
});

await expect(aliceClient.crypto.decryptEvent(new MatrixEvent({
await expect(aliceClient.crypto!.decryptEvent(new MatrixEvent({
type: "m.room.encrypted",
sender: "@bob:example.com",
event_id: "$event",
Expand Down Expand Up @@ -820,7 +915,7 @@ describe("MegolmDecryption", function() {
setTimeout(resolve, 100);
});

await expect(aliceClient.crypto.decryptEvent(new MatrixEvent({
await expect(aliceClient.crypto!.decryptEvent(new MatrixEvent({
type: "m.room.encrypted",
sender: "@bob:example.com",
event_id: "$event",
Expand Down Expand Up @@ -850,10 +945,10 @@ describe("MegolmDecryption", function() {
bobClient.initCrypto(),
]);
const aliceEventEmitter = new TypedEventEmitter<ClientEvent.ToDeviceEvent, any>();
aliceClient.crypto.registerEventHandlers(aliceEventEmitter);
aliceClient.crypto!.registerEventHandlers(aliceEventEmitter);

const bobDevice = bobClient.crypto.olmDevice;
aliceClient.crypto.downloadKeys = jest.fn();
const bobDevice = bobClient.crypto!.olmDevice;
aliceClient.crypto!.downloadKeys = jest.fn();

const roomId = "!someroom";

Expand All @@ -875,7 +970,7 @@ describe("MegolmDecryption", function() {
setTimeout(resolve, 100);
});

await expect(aliceClient.crypto.decryptEvent(new MatrixEvent({
await expect(aliceClient.crypto!.decryptEvent(new MatrixEvent({
type: "m.room.encrypted",
sender: "@bob:example.com",
event_id: "$event",
Expand Down
4 changes: 4 additions & 0 deletions src/@types/event.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export enum EventType {
KeyVerificationCancel = "m.key.verification.cancel",
KeyVerificationMac = "m.key.verification.mac",
KeyVerificationDone = "m.key.verification.done",
KeyVerificationKey = "m.key.verification.key",
KeyVerificationAccept = "m.key.verification.accept",
// XXX this event is not yet supported by js-sdk
KeyVerificationReady = "m.key.verification.ready",
// use of this is discouraged https://matrix.org/docs/spec/client_server/r0.6.1#m-room-message-feedback
RoomMessageFeedback = "m.room.message.feedback",
Reaction = "m.reaction",
Expand Down
37 changes: 34 additions & 3 deletions src/crypto/algorithms/megolm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { DeviceInfo } from "../deviceinfo";
import { IOlmSessionResult } from "../olmlib";
import { DeviceInfoMap } from "../DeviceList";
import { MatrixEvent } from "../../models/event";
import { EventType, MsgType } from '../../@types/event';
import { IEventDecryptionResult, IMegolmSessionData, IncomingRoomKeyRequest } from "../index";
import { RoomKeyRequestState } from '../OutgoingRoomKeyRequestManager';
import { OlmGroupSessionExtraData } from "../../@types/crypto";
Expand Down Expand Up @@ -1019,7 +1020,12 @@ class MegolmEncryption extends EncryptionAlgorithm {
}
}

const [devicesInRoom, blocked] = await this.getDevicesInRoom(room);
/**
* When using in-room messages and the room has encryption enabled,
* clients should ensure that encryption does not hinder the verification.
*/
const forceDistributeToUnverified = this.isVerificationEvent(eventType, content);
const [devicesInRoom, blocked] = await this.getDevicesInRoom(room, forceDistributeToUnverified);

// check if any of these devices are not yet known to the user.
// if so, warn the user so they can verify or ignore.
Expand Down Expand Up @@ -1053,6 +1059,26 @@ class MegolmEncryption extends EncryptionAlgorithm {
return encryptedContent;
}

private isVerificationEvent(eventType: string, content: object): boolean {
switch (eventType) {
case EventType.KeyVerificationCancel:
case EventType.KeyVerificationDone:
case EventType.KeyVerificationMac:
case EventType.KeyVerificationStart:
case EventType.KeyVerificationKey:
case EventType.KeyVerificationReady:
case EventType.KeyVerificationAccept: {
return true;
}
case EventType.RoomMessage: {
return content['msgtype'] === MsgType.KeyVerificationRequest;
}
default: {
return false;
}
}
}

/**
* Forces the current outbound group session to be discarded such
* that another one will be created next time an event is sent.
Expand Down Expand Up @@ -1119,14 +1145,19 @@ class MegolmEncryption extends EncryptionAlgorithm {
* Get the list of unblocked devices for all users in the room
*
* @param {module:models/room} room
* @param forceDistributeToUnverified if set to true will include the unverified devices
* even if setting is set to block them (useful for verification)
*
* @return {Promise} Promise which resolves to an array whose
* first element is a map from userId to deviceId to deviceInfo indicating
* the devices that messages should be encrypted to, and whose second
* element is a map from userId to deviceId to data indicating the devices
* that are in the room but that have been blocked
*/
private async getDevicesInRoom(room: Room): Promise<[DeviceInfoMap, IBlockedMap]> {
private async getDevicesInRoom(
room: Room,
forceDistributeToUnverified = false,
): Promise<[DeviceInfoMap, IBlockedMap]> {
const members = await room.getEncryptionTargetMembers();
const roomMembers = members.map(function(u) {
return u.userId;
Expand Down Expand Up @@ -1161,7 +1192,7 @@ class MegolmEncryption extends EncryptionAlgorithm {
const deviceTrust = this.crypto.checkDeviceTrust(userId, deviceId);

if (userDevices[deviceId].isBlocked() ||
(!deviceTrust.isVerified() && isBlacklisting)
(!deviceTrust.isVerified() && isBlacklisting && !forceDistributeToUnverified)
) {
if (!blocked[userId]) {
blocked[userId] = {};
Expand Down
13 changes: 7 additions & 6 deletions src/crypto/verification/Base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ limitations under the License.
*/

import { MatrixEvent } from '../../models/event';
import { EventType } from '../../@types/event';
import { logger } from '../../logger';
import { DeviceInfo } from '../deviceinfo';
import { newTimeoutError } from "./Error";
Expand Down Expand Up @@ -182,13 +183,13 @@ export class VerificationBase<
} else if (e.getType() === this.expectedEvent) {
// if we receive an expected m.key.verification.done, then just
// ignore it, since we don't need to do anything about it
if (this.expectedEvent !== "m.key.verification.done") {
if (this.expectedEvent !== EventType.KeyVerificationDone) {
this.expectedEvent = undefined;
this.rejectEvent = undefined;
this.resetTimer();
this.resolveEvent(e);
}
} else if (e.getType() === "m.key.verification.cancel") {
} else if (e.getType() === EventType.KeyVerificationCancel) {
const reject = this.reject;
this.reject = undefined;
// there is only promise to reject if verify has been called
Expand Down Expand Up @@ -241,20 +242,20 @@ export class VerificationBase<
const sender = e.getSender();
if (sender !== this.userId) {
const content = e.getContent();
if (e.getType() === "m.key.verification.cancel") {
if (e.getType() === EventType.KeyVerificationCancel) {
content.code = content.code || "m.unknown";
content.reason = content.reason || content.body
|| "Unknown reason";
this.send("m.key.verification.cancel", content);
this.send(EventType.KeyVerificationCancel, content);
} else {
this.send("m.key.verification.cancel", {
this.send(EventType.KeyVerificationCancel, {
code: "m.unknown",
reason: content.body || "Unknown reason",
});
}
}
} else {
this.send("m.key.verification.cancel", {
this.send(EventType.KeyVerificationCancel, {
code: "m.unknown",
reason: e.toString(),
});
Expand Down
Loading

0 comments on commit 1c3dd0e

Please sign in to comment.