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

Element-R: fix repeated requests to enter 4S key during cross-signing reset #12059

Merged
merged 6 commits into from
Dec 15, 2023
Merged

Element-R: fix repeated requests to enter 4S key during cross-signing reset #12059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5bdd12
Remove redundant `forceReset` parameter
richvdh Dec 15, 2023
50d9fc5
Factor out new `withSecretStorageKeyCache` helper
richvdh Dec 15, 2023
ab2e125
Cache secret storage key during cross-signing reset
richvdh Dec 15, 2023
1a6e0c8
Playwright test for resetting cross-signing
richvdh Dec 15, 2023
cf16101
CrossSigningPanel: Silence annoying react warnings
richvdh Dec 15, 2023
a5b919f
Simple unit test of reset button
richvdh Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions playwright/e2e/crypto/crypto.spec.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading