Skip to content

Commit

Permalink
encryption should not hinder verification
Browse files Browse the repository at this point in the history
  • Loading branch information
BillCarsonFr committed Oct 5, 2022
1 parent ff720e3 commit 4248358
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 26 deletions.
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
34 changes: 31 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.
*/
let 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,16 @@ 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: boolean = false): Promise<[DeviceInfoMap, IBlockedMap]> {
const members = await room.getEncryptionTargetMembers();
const roomMembers = members.map(function(u) {
return u.userId;
Expand Down Expand Up @@ -1161,7 +1189,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

0 comments on commit 4248358

Please sign in to comment.