Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Element-R: fix repeated requests to enter 4S key during cross-signing…
Browse files Browse the repository at this point in the history 
… reset (#12059)

* Remove redundant `forceReset` parameter

This was always true, so let's get rid of it.

Also some function renames.

* Factor out new `withSecretStorageKeyCache` helper

... so that we can use the cache without the whole of `accessSecretStorage`.

* Cache secret storage key during cross-signing reset

* Playwright test for resetting cross-signing

* CrossSigningPanel: Silence annoying react warnings

React complains if we don't include an explicit `tbody`.

* Simple unit test of reset button

(cherry picked from commit de5931d)
  • Loading branch information
@richvdh @t3chguy
richvdh authored and t3chguy committed Jan 4, 2024
1 parent 951c0d8 commit 46b28d7
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 143 deletions.
37 changes: 37 additions & 0 deletions playwright/e2e/crypto/crypto.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,43 @@ test.describe("Cryptography", function () {
});
}

test("Can reset cross-signing keys", async ({ page, app, user: aliceCredentials }) => {
const secretStorageKey = await enableKeyBackup(app);

// Fetch the current cross-signing keys
async function fetchMasterKey() {
return await test.step("Fetch master key from server", async () => {
const k = await app.client.evaluate(async (cli) => {
const userId = cli.getUserId();
const keys = await cli.downloadKeysForUsers([userId]);
return Object.values(keys.master_keys[userId].keys)[0];
});
console.log(`fetchMasterKey: ${k}`);
return k;
});
}
const masterKey1 = await fetchMasterKey();

// Find the "reset cross signing" button, and click it
await app.settings.openUserSettings("Security & Privacy");
await page.locator("div.mx_CrossSigningPanel_buttonRow").getByRole("button", { name: "Reset" }).click();

// Confirm
await page.getByRole("button", { name: "Clear cross-signing keys" }).click();

// Enter the 4S key
await page.getByPlaceholder("Security Key").fill(secretStorageKey);
await page.getByRole("button", { name: "Continue" }).click();

await expect(async () => {
const masterKey2 = await fetchMasterKey();
expect(masterKey1).not.toEqual(masterKey2);
}).toPass();

// The dialog should have gone away
await expect(page.locator(".mx_Dialog")).toHaveCount(1);
});

