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: pass device list change notifications into rust crypto-sdk #3254

Merged
merged 6 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 5 additions & 1 deletion spec/integ/sliding-sync-sdk.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -662,13 +662,17 @@ describe("SlidingSyncSdk", () => {
});

it("can update device lists", () => {
client!.crypto!.processDeviceLists = jest.fn();
ext.onResponse({
device_lists: {
changed: ["@alice:localhost"],
left: ["@bob:localhost"],
},
});
// TODO: more assertions?
expect(client!.crypto!.processDeviceLists).toHaveBeenCalledWith({
changed: ["@alice:localhost"],
left: ["@bob:localhost"],
});
});

it("can update OTK counts and unused fallback keys", () => {
Expand Down
10 changes: 9 additions & 1 deletion src/common-crypto/CryptoBackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import type { IToDeviceEvent } from "../sync-accumulator";
import type { IDeviceLists, IToDeviceEvent } from "../sync-accumulator";
import { MatrixEvent } from "../models/event";
import { Room } from "../models/room";
import { CryptoApi } from "../crypto-api";
Expand Down Expand Up @@ -113,6 +113,14 @@ export interface SyncCryptoCallbacks {
*/
processKeyCounts(oneTimeKeysCounts?: Record<string, number>, unusedFallbackKeys?: string[]): Promise<void>;

/**
* Handle the notification from /sync that device lists have
* been changed.
*
* @param deviceLists - device_lists field from /sync
*/
processDeviceLists(deviceLists: IDeviceLists): Promise<void>;

/**
* Called by the /sync loop whenever an m.room.encryption event is received.
*
Expand Down
19 changes: 5 additions & 14 deletions src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import { CryptoStore } from "./store/base";
import { IVerificationChannel } from "./verification/request/Channel";
import { TypedEventEmitter } from "../models/typed-event-emitter";
import { IContent } from "../models/event";
import { ISyncResponse, IToDeviceEvent } from "../sync-accumulator";
import { IDeviceLists, ISyncResponse, IToDeviceEvent } from "../sync-accumulator";
import { ISignatures } from "../@types/signed";
import { IMessage } from "./algorithms/olm";
import { CryptoBackend, OnSyncCompletedData } from "../common-crypto/CryptoBackend";
Expand Down Expand Up @@ -2893,21 +2893,12 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
}

/**
* Handle the notification from /sync or /keys/changes that device lists have
* Handle the notification from /sync that device lists have
* been changed.
*
* @param syncData - Object containing sync tokens associated with this sync
* @param syncDeviceLists - device_lists field from /sync, or response from
* /keys/changes
* @param deviceLists - device_lists field from /sync
*/
public async handleDeviceListChanges(
syncData: ISyncStateData,
syncDeviceLists: Required<ISyncResponse>["device_lists"],
): Promise<void> {
// Initial syncs don't have device change lists. We'll either get the complete list
// of changes for the interval or will have invalidated everything in willProcessSync
if (!syncData.oldSyncToken) return;

public async processDeviceLists(deviceLists: IDeviceLists): Promise<void> {
// Here, we're relying on the fact that we only ever save the sync data after
// sucessfully saving the device list data, so we're guaranteed that the device
// list store is at least as fresh as the sync token from the sync store, ie.
Expand All @@ -2916,7 +2907,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
// If we didn't make this assumption, we'd have to use the /keys/changes API
// to get key changes between the sync token in the device list and the 'old'
// sync token used here to make sure we didn't miss any.
await this.evalDeviceListChanges(syncDeviceLists);
await this.evalDeviceListChanges(deviceLists);
}

/**
Expand Down
17 changes: 15 additions & 2 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-js";

import type { IEventDecryptionResult, IMegolmSessionData } from "../@types/crypto";
import type { IToDeviceEvent } from "../sync-accumulator";
import type { IDeviceLists, IToDeviceEvent } from "../sync-accumulator";
import type { IEncryptedEventInfo } from "../crypto/api";
import { MatrixEvent } from "../models/event";
import { Room } from "../models/room";
Expand Down Expand Up @@ -176,20 +176,23 @@ export class RustCrypto implements CryptoBackend {
* @param events - the received to-device messages
* @param oneTimeKeysCounts - the received one time key counts
* @param unusedFallbackKeys - the received unused fallback keys
* @param devices - the received device list updates
* @returns A list of preprocessed to-device messages.
*/
private async receiveSyncChanges({
events,
oneTimeKeysCounts = new Map<string, number>(),
unusedFallbackKeys = new Set<string>(),
devices = new RustSdkCryptoJs.DeviceLists(),
}: {
events?: IToDeviceEvent[];
oneTimeKeysCounts?: Map<string, number>;
unusedFallbackKeys?: Set<string>;
devices?: RustSdkCryptoJs.DeviceLists;
}): Promise<IToDeviceEvent[]> {
const result = await this.olmMachine.receiveSyncChanges(
events ? JSON.stringify(events) : "[]",
new RustSdkCryptoJs.DeviceLists(),
devices,
oneTimeKeysCounts,
unusedFallbackKeys,
);
Expand Down Expand Up @@ -229,6 +232,16 @@ export class RustCrypto implements CryptoBackend {
}
}

/** called by the sync loop to process the notification that device lists have
* been changed.
*
* @param deviceLists - device_lists field from /sync
*/
public async processDeviceLists(deviceLists: IDeviceLists): Promise<void> {
const devices = new RustSdkCryptoJs.DeviceLists(deviceLists.changed, deviceLists.left);
await this.receiveSyncChanges({ devices });
}

/** called by the sync loop on m.room.encrypted events
*
* @param room - in which the event was received
Expand Down
9 changes: 2 additions & 7 deletions src/sliding-sync-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,8 @@ class ExtensionE2EE implements Extension<ExtensionE2EERequest, ExtensionE2EEResp

public async onResponse(data: ExtensionE2EEResponse): Promise<void> {
// Handle device list updates
if (data["device_lists"]) {
await this.crypto.handleDeviceListChanges(
{
oldSyncToken: "yep", // XXX need to do this so the device list changes get processed :(
},
data["device_lists"],
);
if (data.device_lists) {
await this.crypto.processDeviceLists(data.device_lists);
}

// Handle one_time_keys_count and unused_fallback_key_types
Expand Down
2 changes: 1 addition & 1 deletion src/sync-accumulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ interface IToDevice {
events: IToDeviceEvent[];
}

interface IDeviceLists {
export interface IDeviceLists {
changed?: string[];
left?: string[];
}
Expand Down
4 changes: 2 additions & 2 deletions src/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1515,8 +1515,8 @@ export class SyncApi {

// Handle device list updates
if (data.device_lists) {
if (this.syncOpts.crypto) {
await this.syncOpts.crypto.handleDeviceListChanges(syncEventData, data.device_lists);
if (this.syncOpts.cryptoCallbacks) {
await this.syncOpts.cryptoCallbacks.processDeviceLists(data.device_lists);
} else {
// FIXME if we *don't* have a crypto module, we still need to
// invalidate the device lists. But that would require a
Expand Down