From 64634ebf76a5fa02ee2cc7aa32fd77437cdc5168 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 19 Sep 2024 07:50:42 +0100 Subject: [PATCH 1/4] Implement `UserVerificationStatus.needsUserApproval` Expose the `identityNeedsUserApproval` flag from the rust crypto crate. --- src/crypto-api/index.ts | 20 ++++++++++++++++++++ src/rust-crypto/rust-crypto.ts | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/crypto-api/index.ts b/src/crypto-api/index.ts index 35e4a68976..2d8f8f081a 100644 --- a/src/crypto-api/index.ts +++ b/src/crypto-api/index.ts @@ -711,6 +711,7 @@ export class UserVerificationStatus { private readonly crossSigningVerified: boolean, private readonly crossSigningVerifiedBefore: boolean, private readonly tofu: boolean, + private readonly _needsUserApproval: boolean = false, ) {} /** @@ -737,10 +738,29 @@ 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; } + + /** + * Check 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 needsUserApproval(): boolean { + return this._needsUserApproval; + } } export class DeviceVerificationStatus { diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 491fd04fc9..fcaacbc0ee 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -654,9 +654,13 @@ export class RustCrypto extends TypedEventEmitter Date: Thu, 19 Sep 2024 07:51:41 +0100 Subject: [PATCH 2/4] Add CryptoApi.pinCurrentUserIdentity Expose `pinCurrentMasterKey` from the rust crypto api. --- src/crypto-api/index.ts | 10 ++++++++++ src/crypto/index.ts | 7 +++++++ src/rust-crypto/rust-crypto.ts | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/crypto-api/index.ts b/src/crypto-api/index.ts index 2d8f8f081a..b59a069229 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. * diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 90638aea1a..065c7e522f 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 fcaacbc0ee..7f2db50206 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -663,6 +663,24 @@ 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(); + } + /** * Implementation of {@link CryptoApi#isCrossSigningReady} */ From 8d6124eb45a58c477d25a568e28a60f82c255c29 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 19 Sep 2024 07:52:41 +0100 Subject: [PATCH 3/4] Test data: add second cross-signing key for Bob --- .../test-data/generate-test-data.py | 22 ++++++--- spec/test-utils/test-data/index.ts | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/spec/test-utils/test-data/generate-test-data.py b/spec/test-utils/test-data/generate-test-data.py index 2e85a6c821..eb901f7d46 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,21 @@ 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""" -def build_cross_signing_keys_data(user_data) -> dict: +/** 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, 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 44245b4a6e..beb66ff2e4 100644 --- a/spec/test-utils/test-data/index.ts +++ b/spec/test-utils/test-data/index.ts @@ -449,3 +449,51 @@ 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" + } + } + } + } +}; + From b3230189f862e009e8fd7f19cf1bffa788fb1981 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 19 Sep 2024 07:54:19 +0100 Subject: [PATCH 4/4] Add tests for verification status --- spec/integ/crypto/crypto.spec.ts | 65 +++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 41b52dd53c..b5eccf887c 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); + } }); });