diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 4120b5f53..c6a916bee 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -74,7 +74,7 @@ import { mockSetupCrossSigningRequests, mockSetupMegolmBackupRequests, } from "../../test-utils/mockEndpoints"; -import { AddSecretStorageKeyOpts } from "../../../src/secret-storage"; +import { SecretStorageKeyDescription } from "../../../src/secret-storage"; import { CrossSigningKey, CryptoCallbacks, @@ -340,7 +340,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, function createCryptoCallbacks(): CryptoCallbacks { // Store the cached secret storage key and return it when `getSecretStorageKey` is called let cachedKey: { keyId: string; key: Uint8Array }; - const cacheSecretStorageKey = (keyId: string, keyInfo: AddSecretStorageKeyOpts, key: Uint8Array) => { + const cacheSecretStorageKey = (keyId: string, keyInfo: SecretStorageKeyDescription, key: Uint8Array) => { cachedKey = { keyId, key, @@ -2567,6 +2567,30 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, await backupStatusUpdate; } + describe("Generate 4S recovery keys", () => { + it("should create a random recovery key", async () => { + const generatedKey = await aliceClient.getCrypto()!.createRecoveryKeyFromPassphrase(); + expect(generatedKey.privateKey).toBeDefined(); + expect(generatedKey.privateKey).toBeInstanceOf(Uint8Array); + expect(generatedKey.privateKey.length).toBe(32); + expect(generatedKey.keyInfo?.passphrase).toBeUndefined(); + expect(generatedKey.encodedPrivateKey).toBeDefined(); + expect(generatedKey.encodedPrivateKey!.indexOf("Es")).toBe(0); + }); + + it("should create a recovery key from passphrase", async () => { + const generatedKey = await aliceClient.getCrypto()!.createRecoveryKeyFromPassphrase("mypassphrase"); + expect(generatedKey.privateKey).toBeDefined(); + expect(generatedKey.privateKey).toBeInstanceOf(Uint8Array); + expect(generatedKey.privateKey.length).toBe(32); + expect(generatedKey.keyInfo?.passphrase?.algorithm).toBe("m.pbkdf2"); + expect(generatedKey.keyInfo?.passphrase?.iterations).toBe(500000); + + expect(generatedKey.encodedPrivateKey).toBeDefined(); + expect(generatedKey.encodedPrivateKey!.indexOf("Es")).toBe(0); + }); + }); + describe("bootstrapSecretStorage", () => { // Doesn't work with legacy crypto, which will try to bootstrap even without private key, which is buggy. newBackendOnly( @@ -2592,6 +2616,14 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Wait for the key to be uploaded in the account data const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + // check that the key content contains the key check info + const keyContent = accountDataAccumulator.accountDataEvents.get( + `m.secret_storage.key.${secretStorageKey}`, + )!; + // In order to verify if the key is valid, a zero secret is encrypted with the key + expect(keyContent.iv).toBeDefined(); + expect(keyContent.mac).toBeDefined(); + // Return the newly created key in the sync response accountDataAccumulator.sendSyncResponseWithUpdatedAccountData(syncResponder); diff --git a/spec/unit/crypto/crypto-utils.ts b/spec/unit/crypto/crypto-utils.ts index 1dd0065de..39a53562a 100644 --- a/spec/unit/crypto/crypto-utils.ts +++ b/spec/unit/crypto/crypto-utils.ts @@ -33,12 +33,10 @@ export async function resetCrossSigningKeys( export async function createSecretStorageKey(): Promise { const decryption = new global.Olm.PkDecryption(); - const storagePublicKey = decryption.generate_key(); + decryption.generate_key(); const storagePrivateKey = decryption.get_private_key(); decryption.free(); return { - // `pubkey` not used anymore with symmetric 4S - keyInfo: { pubkey: storagePublicKey, key: undefined! }, privateKey: storagePrivateKey, }; } diff --git a/spec/unit/crypto/secrets.spec.ts b/spec/unit/crypto/secrets.spec.ts index 907280d17..27842e877 100644 --- a/spec/unit/crypto/secrets.spec.ts +++ b/spec/unit/crypto/secrets.spec.ts @@ -190,10 +190,7 @@ describe("Secrets", function () { }; resetCrossSigningKeys(alice); - const { keyId: newKeyId } = await alice.addSecretStorageKey(SECRET_STORAGE_ALGORITHM_V1_AES, { - pubkey: undefined, - key: undefined, - }); + const { keyId: newKeyId } = await alice.addSecretStorageKey(SECRET_STORAGE_ALGORITHM_V1_AES, { key }); // we don't await on this because it waits for the event to come down the sync // which won't happen in the test setup alice.setDefaultSecretStorageKeyId(newKeyId); @@ -335,7 +332,6 @@ describe("Secrets", function () { it("bootstraps when cross-signing keys in secret storage", async function () { const decryption = new global.Olm.PkDecryption(); - const storagePublicKey = decryption.generate_key(); const storagePrivateKey = decryption.get_private_key(); const bob: MatrixClient = await makeTestClient( @@ -378,8 +374,6 @@ describe("Secrets", function () { }); await bob.bootstrapSecretStorage({ createSecretStorageKey: async () => ({ - // `pubkey` not used anymore with symmetric 4S - keyInfo: { pubkey: storagePublicKey }, privateKey: storagePrivateKey, }), }); diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index 5480ffc1b..2fa73b133 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -739,8 +739,8 @@ describe("RustCrypto", () => { // Expect the private key to be an Uint8Array with a length of 32 expect(recoveryKey.privateKey).toBeInstanceOf(Uint8Array); expect(recoveryKey.privateKey.length).toBe(32); - // Expect keyInfo to be empty - expect(Object.keys(recoveryKey.keyInfo!).length).toBe(0); + // Expect passphrase info to be absent + expect(recoveryKey.keyInfo?.passphrase).toBeUndefined(); }); it("should create a recovery key with password", async () => { diff --git a/spec/unit/secret-storage.spec.ts b/spec/unit/secret-storage.spec.ts index 41666f91e..e2f07b79d 100644 --- a/spec/unit/secret-storage.spec.ts +++ b/spec/unit/secret-storage.spec.ts @@ -33,7 +33,9 @@ describe("ServerSideSecretStorageImpl", function () { it("should allow storing a default key", async function () { const accountDataAdapter = mockAccountDataClient(); const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {}); - const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2"); + const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2", { + key: new Uint8Array(32), + }); // it should have made up a 32-character key id expect(result.keyId.length).toEqual(32); @@ -46,7 +48,13 @@ describe("ServerSideSecretStorageImpl", function () { it("should allow storing a key with an explicit id", async function () { const accountDataAdapter = mockAccountDataClient(); const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {}); - const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2", {}, "myKeyId"); + const result = await secretStorage.addKey( + "m.secret_storage.v1.aes-hmac-sha2", + { + key: new Uint8Array(32), + }, + "myKeyId", + ); // it should have made up a 32-character key id expect(result.keyId).toEqual("myKeyId"); @@ -59,7 +67,10 @@ describe("ServerSideSecretStorageImpl", function () { it("should allow storing a key with a name", async function () { const accountDataAdapter = mockAccountDataClient(); const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {}); - const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2", { name: "mykey" }); + const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2", { + name: "mykey", + key: new Uint8Array(32), + }); expect(result.keyInfo.name).toEqual("mykey"); @@ -80,6 +91,7 @@ describe("ServerSideSecretStorageImpl", function () { }; const result = await secretStorage.addKey("m.secret_storage.v1.aes-hmac-sha2", { passphrase, + key: new Uint8Array(32), }); expect(result.keyInfo.passphrase).toEqual(passphrase); @@ -93,7 +105,9 @@ describe("ServerSideSecretStorageImpl", function () { it("should complain about invalid algorithm", async function () { const accountDataAdapter = mockAccountDataClient(); const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {}); - await expect(() => secretStorage.addKey("bad_alg")).rejects.toThrow("Unknown key algorithm"); + await expect(() => secretStorage.addKey("bad_alg", { key: new Uint8Array(32) })).rejects.toThrow( + "Unknown key algorithm", + ); }); }); diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 7a4647c21..163c49197 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -18,7 +18,7 @@ import type { IMegolmSessionData } from "./@types/crypto"; import { Room } from "./models/room"; import { DeviceMap } from "./models/device"; import { UIAuthCallback } from "./interactive-auth"; -import { AddSecretStorageKeyOpts, SecretStorageCallbacks, SecretStorageKeyDescription } from "./secret-storage"; +import { PassphraseInfo, SecretStorageCallbacks, SecretStorageKeyDescription } from "./secret-storage"; import { VerificationRequest } from "./crypto-api/verification"; import { BackupTrustInfo, KeyBackupCheck, KeyBackupInfo } from "./crypto-api/keybackup"; import { ISignatures } from "./@types/signed"; @@ -710,10 +710,15 @@ export interface CrossSigningKeyInfo { } /** - * Recovery key created by {@link CryptoApi#createRecoveryKeyFromPassphrase} + * Recovery key created by {@link CryptoApi#createRecoveryKeyFromPassphrase} or {@link CreateSecretStorageOpts#createSecretStorageKey}. */ export interface GeneratedSecretStorageKey { - keyInfo?: AddSecretStorageKeyOpts; + keyInfo?: { + /** If the key was derived from a passphrase, information (algorithm, salt, etc) on that derivation. */ + passphrase?: PassphraseInfo; + /** Optional human-readable name for the key, to be stored in account_data. */ + name?: string; + }; /** The raw generated private key. */ privateKey: Uint8Array; /** The generated key, encoded for display to the user per https://spec.matrix.org/v1.7/client-server-api/#key-representation. */ diff --git a/src/crypto/SecretStorage.ts b/src/crypto/SecretStorage.ts index 1d91fa189..452d1fde9 100644 --- a/src/crypto/SecretStorage.ts +++ b/src/crypto/SecretStorage.ts @@ -73,11 +73,7 @@ export class SecretStorage im /** * Add a key for encrypting secrets. */ - public addKey( - algorithm: string, - opts: AddSecretStorageKeyOpts = {}, - keyId?: string, - ): Promise { + public addKey(algorithm: string, opts: AddSecretStorageKeyOpts, keyId?: string): Promise { return this.storageImpl.addKey(algorithm, opts, keyId); } diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 1b3cc228c..1172e00e3 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -708,25 +708,30 @@ export class Crypto extends TypedEventEmitter { const decryption = new global.Olm.PkDecryption(); try { - const keyInfo: Partial = {}; if (password) { const derivation = await keyFromPassphrase(password); - keyInfo.passphrase = { - algorithm: "m.pbkdf2", - iterations: derivation.iterations, - salt: derivation.salt, + + decryption.init_with_private_key(derivation.key); + const privateKey = decryption.get_private_key(); + return { + keyInfo: { + passphrase: { + algorithm: "m.pbkdf2", + iterations: derivation.iterations, + salt: derivation.salt, + }, + }, + privateKey: privateKey, + encodedPrivateKey: encodeRecoveryKey(privateKey), }; - keyInfo.pubkey = decryption.init_with_private_key(derivation.key); } else { - keyInfo.pubkey = decryption.generate_key(); + decryption.generate_key(); + const privateKey = decryption.get_private_key(); + return { + privateKey: privateKey, + encodedPrivateKey: encodeRecoveryKey(privateKey), + }; } - const privateKey = decryption.get_private_key(); - const encodedPrivateKey = encodeRecoveryKey(privateKey); - return { - keyInfo: keyInfo as IRecoveryKey["keyInfo"], - encodedPrivateKey, - privateKey, - }; } finally { decryption?.free(); } @@ -986,17 +991,11 @@ export class Crypto extends TypedEventEmitter => { - if (privateKey) { - opts.key = privateKey; - } - + const createSSSS = async (opts: AddSecretStorageKeyOpts): Promise => { const { keyId, keyInfo } = await secretStorage.addKey(SECRET_STORAGE_ALGORITHM_V1_AES, opts); - if (privateKey) { - // make the private key available to encrypt 4S secrets - builder.ssssCryptoCallbacks.addPrivateKey(keyId, keyInfo, privateKey); - } + // make the private key available to encrypt 4S secrets + builder.ssssCryptoCallbacks.addPrivateKey(keyId, keyInfo, opts.key); await secretStorage.setDefaultKeyId(keyId); return keyId; @@ -1060,8 +1059,8 @@ export class Crypto extends TypedEventEmitter { - // keyInfo is required to continue - if (!secretStorageKey.keyInfo) { - throw new Error("missing keyInfo field in the secret storage key"); - } - - const secretStorageKeyObject = await this.secretStorage.addKey( - SECRET_STORAGE_ALGORITHM_V1_AES, - secretStorageKey.keyInfo, - ); + const secretStorageKeyObject = await this.secretStorage.addKey(SECRET_STORAGE_ALGORITHM_V1_AES, { + passphrase: secretStorageKey.keyInfo?.passphrase, + name: secretStorageKey.keyInfo?.name, + key: secretStorageKey.privateKey, + }); await this.secretStorage.setDefaultKeyId(secretStorageKeyObject.keyId); @@ -817,30 +813,29 @@ export class RustCrypto extends TypedEventEmitter { - let key: Uint8Array; - - const keyInfo: AddSecretStorageKeyOpts = {}; if (password) { // Generate the key from the passphrase const derivation = await keyFromPassphrase(password); - keyInfo.passphrase = { - algorithm: "m.pbkdf2", - iterations: derivation.iterations, - salt: derivation.salt, + return { + keyInfo: { + passphrase: { + algorithm: "m.pbkdf2", + iterations: derivation.iterations, + salt: derivation.salt, + }, + }, + privateKey: derivation.key, + encodedPrivateKey: encodeRecoveryKey(derivation.key), }; - key = derivation.key; } else { // Using the navigator crypto API to generate the private key - key = new Uint8Array(32); + const key = new Uint8Array(32); crypto.getRandomValues(key); + return { + privateKey: key, + encodedPrivateKey: encodeRecoveryKey(key), + }; } - - const encodedPrivateKey = encodeRecoveryKey(key); - return { - keyInfo, - encodedPrivateKey, - privateKey: key, - }; } /** diff --git a/src/secret-storage.ts b/src/secret-storage.ts index 6a631415b..71ef30856 100644 --- a/src/secret-storage.ts +++ b/src/secret-storage.ts @@ -100,10 +100,12 @@ export interface PassphraseInfo { * Options for {@link ServerSideSecretStorageImpl#addKey}. */ export interface AddSecretStorageKeyOpts { - pubkey?: string; + /** Information for deriving the key from a passphrase if any. */ passphrase?: PassphraseInfo; + /** Optional name of the key. */ name?: string; - key?: Uint8Array; + /** The private key. Will be used to generate the key check values in the key info; it will not be stored on the server */ + key: Uint8Array; } /** @@ -380,7 +382,7 @@ export class ServerSideSecretStorageImpl implements ServerSideSecretStorage { */ public async addKey( algorithm: string, - opts: AddSecretStorageKeyOpts = {}, + opts: AddSecretStorageKeyOpts, keyId?: string, ): Promise { if (algorithm !== SECRET_STORAGE_ALGORITHM_V1_AES) { @@ -396,11 +398,10 @@ export class ServerSideSecretStorageImpl implements ServerSideSecretStorage { if (opts.passphrase) { keyInfo.passphrase = opts.passphrase; } - if (opts.key) { - const { iv, mac } = await calculateKeyCheck(opts.key); - keyInfo.iv = iv; - keyInfo.mac = mac; - } + + const { iv, mac } = await calculateKeyCheck(opts.key); + keyInfo.iv = iv; + keyInfo.mac = mac; // Create a unique key id. XXX: this is racey. if (!keyId) {