From d22a39f5d7b40285ae6f06cdae381b3d4a4d3eb6 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 17 Apr 2024 15:36:52 +0200 Subject: [PATCH] Element R: fix `isCrossSigningReady` not checking identity trust (#4156) * Fix inconsistency between rust and legacy * Add tests * Review: better comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * review: Better doc Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Simplify test data and some comments --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- spec/integ/crypto/cross-signing.spec.ts | 61 +++++++++++++++++++++++++ src/rust-crypto/rust-crypto.ts | 10 ++-- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index 8a08eebdbc5..b81bf6f00f4 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -347,6 +347,67 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s expect(isCrossSigningReady).toBeTruthy(); }); + + it("should return false if identity is not trusted, even if the secrets are in 4S", async () => { + e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA); + + // Complete initial sync, to get the 4S account_data events stored + mockInitialApiRequests(aliceClient.getHomeserverUrl()); + + // For this test we need to have a well-formed 4S setup. + const mockSecretInfo = { + encrypted: { + // Don't care about the actual values here, just need to be present for validation + KeyId: { + iv: "IVIVIVIVIVIVIV", + ciphertext: "CIPHERTEXTB64", + mac: "MACMACMAC", + }, + }, + }; + syncResponder.sendOrQueueSyncResponse({ + next_batch: 1, + account_data: { + events: [ + { + type: "m.secret_storage.key.KeyId", + content: { + algorithm: "m.secret_storage.v1.aes-hmac-sha2", + // iv and mac not relevant for this test + }, + }, + { + type: "m.secret_storage.default_key", + content: { + key: "KeyId", + }, + }, + { + type: "m.cross_signing.master", + content: mockSecretInfo, + }, + { + type: "m.cross_signing.user_signing", + content: mockSecretInfo, + }, + { + type: "m.cross_signing.self_signing", + content: mockSecretInfo, + }, + ], + }, + }); + await aliceClient.startClient(); + await syncPromise(aliceClient); + + // Sanity: ensure that the secrets are in 4S + const status = await aliceClient.getCrypto()!.getCrossSigningStatus(); + expect(status.privateKeysInSecretStorage).toBeTruthy(); + + const isCrossSigningReady = await aliceClient.getCrypto()!.isCrossSigningReady(); + + expect(isCrossSigningReady).toBeFalsy(); + }); }); describe("getCrossSigningKeyId", () => { diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index ee00aba4c88..fd06d82018b 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -610,15 +610,17 @@ export class RustCrypto extends TypedEventEmitter { - const { publicKeysOnDevice, privateKeysInSecretStorage, privateKeysCachedLocally } = - await this.getCrossSigningStatus(); + const { privateKeysInSecretStorage, privateKeysCachedLocally } = await this.getCrossSigningStatus(); const hasKeysInCache = Boolean(privateKeysCachedLocally.masterKey) && Boolean(privateKeysCachedLocally.selfSigningKey) && Boolean(privateKeysCachedLocally.userSigningKey); - // The cross signing is ready if the public and private keys are available - return publicKeysOnDevice && (hasKeysInCache || privateKeysInSecretStorage); + const identity = await this.getOwnIdentity(); + + // Cross-signing is ready if the public identity is trusted, and the private keys + // are either cached, or accessible via secret-storage. + return !!identity?.isVerified() && (hasKeysInCache || privateKeysInSecretStorage); } /**