diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index e7e4ff7e3cd..3c515dff84a 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -38,7 +38,7 @@ import { idbLoad, idbSave, idbDelete } from "./utils/StorageAccess"; import { ViewRoomPayload } from "./dispatcher/payloads/ViewRoomPayload"; import { IConfigOptions } from "./IConfigOptions"; import SdkConfig from "./SdkConfig"; -import { buildAndEncodePickleKey, getPickleAdditionalData } from "./utils/tokens/pickling"; +import { buildAndEncodePickleKey, encryptPickleKey } from "./utils/tokens/pickling"; export const SSO_HOMESERVER_URL_KEY = "mx_sso_hs_url"; export const SSO_ID_SERVER_URL_KEY = "mx_sso_is_url"; @@ -378,24 +378,16 @@ export default abstract class BasePlatform { * support storing pickle keys. */ public async createPickleKey(userId: string, deviceId: string): Promise { - if (!window.crypto || !window.crypto.subtle) { - return null; - } - const crypto = window.crypto; const randomArray = new Uint8Array(32); crypto.getRandomValues(randomArray); - const cryptoKey = await crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, false, [ - "encrypt", - "decrypt", - ]); - const iv = new Uint8Array(32); - crypto.getRandomValues(iv); - - const additionalData = getPickleAdditionalData(userId, deviceId); - const encrypted = await crypto.subtle.encrypt({ name: "AES-GCM", iv, additionalData }, cryptoKey, randomArray); + const data = await encryptPickleKey(randomArray, userId, deviceId); + if (data === undefined) { + // no crypto support + return null; + } try { - await idbSave("pickleKey", [userId, deviceId], { encrypted, iv, cryptoKey }); + await idbSave("pickleKey", [userId, deviceId], data); } catch (e) { return null; } diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 8b04f74afcb..90f320409fc 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -18,12 +18,12 @@ limitations under the License. */ import { ReactNode } from "react"; -import { createClient, MatrixClient, SSOAction, OidcTokenRefresher } from "matrix-js-sdk/src/matrix"; +import { createClient, MatrixClient, SSOAction, OidcTokenRefresher, decodeBase64 } from "matrix-js-sdk/src/matrix"; import { IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; import { QueryDict } from "matrix-js-sdk/src/utils"; import { logger } from "matrix-js-sdk/src/logger"; -import { IMatrixClientCreds, MatrixClientPeg } from "./MatrixClientPeg"; +import { IMatrixClientCreds, MatrixClientPeg, MatrixClientPegAssignOpts } from "./MatrixClientPeg"; import { ModuleRunner } from "./modules/ModuleRunner"; import EventIndexPeg from "./indexing/EventIndexPeg"; import createMatrixClient from "./utils/createMatrixClient"; @@ -422,6 +422,7 @@ async function onSuccessfulDelegatedAuthLogin(credentials: IMatrixClientCreds): } type TryAgainFunction = () => void; + /** * Display a friendly error to the user when token login or OIDC authorization fails * @param description error description @@ -821,7 +822,23 @@ async function doSetLoggedIn(credentials: IMatrixClientCreds, clearStorageEnable checkSessionLock(); dis.fire(Action.OnLoggedIn); - await startMatrixClient(client, /*startSyncing=*/ !softLogout); + + const clientPegOpts: MatrixClientPegAssignOpts = {}; + if (credentials.pickleKey) { + // The pickleKey, if provided, is probably a base64-encoded 256-bit key, so can be used for the crypto store. + if (credentials.pickleKey.length === 43) { + clientPegOpts.rustCryptoStoreKey = decodeBase64(credentials.pickleKey); + } else { + // We have some legacy pickle key. Continue using it as a password. + clientPegOpts.rustCryptoStorePassword = credentials.pickleKey; + } + } + + try { + await startMatrixClient(client, /*startSyncing=*/ !softLogout, clientPegOpts); + } finally { + clientPegOpts.rustCryptoStoreKey?.fill(0); + } return client; } @@ -955,11 +972,16 @@ export function isLoggingOut(): boolean { /** * Starts the matrix client and all other react-sdk services that * listen for events while a session is logged in. + * * @param client the matrix client to start - * @param {boolean} startSyncing True (default) to actually start - * syncing the client. + * @param startSyncing - `true` to actually start syncing the client. + * @param clientPegOpts - Options to pass through to {@link MatrixClientPeg.start}. */ -async function startMatrixClient(client: MatrixClient, startSyncing = true): Promise { +async function startMatrixClient( + client: MatrixClient, + startSyncing: boolean, + clientPegOpts: MatrixClientPegAssignOpts, +): Promise { logger.log(`Lifecycle: Starting MatrixClient`); // dispatch this before starting the matrix client: it's used @@ -990,10 +1012,10 @@ async function startMatrixClient(client: MatrixClient, startSyncing = true): Pro // index (e.g. the FilePanel), therefore initialize the event index // before the client. await EventIndexPeg.init(); - await MatrixClientPeg.start(); + await MatrixClientPeg.start(clientPegOpts); } else { logger.warn("Caller requested only auxiliary services be started"); - await MatrixClientPeg.assign(); + await MatrixClientPeg.assign(clientPegOpts); } checkSessionLock(); diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index a1c277fc28b..72340cb35fc 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -66,6 +66,27 @@ export interface IMatrixClientCreds { freshLogin?: boolean; } +export interface MatrixClientPegAssignOpts { + /** + * If we are using Rust crypto, a key with which to encrypt the indexeddb. + * + * If provided, it must be exactly 32 bytes of data. If both this and + * {@link MatrixClientPegAssignOpts.rustCryptoStorePassword} are undefined, + * the store will be unencrypted. + */ + rustCryptoStoreKey?: Uint8Array; + + /** + * If we are using Rust crypto, a password which will be used to derive a key to encrypt the store with. + * + * An alternative to {@link MatrixClientPegAssignOpts.rustCryptoStoreKey}. Ignored if `rustCryptoStoreKey` is set. + * + * Deriving a key from a password is (deliberately) a slow operation, so prefer to pass a `rustCryptoStoreKey` + * directly where possible. + */ + rustCryptoStorePassword?: string; +} + /** * Holds the current instance of the `MatrixClient` to use across the codebase. * Looking for an `MatrixClient`? Just look for the `MatrixClientPeg` on the peg @@ -94,14 +115,14 @@ export interface IMatrixClientPeg { unset(): void; /** - * Prepare the MatrixClient for use, including initialising the store and crypto, but do not start it + * Prepare the MatrixClient for use, including initialising the store and crypto, but do not start it. */ - assign(): Promise; + assign(opts?: MatrixClientPegAssignOpts): Promise; /** - * Prepare the MatrixClient for use, including initialising the store and crypto, and start it + * Prepare the MatrixClient for use, including initialising the store and crypto, and start it. */ - start(): Promise; + start(opts?: MatrixClientPegAssignOpts): Promise; /** * If we've registered a user ID we set this to the ID of the @@ -248,7 +269,10 @@ class MatrixClientPegClass implements IMatrixClientPeg { PlatformPeg.get()?.reload(); }; - public async assign(): Promise { + /** + * Implementation of {@link IMatrixClientPeg.assign}. + */ + public async assign(assignOpts: MatrixClientPegAssignOpts = {}): Promise { if (!this.matrixClient) { throw new Error("createClient must be called first"); } @@ -275,7 +299,7 @@ class MatrixClientPegClass implements IMatrixClientPeg { // try to initialise e2e on the new client if (!SettingsStore.getValue("lowBandwidth")) { - await this.initClientCrypto(); + await this.initClientCrypto(assignOpts.rustCryptoStoreKey, assignOpts.rustCryptoStorePassword); } const opts = utils.deepCopy(this.opts); @@ -301,8 +325,16 @@ class MatrixClientPegClass implements IMatrixClientPeg { /** * Attempt to initialize the crypto layer on a newly-created MatrixClient + * + * @param rustCryptoStoreKey - If we are using Rust crypto, a key with which to encrypt the indexeddb. + * If provided, it must be exactly 32 bytes of data. If both this and `rustCryptoStorePassword` are + * undefined, the store will be unencrypted. + * + * @param rustCryptoStorePassword - An alternative to `rustCryptoStoreKey`. Ignored if `rustCryptoStoreKey` is set. + * A password which will be used to derive a key to encrypt the store with. Deriving a key from a password is + * (deliberately) a slow operation, so prefer to pass a `rustCryptoStoreKey` directly where possible. */ - private async initClientCrypto(): Promise { + private async initClientCrypto(rustCryptoStoreKey?: Uint8Array, rustCryptoStorePassword?: string): Promise { if (!this.matrixClient) { throw new Error("createClient must be called first"); } @@ -338,7 +370,13 @@ class MatrixClientPegClass implements IMatrixClientPeg { // Now we can initialise the right crypto impl. if (useRustCrypto) { - await this.matrixClient.initRustCrypto(); + if (!rustCryptoStoreKey && !rustCryptoStorePassword) { + logger.error("Warning! Not using an encryption key for rust crypto store."); + } + await this.matrixClient.initRustCrypto({ + storageKey: rustCryptoStoreKey, + storagePassword: rustCryptoStorePassword, + }); StorageManager.setCryptoInitialised(true); // TODO: device dehydration and whathaveyou @@ -367,8 +405,11 @@ class MatrixClientPegClass implements IMatrixClientPeg { } } - public async start(): Promise { - const opts = await this.assign(); + /** + * Implementation of {@link IMatrixClientPeg.start}. + */ + public async start(assignOpts?: MatrixClientPegAssignOpts): Promise { + const opts = await this.assign(assignOpts); logger.log(`MatrixClientPeg: really starting MatrixClient`); await this.matrixClient!.startClient(opts); diff --git a/src/utils/tokens/pickling.ts b/src/utils/tokens/pickling.ts index c113559a69a..9e096bedef4 100644 --- a/src/utils/tokens/pickling.ts +++ b/src/utils/tokens/pickling.ts @@ -20,6 +20,20 @@ limitations under the License. import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; +/** + * Encrypted format of a pickle key, as stored in IndexedDB. + */ +export interface EncryptedPickleKey { + /** The encrypted payload. */ + encrypted?: BufferSource; + + /** Initialisation vector for the encryption. */ + iv?: BufferSource; + + /** The encryption key which was used to encrypt the payload. */ + cryptoKey?: CryptoKey; +} + /** * Calculates the `additionalData` for the AES-GCM key used by the pickling processes. This * additional data is *not* encrypted, but *is* authenticated. The additional data is constructed @@ -46,6 +60,32 @@ export function getPickleAdditionalData(userId: string, deviceId: string): Uint8 return additionalData; } +/** + * Encrypt the given pickle key, ready for storage in the database. + * + * @param pickleKey - The key to be encrypted. + * @param userId - The user ID the pickle key belongs to. + * @param deviceId - The device ID the pickle key belongs to. + * + * @returns Data object ready for storing in indexeddb. + */ +export async function encryptPickleKey( + pickleKey: Uint8Array, + userId: string, + deviceId: string, +): Promise { + if (!crypto?.subtle) { + return undefined; + } + const cryptoKey = await crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, false, ["encrypt", "decrypt"]); + const iv = new Uint8Array(32); + crypto.getRandomValues(iv); + + const additionalData = getPickleAdditionalData(userId, deviceId); + const encrypted = await crypto.subtle.encrypt({ name: "AES-GCM", iv, additionalData }, cryptoKey, pickleKey); + return { encrypted, iv, cryptoKey }; +} + /** * Decrypts the provided data into a pickle key and base64-encodes it ready for use elsewhere. * @@ -59,7 +99,7 @@ export function getPickleAdditionalData(userId: string, deviceId: string): Uint8 * @returns A promise that resolves to the encoded pickle key, or undefined if the key cannot be built and encoded. */ export async function buildAndEncodePickleKey( - data: { encrypted?: BufferSource; iv?: BufferSource; cryptoKey?: CryptoKey } | undefined, + data: EncryptedPickleKey | undefined, userId: string, deviceId: string, ): Promise { diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 4a6122f4709..55fbb0f1ff1 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -17,9 +17,10 @@ limitations under the License. import { Crypto } from "@peculiar/webcrypto"; import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; +import { decodeBase64, encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix"; import { setCrypto } from "matrix-js-sdk/src/crypto/crypto"; import * as MatrixCryptoAes from "matrix-js-sdk/src/crypto/aes"; -import { MockedObject } from "jest-mock"; +import { mocked, MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; @@ -27,11 +28,15 @@ import { logout, restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; import * as StorageAccess from "../src/utils/StorageAccess"; +import { idbSave } from "../src/utils/StorageAccess"; import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser, mockPlatformPeg } from "./test-utils"; import { OidcClientStore } from "../src/stores/oidc/OidcClientStore"; import { makeDelegatedAuthConfig } from "./test-utils/oidc"; import { persistOidcAuthenticatedSettings } from "../src/utils/oidc/persistOidcSettings"; import { Action } from "../src/dispatcher/actions"; +import PlatformPeg from "../src/PlatformPeg"; +import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../src/utils/tokens/tokens"; +import { encryptPickleKey } from "../src/utils/tokens/pickling"; const webCrypto = new Crypto(); @@ -220,22 +225,18 @@ describe("Lifecycle", () => { }); describe("when session is found in storage", () => { - beforeEach(() => { - initLocalStorageMock(localStorageSession); - initIdbMock(idbStorageSession); - }); - describe("guest account", () => { - it("should ignore guest accounts when ignoreGuest is true", async () => { + beforeEach(() => { initLocalStorageMock({ ...localStorageSession, mx_is_guest: "true" }); + initIdbMock(idbStorageSession); + }); + it("should ignore guest accounts when ignoreGuest is true", async () => { expect(await restoreFromLocalStorage({ ignoreGuest: true })).toEqual(false); expect(logger.log).toHaveBeenCalledWith(`Ignoring stored guest account: ${userId}`); }); it("should restore guest accounts when ignoreGuest is false", async () => { - initLocalStorageMock({ ...localStorageSession, mx_is_guest: "true" }); - expect(await restoreFromLocalStorage({ ignoreGuest: false })).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( @@ -250,6 +251,11 @@ describe("Lifecycle", () => { }); describe("without a pickle key", () => { + beforeEach(() => { + initLocalStorageMock(localStorageSession); + initIdbMock(idbStorageSession); + }); + it("should persist credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); @@ -272,7 +278,7 @@ describe("Lifecycle", () => { expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken); }); - it("should create new matrix client with credentials", async () => { + it("should create and start new matrix client with credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( @@ -288,6 +294,8 @@ describe("Lifecycle", () => { }, undefined, ); + + expect(MatrixClientPeg.start).toHaveBeenCalledWith({}); }); it("should remove fresh login flag from session storage", async () => { @@ -341,12 +349,20 @@ describe("Lifecycle", () => { }); }); - describe("with a pickle key", () => { + describe("with a normal pickle key", () => { + let pickleKey: string; + beforeEach(async () => { - initLocalStorageMock({}); + initLocalStorageMock(localStorageSession); initIdbMock({}); - // setup storage with a session with encrypted token - await setLoggedIn(credentials); + + // Create a pickle key, and store it, encrypted, in IDB. + pickleKey = (await PlatformPeg.get()!.createPickleKey(credentials.userId, credentials.deviceId))!; + + // Indicate that we should have a pickle key + localStorage.setItem("mx_has_pickle_key", "true"); + + await persistAccessTokenInStorage(credentials.accessToken, pickleKey); }); it("should persist credentials", async () => { @@ -383,9 +399,17 @@ describe("Lifecycle", () => { expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken); }); - it("should create new matrix client with credentials", async () => { + it("should create and start new matrix client with credentials", async () => { + // Check that the rust crypto key is as expected. We have to do this during the call, as + // the buffer is cleared afterwards. + mocked(MatrixClientPeg.start).mockImplementation(async (opts) => { + expect(opts?.rustCryptoStoreKey).toEqual(decodeBase64(pickleKey)); + }); + + // Perform the restore expect(await restoreFromLocalStorage()).toEqual(true); + // Ensure that the expected calls were made expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( { userId, @@ -394,23 +418,19 @@ describe("Lifecycle", () => { homeserverUrl, identityServerUrl, deviceId, - freshLogin: true, + freshLogin: false, guest: false, - pickleKey: expect.any(String), + pickleKey, }, undefined, ); + + expect(MatrixClientPeg.start).toHaveBeenCalledWith({ rustCryptoStoreKey: expect.any(Buffer) }); }); describe("with a refresh token", () => { beforeEach(async () => { - initLocalStorageMock({}); - initIdbMock({}); - // setup storage with a session with encrypted token - await setLoggedIn({ - ...credentials, - refreshToken, - }); + await persistRefreshTokenInStorage(refreshToken, pickleKey); }); it("should persist credentials", async () => { @@ -439,7 +459,7 @@ describe("Lifecycle", () => { deviceId, freshLogin: false, guest: false, - pickleKey: expect.any(String), + pickleKey: pickleKey, }, undefined, ); @@ -447,7 +467,60 @@ describe("Lifecycle", () => { }); }); + describe("with a non-standard pickle key", () => { + // Most pickle keys are 43 bytes of base64. Test what happens when it is something else. + let pickleKey: string; + + beforeEach(async () => { + initLocalStorageMock(localStorageSession); + initIdbMock({}); + + // Generate the pickle key. I don't *think* it's possible for there to be a pickle key + // which is not some amount of base64. + const rawPickleKey = new Uint8Array(10); + crypto.getRandomValues(rawPickleKey); + pickleKey = encodeUnpaddedBase64(rawPickleKey); + + // Store it, encrypted, in the db + await idbSave( + "pickleKey", + [userId, deviceId], + (await encryptPickleKey(rawPickleKey, userId, deviceId))!, + ); + + // Indicate that we should have a pickle key + localStorage.setItem("mx_has_pickle_key", "true"); + + await persistAccessTokenInStorage(credentials.accessToken, pickleKey); + }); + + it("should create and start new matrix client with credentials", async () => { + // Perform the restore + expect(await restoreFromLocalStorage()).toEqual(true); + + // Ensure that the expected calls were made + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + // decrypted accessToken + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey, + }, + undefined, + ); + + expect(MatrixClientPeg.start).toHaveBeenCalledWith({ rustCryptoStorePassword: pickleKey }); + }); + }); + it("should proceed if server is not accessible", async () => { + initLocalStorageMock(localStorageSession); + initIdbMock(idbStorageSession); mockClient.isVersionSupported.mockRejectedValue(new Error("Oh, noes, the server is down!")); expect(await restoreFromLocalStorage()).toEqual(true); diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 2ed08e0a21f..e5585f8cc3c 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -217,7 +217,7 @@ describe("MatrixClientPeg", () => { testPeg.safeGet().store.on = emitter.on.bind(emitter); const platform: any = { reload: jest.fn() }; PlatformPeg.set(platform); - await testPeg.assign(); + await testPeg.assign({}); emitter.emit("closed" as any); expect(platform.reload).toHaveBeenCalled(); }); @@ -229,7 +229,7 @@ describe("MatrixClientPeg", () => { PlatformPeg.set(platform); testPeg.safeGet().store.on = emitter.on.bind(emitter); const spy = jest.spyOn(Modal, "createDialog"); - await testPeg.assign(); + await testPeg.assign({}); emitter.emit("closed" as any); expect(spy).toHaveBeenCalled(); }); @@ -243,9 +243,10 @@ describe("MatrixClientPeg", () => { const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); - await testPeg.start(); + const cryptoStoreKey = new Uint8Array([1, 2, 3, 4]); + await testPeg.start({ rustCryptoStoreKey: cryptoStoreKey }); expect(mockInitCrypto).not.toHaveBeenCalled(); - expect(mockInitRustCrypto).toHaveBeenCalledTimes(1); + expect(mockInitRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey }); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); @@ -271,7 +272,7 @@ describe("MatrixClientPeg", () => { await testPeg.start(); expect(mockInitCrypto).toHaveBeenCalled(); - expect(mockInitRustCrypto).not.toHaveBeenCalledTimes(1); + expect(mockInitRustCrypto).not.toHaveBeenCalled(); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, false);