Skip to content

Commit

Permalink
New CryptoApi.getDeviceVerificationStatus api (#3287)
Browse files Browse the repository at this point in the history
* Element-R: implement `{get,set}TrustCrossSignedDevices`

A precursor to element-hq/element-web#25092

* Pull out new `DeviceVerificationStatus`

Define a new base class to replace `DeviceTrustLevel`. The intention is to have
a cleaner interface which is easier to expose from the new crypto impl

* Define, and implement, a new `CryptoApi.getDeviceVerificationStatus`

This is similar to `checkDeviceTrust`, which we're deprecating, but:
 * is `async`, meaning we can implement it in Rust
 * Returns a `DeviceVerificationStatus` instead of a `DeviceTrustLevel`
 * Returns `null` rather than "not verified" if the device is unknown

* add some tests

* Export DeviceVerificationStatus as a proper class

... so that we can instantiate it in tests
  • Loading branch information
richvdh authored Apr 18, 2023
1 parent 8c30a3b commit a03438f
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 39 deletions.
17 changes: 17 additions & 0 deletions spec/unit/crypto/verification/sas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,13 +374,30 @@ describe("SAS verification", function () {
expect(bobDeviceTrust.isLocallyVerified()).toBeTruthy();
expect(bobDeviceTrust.isCrossSigningVerified()).toBeFalsy();

const bobDeviceVerificationStatus = (await alice.client
.getCrypto()!
.getDeviceVerificationStatus("@bob:example.com", "Dynabook"))!;
expect(bobDeviceVerificationStatus.localVerified).toBe(true);
expect(bobDeviceVerificationStatus.crossSigningVerified).toBe(false);

const aliceTrust = bob.client.checkUserTrust("@alice:example.com");
expect(aliceTrust.isCrossSigningVerified()).toBeTruthy();
expect(aliceTrust.isTofu()).toBeTruthy();

const aliceDeviceTrust = bob.client.checkDeviceTrust("@alice:example.com", "Osborne2");
expect(aliceDeviceTrust.isLocallyVerified()).toBeTruthy();
expect(aliceDeviceTrust.isCrossSigningVerified()).toBeFalsy();

const aliceDeviceVerificationStatus = (await bob.client
.getCrypto()!
.getDeviceVerificationStatus("@alice:example.com", "Osborne2"))!;
expect(aliceDeviceVerificationStatus.localVerified).toBe(true);
expect(aliceDeviceVerificationStatus.crossSigningVerified).toBe(false);

const unknownDeviceVerificationStatus = await bob.client
.getCrypto()!
.getDeviceVerificationStatus("@alice:example.com", "xyz");
expect(unknownDeviceVerificationStatus).toBe(null);
});
});

Expand Down
30 changes: 30 additions & 0 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,34 @@ describe("RustCrypto", () => {
expect(rustCrypto.getTrustCrossSignedDevices()).toBe(true);
});
});

describe("getDeviceVerificationStatus", () => {
let rustCrypto: RustCrypto;
let olmMachine: Mocked<RustSdkCryptoJs.OlmMachine>;

beforeEach(() => {
olmMachine = {
getDevice: jest.fn(),
} as unknown as Mocked<RustSdkCryptoJs.OlmMachine>;
rustCrypto = new RustCrypto(olmMachine, {} as MatrixClient["http"], TEST_USER, TEST_DEVICE_ID);
});

it("should call getDevice", async () => {
olmMachine.getDevice.mockResolvedValue({
isCrossSigningTrusted: jest.fn().mockReturnValue(false),
isLocallyTrusted: jest.fn().mockReturnValue(false),
} as unknown as RustSdkCryptoJs.Device);
const res = await rustCrypto.getDeviceVerificationStatus("@user:domain", "device");
expect(olmMachine.getDevice.mock.calls[0][0].toString()).toEqual("@user:domain");
expect(olmMachine.getDevice.mock.calls[0][1].toString()).toEqual("device");
expect(res?.crossSigningVerified).toBe(false);
expect(res?.localVerified).toBe(false);
});

it("should return null for unknown device", async () => {
olmMachine.getDevice.mockResolvedValue(undefined);
const res = await rustCrypto.getDeviceVerificationStatus("@user:domain", "device");
expect(res).toBe(null);
});
});
});
6 changes: 4 additions & 2 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2621,12 +2621,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
*
* @param userId - The ID of the user whose devices is to be checked.
* @param deviceId - The ID of the device to check
*
* @deprecated Use {@link CryptoApi.getDeviceVerificationStatus | `CryptoApi.getDeviceVerificationStatus`}
*/
public checkDeviceTrust(userId: string, deviceId: string): DeviceTrustLevel {
if (!this.cryptoBackend) {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.cryptoBackend.checkDeviceTrust(userId, deviceId);
return this.crypto.checkDeviceTrust(userId, deviceId);
}

/**
Expand Down
12 changes: 1 addition & 11 deletions src/common-crypto/CryptoBackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { IDeviceLists, IToDeviceEvent } from "../sync-accumulator";
import { MatrixEvent } from "../models/event";
import { Room } from "../models/room";
import { CryptoApi } from "../crypto-api";
import { DeviceTrustLevel, UserTrustLevel } from "../crypto/CrossSigning";
import { UserTrustLevel } from "../crypto/CrossSigning";
import { IEncryptedEventInfo } from "../crypto/api";
import { IEventDecryptionResult } from "../@types/crypto";

Expand Down Expand Up @@ -51,16 +51,6 @@ export interface CryptoBackend extends SyncCryptoCallbacks, CryptoApi {
*/
checkUserTrust(userId: string): UserTrustLevel;

/**
* Get the verification level for a given device
*
* TODO: define this better
*
* @param userId - user to be checked
* @param deviceId - device to be checked
*/
checkDeviceTrust(userId: string, deviceId: string): DeviceTrustLevel;

/**
* Encrypt an event according to the configuration of the room.
*
Expand Down
50 changes: 50 additions & 0 deletions src/crypto-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,54 @@ export interface CryptoApi {
* @returns `true` if we trust cross-signed devices, otherwise `false`.
*/
getTrustCrossSignedDevices(): boolean;

/**
* Get the verification status of a given device.
*
* @param userId - The ID of the user whose device is to be checked.
* @param deviceId - The ID of the device to check
*
* @returns Verification status of the device, or `null` if the device is not known
*/
getDeviceVerificationStatus(userId: string, deviceId: string): Promise<DeviceVerificationStatus | null>;
}

export class DeviceVerificationStatus {
public constructor(
/**
* True if this device has been verified via cross signing.
*
* This does *not* take into account `trustCrossSignedDevices`.
*/
public readonly crossSigningVerified: boolean,

/**
* TODO: tofu magic wtf does this do?
*/
public readonly tofu: boolean,

/**
* True if the device has been marked as locally verified.
*/
public readonly localVerified: boolean,

/**
* True if the client has been configured to trust cross-signed devices via {@link CryptoApi#setTrustCrossSignedDevices}.
*/
private readonly trustCrossSignedDevices: boolean,
) {}

/**
* Check if we should consider this device "verified".
*
* A device is "verified" if either:
* * it has been manually marked as such via {@link MatrixClient#setDeviceVerified}.
* * it has been cross-signed with a verified signing key, **and** the client has been configured to trust
* cross-signed devices via {@link CryptoApi#setTrustCrossSignedDevices}.
*
* @returns true if this device is verified via any means.
*/
public isVerified(): boolean {
return this.localVerified || (this.trustCrossSignedDevices && this.crossSigningVerified);
}
}
21 changes: 5 additions & 16 deletions src/crypto/CrossSigning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { ICryptoCallbacks } from ".";
import { ISignatures } from "../@types/signed";
import { CryptoStore, SecretStorePrivateKeys } from "./store/base";
import { ServerSideSecretStorage, SecretStorageKeyDescription } from "../secret-storage";
import { DeviceVerificationStatus } from "../crypto-api";

const KEY_REQUEST_TIMEOUT_MS = 1000 * 60;

Expand Down Expand Up @@ -628,16 +629,11 @@ export class UserTrustLevel {
}