test("creating a DM should work, being e2e-encrypted / user verification", async ({
page,
app,
Expand Down
39 changes: 31 additions & 8 deletions src/SecurityManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,28 @@ export async function promptForBackupPassphrase(): Promise<Uint8Array> {
return key;
}

/**
* Carry out an operation that may require multiple accesses to secret storage, caching the key.
*
* Use this helper to wrap an operation that may require multiple accesses to secret storage; the user will be prompted
* to enter the 4S key or passphrase on the first access, and the key will be cached for the rest of the operation.
*
* @param func - The operation to be wrapped.
*/
export async function withSecretStorageKeyCache<T>(func: () => Promise<T>): Promise<T> {
secretStorageBeingAccessed = true;
try {
return await func();
} finally {
// Clear secret storage key cache now that work is complete
secretStorageBeingAccessed = false;
if (!isCachingAllowed()) {
secretStorageKeys = {};
secretStorageKeyInfo = {};
}
}
}

/**
* This helper should be used whenever you need to access secret storage. It
* ensures that secret storage (and also cross-signing since they each depend on
Expand Down Expand Up @@ -326,7 +348,15 @@ export async function accessSecretStorage(
forceReset = false,
setupNewKeyBackup = true,
): Promise<void> {
secretStorageBeingAccessed = true;
await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup));
}

/** Helper for {@link #accessSecretStorage} */
async function doAccessSecretStorage(
func: () => Promise<void>,
forceReset: boolean,
setupNewKeyBackup: boolean,
): Promise<void> {
try {
const cli = MatrixClientPeg.safeGet();
if (!(await cli.hasSecretStorageKey()) || forceReset) {
Expand Down Expand Up @@ -403,13 +433,6 @@ export async function accessSecretStorage(
logger.error(e);
// Re-throw so that higher level logic can abort as needed
throw e;
} finally {
// Clear secret storage key cache now that work is complete
secretStorageBeingAccessed = false;
if (!isCachingAllowed()) {
secretStorageKeys = {};
secretStorageKeyInfo = {};
}
}
}

Expand Down
153 changes: 78 additions & 75 deletions src/components/views/settings/CrossSigningPanel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import Spinner from "../elements/Spinner";
import InteractiveAuthDialog from "../dialogs/InteractiveAuthDialog";
import ConfirmDestroyCrossSigningDialog from "../dialogs/security/ConfirmDestroyCrossSigningDialog";
import SetupEncryptionDialog from "../dialogs/security/SetupEncryptionDialog";
import { accessSecretStorage } from "../../../SecurityManager";
import { accessSecretStorage, withSecretStorageKeyCache } from "../../../SecurityManager";
import AccessibleButton from "../elements/AccessibleButton";
import { SettingsSubsectionText } from "./shared/SettingsSubsection";

Expand Down Expand Up @@ -118,45 +118,46 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {
}

/**
* Bootstrapping cross-signing take one of these paths:
* 1. Create cross-signing keys locally and store in secret storage (if it
* already exists on the account).
* 2. Access existing secret storage by requesting passphrase and accessing
* cross-signing keys as needed.
* 3. All keys are loaded and there's nothing to do.
* @param {bool} [forceReset] Bootstrap again even if keys already present
* Reset the user's cross-signing keys.
*/
private bootstrapCrossSigning = async ({ forceReset = false }): Promise<void> => {
private async resetCrossSigning(): Promise<void> {
this.setState({ error: false });
try {
const cli = MatrixClientPeg.safeGet();
await cli.bootstrapCrossSigning({
authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
const { finished } = Modal.createDialog(InteractiveAuthDialog, {
title: _t("encryption|bootstrap_title"),
matrixClient: cli,
makeRequest,
});
const [confirmed] = await finished;
if (!confirmed) {
throw new Error("Cross-signing key upload auth canceled");
}
},
setupNewCrossSigning: forceReset,
await withSecretStorageKeyCache(async () => {
await cli.getCrypto()!.bootstrapCrossSigning({
authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
const { finished } = Modal.createDialog(InteractiveAuthDialog, {
title: _t("encryption|bootstrap_title"),
matrixClient: cli,
makeRequest,
});
const [confirmed] = await finished;
if (!confirmed) {
throw new Error("Cross-signing key upload auth canceled");
}
},
setupNewCrossSigning: true,
});
});
} catch (e) {
this.setState({ error: true });
logger.error("Error bootstrapping cross-signing", e);
}
if (this.unmounted) return;
this.getUpdatedStatus();
};
}

private resetCrossSigning = (): void => {
/**
* Callback for when the user clicks the "reset cross signing" button.
*
* Shows a confirmation dialog, and then does the reset if confirmed.
*/
private onResetCrossSigningClick = (): void => {
Modal.createDialog(ConfirmDestroyCrossSigningDialog, {
onFinished: (act) => {
onFinished: async (act) => {
if (!act) return;
this.bootstrapCrossSigning({ forceReset: true });
this.resetCrossSigning();
},
});
};
Expand Down Expand Up @@ -243,7 +244,7 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {

if (keysExistAnywhere) {
actions.push(
<AccessibleButton key="reset" kind="danger" onClick={this.resetCrossSigning}>
<AccessibleButton key="reset" kind="danger" onClick={this.onResetCrossSigningClick}>
{_t("action|reset")}
</AccessibleButton>,
);
Expand All @@ -260,54 +261,56 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {
<details>
<summary className="mx_CrossSigningPanel_advanced">{_t("common|advanced")}</summary>
<table className="mx_CrossSigningPanel_statusList">
<tr>
<th scope="row">{_t("settings|security|cross_signing_public_keys")}</th>
<td>
{crossSigningPublicKeysOnDevice
? _t("settings|security|cross_signing_in_memory")
: _t("settings|security|cross_signing_not_found")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_private_keys")}</th>
<td>
{crossSigningPrivateKeysInStorage
? _t("settings|security|cross_signing_in_4s")
: _t("settings|security|cross_signing_not_in_4s")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_master_private_Key")}</th>
<td>
{masterPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_self_signing_private_key")}</th>
<td>
{selfSigningPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_user_signing_private_key")}</th>
<td>
{userSigningPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_homeserver_support")}</th>
<td>
{homeserverSupportsCrossSigning
? _t("settings|security|cross_signing_homeserver_support_exists")
: _t("settings|security|cross_signing_not_found")}
</td>
</tr>
<tbody>
<tr>
<th scope="row">{_t("settings|security|cross_signing_public_keys")}</th>
<td>
{crossSigningPublicKeysOnDevice
? _t("settings|security|cross_signing_in_memory")
: _t("settings|security|cross_signing_not_found")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_private_keys")}</th>
<td>
{crossSigningPrivateKeysInStorage
? _t("settings|security|cross_signing_in_4s")
: _t("settings|security|cross_signing_not_in_4s")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_master_private_Key")}</th>
<td>
{masterPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_self_signing_private_key")}</th>
<td>
{selfSigningPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_user_signing_private_key")}</th>
<td>
{userSigningPrivateKeyCached
? _t("settings|security|cross_signing_cached")
: _t("settings|security|cross_signing_not_cached")}
</td>
</tr>
<tr>
<th scope="row">{_t("settings|security|cross_signing_homeserver_support")}</th>
<td>
{homeserverSupportsCrossSigning
? _t("settings|security|cross_signing_homeserver_support_exists")
: _t("settings|security|cross_signing_not_found")}
</td>
</tr>
</tbody>
</table>
</details>
{errorSection}
Expand Down
21 changes: 21 additions & 0 deletions test/components/views/settings/CrossSigningPanel-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {
mockClientMethodsCrypto,
mockClientMethodsUser,
} from "../../../test-utils";
import Modal from "../../../../src/Modal";
import ConfirmDestroyCrossSigningDialog from "../../../../src/components/views/dialogs/security/ConfirmDestroyCrossSigningDialog";

describe("<CrossSigningPanel />", () => {
const userId = "@alice:server.org";
Expand All @@ -43,6 +45,10 @@ describe("<CrossSigningPanel />", () => {
mockClient.isCrossSigningReady.mockResolvedValue(false);
});

afterEach(() => {
jest.restoreAllMocks();
});

it("should render a spinner while loading", () => {
getComponent();

Expand Down Expand Up @@ -85,6 +91,21 @@ describe("<CrossSigningPanel />", () => {
expect(screen.getByTestId("summarised-status").innerHTML).toEqual("✅ Cross-signing is ready for use.");
expect(screen.getByText("Cross-signing private keys:").parentElement!).toMatchSnapshot();
});

it("should allow reset of cross-signing", async () => {
mockClient.getCrypto()!.bootstrapCrossSigning = jest.fn().mockResolvedValue(undefined);
getComponent();
await flushPromises();

const modalSpy = jest.spyOn(Modal, "createDialog");

screen.getByRole("button", { name: "Reset" }).click();
expect(modalSpy).toHaveBeenCalledWith(ConfirmDestroyCrossSigningDialog, expect.any(Object));
modalSpy.mock.lastCall![1]!.onFinished(true);
expect(mockClient.getCrypto()!.bootstrapCrossSigning).toHaveBeenCalledWith(
expect.objectContaining({ setupNewCrossSigning: true }),
);
});
});

describe("when cross signing is not ready", () => {
Expand Down
Loading

0 comments on commit 46b28d7

Please sign in to comment.