Skip to content

Commit

Permalink
Implement isSecretStorageReady in rust (#3730)
Browse files Browse the repository at this point in the history 
* Implement isSecretStorageReady in rust

* refactor extract common code to check 4S access

* fix incomplete mocks

* code review

* Remove keyId para from secretStorageCanAccessSecrets

* use map instead of array

* code review
  • Loading branch information
@BillCarsonFr
BillCarsonFr authored Sep 21, 2023
1 parent f134d6d commit 4947a0c
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 80 deletions.
173 changes: 116 additions & 57 deletions spec/integ/crypto/crypto.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { TestClient } from "../../TestClient";
import { logger } from "../../../src/logger";
import {
Category,
ClientEvent,
createClient,
CryptoEvent,
IClaimOTKsResult,
Expand Down Expand Up @@ -68,7 +69,7 @@ import {
mockSetupCrossSigningRequests,
mockSetupMegolmBackupRequests,
} from "../../test-utils/mockEndpoints";
import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage";
import { AddSecretStorageKeyOpts } from "../../../src/secret-storage";
import { CrossSigningKey, CryptoCallbacks, KeyBackupInfo } from "../../../src/crypto-api";
import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
import { DecryptionError } from "../../../src/crypto/algorithms";
Expand Down Expand Up @@ -2283,6 +2284,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});

