Element-R: pass pickleKey in as raw key for indexeddb encryption (#12543)
* Element-R: pass pickleKey in as raw key for indexeddb encryption Currently, we pass the `pickleKey` to the rust library for use as a passphrase for encrypting its crypto store. The Rust libary then passes that passphrase through 200000 rounds of PBKDF2 to generate an encryption key, which is (deliberately) slow. However, the pickleKey is actually 32 bytes of random data (base64-encoded). By passing the raw key into the rust library, we can therefore save the PBKDF operation. Backwards-compatibility with existing sessions is maintained, because if the rust library discovers that the store was previously encrypted with a key based on a PBKDF, it will re-base64 and PBKDF the key we provide, thus reconstructing the right key. * Update src/Lifecycle.ts Co-authored-by: Florian Duros <florianduros@element.io> * Lifecycle-test: clean up test setup Rely less on the unit under test for setting up the test preconditions -- not least because we don't really want to fire up matrix clients and the like during test setup. * Factor out "encryptPickleKey" method For a start it makes it easier to grok what's going on, but also I went to use this in a test * Improve tests for `Lifecycle.restoreFromLocalStorage` --------- Co-authored-by: Florian Duros <florianduros@element.io>
This commit is contained in:
Richard van der Hoff
GitHub
parent
5004456d82
commit
0a01320fca
6 changed files with 233 additions and 64 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue