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

Provide more options for starting dehydration #4664

Merged
merged 2 commits into from
Jan 30, 2025
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
162 changes: 162 additions & 0 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1894,6 +1894,168 @@ describe("RustCrypto", () => {
await rehydrationCompletedPromise;
await rustCrypto2.stop();
});

it("should handle dehydration start options", async () => {
const secretStorageCallbacks = {
getSecretStorageKey: async (keys: any, name: string) => {
return [[...Object.keys(keys.keys)][0], new Uint8Array(32)];
},
} as SecretStorageCallbacks;
const secretStorage = new ServerSideSecretStorageImpl(new DummyAccountDataClient(), secretStorageCallbacks);

const e2eKeyReceiver = new E2EKeyReceiver("http://server");
const e2eKeyResponder = new E2EKeyResponder("http://server");
e2eKeyResponder.addKeyReceiver(TEST_USER, e2eKeyReceiver);
fetchMock.get("path:/_matrix/client/v3/room_keys/version", {
status: 404,
body: {
errcode: "M_NOT_FOUND",
error: "Not found",
},
});
fetchMock.post("path:/_matrix/client/v3/keys/device_signing/upload", {
status: 200,
body: {},
});
fetchMock.post("path:/_matrix/client/v3/keys/signatures/upload", {
status: 200,
body: {},
});
const rustCrypto = await makeTestRustCrypto(makeMatrixHttpApi(), TEST_USER, TEST_DEVICE_ID, secretStorage);

// dehydration requires secret storage and cross signing
async function createSecretStorageKey() {
return {
keyInfo: {} as AddSecretStorageKeyOpts,
privateKey: new Uint8Array(32),
};
}
await rustCrypto.bootstrapCrossSigning({ setupNewCrossSigning: true });
await rustCrypto.bootstrapSecretStorage({
createSecretStorageKey,
setupNewSecretStorage: true,
setupNewKeyBackup: false,
});
// we need to process a sync so that the OlmMachine will upload keys
await rustCrypto.preprocessToDeviceMessages([]);
await rustCrypto.onSyncCompleted({});

// set up mocks needed for device dehydration
let dehydratedDeviceInfo: Record<string, any> | undefined;
const getDehydratedDeviceMock = jest.fn(() => {
if (dehydratedDeviceInfo) {
return {
status: 200,
body: dehydratedDeviceInfo,
};
} else {
return {
status: 404,
body: {
errcode: "M_NOT_FOUND",
error: "Not found",
},
};
}
});
fetchMock.get(
"path:/_matrix/client/unstable/org.matrix.msc3814.v1/dehydrated_device",
getDehydratedDeviceMock,
);
const putDehydratedDeviceMock = jest.fn((path, opts) => {
const content = JSON.parse(opts.body as string);
dehydratedDeviceInfo = {
device_id: content.device_id,
device_data: content.device_data,
};
return {
status: 200,
body: {
device_id: content.device_id,
},
};
});
fetchMock.put(
"path:/_matrix/client/unstable/org.matrix.msc3814.v1/dehydrated_device",
putDehydratedDeviceMock,
);
fetchMock.post(/_matrix\/client\/unstable\/org.matrix.msc3814.v1\/dehydrated_device\/.*\/events/, {
status: 200,
body: {
events: [],
next_batch: "foo",
},
});

fetchMock.mockClear();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the above setup code to be in a function that takes arguments for the exact setup we need e.g. function setup(key_cached: boolean, device_exists: boolean, ...) ... and then separate tests for the things we want to be true:

it("should do nothing if no key is cached and onlyIfKeyCached is true")
it("should create a new key and device if no key or device exist")
it("should rehydrate an existing device using the existing key if it exists")
it("should create a new device if we don't have the key for the existing device")
it("should create a new device even if one exists if rehdrate is false")
it("should create a new device every time if createNewKey is true")

This would really help me understand what the code is supposed to do, and will help us understand what happened if future code changes affect this logic.

This is a preference, so if you have good reasons for this narrative style, you can ignore this request :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

The narrative style made sense when I was writing it, but this is probably clearer.


// Start with testing `onlyIfKeyCached: true`. Since this is our
// first run, there is no key cached, so should do nothing. i.e. it
// should not make any HTTP requests and should not create a new key.
await rustCrypto.startDehydration({ onlyIfKeyCached: true });
expect(getDehydratedDeviceMock).not.toHaveBeenCalled();
expect(putDehydratedDeviceMock).not.toHaveBeenCalled();
expect(await secretStorage.get("org.matrix.msc3814")).toBeFalsy();

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

// With default options and no key is available, should create new
// key and create new dehydrated device (so it will `PUT
// /dehydrated_device`). It won't try to rehydrate the device,
// since it has no dehydration key yet.
await rustCrypto.startDehydration();
expect(putDehydratedDeviceMock).toHaveBeenCalled();
const origDehydrationKey = await secretStorage.get("org.matrix.msc3814");
expect(origDehydrationKey).toBeTruthy();

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

// Start again with default options, but this time it will try to
// rehydrate a device (so it will `GET /dehydrated_device`). It
// should keep the same dehydration key.
await rustCrypto.startDehydration();
expect(getDehydratedDeviceMock).toHaveBeenCalled();
expect(putDehydratedDeviceMock).toHaveBeenCalled();
expect(await secretStorage.get("org.matrix.msc3814")).toEqual(origDehydrationKey);

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

// Again try "onlyIfKeyCached: true", but this time we already have
// a cached key, so it will rehydrate the device and create a new
// dehydrated device. It should keep the same dehydration key.
await rustCrypto.startDehydration({ onlyIfKeyCached: true });
expect(getDehydratedDeviceMock).toHaveBeenCalled();
expect(putDehydratedDeviceMock).toHaveBeenCalled();
expect(await secretStorage.get("org.matrix.msc3814")).toEqual(origDehydrationKey);

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

// With "rehydrate: false", it should not try to rehydrate, but
// still create a new dehydrated device. It should keep the same
// dehydration key.
await rustCrypto.startDehydration({ rehydrate: false });
expect(getDehydratedDeviceMock).not.toHaveBeenCalled();
expect(putDehydratedDeviceMock).toHaveBeenCalled();
expect(await secretStorage.get("org.matrix.msc3814")).toEqual(origDehydrationKey);

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

// With "createNewKey: true", it should create a new dehydration key
await rustCrypto.startDehydration({ createNewKey: true });
expect(getDehydratedDeviceMock).toHaveBeenCalled();
expect(putDehydratedDeviceMock).toHaveBeenCalled();
expect(await secretStorage.get("org.matrix.msc3814")).not.toEqual(origDehydrationKey);

getDehydratedDeviceMock.mockClear();
putDehydratedDeviceMock.mockClear();

rustCrypto.stop();
});
});

describe("import & export secrets bundle", () => {
Expand Down
25 changes: 19 additions & 6 deletions src/crypto-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ import { MatrixEvent } from "../models/event.ts";
* @packageDocumentation
*/

/**
* The options to start device dehydration.
*/
export interface StartDehydrationOpts {
/** Force creation of a new dehydration key. Defaults to `false`. */
createNewKey?: boolean;
/** Only start dehydration if we have a cached key. Defaults to `false`. */
onlyIfKeyCached?: boolean;
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
/** Try to rehydrate a device before creating a new dehydrated device. Defaults to `true`. */
rehydrate?: boolean;
}

/**
* Public interface to the cryptography parts of the js-sdk
*
Expand Down Expand Up @@ -649,10 +661,11 @@ export interface CryptoApi {
/**
* Start using device dehydration.
*
* - Rehydrates a dehydrated device, if one is available.
* - Rehydrates a dehydrated device, if one is available and `opts.rehydrate`
* is `true`.
* - Creates a new dehydration key, if necessary, and stores it in Secret
* Storage.
* - If `createNewKey` is set to true, always creates a new key.
* - If `opts.createNewKey` is set to true, always creates a new key.
* - If a dehydration key is not available, creates a new one.
* - Creates a new dehydrated device, and schedules periodically creating
* new dehydrated devices.
Expand All @@ -661,11 +674,11 @@ export interface CryptoApi {
* `true`, and must not be called until after cross-signing and secret
* storage have been set up.
*
* @param createNewKey - whether to force creation of a new dehydration key.
* This can be used, for example, if Secret Storage is being reset. Defaults
* to false.
* @param opts - options for device dehydration. For backwards compatibility
* with old code, a boolean can be given here, which will be treated as
* the `createNewKey` option. However, this is deprecated.
*/
startDehydration(createNewKey?: boolean): Promise<void>;
startDehydration(opts?: StartDehydrationOpts | boolean): Promise<void>;

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import {
CryptoEventHandlerMap as CryptoApiCryptoEventHandlerMap,
KeyBackupRestoreResult,
KeyBackupRestoreOpts,
StartDehydrationOpts,
} from "../crypto-api/index.ts";
import { Device, DeviceMap } from "../models/device.ts";
import { deviceInfoToDevice } from "./device-converter.ts";
Expand Down Expand Up @@ -4327,7 +4328,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
/**
* Stub function -- dehydration is not implemented here, so throw error
*/
public async startDehydration(createNewKey?: boolean): Promise<void> {
public async startDehydration(createNewKey?: StartDehydrationOpts | boolean): Promise<void> {
throw new Error("Not implemented");
}

Expand Down
39 changes: 25 additions & 14 deletions src/rust-crypto/DehydratedDeviceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IToDeviceEvent } from "../sync-accumulator.ts";
import { ServerSideSecretStorage } from "../secret-storage.ts";
import { decodeBase64 } from "../base64.ts";
import { Logger } from "../logger.ts";
import { CryptoEvent, CryptoEventHandlerMap } from "../crypto-api/index.ts";
import { CryptoEvent, CryptoEventHandlerMap, StartDehydrationOpts } from "../crypto-api/index.ts";
import { TypedEventEmitter } from "../models/typed-event-emitter.ts";

/**
Expand Down Expand Up @@ -121,28 +121,39 @@ export class DehydratedDeviceManager extends TypedEventEmitter<DehydratedDevices
/**
* Start using device dehydration.
*
* - Rehydrates a dehydrated device, if one is available.
* - Rehydrates a dehydrated device, if one is available and `opts.rehydrate`
* is `true`.
* - Creates a new dehydration key, if necessary, and stores it in Secret
* Storage.
* - If `createNewKey` is set to true, always creates a new key.
* - If `opts.createNewKey` is set to true, always creates a new key.
* - If a dehydration key is not available, creates a new one.
* - Creates a new dehydrated device, and schedules periodically creating
* new dehydrated devices.
*
* @param createNewKey - whether to force creation of a new dehydration key.
* This can be used, for example, if Secret Storage is being reset.
* @param opts - options for device dehydration. For backwards compatibility
* with old code, a boolean can be given here, which will be treated as
* the `createNewKey` option. However, this is deprecated.
*/
public async start(createNewKey?: boolean): Promise<void> {
public async start(opts: StartDehydrationOpts | boolean = {}): Promise<void> {
if (typeof opts === "boolean") {
opts = { createNewKey: opts };
}

if (opts.onlyIfKeyCached && !(await this.olmMachine.dehydratedDevices().getDehydratedDeviceKey())) {
return;
}
this.stop();
try {
await this.rehydrateDeviceIfAvailable();
} catch (e) {
// If rehydration fails, there isn't much we can do about it. Log
// the error, and create a new device.
this.logger.info("dehydration: Error rehydrating device:", e);
this.emit(CryptoEvent.RehydrationError, (e as Error).message);
if (opts.rehydrate !== false) {
try {
await this.rehydrateDeviceIfAvailable();
} catch (e) {
// If rehydration fails, there isn't much we can do about it. Log
// the error, and create a new device.
this.logger.info("dehydration: Error rehydrating device:", e);
this.emit(CryptoEvent.RehydrationError, (e as Error).message);
}
}
if (createNewKey) {
if (opts.createNewKey) {
await this.resetKey();
}
await this.scheduleDeviceDehydration();
Expand Down
5 changes: 3 additions & 2 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import {
CryptoEventHandlerMap,
KeyBackupRestoreOpts,
KeyBackupRestoreResult,
StartDehydrationOpts,
} from "../crypto-api/index.ts";
import { deviceKeysToDeviceMap, rustDeviceToJsDevice } from "./device-converter.ts";
import { IDownloadKeyResult, IQueryKeysRequest } from "../client.ts";
Expand Down Expand Up @@ -1425,11 +1426,11 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
/**
* Implementation of {@link CryptoApi#startDehydration}.
*/
public async startDehydration(createNewKey?: boolean): Promise<void> {
public async startDehydration(opts: StartDehydrationOpts | boolean = {}): Promise<void> {
if (!(await this.isCrossSigningReady()) || !(await this.isSecretStorageReady())) {
throw new Error("Device dehydration requires cross-signing and secret storage to be set up");
}
return await this.dehydratedDeviceManager.start(createNewKey);
return await this.dehydratedDeviceManager.start(opts || {});
}

/**
Expand Down
Loading