From 46abd560d5d5feb110b32e6b46095594eee68cfd Mon Sep 17 00:00:00 2001 From: valere Date: Wed, 16 Aug 2023 16:35:37 +0200 Subject: [PATCH 01/14] replace prepareKeyBackupVersion by resetKeyBackup --- src/client.ts | 33 ++++++------ src/crypto-api.ts | 12 +++++ src/crypto/backup.ts | 21 +++++++- src/crypto/index.ts | 28 +++++++++++ src/rust-crypto/backup.ts | 91 ++++++++++++++++++++++++++++++++++ src/rust-crypto/rust-crypto.ts | 48 +++++++++++++++++- 6 files changed, 216 insertions(+), 17 deletions(-) diff --git a/src/client.ts b/src/client.ts index 6b6fe2c227b..dcfd1b9e9d4 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3264,6 +3264,7 @@ export class MatrixClient extends TypedEventEmitter { let res: IKeyBackupInfo; @@ -3335,6 +3336,7 @@ export class MatrixClient extends TypedEventEmitter { + if (!this.cryptoBackend) { + throw new Error("End-to-end encryption disabled"); + } + + await this.cryptoBackend!.resetKeyBackup(); + } + /** * Set up the data required to create a new backup version. The backup version * will not be created and enabled until createKeyBackupVersion is called. @@ -3354,6 +3364,8 @@ export class MatrixClient extends TypedEventEmitter { if (!this.crypto) { @@ -3442,24 +3456,15 @@ export class MatrixClient extends TypedEventEmitter { - if (!this.crypto) { + if (!this.cryptoBackend) { throw new Error("End-to-end encryption disabled"); } - // If we're currently backing up to this backup... stop. - // (We start using it automatically in createKeyBackupVersion - // so this is symmetrical). - // TODO: convert this to use crypto.getActiveSessionBackupVersion. And actually check the version. - if (this.crypto.backupManager.version) { - this.crypto.backupManager.disableKeyBackup(); - } - - const path = utils.encodeUri("/room_keys/version/$version", { - $version: version, - }); - - await this.http.authedRequest(Method.Delete, path, undefined, undefined, { prefix: ClientPrefix.V3 }); + await this.cryptoBackend!.deleteKeyBackupVersion(version); } private makeKeyBackupPath(roomId: undefined, sessionId: undefined, version?: string): IKeyBackupPath; diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 814aacc2f62..8fc3abcd60a 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -225,6 +225,18 @@ export interface CryptoApi { */ bootstrapSecretStorage(opts: CreateSecretStorageOpts): Promise; + /** + * Creates a new keybackup. + * If there are existing keybackups they will be replaced. + * The decryption key will be saved in Secret Storage and the backup engine will be started. + */ + resetKeyBackup(): Promise; + + /** + * Deletes the given key backup. + */ + deleteKeyBackupVersion(version: string): Promise; + /** * Get the status of our cross-signing keys. * diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index 1d892337a0b..87131a7c23d 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -25,7 +25,7 @@ import { MEGOLM_ALGORITHM, verifySignature } from "./olmlib"; import { DeviceInfo } from "./deviceinfo"; import { DeviceTrustLevel } from "./CrossSigning"; import { keyFromPassphrase } from "./key_passphrase"; -import { safeSet, sleep } from "../utils"; +import { encodeUri, safeSet, sleep } from "../utils"; import { IndexedDBCryptoStore } from "./store/indexeddb-crypto-store"; import { encodeRecoveryKey } from "./recoverykey"; import { calculateKeyCheck, decryptAES, encryptAES, IEncryptedPayload } from "./aes"; @@ -39,7 +39,7 @@ import { import { UnstableValue } from "../NamespacedValue"; import { CryptoEvent } from "./index"; import { crypto } from "./crypto"; -import { HTTPError, MatrixError } from "../http-api"; +import { ClientPrefix, HTTPError, MatrixError, Method } from "../http-api"; import { BackupTrustInfo } from "../crypto-api/keybackup"; const KEY_BACKUP_KEYS_PER_REQUEST = 200; @@ -224,6 +224,23 @@ export class BackupManager { this.algorithm = await BackupManager.makeAlgorithm(info, this.getKey); } + public async deleteKeyBackup(): Promise { + // there could be several backup versions, delete all to be safe. + let current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; + while (current != null) { + await this.deleteKeyBackupVersion(current); + current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; + } + } + + public async deleteKeyBackupVersion(version: string): Promise { + this.disableKeyBackup(); + const path = encodeUri("/room_keys/version/$version", { $version: version }); + await this.baseApis.http.authedRequest(Method.Delete, path, undefined, undefined, { + prefix: ClientPrefix.V3, + }); + } + /** * Check the server for an active key backup and * if one is present and has a valid signature from diff --git a/src/crypto/index.ts b/src/crypto/index.ts index d59fbe8d37c..1ee2084661c 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -98,6 +98,7 @@ import { } from "../crypto-api"; import { Device, DeviceMap } from "../models/device"; import { deviceInfoToDevice } from "./device-converter"; +import { ClientPrefix, Method } from "../http-api"; /* re-exports for backwards compatibility */ export type { @@ -1145,6 +1146,33 @@ export class Crypto extends TypedEventEmitter { + // Delete existing ones + await this.backupManager.deleteKeyBackup(); + + // eslint-disable-next-line camelcase + const info = await this.backupManager.prepareKeyBackupVersion(); + + await this.signObject(info.auth_data); + + // add new key backup + const { version } = await this.baseApis.http.authedRequest<{ version: string }>( + Method.Post, + "/room_keys/version", + undefined, + info, + { + prefix: ClientPrefix.V3, + }, + ); + + // write the key ourselves to 4S + const privateKey = decodeRecoveryKey(info.recovery_key); + await this.secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey)); + + logger.log(`Created backup version ${version}`); + } + /** * @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#addKey}. */ diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index c3a790ffa27..680d305400c 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -22,6 +22,27 @@ import { logger } from "../logger"; import { ClientPrefix, IHttpOpts, MatrixError, MatrixHttpApi, Method } from "../http-api"; import { CryptoEvent } from "../crypto"; import { TypedEventEmitter } from "../models/typed-event-emitter"; +import { encodeUri } from "../utils"; + +/* eslint-disable camelcase */ +interface PreparedKeyBackupVersion { + algorithm: string; + auth_data: AuthData; + decryptionKey: RustSdkCryptoJs.BackupDecryptionKey; +} + +type AuthData = KeyBackupInfo["auth_data"]; + +interface BackupCreateResponse { + version: string; +} + +export interface KeyBackupCreationInfo { + version: string; + algorithm: string; + authData: AuthData; + decryptionKey: RustSdkCryptoJs.BackupDecryptionKey; +} /** * @internal @@ -193,6 +214,76 @@ export class RustBackupManager extends TypedEventEmitter Promise): Promise { + // check first if there is an existing backup + // TODO should it be an option? like if there is already a backup we throw unless it's forced? + if (this.activeBackupVersion || (await this.requestKeyBackupVersion()) != null) { + // we want to delete? + await this.deleteKeyBackup(); + } + + const version = await this.prepareKeyBackupVersion(); + await signer(version.auth_data); + + const res = await this.http.authedRequest( + Method.Post, + "/room_keys/version", + undefined, + { + algorithm: version.algorithm, + auth_data: version.auth_data, + }, + { + prefix: ClientPrefix.V3, + }, + ); + + this.olmMachine.saveBackupDecryptionKey(version.decryptionKey, res.version); + + return { + version: res.version, + algorithm: version.algorithm, + authData: version.auth_data, + decryptionKey: version.decryptionKey, + }; + } + + public async deleteKeyBackup(): Promise { + // there could be several backup versions, delete all to be safe. + let current = (await this.requestKeyBackupVersion())?.version ?? null; + while (current != null) { + this.deleteKeyBackupVersion(current); + current = (await this.requestKeyBackupVersion())?.version ?? null; + } + + // Should this also update Secret Storage and delete any existing keys? + } + + public async deleteKeyBackupVersion(version: string): Promise { + await this.disableKeyBackup(); + const path = encodeUri("/room_keys/version/$version", { $version: version }); + await this.http.authedRequest(Method.Delete, path, undefined, undefined, { + prefix: ClientPrefix.V3, + }); + } + + /** + * Prepare the keybackup version data, auth_data not signed at this point + * @returns P + */ + private async prepareKeyBackupVersion(): Promise { + const randomKey = RustSdkCryptoJs.BackupDecryptionKey.createRandomKey(); + const pubKey = randomKey.megolmV1PublicKey; + + const authData = { public_key: pubKey.publicKeyBase64 }; + + return { + algorithm: pubKey.algorithm, + auth_data: authData, + decryptionKey: randomKey, + }; + } } export type RustBackupCryptoEvents = CryptoEvent.KeyBackupStatus; diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index d9cd3466c24..af7d30bc373 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import anotherjson from "another-json"; import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-wasm"; import type { IEventDecryptionResult, IMegolmSessionData } from "../@types/crypto"; @@ -29,7 +30,7 @@ import { UserTrustLevel } from "../crypto/CrossSigning"; import { RoomEncryptor } from "./RoomEncryptor"; import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; import { KeyClaimManager } from "./KeyClaimManager"; -import { MapWithDefault } from "../utils"; +import { MapWithDefault, recursiveMapToObject } from "../utils"; import { BackupTrustInfo, BootstrapCrossSigningOpts, @@ -61,9 +62,15 @@ import { CryptoEvent } from "../crypto"; import { TypedEventEmitter } from "../models/typed-event-emitter"; import { RustBackupCryptoEventMap, RustBackupCryptoEvents, RustBackupManager } from "./backup"; import { TypedReEmitter } from "../ReEmitter"; +import { ISignatures } from "../@types/signed"; const ALL_VERIFICATION_METHODS = ["m.sas.v1", "m.qr_code.scan.v1", "m.qr_code.show.v1", "m.reciprocate.v1"]; +interface ISignableObject { + signatures?: ISignatures; + unsigned?: object; +} + /** * An implementation of {@link CryptoBackend} using the Rust matrix-sdk-crypto. * @@ -507,6 +514,7 @@ export class RustCrypto extends TypedEventEmitter { // If an AES Key is already stored in the secret storage and setupNewSecretStorage is not set // we don't want to create a new key @@ -550,9 +558,47 @@ export class RustCrypto extends TypedEventEmitter { + // check if there is already an existing backup + const backupInfo = await this.backupManager.setUpKeyBackup(this.signObject.bind(this)); + + // we want to store the private key in 4S + // need to check if 4S setup? + if (await this.secretStorageHasAESKey()) { + await this.secretStorage.store("m.megolm_backup.v1", backupInfo.decryptionKey.toBase64()); + } + + // we can check and start async + this.checkKeyBackupAndEnable(); + } + + public async signObject(obj: T): Promise { + const sigs = new Map(Object.entries(obj.signatures || {})); + const unsigned = obj.unsigned; + + delete obj.signatures; + delete obj.unsigned; + + const userSignatures = sigs.get(this.userId) || {}; + + const canonalizedJson = anotherjson.stringify(obj); + const signatures: RustSdkCryptoJs.Signatures = await this.olmMachine.sign(canonalizedJson); + + const map = JSON.parse(signatures.asJSON()); + + sigs.set(this.userId, { ...userSignatures, ...map[this.userId] }); + + if (unsigned !== undefined) obj.unsigned = unsigned; + obj.signatures = recursiveMapToObject(sigs); + } + /** * Add the secretStorage key to the secret storage * - The secret storage key must have the `keyInfo` field filled From 21b8f0f103c341d9c108ae52ecc2f94f3590ae7a Mon Sep 17 00:00:00 2001 From: valere Date: Wed, 16 Aug 2023 17:22:18 +0200 Subject: [PATCH 02/14] Fix missing implementation --- src/crypto/index.ts | 4 ++++ src/rust-crypto/rust-crypto.ts | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 1ee2084661c..8c4e5960691 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -543,6 +543,10 @@ export class Crypto extends TypedEventEmitter { + await this.backupManager.deleteKeyBackupVersion(version); + } + /** * Initialise the crypto module so that it is ready for use * diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index af7d30bc373..4eff7e4d9bb 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -139,6 +139,9 @@ export class RustCrypto extends TypedEventEmitter { + await this.backupManager.deleteKeyBackupVersion(version); + } /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // From 49eaf4b6628296ef631ff8ec828012e344896913 Mon Sep 17 00:00:00 2001 From: valere Date: Thu, 17 Aug 2023 16:45:58 +0200 Subject: [PATCH 03/14] add test for backup setup --- spec/integ/crypto/crypto.spec.ts | 86 +++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index fd578084fb6..5334ab89b8a 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -45,6 +45,7 @@ import { RoomMember, RoomStateEvent, IRoomEvent, + CryptoEvent, } from "../../../src/matrix"; import { DeviceInfo } from "../../../src/crypto/deviceinfo"; import { E2EKeyReceiver, IE2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; @@ -54,7 +55,7 @@ import { downloadDeviceToJsDevice } from "../../../src/rust-crypto/device-conver import { flushPromises } from "../../test-utils/flushPromises"; import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; -import { CryptoCallbacks } from "../../../src/crypto-api"; +import { CryptoCallbacks, KeyBackupInfo } from "../../../src/crypto-api"; import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; afterEach(() => { @@ -2219,6 +2220,20 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); } + function awaitMegolmBackupKeyUpload(): Promise> { + return new Promise((resolve) => { + // Called when the cross signing key is uploaded + fetchMock.put( + `express:/_matrix/client/v3/user/:userId/account_data/m.megolm_backup.v1`, + (url: string, options: RequestInit) => { + const content = JSON.parse(options.body as string); + resolve(content.encrypted); + return {}; + }, + ); + }); + } + /** * Send in the sync response the provided `secretStorageKey` into the account_data field * The key is set for the `m.secret_storage.default_key` and `m.secret_storage.key.${secretStorageKey}` events @@ -2376,6 +2391,75 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(userSigningKey[secretStorageKey]).toBeDefined(); expect(selfSigningKey[secretStorageKey]).toBeDefined(); }); + + newBackendOnly("should create a new megolm backup", async () => { + mockSetupCrossSigningRequests(); + + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { + errcode: "M_NOT_FOUND", + error: "No current backup version", + }, + }); + + const backupVersion = "abc"; + + fetchMock.post("path:/_matrix/client/v3/room_keys/version", (url, request) => { + const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); + backupData.version = backupVersion; + backupData.count = 0; + backupData.etag = "zer"; + fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { + overwriteRoutes: true, + }); + return { + version: backupVersion, + }; + }); + + const backupStatusUpdate = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { + if (enabled) { + resolve(); + } + }); + }); + + // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. + await aliceClient.getCrypto()?.bootstrapCrossSigning({}); + + // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. + const bootstrapPromise = aliceClient + .getCrypto()! + .bootstrapSecretStorage({ + setupNewSecretStorage: true, + createSecretStorageKey, + setupNewKeyBackup: true, + }); + + // Wait for the key to be uploaded in the account data + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); + + // Wait for the cross signing keys to be uploaded + await Promise.all([ + awaitCrossSigningKeyUpload("master"), + awaitCrossSigningKeyUpload("user_signing"), + awaitCrossSigningKeyUpload("self_signing"), + ]); + await awaitMegolmBackupKeyUpload(); + + // Finally, wait for bootstrapSecretStorage to finished + await bootstrapPromise; + await backupStatusUpdate; + + // Expect a backup to be available and used + const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); + expect(activeBackup).toStrictEqual(backupVersion); + }); }); describe("Incoming verification in a DM", () => { From 458240236f8b438a8bbc93761272a78cc8b94746 Mon Sep 17 00:00:00 2001 From: valere Date: Fri, 18 Aug 2023 15:13:29 +0200 Subject: [PATCH 04/14] more test and fix missing await --- spec/integ/crypto/crypto.spec.ts | 220 ++++++++++++++++++++++--------- spec/test-utils/mockEndpoints.ts | 31 +++++ src/crypto/EncryptionSetup.ts | 2 + src/crypto/index.ts | 2 + src/rust-crypto/backup.ts | 3 +- 5 files changed, 193 insertions(+), 65 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 5334ab89b8a..de3bea22d13 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -53,7 +53,11 @@ import { ISyncResponder, SyncResponder } from "../../test-utils/SyncResponder"; import { escapeRegExp } from "../../../src/utils"; import { downloadDeviceToJsDevice } from "../../../src/rust-crypto/device-converter"; import { flushPromises } from "../../test-utils/flushPromises"; -import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; +import { + mockInitialApiRequests, + mockSetupCrossSigningRequests, + mockSetupMegolmBackupRequests, +} from "../../test-utils/mockEndpoints"; import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; import { CryptoCallbacks, KeyBackupInfo } from "../../../src/crypto-api"; import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; @@ -2230,6 +2234,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, resolve(content.encrypted); return {}; }, + { overwriteRoutes: true }, ); }); } @@ -2281,7 +2286,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }, ); - newBackendOnly("should create a new key", async () => { + it("should create a new key", async () => { const bootstrapPromise = aliceClient .getCrypto()! .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); @@ -2324,42 +2329,39 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }, ); - newBackendOnly( - "should create a new key if setupNewSecretStorage is at true even if an AES key is already in the secret storage", - async () => { - let bootstrapPromise = aliceClient - .getCrypto()! - .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); + it("should create a new key if setupNewSecretStorage is at true even if an AES key is already in the secret storage", async () => { + let bootstrapPromise = aliceClient + .getCrypto()! + .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); - // Wait for the key to be uploaded in the account data - let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + // Wait for the key to be uploaded in the account data + let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); - // Return the newly created key in the sync response - sendSyncResponse(secretStorageKey); + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); - // Wait for bootstrapSecretStorage to finished - await bootstrapPromise; + // Wait for bootstrapSecretStorage to finished + await bootstrapPromise; - // Call again bootstrapSecretStorage - bootstrapPromise = aliceClient - .getCrypto()! - .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); + // Call again bootstrapSecretStorage + bootstrapPromise = aliceClient + .getCrypto()! + .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); - // Wait for the key to be uploaded in the account data - secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + // Wait for the key to be uploaded in the account data + secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); - // Return the newly created key in the sync response - sendSyncResponse(secretStorageKey); + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); - // Wait for bootstrapSecretStorage to finished - await bootstrapPromise; + // Wait for bootstrapSecretStorage to finished + await bootstrapPromise; - // createSecretStorageKey should have been called twice, one time every bootstrapSecretStorage call - expect(createSecretStorageKey).toHaveBeenCalledTimes(2); - }, - ); + // createSecretStorageKey should have been called twice, one time every bootstrapSecretStorage call + expect(createSecretStorageKey).toHaveBeenCalledTimes(2); + }); - newBackendOnly("should upload cross signing keys", async () => { + it("should upload cross signing keys", async () => { mockSetupCrossSigningRequests(); // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. @@ -2394,30 +2396,56 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, newBackendOnly("should create a new megolm backup", async () => { mockSetupCrossSigningRequests(); - - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { - errcode: "M_NOT_FOUND", - error: "No current backup version", - }, + const backupVersion = "abc"; + mockSetupMegolmBackupRequests(backupVersion); + const backupStatusUpdate = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { + if (enabled) { + resolve(); + } + }); }); - const backupVersion = "abc"; + const setupPromises = [ + awaitCrossSigningKeyUpload("master"), + awaitCrossSigningKeyUpload("user_signing"), + awaitCrossSigningKeyUpload("self_signing"), + awaitMegolmBackupKeyUpload(), + ]; - fetchMock.post("path:/_matrix/client/v3/room_keys/version", (url, request) => { - const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); - backupData.version = backupVersion; - backupData.count = 0; - backupData.etag = "zer"; - fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { - overwriteRoutes: true, - }); - return { - version: backupVersion, - }; + // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. + await aliceClient.getCrypto()?.bootstrapCrossSigning({}); + + // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. + const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ + setupNewSecretStorage: true, + createSecretStorageKey, + setupNewKeyBackup: true, }); + // Wait for the key to be uploaded in the account data + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); + + // Wait for the cross signing keys to be uploaded + await Promise.all(setupPromises); + + // Finally, wait for bootstrapSecretStorage to finished + await bootstrapPromise; + await backupStatusUpdate; + + // Expect a backup to be available and used + const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); + expect(activeBackup).toStrictEqual(backupVersion); + }); + + it("Reset key backup should create a new backup and update 4S", async () => { + // First setup recovery + mockSetupCrossSigningRequests(); + const backupVersion = "1"; + mockSetupMegolmBackupRequests(backupVersion); const backupStatusUpdate = new Promise((resolve) => { aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { if (enabled) { @@ -2426,17 +2454,22 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); }); + const setupPromises = [ + awaitCrossSigningKeyUpload("master"), + awaitCrossSigningKeyUpload("user_signing"), + awaitCrossSigningKeyUpload("self_signing"), + awaitMegolmBackupKeyUpload(), + ]; + // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. await aliceClient.getCrypto()?.bootstrapCrossSigning({}); // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. - const bootstrapPromise = aliceClient - .getCrypto()! - .bootstrapSecretStorage({ - setupNewSecretStorage: true, - createSecretStorageKey, - setupNewKeyBackup: true, - }); + const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ + setupNewSecretStorage: true, + createSecretStorageKey, + setupNewKeyBackup: true, + }); // Wait for the key to be uploaded in the account data const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); @@ -2445,20 +2478,79 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, sendSyncResponse(secretStorageKey); // Wait for the cross signing keys to be uploaded - await Promise.all([ - awaitCrossSigningKeyUpload("master"), - awaitCrossSigningKeyUpload("user_signing"), - awaitCrossSigningKeyUpload("self_signing"), - ]); - await awaitMegolmBackupKeyUpload(); + await Promise.all(setupPromises); // Finally, wait for bootstrapSecretStorage to finished await bootstrapPromise; await backupStatusUpdate; - // Expect a backup to be available and used - const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); - expect(activeBackup).toStrictEqual(backupVersion); + const currentVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); + const currentBackupKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); + + // we will call reset backup, it should delete the existing one, then setup a new one + // Let's mock for that + + // Mock delete and replace the GET to return 404 as soon as called + const awaitDeleteCalled = new Promise((resolve) => { + fetchMock.delete( + "express:/_matrix/client/v3/room_keys/version/:version", + (url: string, options: RequestInit) => { + fetchMock.get( + "path:/_matrix/client/v3/room_keys/version", + { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "Account data not found." }, + }, + { overwriteRoutes: true }, + ); + resolve(); + return {}; + }, + { overwriteRoutes: true }, + ); + }); + + const newVersion = "2"; + fetchMock.post( + "path:/_matrix/client/v3/room_keys/version", + (url, request) => { + const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); + backupData.version = newVersion; + backupData.count = 0; + backupData.etag = "zer"; + + // update get call with new version + fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { + overwriteRoutes: true, + }); + return { + version: backupVersion, + }; + }, + { overwriteRoutes: true }, + ); + + const newBackupStatusUpdate = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { + if (enabled) { + resolve(); + } + }); + }); + + const new4SUpload = awaitMegolmBackupKeyUpload(); + + await aliceClient.getCrypto()!.resetKeyBackup(); + await awaitDeleteCalled; + await newBackupStatusUpdate; + await new4SUpload; + + const nextVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); + const nextKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); + + expect(nextVersion).toBeDefined(); + expect(nextVersion).not.toEqual(currentVersion); + expect(nextKey).not.toEqual(currentBackupKey); }); }); diff --git a/spec/test-utils/mockEndpoints.ts b/spec/test-utils/mockEndpoints.ts index 4fdcc5b1a81..d7aa00dd1f8 100644 --- a/spec/test-utils/mockEndpoints.ts +++ b/spec/test-utils/mockEndpoints.ts @@ -16,6 +16,8 @@ limitations under the License. import fetchMock from "fetch-mock-jest"; +import { KeyBackupInfo } from "../../src/crypto-api"; + /** * Mock out the endpoints that the js-sdk calls when we call `MatrixClient.start()`. * @@ -56,3 +58,32 @@ export function mockSetupCrossSigningRequests(): void { {}, ); } + +/** + * Returns 404 `M_NOT_FOUND` for GET `/room_keys/version` request until POST `room_keys/version` is called. + * Once the POST is done GET `/room_keys/version` will return the posted backup instead of 404 + * + * @param backupVersion The version of the backup to create + */ +export function mockSetupMegolmBackupRequests(backupVersion: string): void { + 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", (url, request) => { + const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); + backupData.version = backupVersion; + backupData.count = 0; + backupData.etag = "zer"; + fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { + overwriteRoutes: true, + }); + return { + version: backupVersion, + }; + }); +} diff --git a/src/crypto/EncryptionSetup.ts b/src/crypto/EncryptionSetup.ts index 2c838446753..c3e2b652770 100644 --- a/src/crypto/EncryptionSetup.ts +++ b/src/crypto/EncryptionSetup.ts @@ -228,6 +228,8 @@ export class EncryptionSetupOperation { prefix: ClientPrefix.V3, }); } + // enable it + await crypto.backupManager.checkAndStart(); } } } diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 8c4e5960691..4ae8d851a9b 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -1173,7 +1173,9 @@ export class Crypto extends TypedEventEmitter { + logger.debug(`deleteKeyBackupVersion v:${version}`); await this.disableKeyBackup(); const path = encodeUri("/room_keys/version/$version", { $version: version }); await this.http.authedRequest(Method.Delete, path, undefined, undefined, { From 2c6037c61f5596745ac1fad0fc69034efa90c03f Mon Sep 17 00:00:00 2001 From: valere Date: Mon, 21 Aug 2023 17:51:20 +0200 Subject: [PATCH 05/14] cleaning and doc --- src/crypto-api.ts | 2 ++ src/crypto/index.ts | 1 - src/rust-crypto/backup.ts | 24 +++++++++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 8fc3abcd60a..2ec629b7e60 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -234,6 +234,8 @@ export interface CryptoApi { /** * Deletes the given key backup. + * + * @param version - The backup version to delete. */ deleteKeyBackupVersion(version: string): Promise; diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 4ae8d851a9b..e7bf769d31c 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -1154,7 +1154,6 @@ export class Crypto extends TypedEventEmitter Promise): Promise { // check first if there is an existing backup // TODO should it be an option? like if there is already a backup we throw unless it's forced? @@ -336,6 +349,10 @@ export class RustBackupManager extends TypedEventEmitter { // there could be several backup versions, delete all to be safe. let current = (await this.requestKeyBackupVersion())?.version ?? null; @@ -347,6 +364,11 @@ export class RustBackupManager extends TypedEventEmitter { logger.debug(`deleteKeyBackupVersion v:${version}`); await this.disableKeyBackup(); From c95e90ade0cbc0587fdc23b82fe537a595b8b348 Mon Sep 17 00:00:00 2001 From: valere Date: Mon, 21 Aug 2023 17:56:47 +0200 Subject: [PATCH 06/14] fix lint --- src/rust-crypto/backup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index d7e54b9fd9e..10e28cc97c3 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -311,7 +311,7 @@ export class RustBackupManager extends TypedEventEmitter Date: Tue, 29 Aug 2023 11:39:05 +0200 Subject: [PATCH 07/14] added some comments --- src/client.ts | 7 ++++++- src/crypto-api.ts | 7 ++++--- src/crypto/backup.ts | 4 ++++ src/crypto/index.ts | 4 ++++ src/rust-crypto/backup.ts | 8 ++++---- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/client.ts b/src/client.ts index 0117e15114d..9caf004e82d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3341,7 +3341,7 @@ export class MatrixClient extends TypedEventEmitter { if (!this.cryptoBackend) { throw new Error("End-to-end encryption disabled"); diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 57f87dda71b..2fbc3f31ef8 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -226,9 +226,10 @@ export interface CryptoApi { bootstrapSecretStorage(opts: CreateSecretStorageOpts): Promise; /** - * Creates a new keybackup. - * If there are existing keybackups they will be replaced. - * The decryption key will be saved in Secret Storage and the backup engine will be started. + * Creates a new key backup version. + * If there are existing backups they will be replaced. + * The decryption key will be saved in Secret Storage (will prompt for passphrase/key if neeeded) + * and the backup engine will be started. */ resetKeyBackup(): Promise; diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index 4d3e5d5e6af..5994fdeb7a4 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -224,6 +224,10 @@ export class BackupManager { this.algorithm = await BackupManager.makeAlgorithm(info, this.getKey); } + /** + * Deletes all key backups. + * Will call the API to delete active backup until there is no more present. + */ public async deleteKeyBackup(): Promise { // there could be several backup versions, delete all to be safe. let current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; diff --git a/src/crypto/index.ts b/src/crypto/index.ts index e7bf769d31c..97aae3c365d 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -543,6 +543,10 @@ export class Crypto extends TypedEventEmitter { await this.backupManager.deleteKeyBackupVersion(version); } diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index 10e28cc97c3..f4a1a4f4262 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -28,7 +28,7 @@ import { sleep } from "../utils"; interface PreparedKeyBackupVersion { algorithm: string; - /* eslint-disable camelcase */ + /* eslint-disable-next-line camelcase */ auth_data: AuthData; decryptionKey: RustSdkCryptoJs.BackupDecryptionKey; } @@ -41,9 +41,9 @@ interface BackupCreateResponse { /** * Holds information of a created keybackup. - * Usefull to get the generated private key material and save it securely somewhere. + * Useful to get the generated private key material and save it securely somewhere. */ -export interface KeyBackupCreationInfo { +interface KeyBackupCreationInfo { version: string; algorithm: string; authData: AuthData; @@ -307,7 +307,7 @@ export class RustBackupManager extends TypedEventEmitter Date: Tue, 29 Aug 2023 13:45:29 +0200 Subject: [PATCH 08/14] Remove unneeded crypto methods to MatrixClient --- src/client.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/client.ts b/src/client.ts index 9caf004e82d..01a79b7e76b 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3351,19 +3351,6 @@ export class MatrixClient extends TypedEventEmitter { - if (!this.cryptoBackend) { - throw new Error("End-to-end encryption disabled"); - } - - await this.cryptoBackend!.resetKeyBackup(); - } - /** * Set up the data required to create a new backup version. The backup version * will not be created and enabled until createKeyBackupVersion is called. From 511faa0c587180f6182ca040da38ef5ef24ded0a Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 29 Aug 2023 14:04:32 +0200 Subject: [PATCH 09/14] fix bad call to disableKeyBackup --- src/crypto/backup.ts | 2 +- src/rust-crypto/backup.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index 5994fdeb7a4..42ba0104147 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -233,12 +233,12 @@ export class BackupManager { let current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; while (current != null) { await this.deleteKeyBackupVersion(current); + this.disableKeyBackup(); current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; } } public async deleteKeyBackupVersion(version: string): Promise { - this.disableKeyBackup(); const path = encodeUri("/room_keys/version/$version", { $version: version }); await this.baseApis.http.authedRequest(Method.Delete, path, undefined, undefined, { prefix: ClientPrefix.V3, diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index f4a1a4f4262..53af7c1dd57 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -371,7 +371,6 @@ export class RustBackupManager extends TypedEventEmitter { logger.debug(`deleteKeyBackupVersion v:${version}`); - await this.disableKeyBackup(); const path = encodeUri("/room_keys/version/$version", { $version: version }); await this.http.authedRequest(Method.Delete, path, undefined, undefined, { prefix: ClientPrefix.V3, From 8f28611ef4c9a4b547763a7168369d40fb502109 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 29 Aug 2023 14:07:48 +0200 Subject: [PATCH 10/14] missing ts-doc --- src/crypto/backup.ts | 5 +++++ src/rust-crypto/rust-crypto.ts | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index 42ba0104147..10844c90e57 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -238,6 +238,11 @@ export class BackupManager { } } + /** + * Deletes the given key backup. + * + * @param version - The backup version to delete. + */ public async deleteKeyBackupVersion(version: string): Promise { const path = encodeUri("/room_keys/version/$version", { $version: version }); await this.baseApis.http.authedRequest(Method.Delete, path, undefined, undefined, { diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 214361c5e30..9c3ad60f714 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -144,6 +144,12 @@ export class RustCrypto extends TypedEventEmitter { await this.backupManager.deleteKeyBackupVersion(version); } From 319985cad45fd0801a89de3cecbd32910989b9b1 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 29 Aug 2023 14:18:25 +0200 Subject: [PATCH 11/14] ts-doc --- src/rust-crypto/backup.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index 53af7c1dd57..ec7535a1648 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -26,19 +26,22 @@ import { encodeUri } from "../utils"; import { OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; import { sleep } from "../utils"; +/** + * prepareKeyBackupVersion result. + */ interface PreparedKeyBackupVersion { + /** The prepared algorithm version */ algorithm: string; + /** The auth data of the algorithm */ /* eslint-disable-next-line camelcase */ auth_data: AuthData; + /** The generated private key */ decryptionKey: RustSdkCryptoJs.BackupDecryptionKey; } +/** Authentification of the backup info, depends on algorithm */ type AuthData = KeyBackupInfo["auth_data"]; -interface BackupCreateResponse { - version: string; -} - /** * Holds information of a created keybackup. * Useful to get the generated private key material and save it securely somewhere. @@ -326,7 +329,7 @@ export class RustBackupManager extends TypedEventEmitter( + const res = await this.http.authedRequest<{ version: string }>( Method.Post, "/room_keys/version", undefined, @@ -379,7 +382,7 @@ export class RustBackupManager extends TypedEventEmitter { const randomKey = RustSdkCryptoJs.BackupDecryptionKey.createRandomKey(); From 80fce560b01de98ea621207b024c5e34ce763af4 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 29 Aug 2023 14:22:58 +0200 Subject: [PATCH 12/14] remove a todo --- src/rust-crypto/backup.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rust-crypto/backup.ts b/src/rust-crypto/backup.ts index ec7535a1648..ade994ca6f2 100644 --- a/src/rust-crypto/backup.ts +++ b/src/rust-crypto/backup.ts @@ -320,7 +320,6 @@ export class RustBackupManager extends TypedEventEmitter Promise): Promise { // check first if there is an existing backup - // TODO should it be an option? like if there is already a backup we throw unless it's forced? if (this.activeBackupVersion || (await this.requestKeyBackupVersion()) != null) { // we want to delete? await this.deleteKeyBackup(); From 646de150315799ebc8af7c333d92b802c4d5c399 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 30 Aug 2023 18:21:17 +0200 Subject: [PATCH 13/14] code review --- spec/integ/crypto/crypto.spec.ts | 130 ++++++++++++------------------- spec/test-utils/mockEndpoints.ts | 9 ++- src/client.ts | 6 +- src/crypto-api.ts | 5 +- src/crypto/backup.ts | 3 +- src/crypto/index.ts | 17 ++-- src/rust-crypto/backup.ts | 25 +++--- src/rust-crypto/rust-crypto.ts | 28 +++---- 8 files changed, 101 insertions(+), 122 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index f3d8cbd4009..003e8f6faca 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -2235,7 +2235,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, function awaitMegolmBackupKeyUpload(): Promise> { return new Promise((resolve) => { - // Called when the cross signing key is uploaded + // Called when the megolm backup key is uploaded fetchMock.put( `express:/_matrix/client/v3/user/:userId/account_data/m.megolm_backup.v1`, (url: string, options: RequestInit) => { @@ -2248,6 +2248,50 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); } + async function bootstrapSecurity(backupVersion: string): Promise { + mockSetupCrossSigningRequests(); + mockSetupMegolmBackupRequests(backupVersion); + + // promise which will resolve when a `KeyBackupStatus` event is emitted with `enabled: true` + const backupStatusUpdate = new Promise((resolve) => { + aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { + if (enabled) { + resolve(); + } + }); + }); + + const setupPromises = [ + awaitCrossSigningKeyUpload("master"), + awaitCrossSigningKeyUpload("user_signing"), + awaitCrossSigningKeyUpload("self_signing"), + awaitMegolmBackupKeyUpload(), + ]; + + // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. + await aliceClient.getCrypto()!.bootstrapCrossSigning({}); + + // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. + const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ + setupNewSecretStorage: true, + createSecretStorageKey, + setupNewKeyBackup: true, + }); + + // Wait for the key to be uploaded in the account data + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); + + // Wait for the cross signing keys to be uploaded + await Promise.all(setupPromises); + + // Finally, wait for bootstrapSecretStorage to finished + await bootstrapPromise; + await backupStatusUpdate; + } + /** * Send in the sync response the provided `secretStorageKey` into the account_data field * The key is set for the `m.secret_storage.default_key` and `m.secret_storage.key.${secretStorageKey}` events @@ -2374,7 +2418,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, mockSetupCrossSigningRequests(); // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. - await aliceClient.getCrypto()?.bootstrapCrossSigning({}); + await aliceClient.getCrypto()!.bootstrapCrossSigning({}); // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. const bootstrapPromise = aliceClient @@ -2404,46 +2448,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); newBackendOnly("should create a new megolm backup", async () => { - mockSetupCrossSigningRequests(); const backupVersion = "abc"; - mockSetupMegolmBackupRequests(backupVersion); - const backupStatusUpdate = new Promise((resolve) => { - aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { - if (enabled) { - resolve(); - } - }); - }); - - const setupPromises = [ - awaitCrossSigningKeyUpload("master"), - awaitCrossSigningKeyUpload("user_signing"), - awaitCrossSigningKeyUpload("self_signing"), - awaitMegolmBackupKeyUpload(), - ]; - - // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. - await aliceClient.getCrypto()?.bootstrapCrossSigning({}); - - // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. - const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ - setupNewSecretStorage: true, - createSecretStorageKey, - setupNewKeyBackup: true, - }); - - // Wait for the key to be uploaded in the account data - const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); - - // Return the newly created key in the sync response - sendSyncResponse(secretStorageKey); - - // Wait for the cross signing keys to be uploaded - await Promise.all(setupPromises); - - // Finally, wait for bootstrapSecretStorage to finished - await bootstrapPromise; - await backupStatusUpdate; + await bootstrapSecurity(backupVersion); // Expect a backup to be available and used const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); @@ -2451,47 +2457,9 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); it("Reset key backup should create a new backup and update 4S", async () => { - // First setup recovery - mockSetupCrossSigningRequests(); + // First set up recovery const backupVersion = "1"; - mockSetupMegolmBackupRequests(backupVersion); - const backupStatusUpdate = new Promise((resolve) => { - aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { - if (enabled) { - resolve(); - } - }); - }); - - const setupPromises = [ - awaitCrossSigningKeyUpload("master"), - awaitCrossSigningKeyUpload("user_signing"), - awaitCrossSigningKeyUpload("self_signing"), - awaitMegolmBackupKeyUpload(), - ]; - - // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. - await aliceClient.getCrypto()?.bootstrapCrossSigning({}); - - // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. - const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ - setupNewSecretStorage: true, - createSecretStorageKey, - setupNewKeyBackup: true, - }); - - // Wait for the key to be uploaded in the account data - const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); - - // Return the newly created key in the sync response - sendSyncResponse(secretStorageKey); - - // Wait for the cross signing keys to be uploaded - await Promise.all(setupPromises); - - // Finally, wait for bootstrapSecretStorage to finished - await bootstrapPromise; - await backupStatusUpdate; + await bootstrapSecurity(backupVersion); const currentVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); const currentBackupKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); diff --git a/spec/test-utils/mockEndpoints.ts b/spec/test-utils/mockEndpoints.ts index d7aa00dd1f8..c75e758cdda 100644 --- a/spec/test-utils/mockEndpoints.ts +++ b/spec/test-utils/mockEndpoints.ts @@ -60,10 +60,13 @@ export function mockSetupCrossSigningRequests(): void { } /** - * Returns 404 `M_NOT_FOUND` for GET `/room_keys/version` request until POST `room_keys/version` is called. - * Once the POST is done GET `/room_keys/version` will return the posted backup instead of 404 + * Mock out requests to `/room_keys/version`. * - * @param backupVersion The version of the backup to create + * Returns `404 M_NOT_FOUND` for GET requests until `POST room_keys/version` is called. + * Once the POST is done, `GET /room_keys/version` will return the posted backup + * instead of 404. + * + * @param backupVersion - The version of the backup to create */ export function mockSetupMegolmBackupRequests(backupVersion: string): void { fetchMock.get("path:/_matrix/client/v3/room_keys/version", { diff --git a/src/client.ts b/src/client.ts index 347f218e2fe..648f33d141b 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3363,7 +3363,7 @@ export class MatrixClient extends TypedEventEmitter { if (!this.crypto) { @@ -3455,7 +3455,7 @@ export class MatrixClient extends TypedEventEmitter { if (!this.cryptoBackend) { diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 5b419c20f64..fdca02632b8 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -235,8 +235,11 @@ export interface CryptoApi { /** * Creates a new key backup version. + * * If there are existing backups they will be replaced. - * The decryption key will be saved in Secret Storage (will prompt for passphrase/key if neeeded) + * + * The decryption key will be saved in Secret Storage (the `SecretStorageCallbacks.getSecretStorageKey` Crypto + * callback will be called) * and the backup engine will be started. */ resetKeyBackup(): Promise; diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index 10844c90e57..de89bad5d68 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -226,9 +226,10 @@ export class BackupManager { /** * Deletes all key backups. + * * Will call the API to delete active backup until there is no more present. */ - public async deleteKeyBackup(): Promise { + public async deleteAllKeyBackupVersions(): Promise { // there could be several backup versions, delete all to be safe. let current = (await this.baseApis.getKeyBackupVersion())?.version ?? null; while (current != null) { diff --git a/src/crypto/index.ts b/src/crypto/index.ts index b101c3996ab..592af20f79e 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -545,7 +545,8 @@ export class Crypto extends TypedEventEmitter { await this.backupManager.deleteKeyBackupVersion(version); @@ -1097,7 +1098,7 @@ export class Crypto extends TypedEventEmitter { // Delete existing ones - await this.backupManager.deleteKeyBackup(); + // There is no use case for having several key backup version live server side. + // Even if not deleted it would be lost as the key to restore is lost. + // There should be only one backup at a time. + await this.backupManager.deleteAllKeyBackupVersions(); const info = await this.backupManager.prepareKeyBackupVersion(); @@ -1173,13 +1177,14 @@ export class Crypto extends TypedEventEmitter Promise): Promise { - // check first if there is an existing backup - if (this.activeBackupVersion || (await this.requestKeyBackupVersion()) != null) { - // we want to delete? - await this.deleteKeyBackup(); - } + public async setupKeyBackup(signObject: (authData: AuthData) => Promise): Promise { + // Cleanup any existing backup + await this.deleteAllKeyBackupVersions(); const version = await this.prepareKeyBackupVersion(); - await signer(version.auth_data); + await signObject(version.auth_data); const res = await this.http.authedRequest<{ version: string }>( Method.Post, @@ -353,17 +351,18 @@ export class RustBackupManager extends TypedEventEmitter { - // there could be several backup versions, delete all to be safe. + public async deleteAllKeyBackupVersions(): Promise { + // there could be several backup versions. Delete all to be safe. let current = (await this.requestKeyBackupVersion())?.version ?? null; while (current != null) { await this.deleteKeyBackupVersion(current); current = (await this.requestKeyBackupVersion())?.version ?? null; } - // Should this also update Secret Storage and delete any existing keys? + // XXX: Should this also update Secret Storage and delete any existing keys? } /** @@ -387,11 +386,9 @@ export class RustBackupManager extends TypedEventEmitter { - await this.backupManager.deleteKeyBackupVersion(version); - } - /** * Return the OlmMachine only if {@link RustCrypto#stop} has not been called. * @@ -622,12 +613,23 @@ export class RustCrypto extends TypedEventEmitter { + await this.backupManager.deleteKeyBackupVersion(version); + } + + /** + * Implementation of {@link CryptoApi#resetKeyBackup}. + */ public async resetKeyBackup(): Promise { - // check if there is already an existing backup - const backupInfo = await this.backupManager.setUpKeyBackup(this.signObject.bind(this)); + const backupInfo = await this.backupManager.setupKeyBackup((o) => this.signObject(o)); // we want to store the private key in 4S - // need to check if 4S setup? + // need to check if 4S is set up? if (await this.secretStorageHasAESKey()) { await this.secretStorage.store("m.megolm_backup.v1", backupInfo.decryptionKey.toBase64()); } @@ -636,7 +638,7 @@ export class RustCrypto extends TypedEventEmitter(obj: T): Promise { + private async signObject(obj: T): Promise { const sigs = new Map(Object.entries(obj.signatures || {})); const unsigned = obj.unsigned; From 05f80da821371f86dd5291577c6b35bc183d52e2 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 31 Aug 2023 15:52:22 +0200 Subject: [PATCH 14/14] missing ts-doc --- src/crypto/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 592af20f79e..82c2dd3251e 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -1155,6 +1155,9 @@ export class Crypto extends TypedEventEmitter { // Delete existing ones // There is no use case for having several key backup version live server side.