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

Element-R: Basic implementation of SAS verification #3490

Merged
merged 4 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
],
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/matrix-sdk-crypto-js": "^0.1.0-alpha.10",
"@matrix-org/matrix-sdk-crypto-js": "^0.1.0-alpha.11",
"another-json": "^0.2.0",
"bs58": "^5.0.0",
"content-type": "^1.0.4",
Expand Down
80 changes: 57 additions & 23 deletions spec/integ/crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import fetchMock from "fetch-mock-jest";
import { MockResponse } from "fetch-mock";
import "fake-indexeddb/auto";

import { createClient, CryptoEvent, MatrixClient } from "../../../src";
import {
Expand All @@ -41,14 +42,15 @@ import {
} from "../../test-utils/test-data";
import { mockInitialApiRequests } from "../../test-utils/mockEndpoints";
import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
import { E2EKeyReceiver } from "../../test-utils/E2EKeyReceiver";

// The verification flows use javascript timers to set timeouts. We tell jest to use mock timer implementations
// to ensure that we don't end up with dangling timeouts.
jest.useFakeTimers();

let previousCrypto: Crypto | undefined;

beforeAll(() => {
beforeAll(async () => {
// Stub out global.crypto
previousCrypto = global["crypto"];

Expand All @@ -60,6 +62,9 @@ beforeAll(() => {
},
},
});

// we use the libolm primitives in the test, so init the Olm library
await global.Olm.init();
});

// restore the original global.crypto
Expand Down Expand Up @@ -105,6 +110,9 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
/** an object which intercepts `/keys/query` requests from {@link #aliceClient} */
let e2eKeyResponder: E2EKeyResponder;

/** an object which intercepts `/keys/upload` requests from {@link #aliceClient} */
let e2eKeyReceiver: E2EKeyReceiver;

beforeEach(async () => {
// anything that we don't have a specific matcher for silently returns a 404
fetchMock.catch(404);
Expand All @@ -121,14 +129,21 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u

await initCrypto(aliceClient);

e2eKeyReceiver = new E2EKeyReceiver(aliceClient.getHomeserverUrl());
e2eKeyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl());
e2eKeyResponder.addKeyReceiver(TEST_USER_ID, e2eKeyReceiver);

syncResponder = new SyncResponder(aliceClient.getHomeserverUrl());
mockInitialApiRequests(aliceClient.getHomeserverUrl());
await aliceClient.startClient();
});

afterEach(async () => {
await aliceClient.stopClient();

// Allow in-flight things to complete before we tear down the test
await jest.runAllTimersAsync();

fetchMock.mockReset();
});

Expand All @@ -138,7 +153,9 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
e2eKeyResponder.addDeviceKeys(TEST_USER_ID, TEST_DEVICE_ID, SIGNED_TEST_DEVICE_DATA);
});

oldBackendOnly("can verify via SAS", async () => {
it("can verify another device via SAS", async () => {
await waitForDeviceList();

// have alice initiate a verification. She should send a m.key.verification.request
let [requestBody, request] = await Promise.all([
expectSendToDeviceMessage("m.key.verification.request"),
Expand Down Expand Up @@ -189,22 +206,29 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
short_authentication_string: ["decimal", "emoji"],
},
});
await waitForVerificationRequestChanged(request);
expect(request.phase).toEqual(VerificationPhase.Started);
expect(request.otherPartySupportsMethod("m.sas.v1")).toBe(true);
expect(request.chosenMethod).toEqual("m.sas.v1");

// there should now be a verifier
const verifier: Verifier = request.verifier!;
expect(verifier).toBeDefined();
expect(verifier.getShowSasCallbacks()).toBeNull();
// as soon as the Changed event arrives, `verifier` should be defined
const verifier = await new Promise<Verifier>((resolve) => {
function onChange() {
expect(request.phase).toEqual(VerificationPhase.Started);
expect(request.otherPartySupportsMethod("m.sas.v1")).toBe(true);
expect(request.chosenMethod).toEqual("m.sas.v1");

const verifier: Verifier = request.verifier!;
expect(verifier).toBeDefined();
expect(verifier.getShowSasCallbacks()).toBeNull();

resolve(verifier);
}
request.once(VerificationRequestEvent.Change, onChange);
});

// start off the verification process: alice will send an `accept`
const sendToDevicePromise = expectSendToDeviceMessage("m.key.verification.accept");
const verificationPromise = verifier.verify();
// advance the clock, because the devicelist likes to sleep for 5ms during key downloads
jest.advanceTimersByTime(10);

requestBody = await expectSendToDeviceMessage("m.key.verification.accept");
requestBody = await sendToDevicePromise;
toDeviceMessage = requestBody.messages[TEST_USER_ID][TEST_DEVICE_ID];
expect(toDeviceMessage.key_agreement_protocol).toEqual("curve25519-hkdf-sha256");
expect(toDeviceMessage.short_authentication_string).toEqual(["decimal", "emoji"]);
Expand Down Expand Up @@ -281,15 +305,9 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
});

oldBackendOnly("can verify another via QR code with an untrusted cross-signing key", async () => {
e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA);

// QRCode fails if we don't yet have the cross-signing keys, so make sure we have them now.
//
// Completing the initial sync will make the device list download outdated device lists (of which our own
// user will be one).
syncResponder.sendOrQueueSyncResponse({});
// DeviceList has a sleep(5) which we need to make happen
await jest.advanceTimersByTimeAsync(10);
e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA);
await waitForDeviceList();
expect(aliceClient.getStoredCrossSigningForUser(TEST_USER_ID)).toBeTruthy();

// have alice initiate a verification. She should send a m.key.verification.request
Expand Down Expand Up @@ -377,7 +395,9 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
expect(request.phase).toEqual(VerificationPhase.Done);
});

