From 7912c744cc9a352eb256ec708f69defdbcdcbb22 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 3 Aug 2023 17:08:20 +0200 Subject: [PATCH 1/6] Use new crypto-api for cross user verification --- src/DeviceListener.ts | 5 ++++- src/components/views/right_panel/EncryptionPanel.tsx | 2 +- src/components/views/right_panel/UserInfo.tsx | 5 ++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index d7c845943b9..5b1a0932832 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -291,7 +291,10 @@ export default class DeviceListener { // cross signing isn't enabled - nag to enable it // There are 3 different toasts for: - if (!(await crypto.getCrossSigningKeyId()) && cli.getStoredCrossSigningForUser(cli.getSafeUserId())) { + if ( + !(await crypto.getCrossSigningKeyId()) && + (await cli.getStoredCrossSigningForUser(cli.getSafeUserId())) + ) { // Cross-signing on account but this device doesn't trust the master key (verify this session) showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION); this.checkKeyBackupStatus(); diff --git a/src/components/views/right_panel/EncryptionPanel.tsx b/src/components/views/right_panel/EncryptionPanel.tsx index ede0e96c99b..61bcfc0617e 100644 --- a/src/components/views/right_panel/EncryptionPanel.tsx +++ b/src/components/views/right_panel/EncryptionPanel.tsx @@ -111,7 +111,7 @@ const EncryptionPanel: React.FC = (props: IProps) => { if (!roomId) { throw new Error("Unable to create Room for verification"); } - verificationRequest_ = await cli.requestVerificationDM(member.userId, roomId); + verificationRequest_ = await cli.getCrypto()!.requestVerificationDM(member.userId, roomId); } catch (e) { console.error("Error starting verification", e); setRequesting(false); diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index 9a74cc60571..d0b143c7141 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -151,9 +151,8 @@ function useHasCrossSigningKeys( try { // We call it to populate the user keys and devices await cli.getCrypto()?.getUserDeviceInfo([member.userId], true); - const xsi = cli.getStoredCrossSigningForUser(member.userId); - const key = xsi && xsi.getId(); - return !!key; + const xsi = await cli.getStoredCrossSigningForUser(member.userId); + return Boolean(xsi?.getId()); } finally { setUpdating(false); } From ce54b5ad5eec0790636d0e1042fd38969b3860b2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 1 Sep 2023 23:30:18 +0100 Subject: [PATCH 2/6] update verification flow with new APIs --- src/DeviceListener.ts | 5 +---- src/components/views/right_panel/UserInfo.tsx | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 2a8da3a06ab..f947c1b9edb 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -307,10 +307,7 @@ export default class DeviceListener { // cross signing isn't enabled - nag to enable it // There are 3 different toasts for: - if ( - !(await crypto.getCrossSigningKeyId()) && - (await cli.getStoredCrossSigningForUser(cli.getSafeUserId())) - ) { + if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) { // Cross-signing on account but this device doesn't trust the master key (verify this session) showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION); this.checkKeyBackupStatus(); diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index e871e641091..f405056c39d 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -152,10 +152,7 @@ function useHasCrossSigningKeys( } setUpdating(true); try { - // We call it to populate the user keys and devices - await cli.getCrypto()?.getUserDeviceInfo([member.userId], true); - const xsi = await cli.getStoredCrossSigningForUser(member.userId); - return Boolean(xsi?.getId()); + return await cli.getCrypto()?.userHasCrossSigningKeys(member.userId, true); } finally { setUpdating(false); } From 3a9864f05a1e937aadd88cded59a3ba14c7fe5de Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 1 Sep 2023 23:39:32 +0100 Subject: [PATCH 3/6] Replace some calls to `checkUserTrust` A start on https://github.com/vector-im/crypto-internal/issues/147 --- src/components/views/right_panel/UserInfo.tsx | 12 +++++++++--- src/utils/ShieldUtils.ts | 14 ++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index f405056c39d..63bd90e7e43 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -105,9 +105,15 @@ export const disambiguateDevices = (devices: IDevice[]): void => { } }; -export const getE2EStatus = async (cli: MatrixClient, userId: string, devices: IDevice[]): Promise => { +export const getE2EStatus = async ( + cli: MatrixClient, + userId: string, + devices: IDevice[], +): Promise => { + const crypto = cli.getCrypto(); + if (!crypto) return undefined; const isMe = userId === cli.getUserId(); - const userTrust = cli.checkUserTrust(userId); + const userTrust = await crypto.getUserVerificationStatus(userId); if (!userTrust.isCrossSigningVerified()) { return userTrust.wasCrossSigningVerified() ? E2EStatus.Warning : E2EStatus.Normal; } @@ -119,7 +125,7 @@ export const getE2EStatus = async (cli: MatrixClient, userId: string, devices: I // cross-signing so that other users can then safely trust you. // For other people's devices, the more general verified check that // includes locally verified devices can be used. - const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, deviceId); + const deviceTrust = await crypto.getDeviceVerificationStatus(userId, deviceId); return isMe ? !deviceTrust?.crossSigningVerified : !deviceTrust?.isVerified(); }); return anyDeviceUnverified ? E2EStatus.Warning : E2EStatus.Verified; diff --git a/src/utils/ShieldUtils.ts b/src/utils/ShieldUtils.ts index d43f1568a55..fa01778b67f 100644 --- a/src/utils/ShieldUtils.ts +++ b/src/utils/ShieldUtils.ts @@ -37,17 +37,15 @@ export async function shieldStatusForRoom(client: MatrixClient, room: Room): Pro const verified: string[] = []; const unverified: string[] = []; - members - .filter((userId) => userId !== client.getUserId()) - .forEach((userId) => { - (client.checkUserTrust(userId).isCrossSigningVerified() ? verified : unverified).push(userId); - }); + for (const userId of members) { + if (userId === client.getUserId()) continue; + const userTrust = await crypto.getUserVerificationStatus(userId); - /* Alarm if any unverified users were verified before. */ - for (const userId of unverified) { - if (client.checkUserTrust(userId).wasCrossSigningVerified()) { + /* Alarm if any unverified users were verified before. */ + if (userTrust.wasCrossSigningVerified() && !userTrust.isCrossSigningVerified()) { return E2EStatus.Warning; } + (userTrust.isCrossSigningVerified() ? verified : unverified).push(userId); } /* Check all verified user devices. */ From 2fb9ce282808ac6b9e001f4fca20073bccf7b93f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 8 Sep 2023 02:22:41 +0100 Subject: [PATCH 4/6] Enable cypress tests --- cypress/e2e/crypto/crypto.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cypress/e2e/crypto/crypto.spec.ts b/cypress/e2e/crypto/crypto.spec.ts index 8a8fa042e8a..28f6a91de00 100644 --- a/cypress/e2e/crypto/crypto.spec.ts +++ b/cypress/e2e/crypto/crypto.spec.ts @@ -259,7 +259,7 @@ describe("Cryptography", function () { } it("creating a DM should work, being e2e-encrypted / user verification", function (this: CryptoTestContext) { - skipIfRustCrypto(); + skipIfRustCrypto(); // needs working event shields cy.bootstrapCrossSigning(aliceCredentials); startDMWithBob.call(this); // send first message @@ -281,7 +281,6 @@ describe("Cryptography", function () { }); it("should allow verification when there is no existing DM", function (this: CryptoTestContext) { - skipIfRustCrypto(); cy.bootstrapCrossSigning(aliceCredentials); autoJoin(this.bob); From 611b39f5f6a925d4b88a727a08bc726be136c91d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 8 Sep 2023 04:38:33 +0100 Subject: [PATCH 5/6] update tests --- test/DeviceListener-test.ts | 7 +++---- test/components/views/right_panel/UserInfo-test.tsx | 6 +++++- test/utils/ShieldUtils-test.ts | 7 +++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test/DeviceListener-test.ts b/test/DeviceListener-test.ts index 0c399b0d264..e1d7c1414c4 100644 --- a/test/DeviceListener-test.ts +++ b/test/DeviceListener-test.ts @@ -25,7 +25,6 @@ import { ClientStoppedError, } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; -import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup"; @@ -92,6 +91,7 @@ describe("DeviceListener", () => { getUserDeviceInfo: jest.fn().mockResolvedValue(new Map()), isCrossSigningReady: jest.fn().mockResolvedValue(true), isSecretStorageReady: jest.fn().mockResolvedValue(true), + userHasCrossSigningKeys: jest.fn(), } as unknown as Mocked; mockClient = getMockClientWithEventEmitter({ isGuest: jest.fn(), @@ -102,7 +102,6 @@ describe("DeviceListener", () => { isVersionSupported: jest.fn().mockResolvedValue(true), isInitialSyncComplete: jest.fn().mockReturnValue(true), getKeyBackupEnabled: jest.fn(), - getStoredCrossSigningForUser: jest.fn(), waitForClientWellKnown: jest.fn(), isRoomEncrypted: jest.fn(), getClientWellKnown: jest.fn(), @@ -324,7 +323,7 @@ describe("DeviceListener", () => { }); it("shows verify session toast when account has cross signing", async () => { - mockClient!.getStoredCrossSigningForUser.mockReturnValue(new CrossSigningInfo(userId)); + mockCrypto!.userHasCrossSigningKeys.mockResolvedValue(true); await createAndStart(); expect(mockCrypto!.getUserDeviceInfo).toHaveBeenCalled(); @@ -335,7 +334,7 @@ describe("DeviceListener", () => { it("checks key backup status when when account has cross signing", async () => { mockCrypto!.getCrossSigningKeyId.mockResolvedValue(null); - mockClient!.getStoredCrossSigningForUser.mockReturnValue(new CrossSigningInfo(userId)); + mockCrypto!.userHasCrossSigningKeys.mockResolvedValue(true); await createAndStart(); expect(mockClient!.getKeyBackupEnabled).toHaveBeenCalled(); diff --git a/test/components/views/right_panel/UserInfo-test.tsx b/test/components/views/right_panel/UserInfo-test.tsx index 838051e128e..d1a87e01532 100644 --- a/test/components/views/right_panel/UserInfo-test.tsx +++ b/test/components/views/right_panel/UserInfo-test.tsx @@ -37,6 +37,7 @@ import { import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning"; import { defer } from "matrix-js-sdk/src/utils"; import { EventEmitter } from "events"; +import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; import UserInfo, { BanToggleButton, @@ -134,6 +135,8 @@ beforeEach(() => { mockCrypto = mocked({ getDeviceVerificationStatus: jest.fn(), getUserDeviceInfo: jest.fn(), + userHasCrossSigningKeys: jest.fn().mockResolvedValue(false), + getUserVerificationStatus: jest.fn(), } as unknown as CryptoApi); mockClient = mocked({ @@ -161,7 +164,6 @@ beforeEach(() => { setPowerLevel: jest.fn(), downloadKeys: jest.fn(), getCrypto: jest.fn().mockReturnValue(mockCrypto), - getStoredCrossSigningForUser: jest.fn(), } as unknown as MatrixClient); jest.spyOn(MatrixClientPeg, "get").mockReturnValue(mockClient); @@ -378,6 +380,7 @@ describe("", () => { it("renders unverified user info", async () => { mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(false, false, false)); + mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false)); renderComponent({ room: mockRoom }); await act(flushPromises); @@ -389,6 +392,7 @@ describe("", () => { it("renders verified user info", async () => { mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(true, false, false)); + mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, false, false)); renderComponent({ room: mockRoom }); await act(flushPromises); diff --git a/test/utils/ShieldUtils-test.ts b/test/utils/ShieldUtils-test.ts index 4a5095c0c75..40890a5c643 100644 --- a/test/utils/ShieldUtils-test.ts +++ b/test/utils/ShieldUtils-test.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { MatrixClient, Room } from "matrix-js-sdk/src/matrix"; +import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; import { shieldStatusForRoom } from "../../src/utils/ShieldUtils"; import DMRoomMap from "../../src/utils/DMRoomMap"; @@ -30,10 +31,8 @@ function mkClient(selfTrust = false) { getUserDeviceInfo: async (userIds: string[]) => { return new Map(userIds.map((u) => [u, new Map([["DEVICE", {}]])])); }, - }), - checkUserTrust: (userId: string) => ({ - isCrossSigningVerified: () => userId[1] == "T", - wasCrossSigningVerified: () => userId[1] == "T" || userId[1] == "W", + getUserVerificationStatus: async (userId: string): Promise => + new UserVerificationStatus(userId[1] == "T", userId[1] == "T" || userId[1] == "W", false), }), } as unknown as MatrixClient; } From 508dce429ad902f4633f4d0cba1a094100d26f66 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 18 Sep 2023 15:22:20 +0100 Subject: [PATCH 6/6] fix tests --- test/utils/ShieldUtils-test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/utils/ShieldUtils-test.ts b/test/utils/ShieldUtils-test.ts index 40890a5c643..c83766e1b32 100644 --- a/test/utils/ShieldUtils-test.ts +++ b/test/utils/ShieldUtils-test.ts @@ -49,8 +49,9 @@ describe("mkClient self-test", function () { ["@TF:h", true], ["@FT:h", false], ["@FF:h", false], - ])("behaves well for user trust %s", (userId, trust) => { - expect(mkClient().checkUserTrust(userId).isCrossSigningVerified()).toBe(trust); + ])("behaves well for user trust %s", async (userId, trust) => { + const status = await mkClient().getCrypto()?.getUserVerificationStatus(userId); + expect(status!.isCrossSigningVerified()).toBe(trust); }); test.each([