describe("Secret Storage and Key Backup", () => {
/**
* The account data events to be returned by the sync.
* Will be updated when fecthMock intercepts calls to PUT `/_matrix/client/v3/user/:userId/account_data/`.
* Will be used by `sendSyncResponseWithUpdatedAccountData`
*/
let accountDataEvents: Map<String, any>;

/**
* Create a fake secret storage key
* Async because `bootstrapSecretStorage` expect an async method
Expand All @@ -2294,11 +2302,35 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,

beforeEach(async () => {
createSecretStorageKey.mockClear();

accountDataEvents = new Map();
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
});

function mockGetAccountData() {
fetchMock.get(
`path:/_matrix/client/v3/user/:userId/account_data/:type`,
(url) => {
const type = url.split("/").pop();
const existing = accountDataEvents.get(type!);
if (existing) {
// return it
return {
status: 200,
body: existing.content,
};
} else {
// 404
return {
status: 404,
body: { errcode: "M_NOT_FOUND", error: "Account data not found." },
};
}
},
{ overwriteRoutes: true },
);
}

/**
* Create a mock to respond to the PUT request `/_matrix/client/v3/user/:userId/account_data/m.cross_signing.${key}`
* Resolved when the cross signing key is uploaded
Expand All @@ -2311,6 +2343,9 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
`express:/_matrix/client/v3/user/:userId/account_data/m.cross_signing.${key}`,
(url: string, options: RequestInit) => {
const content = JSON.parse(options.body as string);
const type = url.split("/").pop();
// update account data for sync response
accountDataEvents.set(type!, content);
resolve(content.encrypted);
return {};
},
Expand All @@ -2319,32 +2354,24 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
}

/**
* Send in the sync response the provided `secretStorageKey` into the account_data field
* The key is set for the `m.secret_storage.default_key` and `m.secret_storage.key.${secretStorageKey}` events
* https://spec.matrix.org/v1.6/client-server-api/#get_matrixclientv3sync
* @param secretStorageKey
* Send in the sync response the current account data events, as stored by `accountDataEvents`.
*/
function sendSyncResponse(secretStorageKey: string) {
syncResponder.sendOrQueueSyncResponse({
next_batch: 1,
account_data: {
events: [
{
type: "m.secret_storage.default_key",
content: {
key: secretStorageKey,
},
},
// Needed for secretStorage.getKey or secretStorage.hasKey
{
type: `m.secret_storage.key.${secretStorageKey}`,
content: {
algorithm: SECRET_STORAGE_ALGORITHM_V1_AES,
},
},
],
},
});
function sendSyncResponseWithUpdatedAccountData() {
try {
syncResponder.sendOrQueueSyncResponse({
next_batch: 1,
account_data: {
events: Array.from(accountDataEvents, ([type, content]) => ({
type: type,
content: content,
})),
},
});
} catch (err) {
// Might fail with "Cannot queue more than one /sync response" if called too often.
// It's ok if it fails here, the sync response is cumulative and will contain
// the latest account data.
}
}

/**
Expand All @@ -2359,10 +2386,16 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
fetchMock.put(
"express:/_matrix/client/v3/user/:userId/account_data/:type(m.secret_storage.*)",
(url: string, options: RequestInit) => {
const type = url.split("/").pop();
const content = JSON.parse(options.body as string);

// update account data for sync response
accountDataEvents.set(type!, content);

if (content.key) {
resolve(content.key);
}
sendSyncResponseWithUpdatedAccountData();
return {};
},
{ overwriteRoutes: true },
Expand All @@ -2377,6 +2410,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
`express:/_matrix/client/v3/user/:userId/account_data/m.megolm_backup.v1`,
(url: string, options: RequestInit) => {
const content = JSON.parse(options.body as string);
// update account data for sync response
accountDataEvents.set("m.megolm_backup.v1", content);
resolve(content.encrypted);
return {};
},
Expand All @@ -2385,6 +2420,16 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});
}

function awaitAccountDataUpdate(type: string): Promise<void> {
return new Promise((resolve) => {
aliceClient.on(ClientEvent.AccountData, (ev: MatrixEvent): void => {
if (ev.getType() === type) {
resolve();
}
});
});
}

/**
* Add all mocks needed to setup cross-signing, key backup, 4S and then
* configure the account to have recovery.
Expand Down Expand Up @@ -2422,32 +2467,41 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});

// Wait for the key to be uploaded in the account data
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
await awaitSecretStorageKeyStoredInAccountData();

// Wait for the cross signing keys to be uploaded
await Promise.all(setupPromises);

// wait for bootstrapSecretStorage to finished
await bootstrapPromise;

// Return the newly created key in the sync response
sendSyncResponseWithUpdatedAccountData();

// Finally ensure backup is working
await aliceClient.getCrypto()!.checkKeyBackupAndEnable();

await backupStatusUpdate;
}

describe("bootstrapSecretStorage", () => {
// Doesn't work with legacy crypto, which will try to bootstrap even without private key, which is buggy.
newBackendOnly(
"should throw an error if we are unable to create a key because createSecretStorageKey is not set",
async () => {
await expect(
aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }),
).rejects.toThrow("unable to create a new secret storage key, createSecretStorageKey is not set");

expect(await aliceClient.getCrypto()!.isSecretStorageReady()).toStrictEqual(false);
},
);

it("should create a new key", async () => {
it("Should create a 4S key", async () => {
mockGetAccountData();

const awaitAccountData = awaitAccountDataUpdate("m.secret_storage.default_key");

const bootstrapPromise = aliceClient
.getCrypto()!
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey });
Expand All @@ -2456,52 +2510,55 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
sendSyncResponseWithUpdatedAccountData();

// Finally, wait for bootstrapSecretStorage to finished
await bootstrapPromise;

// await account data updated before getting default key.
await awaitAccountData;

const defaultKeyId = await aliceClient.secretStorage.getDefaultKeyId();
// Check that the uploaded key in stored in the secret storage
expect(await aliceClient.secretStorage.hasKey(secretStorageKey)).toBeTruthy();
// Check that the uploaded key is the default key
expect(defaultKeyId).toBe(secretStorageKey);
});

newBackendOnly(
"should do nothing if an AES key is already in the secret storage and setupNewSecretStorage is not set",
async () => {
const bootstrapPromise = aliceClient
.getCrypto()!
.bootstrapSecretStorage({ createSecretStorageKey });
it("should do nothing if an AES key is already in the secret storage and setupNewSecretStorage is not set", async () => {
const awaitAccountDataClientUpdate = awaitAccountDataUpdate("m.secret_storage.default_key");

const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey });

// Wait for the key to be uploaded in the account data
await awaitSecretStorageKeyStoredInAccountData();

// Wait for the key to be uploaded in the account data
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();
// Return the newly created key in the sync response
sendSyncResponseWithUpdatedAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
// Wait for bootstrapSecretStorage to finished
await bootstrapPromise;

// Wait for bootstrapSecretStorage to finished
await bootstrapPromise;
// On legacy crypto we need to wait for ClientEvent.AccountData before calling bootstrap again.
await awaitAccountDataClientUpdate;

// Call again bootstrapSecretStorage
await aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey });
// Call again bootstrapSecretStorage
await aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey });

// createSecretStorageKey should be called only on the first run of bootstrapSecretStorage
expect(createSecretStorageKey).toHaveBeenCalledTimes(1);
},
);
// createSecretStorageKey should be called only on the first run of bootstrapSecretStorage
expect(createSecretStorageKey).toHaveBeenCalledTimes(1);
});

it("should create a new key if setupNewSecretStorage is at true even if an AES key is already in the secret storage", async () => {
let bootstrapPromise = aliceClient
.getCrypto()!
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey });

// Wait for the key to be uploaded in the account data
let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();
await awaitSecretStorageKeyStoredInAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
sendSyncResponseWithUpdatedAccountData();

// Wait for bootstrapSecretStorage to finished
await bootstrapPromise;
Expand All @@ -2512,10 +2569,10 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey });

// Wait for the key to be uploaded in the account data
secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();
await awaitSecretStorageKeyStoredInAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
sendSyncResponseWithUpdatedAccountData();

// Wait for bootstrapSecretStorage to finished
await bootstrapPromise;
Expand All @@ -2539,7 +2596,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();

// Return the newly created key in the sync response
sendSyncResponse(secretStorageKey);
sendSyncResponseWithUpdatedAccountData();

// Wait for the cross signing keys to be uploaded
const [masterKey, userSigningKey, selfSigningKey] = await Promise.all([
Expand All @@ -2561,6 +2618,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
const backupVersion = "abc";
await bootstrapSecurity(backupVersion);

expect(await aliceClient.getCrypto()!.isSecretStorageReady()).toStrictEqual(true);

// Expect a backup to be available and used
const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion();
expect(activeBackup).toStrictEqual(backupVersion);
Expand Down Expand Up @@ -2596,7 +2655,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
"path:/_matrix/client/v3/room_keys/version",
{
status: 404,
body: { errcode: "M_NOT_FOUND", error: "Account data not found." },
body: { errcode: "M_NOT_FOUND", error: "No current backup version." },
},
{ overwriteRoutes: true },
);
Expand Down
7 changes: 6 additions & 1 deletion spec/unit/rust-crypto/rust-crypto.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ describe("RustCrypto", () => {
it("returns sensible values on a default client", async () => {
const secretStorage = {
isStored: jest.fn().mockResolvedValue(null),
getDefaultKeyId: jest.fn().mockResolvedValue("key"),
} as unknown as Mocked<ServerSideSecretStorage>;
const rustCrypto = await makeTestRustCrypto(undefined, undefined, undefined, secretStorage);

Expand All @@ -232,6 +233,7 @@ describe("RustCrypto", () => {
it("throws if `stop` is called mid-call", async () => {
const secretStorage = {
isStored: jest.fn().mockResolvedValue(null),
getDefaultKeyId: jest.fn().mockResolvedValue(null),
} as unknown as Mocked<ServerSideSecretStorage>;
const rustCrypto = await makeTestRustCrypto(undefined, undefined, undefined, secretStorage);

Expand All @@ -258,7 +260,10 @@ describe("RustCrypto", () => {
});

it("isSecretStorageReady", async () => {
const rustCrypto = await makeTestRustCrypto();
const mockSecretStorage = {
getDefaultKeyId: jest.fn().mockResolvedValue(null),
} as unknown as Mocked<ServerSideSecretStorage>;
const rustCrypto = await makeTestRustCrypto(undefined, undefined, undefined, mockSecretStorage);
await expect(rustCrypto.isSecretStorageReady()).resolves.toBe(false);
});

Expand Down
Loading

0 comments on commit 4947a0c

Please sign in to comment.