/**
* Represents the ways in which we trust a device
* Represents the ways in which we trust a device.
*
* @deprecated Use {@link DeviceVerificationStatus}.
*/
export class DeviceTrustLevel {
public constructor(
public readonly crossSigningVerified: boolean,
public readonly tofu: boolean,
private readonly localVerified: boolean,
private readonly trustCrossSignedDevices: boolean,
) {}

export class DeviceTrustLevel extends DeviceVerificationStatus {
public static fromUserTrustLevel(
userTrustLevel: UserTrustLevel,
localVerified: boolean,
Expand All @@ -651,13 +647,6 @@ export class DeviceTrustLevel {
);
}

/**
* @returns true if this device is verified via any means
*/
public isVerified(): boolean {
return Boolean(this.isLocallyVerified() || (this.trustCrossSignedDevices && this.isCrossSigningVerified()));
}

/**
* @returns true if this device is verified via cross signing
*/
Expand Down
21 changes: 17 additions & 4 deletions src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {
ServerSideSecretStorageImpl,
} from "../secret-storage";
import { ISecretRequest } from "./SecretSharing";
import { DeviceVerificationStatus } from "../crypto-api";

const DeviceVerification = DeviceInfo.DeviceVerification;

Expand Down Expand Up @@ -1452,10 +1453,22 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
/**
* Check whether a given device is trusted.
*
* @param userId - The ID of the user whose devices is to be checked.
* @param userId - The ID of the user whose device is to be checked.
* @param deviceId - The ID of the device to check
*
* @returns
*/
public async getDeviceVerificationStatus(
userId: string,
deviceId: string,
): Promise<DeviceVerificationStatus | null> {
const device = this.deviceList.getStoredDevice(userId, deviceId);
if (!device) {
return null;
}
return this.checkDeviceInfoTrust(userId, device);
}

/**
* @deprecated Use {@link CryptoApi.getDeviceVerificationStatus}.
*/
public checkDeviceTrust(userId: string, deviceId: string): DeviceTrustLevel {
const device = this.deviceList.getStoredDevice(userId, deviceId);
Expand All @@ -1468,7 +1481,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
* @param userId - The ID of the user whose devices is to be checked.
* @param device - The device info object to check
*
* @returns
* @deprecated Use {@link CryptoApi.getDeviceVerificationStatus}.
*/
public checkDeviceInfoTrust(userId: string, device?: DeviceInfo): DeviceTrustLevel {
const trustedLocally = !!device?.isVerified();
Expand Down
1 change: 1 addition & 0 deletions src/matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export type { MatrixCall } from "./webrtc/call";
export { GroupCallEvent, GroupCallIntent, GroupCallState, GroupCallType } from "./webrtc/groupCall";
export type { GroupCall } from "./webrtc/groupCall";
export type { CryptoApi } from "./crypto-api";
export { DeviceVerificationStatus } from "./crypto-api";
export { CryptoEvent } from "./crypto";

let cryptoStoreFactory = (): CryptoStore => new MemoryCryptoStore();
Expand Down
30 changes: 24 additions & 6 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ import { RoomMember } from "../models/room-member";
import { CryptoBackend, OnSyncCompletedData } from "../common-crypto/CryptoBackend";
import { logger } from "../logger";
import { IHttpOpts, MatrixHttpApi } from "../http-api";
import { DeviceTrustLevel, UserTrustLevel } from "../crypto/CrossSigning";
import { UserTrustLevel } from "../crypto/CrossSigning";
import { RoomEncryptor } from "./RoomEncryptor";
import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProcessor";
import { KeyClaimManager } from "./KeyClaimManager";
import { MapWithDefault } from "../utils";
import { DeviceVerificationStatus } from "../crypto-api";

/**
* An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto.
Expand Down Expand Up @@ -131,11 +132,6 @@ export class RustCrypto implements CryptoBackend {
return new UserTrustLevel(false, false, false);
}

public checkDeviceTrust(userId: string, deviceId: string): DeviceTrustLevel {
// TODO
return new DeviceTrustLevel(false, false, false, false);
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
// CryptoApi implementation
Expand Down Expand Up @@ -182,6 +178,28 @@ export class RustCrypto implements CryptoBackend {
// events. Maybe we need to do the same?
}

/**
* Implementation of {@link CryptoApi#getDeviceVerificationStatus}.
*/
public async getDeviceVerificationStatus(
userId: string,
deviceId: string,
): Promise<DeviceVerificationStatus | null> {
const device: RustSdkCryptoJs.Device | undefined = await this.olmMachine.getDevice(
new RustSdkCryptoJs.UserId(userId),
new RustSdkCryptoJs.DeviceId(deviceId),
);

if (!device) return null;

return new DeviceVerificationStatus(
device.isCrossSigningTrusted(),
false, // tofu
device.isLocallyTrusted(),
this._trustCrossSignedDevices,
);
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
// SyncCryptoCallbacks implementation
Expand Down

0 comments on commit a03438f

Please sign in to comment.