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

Encryption should not hinder verification #2734

Merged
merged 8 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions src/@types/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,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
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
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
3 changes: 2 additions & 1 deletion src/crypto/verification/Error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ limitations under the License.
*/

import { MatrixEvent } from "../../models/event";
import { EventType } from '../../@types/event';

export function newVerificationError(code: string, reason: string, extraData: Record<string, any>): MatrixEvent {
const content = Object.assign({}, { code, reason }, extraData);
return new MatrixEvent({
type: "m.key.verification.cancel",
type: EventType.KeyVerificationCancel,
content,
});
}
Expand Down
31 changes: 16 additions & 15 deletions src/crypto/verification/SAS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ import {
} from './Error';
import { logger } from '../../logger';
import { IContent, MatrixEvent } from "../../models/event";
import { EventType } from '../../@types/event';

const START_TYPE = "m.key.verification.start";
const START_TYPE = EventType.KeyVerificationStart;

const EVENTS = [
"m.key.verification.accept",
"m.key.verification.key",
"m.key.verification.mac",
EventType.KeyVerificationAccept,
EventType.KeyVerificationKey,
EventType.KeyVerificationMac,
];

let olmutil: Utility;
Expand Down Expand Up @@ -329,7 +330,7 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {

let e;
try {
e = await this.waitForEvent("m.key.verification.accept");
e = await this.waitForEvent(EventType.KeyVerificationAccept);
} finally {
this.waitingForAccept = false;
}
Expand All @@ -351,11 +352,11 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
const olmSAS = new global.Olm.SAS();
try {
this.ourSASPubKey = olmSAS.get_pubkey();
await this.send("m.key.verification.key", {
await this.send(EventType.KeyVerificationKey, {
key: this.ourSASPubKey,
});

e = await this.waitForEvent("m.key.verification.key");
e = await this.waitForEvent(EventType.KeyVerificationKey);
// FIXME: make sure event is properly formed
content = e.getContent();
const commitmentStr = content.key + anotherjson.stringify(startContent);
Expand Down Expand Up @@ -385,12 +386,12 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
});

[e] = await Promise.all([
this.waitForEvent("m.key.verification.mac")
this.waitForEvent(EventType.KeyVerificationMac)
.then((e) => {
// we don't expect any more messages from the other
// party, and they may send a m.key.verification.done
// when they're done on their end
this.expectedEvent = "m.key.verification.done";
this.expectedEvent = EventType.KeyVerificationDone;
return e;
}),
verifySAS,
Expand Down Expand Up @@ -423,7 +424,7 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
const olmSAS = new global.Olm.SAS();
try {
const commitmentStr = olmSAS.get_pubkey() + anotherjson.stringify(content);
await this.send("m.key.verification.accept", {
await this.send(EventType.KeyVerificationAccept, {
key_agreement_protocol: keyAgreement,
hash: hashMethod,
message_authentication_code: macMethod,
Expand All @@ -432,13 +433,13 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
commitment: olmutil.sha256(commitmentStr),
});

let e = await this.waitForEvent("m.key.verification.key");
let e = await this.waitForEvent(EventType.KeyVerificationKey);
// FIXME: make sure event is properly formed
content = e.getContent();
this.theirSASPubKey = content.key;
olmSAS.set_their_key(content.key);
this.ourSASPubKey = olmSAS.get_pubkey();
await this.send("m.key.verification.key", {
await this.send(EventType.KeyVerificationKey, {
key: this.ourSASPubKey,
});

Expand All @@ -461,12 +462,12 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
});

[e] = await Promise.all([
this.waitForEvent("m.key.verification.mac")
this.waitForEvent(EventType.KeyVerificationMac)
.then((e) => {
// we don't expect any more messages from the other
// party, and they may send a m.key.verification.done
// when they're done on their end
this.expectedEvent = "m.key.verification.done";
this.expectedEvent = EventType.KeyVerificationDone;
return e;
}),
verifySAS,
Expand Down Expand Up @@ -507,7 +508,7 @@ export class SAS extends Base<SasEvent, EventHandlerMap> {
keyList.sort().join(","),
baseInfo + "KEY_IDS",
);
return this.send("m.key.verification.mac", { mac, keys });
return this.send(EventType.KeyVerificationMac, { mac, keys });
}

private async checkMAC(olmSAS: OlmSAS, content: IContent, method: string): Promise<void> {
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/verification/request/VerificationRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { QRCodeData, SCAN_QR_CODE_METHOD } from "../QRCode";
import { IVerificationChannel } from "./Channel";
import { MatrixClient } from "../../../client";
import { MatrixEvent } from "../../../models/event";
import { EventType } from '../../../@types/event';
import { VerificationBase } from "../Base";
import { VerificationMethod } from "../../index";
import { TypedEventEmitter } from "../../../models/typed-event-emitter";
Expand Down Expand Up @@ -931,7 +932,7 @@ export class VerificationRequest<
}

public onVerifierFinished(): void {
this.channel.send("m.key.verification.done", {});
this.channel.send(EventType.KeyVerificationDone, {});
this.verifierHasFinished = true;
// move to .done phase
const newTransitions = this.applyPhaseTransitions();
Expand Down