oldBackendOnly("can cancel during the SAS phase", async () => {
it("can cancel during the SAS phase", async () => {
await waitForDeviceList();

// have alice initiate a verification. She should send a m.key.verification.request
const [, request] = await Promise.all([
expectSendToDeviceMessage("m.key.verification.request"),
Expand Down Expand Up @@ -419,12 +439,13 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
expect(verifier.hasBeenCancelled).toBe(false);

// start off the verification process: alice will send an `accept`
const sendToDevicePromise = expectSendToDeviceMessage("m.key.verification.accept");
const verificationPromise = verifier.verify();
// advance the clock, because the devicelist likes to sleep for 5ms during key downloads
jest.advanceTimersByTime(10);
await expectSendToDeviceMessage("m.key.verification.accept");
await sendToDevicePromise;

// now we unceremoniously cancel
// now we unceremoniously cancel. We expect the verificatationPromise to reject.
const requestPromise = expectSendToDeviceMessage("m.key.verification.cancel");
verifier.cancel(new Error("blah"));
await requestPromise;
Expand Down Expand Up @@ -479,6 +500,19 @@ function runTests(backend: string, initCrypto: InitCrypto, methods: string[] | u
});
});

/** make sure that the client knows about the dummy device */
async function waitForDeviceList(): Promise<void> {
// Completing the initial sync will make the device list download outdated device lists (of which our own
// user will be one).
syncResponder.sendOrQueueSyncResponse({});
// DeviceList has a sleep(5) which we need to make happen
await jest.advanceTimersByTimeAsync(10);

// The client should now know about the dummy device
const devices = await aliceClient.getCrypto()!.getUserDeviceInfo([TEST_USER_ID]);
expect(devices.get(TEST_USER_ID)!.keys()).toContain(TEST_DEVICE_ID);
}

function returnToDeviceMessageFromSync(ev: { type: string; content: object; sender?: string }): void {
ev.sender ??= TEST_USER_ID;
syncResponder.sendOrQueueSyncResponse({ to_device: { events: [ev] } });
Expand Down
7 changes: 7 additions & 0 deletions spec/test-utils/E2EKeyReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ export class E2EKeyReceiver implements IE2EKeyReceiver {
return this.deviceKeys.keys[keyIds[0]];
}

/**
* If the device keys have already been uploaded, return them. Else return null.
*/
public getUploadedDeviceKeys(): IDeviceKeys | null {
return this.deviceKeys;
}

/**
* If one-time keys have already been uploaded, return them. Otherwise,
* set up an expectation that the keys will be uploaded, and wait for
Expand Down
24 changes: 24 additions & 0 deletions spec/test-utils/E2EKeyResponder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import fetchMock from "fetch-mock-jest";
import { MapWithDefault } from "../../src/utils";
import { IDownloadKeyResult } from "../../src";
import { IDeviceKeys } from "../../src/@types/crypto";
import { E2EKeyReceiver } from "./E2EKeyReceiver";

/**
* An object which intercepts `/keys/query` fetches via fetch-mock.
*/
export class E2EKeyResponder {
private deviceKeysByUserByDevice = new MapWithDefault<string, Map<string, any>>(() => new Map());
private e2eKeyReceiversByUser = new Map<string, E2EKeyReceiver>();
private masterKeysByUser: Record<string, any> = {};
private selfSigningKeysByUser: Record<string, any> = {};
private userSigningKeysByUser: Record<string, any> = {};
Expand Down Expand Up @@ -61,6 +63,16 @@ export class E2EKeyResponder {
if (userKeys !== undefined) {
response.device_keys[user] = Object.fromEntries(userKeys.entries());
}

const e2eKeyReceiver = this.e2eKeyReceiversByUser.get(user);
if (e2eKeyReceiver !== undefined) {
const deviceKeys = e2eKeyReceiver.getUploadedDeviceKeys();
if (deviceKeys !== null) {
response.device_keys[user] ??= {};
response.device_keys[user][deviceKeys.device_id] = deviceKeys;
}
}

if (this.masterKeysByUser.hasOwnProperty(user)) {
response.master_keys[user] = this.masterKeysByUser[user];
}
Expand Down Expand Up @@ -96,4 +108,16 @@ export class E2EKeyResponder {
Object.assign(this.selfSigningKeysByUser, data.self_signing_keys);
Object.assign(this.userSigningKeysByUser, data.user_signing_keys);
}

/**
* Add an E2EKeyReceiver to poll for uploaded keys
*
* Any keys which have been uploaded to the given `E2EKeyReceiver` at the time of the `/keys/query` request will
* be added to the response.
*
* @param e2eKeyReceiver
*/
public addKeyReceiver(userId: string, e2eKeyReceiver: E2EKeyReceiver) {
this.e2eKeyReceiversByUser.set(userId, e2eKeyReceiver);
}
}
9 changes: 9 additions & 0 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,15 @@ describe("RustCrypto", () => {
expect(deviceMap.has(testData.TEST_DEVICE_ID)).toBe(true);
rustCrypto.stop();
});

describe("requestDeviceVerification", () => {
it("throws an error if the device is unknown", async () => {
const rustCrypto = await makeTestRustCrypto();
await expect(() => rustCrypto.requestDeviceVerification(TEST_USER, "unknown")).rejects.toThrow(
"Not a known device",
);
});
});
});

/** build a basic RustCrypto instance for testing
Expand Down
2 changes: 2 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
this.secretStorage,
this.cryptoCallbacks,
);
rustCrypto.supportedVerificationMethods = this.verificationMethods;

this.cryptoBackend = rustCrypto;

// attach the event listeners needed by RustCrypto
Expand Down
41 changes: 37 additions & 4 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { secretStorageContainsCrossSigningKeys } from "./secret-storage";
import { keyFromPassphrase } from "../crypto/key_passphrase";
import { encodeRecoveryKey } from "../crypto/recoverykey";
import { crypto } from "../crypto/crypto";
import { RustVerificationRequest, verificationMethodIdentifierToMethod } from "./verification";

/**
* An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto.
Expand Down Expand Up @@ -562,6 +563,13 @@ export class RustCrypto implements CryptoBackend {
return;
}

/**
* The verification methods we offer to the other side during an interactive verification.
*
* If `undefined`, we will offer all the methods supported by the Rust SDK.
*/
public supportedVerificationMethods: string[] | undefined;

/**
* Send a verification request to our other devices.
*
Expand All @@ -571,7 +579,18 @@ export class RustCrypto implements CryptoBackend {
*
* @returns a VerificationRequest when the request has been sent to the other party.
*/
public requestOwnUserVerification(): Promise<VerificationRequest> {
public async requestOwnUserVerification(): Promise<VerificationRequest> {
/* something like this, but currently untested
const user: RustSdkCryptoJs.OwnUserIdentity = await this.olmMachine.getIdentity(
new RustSdkCryptoJs.UserId(this.userId),
);
const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await user.requestVerification(
this.supportedVerificationMethods?.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor);
*/
richvdh marked this conversation as resolved.
Show resolved Hide resolved
richvdh marked this conversation as resolved.
Show resolved Hide resolved
throw new Error("not implemented");
}

Expand All @@ -580,15 +599,29 @@ export class RustCrypto implements CryptoBackend {
*
* If a verification is already in flight, returns it. Otherwise, initiates a new one.
*
* Implementation of {@link CryptoApi#requestDeviceVerification }.
* Implementation of {@link CryptoApi#requestDeviceVerification}.
*
* @param userId - ID of the owner of the device to verify
* @param deviceId - ID of the device to verify
*
* @returns a VerificationRequest when the request has been sent to the other party.
*/
public requestDeviceVerification(userId: string, deviceId: string): Promise<VerificationRequest> {
throw new Error("not implemented");
public async requestDeviceVerification(userId: string, deviceId: string): Promise<VerificationRequest> {
const device: RustSdkCryptoJs.Device | undefined = await this.olmMachine.getDevice(
new RustSdkCryptoJs.UserId(userId),
new RustSdkCryptoJs.DeviceId(deviceId),
);

if (!device) {
throw new Error("Not a known device");
}

const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await device.requestVerification(
this.supportedVerificationMethods?.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor);
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
Loading