Skip to content

Commit

Permalink
Rust-crypto: fix bootstrapCrossSigning on second call (#3912)
Browse files Browse the repository at this point in the history
* Rust-crypto: fix `bootstrapCrossSigning` on second call

Currently, `bootstrapCrossSigning` raises an exception if it is called a second
time before secret storage is set up. It is easily fixed by checking that 4S is
set up before trying to export to 4S.

Also a few logging fixes while we're in the area.

* Factor out an `AccountDataAccumulator`

* Another test for bootstrapCrossSigning
  • Loading branch information
richvdh authored Dec 1, 2023
1 parent f4b6f91 commit b515cdb
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 111 deletions.
48 changes: 48 additions & 0 deletions spec/integ/crypto/cross-signing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
} from "../../test-utils/test-data";
import * as testData from "../../test-utils/test-data";
import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
import { AccountDataAccumulator } from "../../test-utils/AccountDataAccumulator";

afterEach(() => {
// reset fake-indexeddb after each test, to make sure we don't leak connections
Expand Down Expand Up @@ -244,6 +245,53 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s
`[${TEST_USER_ID}].[${TEST_DEVICE_ID}].signatures.[${TEST_USER_ID}].[ed25519:${SELF_CROSS_SIGNING_PUBLIC_KEY_BASE64}]`,
);
});

it("can bootstrapCrossSigning twice", async () => {
mockSetupCrossSigningRequests();

const authDict = { type: "test" };
await bootstrapCrossSigning(authDict);

// a second call should do nothing except GET requests
fetchMock.mockClear();
await bootstrapCrossSigning(authDict);
const calls = fetchMock.calls((url, opts) => opts.method != "GET");
expect(calls.length).toEqual(0);
});

newBackendOnly("will upload existing cross-signing keys to an established secret storage", async () => {
// This rather obscure codepath covers the case that:
// - 4S is set up and working
// - our device has private cross-signing keys, but has not published them to 4S
//
// To arrange that, we call `bootstrapCrossSigning` on our main device, and then (pretend to) set up 4S from
// a *different* device. Then, when we call `bootstrapCrossSigning` again, it should do the honours.

mockSetupCrossSigningRequests();
const accountDataAccumulator = new AccountDataAccumulator();
accountDataAccumulator.interceptGetAccountData();

const authDict = { type: "test" };
await bootstrapCrossSigning(authDict);

// Pretend that another device has uploaded a 4S key
accountDataAccumulator.accountDataEvents.set("m.secret_storage.default_key", { key: "key_id" });
accountDataAccumulator.accountDataEvents.set("m.secret_storage.key.key_id", {
key: "keykeykey",
algorithm: SECRET_STORAGE_ALGORITHM_V1_AES,
});

// Prepare for the cross-signing keys
const p = accountDataAccumulator.interceptSetAccountData(":type(m.cross_signing..*)");

await bootstrapCrossSigning(authDict);
await p;

// The cross-signing keys should have been uploaded
expect(accountDataAccumulator.accountDataEvents.has("m.cross_signing.master")).toBeTruthy();
expect(accountDataAccumulator.accountDataEvents.has("m.cross_signing.self_signing")).toBeTruthy();
expect(accountDataAccumulator.accountDataEvents.has("m.cross_signing.user_signing")).toBeTruthy();
});
});

