From 1a8ea3d6855cb74b476f7307c9ac48388ff81b48 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 24 Sep 2024 17:38:18 +0100 Subject: [PATCH] Add `CryptoApi.pinCurrentUserIdentity` and `UserIdentity.needsUserApproval` (#4415) * Implement `UserVerificationStatus.needsUserApproval` Expose the `identityNeedsUserApproval` flag from the rust crypto crate. * Add CryptoApi.pinCurrentUserIdentity Expose `pinCurrentMasterKey` from the rust crypto api. * Test data: add second cross-signing key for Bob * Add tests for verification status --- spec/integ/crypto/crypto.spec.ts | 65 +++++++++++++++++-- .../test-data/generate-test-data.py | 21 ++++-- spec/test-utils/test-data/index.ts | 47 ++++++++++++++ spec/unit/rust-crypto/rust-crypto.spec.ts | 43 +++++++++++- src/crypto-api/index.ts | 32 ++++++++- src/crypto/index.ts | 7 ++ src/rust-crypto/rust-crypto.ts | 24 ++++++- 7 files changed, 223 insertions(+), 16 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 41b52dd53cb..e6b6951a4aa 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -3261,15 +3261,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); }); - describe("Check if the cross signing keys are available for a user", () => { + describe("User identity", () => { + let keyResponder: E2EKeyResponder; beforeEach(async () => { - // anything that we don't have a specific matcher for silently returns a 404 - fetchMock.catch(404); - - const keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl()); + keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl()); keyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA); keyResponder.addDeviceKeys(SIGNED_TEST_DEVICE_DATA); - keyResponder.addKeyReceiver(BOB_TEST_USER_ID, keyReceiver); + keyResponder.addKeyReceiver(TEST_USER_ID, keyReceiver); keyResponder.addCrossSigningData(BOB_SIGNED_CROSS_SIGNING_KEYS_DATA); keyResponder.addDeviceKeys(BOB_SIGNED_TEST_DEVICE_DATA); @@ -3285,6 +3283,12 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, .getCrypto()! .userHasCrossSigningKeys(BOB_TEST_USER_ID, true); expect(hasCrossSigningKeysForUser).toBe(true); + + const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID); + expect(verificationStatus.isVerified()).toBe(false); + expect(verificationStatus.isCrossSigningVerified()).toBe(false); + expect(verificationStatus.wasCrossSigningVerified()).toBe(false); + expect(verificationStatus.needsUserApproval).toBe(false); }); it("Cross signing keys are available for a tracked user", async () => { @@ -3295,11 +3299,60 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Alice is the local user and should be tracked ! const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys(TEST_USER_ID); expect(hasCrossSigningKeysForUser).toBe(true); + + const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID); + expect(verificationStatus.isVerified()).toBe(false); + expect(verificationStatus.isCrossSigningVerified()).toBe(false); + expect(verificationStatus.wasCrossSigningVerified()).toBe(false); + expect(verificationStatus.needsUserApproval).toBe(false); }); it("Cross signing keys are not available for an unknown user", async () => { const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys("@unknown:xyz"); expect(hasCrossSigningKeysForUser).toBe(false); + + const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID); + expect(verificationStatus.isVerified()).toBe(false); + expect(verificationStatus.isCrossSigningVerified()).toBe(false); + expect(verificationStatus.wasCrossSigningVerified()).toBe(false); + expect(verificationStatus.needsUserApproval).toBe(false); + }); + + newBackendOnly("An unverified user changes identity", async () => { + // We have to be tracking Bob's keys, which means we need to share a room with him + syncResponder.sendOrQueueSyncResponse({ + ...getSyncResponse([BOB_TEST_USER_ID]), + device_lists: { changed: [BOB_TEST_USER_ID] }, + }); + await syncPromise(aliceClient); + + const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys(BOB_TEST_USER_ID); + expect(hasCrossSigningKeysForUser).toBe(true); + + // Bob changes his cross-signing keys + keyResponder.addCrossSigningData(testData.BOB_ALT_SIGNED_CROSS_SIGNING_KEYS_DATA); + syncResponder.sendOrQueueSyncResponse({ + next_batch: "2", + device_lists: { changed: [BOB_TEST_USER_ID] }, + }); + await syncPromise(aliceClient); + + await aliceClient.getCrypto()!.userHasCrossSigningKeys(BOB_TEST_USER_ID, true); + + { + const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID); + expect(verificationStatus.isVerified()).toBe(false); + expect(verificationStatus.isCrossSigningVerified()).toBe(false); + expect(verificationStatus.wasCrossSigningVerified()).toBe(false); + expect(verificationStatus.needsUserApproval).toBe(true); + } + + // Pinning the new identity should clear the needsUserApproval flag. + await aliceClient.getCrypto()!.pinCurrentUserIdentity(BOB_TEST_USER_ID); + { + const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID); + expect(verificationStatus.needsUserApproval).toBe(false); + } }); }); diff --git a/spec/test-utils/test-data/generate-test-data.py b/spec/test-utils/test-data/generate-test-data.py index 2e85a6c8218..5a80a4a1a41 100755 --- a/spec/test-utils/test-data/generate-test-data.py +++ b/spec/test-utils/test-data/generate-test-data.py @@ -66,6 +66,7 @@ "TEST_DEVICE_CURVE_PRIVATE_KEY_BYTES": b"Deadmuledeadmuledeadmuledeadmule", "MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Doyouspeakwhaaaaaaaaaaaaaaaaaale", + "ALT_MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"DoYouSpeakWhaaaaaaaaaaaaaaaaaale", "USER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Useruseruseruseruseruseruseruser", "SELF_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Selfselfselfselfselfselfselfself", @@ -208,7 +209,7 @@ def build_test_data(user_data, prefix = "") -> str: backup_recovery_key = export_recovery_key(user_data["B64_BACKUP_DECRYPTION_KEY"]) - return f"""\ + result = f"""\ export const {prefix}TEST_USER_ID = "{user_data['TEST_USER_ID']}"; export const {prefix}TEST_DEVICE_ID = "{user_data['TEST_DEVICE_ID']}"; export const {prefix}TEST_ROOM_ID = "{user_data['TEST_ROOM_ID']}"; @@ -239,7 +240,7 @@ def build_test_data(user_data, prefix = "") -> str: /** Signed cross-signing keys data, also suitable for returning from a `/keys/query` call */ export const {prefix}SIGNED_CROSS_SIGNING_KEYS_DATA: Partial = { - json.dumps(build_cross_signing_keys_data(user_data), indent=4) + json.dumps(build_cross_signing_keys_data(user_data, user_data["MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES"]), indent=4) }; /** Signed OTKs, returned by `POST /keys/claim` */ @@ -279,12 +280,20 @@ def build_test_data(user_data, prefix = "") -> str: export const {prefix}ENCRYPTED_EVENT: Partial = {json.dumps(encrypted_event, indent=4)}; """ + alt_master_key = user_data.get("ALT_MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES") + if alt_master_key is not None: + result += f""" +/** A second set of signed cross-signing keys data, also suitable for returning from a `/keys/query` call */ +export const {prefix}ALT_SIGNED_CROSS_SIGNING_KEYS_DATA: Partial = { + json.dumps(build_cross_signing_keys_data(user_data, alt_master_key), indent=4) +}; +""" + + return result -def build_cross_signing_keys_data(user_data) -> dict: +def build_cross_signing_keys_data(user_data, master_key_bytes) -> dict: """Build the signed cross-signing-keys data for return from /keys/query""" - master_private_key = ed25519.Ed25519PrivateKey.from_private_bytes( - user_data["MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES"] - ) + master_private_key = ed25519.Ed25519PrivateKey.from_private_bytes(master_key_bytes) b64_master_public_key = encode_base64( master_private_key.public_key().public_bytes(Encoding.Raw, PublicFormat.Raw) ) diff --git a/spec/test-utils/test-data/index.ts b/spec/test-utils/test-data/index.ts index 44245b4a6eb..77a2687b84e 100644 --- a/spec/test-utils/test-data/index.ts +++ b/spec/test-utils/test-data/index.ts @@ -449,3 +449,50 @@ export const BOB_ENCRYPTED_EVENT: Partial = { "origin_server_ts": 1507753886000 }; +/** A second set of signed cross-signing keys data, also suitable for returning from a `/keys/query` call */ +export const BOB_ALT_SIGNED_CROSS_SIGNING_KEYS_DATA: Partial = { + "master_keys": { + "@bob:xyz": { + "keys": { + "ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E" + }, + "user_id": "@bob:xyz", + "usage": [ + "master" + ] + } + }, + "self_signing_keys": { + "@bob:xyz": { + "keys": { + "ed25519:DaScI3WulBvDjf/d2vdyP5Cgjdypn1c/PSDX23MgN+A": "DaScI3WulBvDjf/d2vdyP5Cgjdypn1c/PSDX23MgN+A" + }, + "user_id": "@bob:xyz", + "usage": [ + "self_signing" + ], + "signatures": { + "@bob:xyz": { + "ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "eDZETBRUw9yW0WJnBZ7vxo12TW09Yb7/47qBPKZzPZzZEvs9M82dnAOtWUv00mcTdp2K9GpeFYDQJ6qLQgxaCA" + } + } + } + }, + "user_signing_keys": { + "@bob:xyz": { + "keys": { + "ed25519:lXP89FP6zvFH9TSbU1S8uSdAsVawm1NmV6z+Rfr3lEw": "lXP89FP6zvFH9TSbU1S8uSdAsVawm1NmV6z+Rfr3lEw" + }, + "user_id": "@bob:xyz", + "usage": [ + "user_signing" + ], + "signatures": { + "@bob:xyz": { + "ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "Q1CbIXvp2BxBsu3F/eZ1ZpuR5rXIt0+FrrA/l6itskpW748xwMoIKxQRVQqs87kh7pCsWEoTy6FzIL8nV+P6BQ" + } + } + } + } +}; + diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index 2b161c778f7..07501a47c95 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -1362,13 +1362,52 @@ describe("RustCrypto", () => { }); it("returns a verified UserVerificationStatus when the UserIdentity is verified", async () => { - olmMachine.getIdentity.mockResolvedValue({ free: jest.fn(), isVerified: jest.fn().mockReturnValue(true) }); + olmMachine.getIdentity.mockResolvedValue({ + free: jest.fn(), + isVerified: jest.fn().mockReturnValue(true), + wasPreviouslyVerified: jest.fn().mockReturnValue(true), + }); const userVerificationStatus = await rustCrypto.getUserVerificationStatus(testData.TEST_USER_ID); expect(userVerificationStatus.isVerified()).toBeTruthy(); expect(userVerificationStatus.isTofu()).toBeFalsy(); expect(userVerificationStatus.isCrossSigningVerified()).toBeTruthy(); - expect(userVerificationStatus.wasCrossSigningVerified()).toBeFalsy(); + expect(userVerificationStatus.wasCrossSigningVerified()).toBeTruthy(); + }); + }); + + describe("pinCurrentIdentity", () => { + let rustCrypto: RustCrypto; + let olmMachine: Mocked; + + beforeEach(() => { + olmMachine = { + getIdentity: jest.fn(), + } as unknown as Mocked; + rustCrypto = new RustCrypto( + logger, + olmMachine, + {} as MatrixClient["http"], + TEST_USER, + TEST_DEVICE_ID, + {} as ServerSideSecretStorage, + {} as CryptoCallbacks, + ); + }); + + it("throws an error for an unknown user", async () => { + await expect(rustCrypto.pinCurrentUserIdentity("@alice:example.com")).rejects.toThrow( + "Cannot pin identity of unknown user", + ); + }); + + it("throws an error for our own user", async () => { + const ownIdentity = new RustSdkCryptoJs.OwnUserIdentity(); + olmMachine.getIdentity.mockResolvedValue(ownIdentity); + + await expect(rustCrypto.pinCurrentUserIdentity("@alice:example.com")).rejects.toThrow( + "Cannot pin identity of own user", + ); }); }); diff --git a/src/crypto-api/index.ts b/src/crypto-api/index.ts index 35e4a689762..39038290f34 100644 --- a/src/crypto-api/index.ts +++ b/src/crypto-api/index.ts @@ -197,6 +197,16 @@ export interface CryptoApi { */ getUserVerificationStatus(userId: string): Promise; + /** + * "Pin" the current identity of the given user, accepting it as genuine. + * + * This is useful if the user has changed identity since we first saw them (leading to + * {@link UserVerificationStatus.needsUserApproval}), and we are now accepting their new identity. + * + * Throws an error if called on our own user ID, or on a user ID that we don't have an identity for. + */ + pinCurrentUserIdentity(userId: string): Promise; + /** * Get the verification status of a given device. * @@ -707,11 +717,29 @@ export interface BootstrapCrossSigningOpts { * Represents the ways in which we trust a user */ export class UserVerificationStatus { + /** + * Indicates if the identity has changed in a way that needs user approval. + * + * This happens if the identity has changed since we first saw it, *unless* the new identity has also been verified + * by our user (eg via an interactive verification). + * + * To rectify this, either: + * + * * Conduct a verification of the new identity via {@link CryptoApi.requestVerificationDM}. + * * Pin the new identity, via {@link CryptoApi.pinCurrentUserIdentity}. + * + * @returns true if the identity has changed in a way that needs user approval. + */ + public readonly needsUserApproval: boolean; + public constructor( private readonly crossSigningVerified: boolean, private readonly crossSigningVerifiedBefore: boolean, private readonly tofu: boolean, - ) {} + needsUserApproval: boolean = false, + ) { + this.needsUserApproval = needsUserApproval; + } /** * @returns true if this user is verified via any means @@ -737,6 +765,8 @@ export class UserVerificationStatus { /** * @returns true if this user's key is trusted on first use + * + * @deprecated No longer supported, with the Rust crypto stack. */ public isTofu(): boolean { return this.tofu; diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 90638aea1a0..38bf03c7fd8 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -1609,6 +1609,13 @@ export class Crypto extends TypedEventEmitter { + throw new Error("not implemented"); + } + /** * Check whether a given device is trusted. * diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index e1e9ecef492..15fa6371f31 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -654,9 +654,31 @@ export class RustCrypto extends TypedEventEmitter { + const userIdentity: RustSdkCryptoJs.UserIdentity | RustSdkCryptoJs.OwnUserIdentity | undefined = + await this.getOlmMachineOrThrow().getIdentity(new RustSdkCryptoJs.UserId(userId)); + + if (userIdentity === undefined) { + throw new Error("Cannot pin identity of unknown user"); + } + + if (userIdentity instanceof RustSdkCryptoJs.OwnUserIdentity) { + throw new Error("Cannot pin identity of own user"); + } + + await userIdentity.pinCurrentMasterKey(); } /**