diff --git a/src/appservice/Intent.ts b/src/appservice/Intent.ts index 61a9917e..38c7d1ce 100644 --- a/src/appservice/Intent.ts +++ b/src/appservice/Intent.ts @@ -97,10 +97,12 @@ export class Intent { /** * Sets up crypto on the client if it hasn't already been set up. + * @param providedDeviceId Optional device ID. If given, this will used instead of trying to + * masquerade as the first non-key enabled device. * @returns {Promise} Resolves when complete. */ @timedIntentFunctionCall() - public async enableEncryption(): Promise { + public async enableEncryption(providedDeviceId?: string): Promise { if (!this.cryptoSetupPromise) { // eslint-disable-next-line no-async-promise-executor this.cryptoSetupPromise = new Promise(async (resolve, reject) => { @@ -116,24 +118,38 @@ export class Intent { throw new Error("Failed to create crypto store"); } - // Try to impersonate a device ID - const ownDevices = await this.client.getOwnDevices(); let deviceId = await cryptoStore.getDeviceId(); - if (!deviceId || !ownDevices.some(d => d.device_id === deviceId)) { - const deviceKeys = await this.client.getUserDevices([this.userId]); - const userDeviceKeys = deviceKeys.device_keys[this.userId]; - if (userDeviceKeys) { - // We really should be validating signatures here, but we're actively looking - // for devices without keys to impersonate, so it should be fine. In theory, - // those devices won't even be present but we're cautious. - const devicesWithKeys = Array.from(Object.entries(userDeviceKeys)) - .filter(d => d[0] === d[1].device_id && !!d[1].keys?.[`${DeviceKeyAlgorithm.Curve25519}:${d[1].device_id}`]) - .map(t => t[0]); // grab device ID from tuple - deviceId = ownDevices.find(d => !devicesWithKeys.includes(d.device_id))?.device_id; + if (!providedDeviceId) { + // Try to impersonate a device ID + const ownDevices = await this.client.getOwnDevices(); + let deviceId = await cryptoStore.getDeviceId(); + if (!deviceId || !ownDevices.some(d => d.device_id === deviceId)) { + const deviceKeys = await this.client.getUserDevices([this.userId]); + const userDeviceKeys = deviceKeys.device_keys[this.userId]; + if (userDeviceKeys) { + // We really should be validating signatures here, but we're actively looking + // for devices without keys to impersonate, so it should be fine. In theory, + // those devices won't even be present but we're cautious. + const devicesWithKeys = Array.from(Object.entries(userDeviceKeys)) + .filter(d => d[0] === d[1].device_id && !!d[1].keys?.[`${DeviceKeyAlgorithm.Curve25519}:${d[1].device_id}`]) + .map(t => t[0]); // grab device ID from tuple + deviceId = ownDevices.find(d => !devicesWithKeys.includes(d.device_id))?.device_id; + } } + } else { + if (deviceId && deviceId !== providedDeviceId) { + LogService.warn(`Storage already configured with an existing device ${deviceId}. Old storage will be cleared.`); + } + deviceId = providedDeviceId; } let prepared = false; + if (deviceId) { + const cryptoStore = this.cryptoStorage?.storageForUser(this.userId); + const existingDeviceId = await cryptoStore.getDeviceId(); + if (existingDeviceId && existingDeviceId !== deviceId) { + LogService.warn("Intent", `Device ID has changed for user ${this.userId} from ${existingDeviceId} to ${deviceId}`); + } this.makeClient(true); this.client.impersonateUserId(this.userId, deviceId); diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 110fa090..be8b789d 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -72,23 +72,25 @@ export class CryptoClient { if (this.ready) return; // stop re-preparing here const storedDeviceId = await this.client.cryptoStore.getDeviceId(); - if (storedDeviceId) { - this.deviceId = storedDeviceId; - } else { - const deviceId = (await this.client.getWhoAmI())['device_id']; - if (!deviceId) { - throw new Error("Encryption not possible: server not revealing device ID"); - } - this.deviceId = deviceId; - await this.client.cryptoStore.setDeviceId(this.deviceId); + const { user_id: userId, device_id: deviceId } = (await this.client.getWhoAmI()); + + if (!deviceId) { + throw new Error("Encryption not possible: server not revealing device ID"); + } + + const storagePath = await this.storage.getMachineStoragePath(deviceId); + + if (storedDeviceId !== deviceId) { + this.client.cryptoStore.setDeviceId(deviceId); } + this.deviceId = deviceId; - LogService.debug("CryptoClient", "Starting with device ID:", this.deviceId); + LogService.debug("CryptoClient", `Starting ${userId} with device ID:`, this.deviceId); const machine = await OlmMachine.initialize( - new UserId(await this.client.getUserId()), + new UserId(userId), new DeviceId(this.deviceId), - this.storage.storagePath, "", + storagePath, "", this.storage.storageType, ); this.engine = new RustEngine(machine, this.client); diff --git a/src/storage/IAppserviceStorageProvider.ts b/src/storage/IAppserviceStorageProvider.ts index da9eb2c0..aca046bb 100644 --- a/src/storage/IAppserviceStorageProvider.ts +++ b/src/storage/IAppserviceStorageProvider.ts @@ -47,8 +47,8 @@ export interface IAppserviceStorageProvider { export interface IAppserviceCryptoStorageProvider { /** * Gets a storage provider to use for the given user ID. - * @param {string} userId The user ID. - * @returns {ICryptoStorageProvider} The storage provider. + * @param userId The user ID. + * @returns The storage provider. */ storageForUser(userId: string): ICryptoStorageProvider; } diff --git a/src/storage/RustSdkCryptoStorageProvider.ts b/src/storage/RustSdkCryptoStorageProvider.ts index 200845f5..ce67976b 100644 --- a/src/storage/RustSdkCryptoStorageProvider.ts +++ b/src/storage/RustSdkCryptoStorageProvider.ts @@ -2,6 +2,8 @@ import * as lowdb from "lowdb"; import * as FileSync from "lowdb/adapters/FileSync"; import * as mkdirp from "mkdirp"; import * as path from "path"; +import { stat, rename, mkdir } from "fs/promises"; +import { PathLike } from "fs"; import * as sha512 from "hash.js/lib/hash/sha/512"; import * as sha256 from "hash.js/lib/hash/sha/256"; import { StoreType as RustSdkCryptoStoreType } from "@matrix-org/matrix-sdk-crypto-nodejs"; @@ -9,9 +11,14 @@ import { StoreType as RustSdkCryptoStoreType } from "@matrix-org/matrix-sdk-cryp import { ICryptoStorageProvider } from "./ICryptoStorageProvider"; import { IAppserviceCryptoStorageProvider } from "./IAppserviceStorageProvider"; import { ICryptoRoomInformation } from "../e2ee/ICryptoRoomInformation"; +import { LogService } from "../logging/LogService"; export { RustSdkCryptoStoreType }; +async function doesFileExist(path: PathLike) { + return stat(path).then(() => true).catch(() => false); +} + /** * A crypto storage provider for the file-based rust-sdk store. * @category Storage providers @@ -40,6 +47,37 @@ export class RustSdkCryptoStorageProvider implements ICryptoStorageProvider { }); } + public async getMachineStoragePath(deviceId: string): Promise { + const newPath = path.join(this.storagePath, sha256().update(deviceId).digest('hex')); + if (await doesFileExist(newPath)) { + // Already exists, short circuit. + return newPath; + } // else: If the path does NOT exist we might need to perform a migration. + + const legacyFilePath = path.join(this.storagePath, 'matrix-sdk-crypto.sqlite3'); + // XXX: Slightly gross cross-dependency file name expectations. + if (await doesFileExist(legacyFilePath) === false) { + // No machine files at all, we can skip. + return newPath; + } + const legacyDeviceId = await this.getDeviceId(); + // We need to move the file. + const previousDevicePath = path.join(this.storagePath, sha256().update(legacyDeviceId).digest('hex')); + LogService.warn("RustSdkCryptoStorageProvider", `Migrating path for SDK database for legacy device ${legacyDeviceId}`); + await mkdir(previousDevicePath); + await rename(legacyFilePath, path.join(previousDevicePath, 'matrix-sdk-crypto.sqlite3')).catch((ex) => + LogService.warn("RustSdkCryptoStorageProvider", `Could not migrate matrix-sdk-crypto.sqlite3`, ex), + ); + await rename(legacyFilePath, path.join(previousDevicePath, 'matrix-sdk-crypto.sqlite3-shm')).catch((ex) => + LogService.warn("RustSdkCryptoStorageProvider", `Could not migrate matrix-sdk-crypto.sqlite3-shm`, ex), + ); + await rename(legacyFilePath, path.join(previousDevicePath, 'matrix-sdk-crypto.sqlite3-wal')).catch((ex) => + LogService.warn("RustSdkCryptoStorageProvider", `Could not migrate matrix-sdk-crypto.sqlite3-wal`, ex), + ); + + return newPath; + } + public async getDeviceId(): Promise { return this.db.get('deviceId').value(); } @@ -75,7 +113,7 @@ export class RustSdkAppserviceCryptoStorageProvider extends RustSdkCryptoStorage public storageForUser(userId: string): ICryptoStorageProvider { // sha256 because sha512 is a bit big for some operating systems - const key = sha256().update(userId).digest('hex'); - return new RustSdkCryptoStorageProvider(path.join(this.baseStoragePath, key), this.storageType); + const storagePath = path.join(this.baseStoragePath, sha256().update(userId).digest('hex')); + return new RustSdkCryptoStorageProvider(storagePath, this.storageType); } } diff --git a/test/MatrixClientTest.ts b/test/MatrixClientTest.ts index 33343809..99c3c404 100644 --- a/test/MatrixClientTest.ts +++ b/test/MatrixClientTest.ts @@ -1043,7 +1043,7 @@ describe('MatrixClient', () => { }); it('should request the user ID if it is not known', async () => { - const { client, http } = createTestClient(); + const { client, http } = createTestClient(undefined, undefined, undefined, { handleWhoAmI: false }); const userId = "@example:example.org"; const response = { @@ -1061,7 +1061,7 @@ describe('MatrixClient', () => { describe('getWhoAmI', () => { it('should call the right endpoint', async () => { - const { client, http } = createTestClient(); + const { client, http } = createTestClient(undefined, undefined, undefined, { handleWhoAmI: false }); const response = { user_id: "@user:example.org", diff --git a/test/SynapseAdminApisTest.ts b/test/SynapseAdminApisTest.ts index 1dcd8541..ee3b266a 100644 --- a/test/SynapseAdminApisTest.ts +++ b/test/SynapseAdminApisTest.ts @@ -24,7 +24,7 @@ export function createTestSynapseAdminClient( hsUrl: string; accessToken: string; } { - const result = createTestClient(storage); + const result = createTestClient(storage, undefined, undefined, { handleWhoAmI: false }); const mxClient = result.client; const client = new SynapseAdminApis(mxClient); diff --git a/test/TestUtils.ts b/test/TestUtils.ts index f340e63e..e8d2cfa3 100644 --- a/test/TestUtils.ts +++ b/test/TestUtils.ts @@ -31,6 +31,7 @@ export function createTestClient( storage: IStorageProvider = null, userId: string = null, cryptoStoreType?: StoreType, + opts = { handleWhoAmI: true }, ): { client: MatrixClient; http: HttpBackend; @@ -44,6 +45,11 @@ export function createTestClient( (client).userId = userId; // private member access setRequestFn(http.requestFn); + if (opts.handleWhoAmI) { + // Ensure we always respond to a whoami + client.getWhoAmI = () => Promise.resolve({ user_id: userId, device_id: TEST_DEVICE_ID }); + } + return { http, hsUrl, accessToken, client }; } diff --git a/test/encryption/CryptoClientTest.ts b/test/encryption/CryptoClientTest.ts index f097be90..e5d17322 100644 --- a/test/encryption/CryptoClientTest.ts +++ b/test/encryption/CryptoClientTest.ts @@ -8,7 +8,6 @@ describe('CryptoClient', () => { it('should not have a device ID or be ready until prepared', () => testCryptoStores(async (cryptoStoreType) => { const userId = "@alice:example.org"; const { client, http } = createTestClient(null, userId, cryptoStoreType); - client.getWhoAmI = () => Promise.resolve({ user_id: userId, device_id: TEST_DEVICE_ID }); expect(client.crypto).toBeDefined(); @@ -46,8 +45,9 @@ describe('CryptoClient', () => { const { client, http } = createTestClient(null, userId, cryptoStoreType); await client.cryptoStore.setDeviceId(TEST_DEVICE_ID); + const CORRECT_DEVICE = "new_device"; - const whoamiSpy = simple.stub().callFn(() => Promise.resolve({ user_id: userId, device_id: "wrong" })); + const whoamiSpy = simple.stub().callFn(() => Promise.resolve({ user_id: userId, device_id: CORRECT_DEVICE })); client.getWhoAmI = whoamiSpy; bindNullEngine(http); @@ -55,8 +55,10 @@ describe('CryptoClient', () => { client.crypto.prepare(), http.flushAllExpected(), ]); - expect(whoamiSpy.callCount).toEqual(0); - expect(client.crypto.clientDeviceId).toEqual(TEST_DEVICE_ID); + // This should be called to check + expect(whoamiSpy.callCount).toEqual(1); + expect(client.crypto.clientDeviceId).toEqual(CORRECT_DEVICE); + expect(await client.cryptoStore.getDeviceId()).toEqual(CORRECT_DEVICE); })); });