From 4ee3e591bf6498eecc1a92aad92b2ff3fc604fb8 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 20 Apr 2026 17:48:51 -0400 Subject: [PATCH] Handle secret pushing for key backups (#5189) * push backup key to other verified devices when we reset backup * handle receiving pushed backup keys - make sure that backup gets enabled after we receive a pushed key that matches the current, valid backup * apply requested changes from review --- package.json | 2 +- pnpm-lock.yaml | 10 +-- spec/integ/crypto/crypto.spec.ts | 104 ++++++++++++++++++++++ spec/unit/rust-crypto/rust-crypto.spec.ts | 3 + src/rust-crypto/backup.ts | 24 +++-- src/rust-crypto/rust-crypto.ts | 20 ++++- 6 files changed, 149 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 00f7505b2..1fa92f60d 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ ], "dependencies": { "@babel/runtime": "^7.12.5", - "@matrix-org/matrix-sdk-crypto-wasm": "^18.0.0", + "@matrix-org/matrix-sdk-crypto-wasm": "^18.1.0", "another-json": "^0.2.0", "bs58": "^6.0.0", "content-type": "^1.0.4", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9a5a79c78..a9830f2ac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -19,8 +19,8 @@ importers: specifier: ^7.12.5 version: 7.29.2 '@matrix-org/matrix-sdk-crypto-wasm': - specifier: ^18.0.0 - version: 18.0.0 + specifier: ^18.1.0 + version: 18.1.0 another-json: specifier: ^0.2.0 version: 0.2.0 @@ -1071,8 +1071,8 @@ packages: '@jridgewell/trace-mapping@0.3.31': resolution: {integrity: sha512-zzNR+SdQSDJzc8joaeP8QQoCQr8NuYx2dIIytl1QeBEZHJ9uW6hebsrYgbz8hJwUQao3TWCMtmfV8Nu1twOLAw==} - '@matrix-org/matrix-sdk-crypto-wasm@18.0.0': - resolution: {integrity: sha512-88+n+dvxLI1cjS10UIlKXVYK7TGWbpAnnaDC9fow7ch/hCvdu3dFhJ3tS3/13N9s9+1QFXB4FFuommj+tHJPhQ==} + '@matrix-org/matrix-sdk-crypto-wasm@18.1.0': + resolution: {integrity: sha512-GxXK2U39+2qWNvR3fXJY7nxdikvpiT17RaS0/Dktk6R8FMKDk3vm79Hq65yrCWLBmT7pJZoerfILNZqhrcUHrg==} engines: {node: '>= 18'} '@matrix-org/olm@3.2.15': @@ -5019,7 +5019,7 @@ snapshots: '@jridgewell/resolve-uri': 3.1.2 '@jridgewell/sourcemap-codec': 1.5.5 - '@matrix-org/matrix-sdk-crypto-wasm@18.0.0': {} + '@matrix-org/matrix-sdk-crypto-wasm@18.1.0': {} '@matrix-org/olm@3.2.15': {} diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index cc04edb4d..3bf4ec3d8 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -86,6 +86,7 @@ import { encryptGroupSessionKey, encryptMegolmEvent, encryptMegolmEventRawPlainText, + encryptOlmEvent, establishOlmSession, getTestOlmAccountKeys, expectSendRoomKey, @@ -2315,4 +2316,107 @@ describe("crypto", () => { ); } }); + + describe("secret pushing", () => { + it("should push a new backup key when a new backup key is set", async () => { + // setup: alice has another device, DEVICE_ID, which is verified + const crypto = aliceClient.getCrypto()!; + expectAliceKeyQuery(getTestKeysQueryResponse("@alice:localhost")); + await startClientAndAwaitFirstSync(); + const devices = await aliceClient.getCrypto()!.getUserDeviceInfo(["@alice:localhost"]); + expect(devices.get("@alice:localhost")!.keys()).toContain("DEVICE_ID"); + await crypto.setDeviceVerified("@alice:localhost", "DEVICE_ID"); + + expectAliceKeyClaim(getTestKeysClaimResponse("@alice:localhost")); + + // when we set a new backup key + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + fetchMock.post("path:/_matrix/client/v3/room_keys/version", { + status: 200, + body: { version: "1" }, + }); + const secretPushPromise = new Promise((resolve) => { + fetchMock.putOnce(new RegExp("/sendToDevice/m.room.encrypted/"), (callLog): RouteResponse => { + const content = JSON.parse(callLog.options.body as string); + resolve(content); + return {}; + }); + }); + + await crypto.resetKeyBackup(); + + // we expect the other device to get a secret push + const content = await secretPushPromise; + const curve25519key = JSON.parse(testOlmAccount.identity_keys()).curve25519; + const ciphertext = content.messages["@alice:localhost"].DEVICE_ID.ciphertext[curve25519key]; + const olmSession = new Olm.Session(); + olmSession.create_inbound(testOlmAccount, ciphertext.body); + const decrypted = JSON.parse(olmSession.decrypt(0, ciphertext.body)); + expect(decrypted.type).toBe("io.element.msc4385.secret.push"); + expect(decrypted.content.name).toBe("m.megolm_backup.v1"); + }); + + it("should receive pushed backup key", async () => { + // setup: alice has another device, DEVICE_ID, which is verified, + // and has a key backup set up and signed by DEVICE_ID + const crypto = aliceClient.getCrypto()!; + expectAliceKeyQuery(getTestKeysQueryResponse("@alice:localhost")); + fetchMock.get("path:/_matrix/client/v3/room_keys/version", testData.SIGNED_BACKUP_DATA); + await startClientAndAwaitFirstSync(); + const devices = await aliceClient.getCrypto()!.getUserDeviceInfo(["@alice:localhost"]); + expect(devices.get("@alice:localhost")!.keys()).toContain("DEVICE_ID"); + await crypto.setDeviceVerified("@alice:localhost", "DEVICE_ID"); + + expectAliceKeyClaim(getTestKeysClaimResponse("@alice:localhost")); + + // after we push the backup key to alice... + + const senderIdentityKeys = JSON.parse(testOlmAccount.identity_keys()); + const aliceDeviceKeys = await crypto.getOwnDeviceKeys(); + const p2pSession = await createOlmSession(testOlmAccount, keyReceiver); + const secretPush = encryptOlmEvent({ + sender: "@alice:localhost", + senderKey: senderIdentityKeys.curve25519, + senderSigningKey: senderIdentityKeys.ed25519, + p2pSession, + recipient: "@alice:localhost", + recipientCurve25519Key: aliceDeviceKeys.curve25519, + recipientEd25519Key: aliceDeviceKeys.ed25519, + plaincontent: { + secret: testData.BACKUP_DECRYPTION_KEY_BASE64, + name: "m.megolm_backup.v1", + }, + plaintype: "io.element.msc4385.secret.push", + }); + + const syncResponse = { + next_batch: 1, + to_device: { + events: [secretPush], + }, + }; + + const backupKeyReceivedPromise = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupDecryptionKeyCached, resolve); + }); + const keyBackupEnabledPromise = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { + if (enabled) { + resolve(); + } + }); + }); + + syncResponder.sendOrQueueSyncResponse(syncResponse); + await syncPromise(aliceClient); + + // alice should be using backup now + expect(await backupKeyReceivedPromise).toBe(testData.SIGNED_BACKUP_DATA.version); + await keyBackupEnabledPromise; + expect(await crypto.getActiveSessionBackupVersion()).toBe(testData.SIGNED_BACKUP_DATA.version); + }); + }); }); diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index d5d65b748..91d31dfea 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -790,6 +790,7 @@ describe("RustCrypto", () => { undefined, secretStorage, ); + vi.spyOn(rustCrypto, "pushSecretToVerifiedDevices").mockResolvedValue(); async function createSecretStorageKey() { return { @@ -837,6 +838,7 @@ describe("RustCrypto", () => { {} as CryptoCallbacks, false, ); + vi.spyOn(rustCrypto, "pushSecretToVerifiedDevices").mockResolvedValue(); async function createSecretStorageKey() { return { @@ -2324,6 +2326,7 @@ describe("RustCrypto", () => { }); const rustCrypto = await makeTestRustCrypto(makeMatrixHttpApi(), undefined, undefined, secretStorage); + vi.spyOn(rustCrypto, "pushSecretToVerifiedDevices").mockResolvedValue(); // We have a key backup await waitFor(async () => expect(await rustCrypto.getActiveSessionBackupVersion()).not.toBeNull()); diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index 62a8f92cb..c3e4a3717 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -161,7 +161,7 @@ export class RustBackupManager extends TypedEventEmitter { @@ -206,6 +206,15 @@ export class RustBackupManager extends TypedEventEmitter { + this.keyBackupCheckInProgress = null; + }); + await this.keyBackupCheckInProgress; return true; } catch (e) { this.logger.warn("handleBackupSecretReceived: Unable to validate backup decryption key", e); @@ -288,12 +297,17 @@ export class RustBackupManager extends TypedEventEmitter | null = null; - /** Helper for `checkKeyBackup` */ - private async doCheckKeyBackup(): Promise { + /** Helper to check the key backup status, and enable/disable it as appropriate + * + * A KeyBackupInfo can be passed if it was fetched recently, to avoid trying to + * re-fetch it from the server. + */ + private async doCheckKeyBackup(backupInfo?: KeyBackupInfo | null | undefined): Promise { this.logger.debug("Checking key backup status..."); - let backupInfo: KeyBackupInfo | null | undefined; try { - backupInfo = await this.requestKeyBackupVersion(); + if (!backupInfo) { + backupInfo = await this.requestKeyBackupVersion(); + } } catch (e) { this.logger.warn("Error checking for active key backup", e); this.serverBackupInfo = undefined; diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 31860d33a..de49196d6 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -1376,6 +1376,8 @@ export class RustCrypto extends TypedEventEmitter { const backupInfo = await this.backupManager.setupKeyBackup((o) => this.signObject(o)); + await this.pushSecretToVerifiedDevices("m.megolm_backup.v1"); + // we want to store the private key in 4S // need to check if 4S is set up? if (await this.secretStorageHasAESKey()) { @@ -2104,9 +2106,9 @@ export class RustCrypto extends TypedEventEmitter { + const logger = new LogSpan(this.logger, "pushSecretToVerifiedDevices"); + await this.keyClaimManager.ensureSessionsForUsers(logger, [new RustSdkCryptoJs.UserId(this.userId)]); + await this.olmMachine.pushSecretToVerifiedDevices(name); + this.outgoingRequestsManager.doProcessOutgoingRequests().catch((e) => { + logger.warn("pushSecretToVerifiedDevices: Error processing outgoing requests", e); + }); + } } class EventDecryptor {