describe("getCrossSigningStatus()", () => {
Expand Down
123 changes: 26 additions & 97 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ import {
getTestOlmAccountKeys,
} from "./olm-utils";
import { ToDevicePayload } from "../../../src/models/ToDeviceMessage";
import { AccountDataAccumulator } from "../../test-utils/AccountDataAccumulator";

afterEach(() => {
// reset fake-indexeddb after each test, to make sure we don't leak connections
Expand Down Expand Up @@ -2438,12 +2439,7 @@ 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>;
let accountDataAccumulator: AccountDataAccumulator;

/**
* Create a fake secret storage key
Expand All @@ -2456,105 +2452,38 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,

beforeEach(async () => {
createSecretStorageKey.mockClear();
accountDataEvents = new Map();
accountDataAccumulator = new AccountDataAccumulator();
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
* https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype
*/
function awaitCrossSigningKeyUpload(key: string): Promise<Record<string, {}>> {
return new Promise((resolve) => {
// Called when the cross signing key is uploaded
fetchMock.put(
`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 {};
},
);
});
}

/**
* Send in the sync response the current account data events, as stored by `accountDataEvents`.
*/
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.
}
async function awaitCrossSigningKeyUpload(key: string): Promise<Record<string, {}>> {
const content = await accountDataAccumulator.interceptSetAccountData(`m.cross_signing.${key}`);
return content.encrypted;
}

/**
* Create a mock to respond to the PUT request `/_matrix/client/v3/user/:userId/account_data/:type(m.secret_storage.*)`
* Resolved when a key is uploaded (ie in `body.content.key`)
* https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype
*/
function awaitSecretStorageKeyStoredInAccountData(): Promise<string> {
return new Promise((resolve) => {
// This url is called multiple times during the secret storage bootstrap process
// When we received the newly generated key, we return it
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 },
);
});
async function awaitSecretStorageKeyStoredInAccountData(): Promise<string> {
// eslint-disable-next-line no-constant-condition
while (true) {
const content = await accountDataAccumulator.interceptSetAccountData(":type(m.secret_storage.*)", {
repeat: 1,
overwriteRoutes: true,
});
accountDataAccumulator.sendSyncResponseWithUpdatedAccountData(syncResponder);
if (content.key) {
return content.key;
}
}
}

function awaitMegolmBackupKeyUpload(): Promise<Record<string, {}>> {
Expand All @@ -2565,7 +2494,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
(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);
accountDataAccumulator.accountDataEvents.set("m.megolm_backup.v1", content);
resolve(content.encrypted);
return {};
},
Expand Down Expand Up @@ -2630,7 +2559,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
await bootstrapPromise;

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

// Finally ensure backup is working
await aliceClient.getCrypto()!.checkKeyBackupAndEnable();
Expand All @@ -2652,7 +2581,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
);

it("Should create a 4S key", async () => {
mockGetAccountData();
accountDataAccumulator.interceptGetAccountData();

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

Expand All @@ -2664,7 +2593,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData();

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

// Finally, wait for bootstrapSecretStorage to finished
await bootstrapPromise;
Expand All @@ -2688,7 +2617,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
await awaitSecretStorageKeyStoredInAccountData();

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

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

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

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

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

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

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

// Wait for the cross signing keys to be uploaded
const [masterKey, userSigningKey, selfSigningKey] = await Promise.all([
Expand Down
108 changes: 108 additions & 0 deletions spec/test-utils/AccountDataAccumulator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import fetchMock from "fetch-mock-jest";
import { MockOptionsMethodPut } from "fetch-mock";

import { ISyncResponder } from "./SyncResponder";

/**
* An object which intercepts `account_data` get and set requests via fetch-mock.
*/
export class AccountDataAccumulator {
/**
* The account data events to be returned by the sync.
* Will be updated when fetchMock intercepts calls to PUT `/_matrix/client/v3/user/:userId/account_data/`.
* Will be used by `sendSyncResponseWithUpdatedAccountData`
*/
public accountDataEvents: Map<String, any> = new Map();

/**
* Intercept requests to set a particular type of account data.
*
* Once it is set, its data is stored (for future return by `interceptGetAccountData` etc) and the resolved promise is
* resolved.
*
* @param accountDataType - type of account data to be intercepted
* @param opts - options to pass to fetchMock
* @returns a Promise which will resolve (with the content of the account data) once it is set.
*/
public interceptSetAccountData(accountDataType: string, opts?: MockOptionsMethodPut): Promise<any> {
return new Promise((resolve) => {
// Called when the cross signing key is uploaded
fetchMock.put(
`express:/_matrix/client/v3/user/:userId/account_data/${accountDataType}`,
(url: string, options: RequestInit) => {
const content = JSON.parse(options.body as string);
const type = url.split("/").pop();
// update account data for sync response
this.accountDataEvents.set(type!, content);
resolve(content);
return {};
},
opts,
);
});
}

/**
* Intercept all requests to get account data
*/
public interceptGetAccountData(): void {
fetchMock.get(
`express:/_matrix/client/v3/user/:userId/account_data/:type`,
(url) => {
const type = url.split("/").pop();
const existing = this.accountDataEvents.get(type!);
if (existing) {
// return it
return {
status: 200,
body: existing,
};
} else {
// 404
return {
status: 404,
body: { errcode: "M_NOT_FOUND", error: "Account data not found." },
};
}
},
{ overwriteRoutes: true },
);
}

/**
* Send a sync response the current account data events.
*/
public sendSyncResponseWithUpdatedAccountData(syncResponder: ISyncResponder): void {
try {
syncResponder.sendOrQueueSyncResponse({
next_batch: 1,
account_data: {
events: Array.from(this.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.
}
}
}
Loading

0 comments on commit b515cdb

Please sign in to comment.