matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-07-31 15:24:23 +03:00
Code Activity

Expose ServerSideSecretStorage independently of Crypto (#3280)

Browse Source 
There is no reason to indirect secret storage via the Crypto layer, and
exposing it directly means it will work for Element-R.

Fixes: https://github.com/vector-im/element-web/issues/24982
This commit is contained in:
Richard van der Hoff
2023-04-13 17:21:38 +01:00
committed by GitHub
parent f400a7b1b2
commit 1e1b571b28
5 changed files with 122 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
spec/unit/crypto.spec.ts
View File

@ -1011,7 +1011,6 @@ describe("Crypto", function () {
jest.setTimeout(10000);
const client = new TestClient("@a:example.com", "dev").client;
await client.initCrypto();
client.crypto!.getSecretStorageKey = jest.fn().mockResolvedValue(null);
client.crypto!.isCrossSigningReady = async () => false;
client.crypto!.baseApis.uploadDeviceSigningKeys = jest.fn().mockResolvedValue(null);
client.crypto!.baseApis.setAccountData = jest.fn().mockResolvedValue(null);

41
spec/unit/matrix-client.spec.ts
View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { mocked } from "jest-mock";
import { Mocked, mocked } from "jest-mock";
import { logger } from "../../src/logger";
import { ClientEvent, IMatrixClientCreateOpts, ITurnServerResponse, MatrixClient, Store } from "../../src/client";
@ -63,6 +63,7 @@ import { QueryDict } from "../../src/utils";
import { SyncState } from "../../src/sync";
import * as featureUtils from "../../src/feature";
import { StubStore } from "../../src/store/stub";
import { SecretStorageKeyDescriptionAesV1, ServerSideSecretStorageImpl } from "../../src/secret-storage";
jest.useFakeTimers();
@ -2704,4 +2705,42 @@ describe("MatrixClient", function () {
});
});
});
// these wrappers are deprecated, but we need coverage of them to pass the quality gate
describe("SecretStorage wrappers", () => {
let mockSecretStorage: Mocked<ServerSideSecretStorageImpl>;
beforeEach(() => {
mockSecretStorage = {
getDefaultKeyId: jest.fn(),
hasKey: jest.fn(),
isStored: jest.fn(),
} as unknown as Mocked<ServerSideSecretStorageImpl>;
client["_secretStorage"] = mockSecretStorage;
});
it("hasSecretStorageKey", async () => {
mockSecretStorage.hasKey.mockResolvedValue(false);
expect(await client.hasSecretStorageKey("mykey")).toBe(false);
expect(mockSecretStorage.hasKey).toHaveBeenCalledWith("mykey");
});
it("isSecretStored", async () => {
const mockResult = { key: {} as SecretStorageKeyDescriptionAesV1 };
mockSecretStorage.isStored.mockResolvedValue(mockResult);
expect(await client.isSecretStored("mysecret")).toBe(mockResult);
expect(mockSecretStorage.isStored).toHaveBeenCalledWith("mysecret");
});
it("getDefaultSecretStorageKeyId", async () => {
mockSecretStorage.getDefaultKeyId.mockResolvedValue("bzz");
expect(await client.getDefaultSecretStorageKeyId()).toEqual("bzz");
});
it("isKeyBackupKeyStored", async () => {
mockSecretStorage.isStored.mockResolvedValue(null);
expect(await client.isKeyBackupKeyStored()).toBe(null);
expect(mockSecretStorage.isStored).toHaveBeenCalledWith("m.megolm_backup.v1");
});
});
});

87
src/client.ts
View File

@ -206,7 +206,12 @@ import { CryptoBackend } from "./common-crypto/CryptoBackend";
import { RUST_SDK_STORE_PREFIX } from "./rust-crypto/constants";
import { CryptoApi } from "./crypto-api";
import { DeviceInfoMap } from "./crypto/DeviceList";
import { AddSecretStorageKeyOpts, SecretStorageKeyDescription } from "./secret-storage";
import {
AddSecretStorageKeyOpts,
SecretStorageKeyDescription,
ServerSideSecretStorage,
ServerSideSecretStorageImpl,
} from "./secret-storage";
export type Store = IStore;
@ -1252,6 +1257,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
private useE2eForGroupCall = true;
private toDeviceMessageQueue: ToDeviceMessageQueue;
private _secretStorage: ServerSideSecretStorageImpl;
// A manager for determining which invites should be ignored.
public readonly ignoredInvites: IgnoredInvites;
@ -1417,6 +1424,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
});
this.ignoredInvites = new IgnoredInvites(this);
this._secretStorage = new ServerSideSecretStorageImpl(this, opts.cryptoCallbacks ?? {});
}
/**
@ -2226,6 +2234,13 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
this.on(RoomMemberEvent.Membership, rustCrypto.onRoomMembership.bind(rustCrypto));
}
/**
* Access the server-side secret storage API for this client.
*/
public get secretStorage(): ServerSideSecretStorage {
return this._secretStorage;
}
/**
* Access the crypto API for this client.
*
@ -2472,11 +2487,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
return this.crypto.beginKeyVerification(method, userId, deviceId);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#checkKey}.
*/
public checkSecretStorageKey(key: Uint8Array, info: SecretStorageKeyDescription): Promise<boolean> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.checkSecretStorageKey(key, info);
return this.secretStorage.checkKey(key, info);
}
/**
@ -2867,16 +2882,15 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @returns An object with:
* keyId: the ID of the key
* keyInfo: details about the key (iv, mac, passphrase)
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#addKey}.
*/
public addSecretStorageKey(
algorithm: string,
opts: AddSecretStorageKeyOpts,
keyName?: string,
): Promise<{ keyId: string; keyInfo: SecretStorageKeyDescription }> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.addSecretStorageKey(algorithm, opts, keyName);
return this.secretStorage.addKey(algorithm, opts, keyName);
}
/**
@ -2887,12 +2901,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @param keyId - The ID of the key to check
* for. Defaults to the default key ID if not provided.
* @returns Whether we have the key.
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#hasKey}.
*/
public hasSecretStorageKey(keyId?: string): Promise<boolean> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.hasSecretStorageKey(keyId);
return this.secretStorage.hasKey(keyId);
}
/**
@ -2904,12 +2917,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @param secret - The secret contents.
* @param keys - The IDs of the keys to use to encrypt the secret or null/undefined
* to use the default (will throw if no default key is set).
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#store}.
*/
public storeSecret(name: string, secret: string, keys?: string[]): Promise<void> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.storeSecret(name, secret, keys);
return this.secretStorage.store(name, secret, keys);
}
/**
@ -2920,12 +2932,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @param name - the name of the secret
*
* @returns the contents of the secret
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#get}.
*/
public getSecret(name: string): Promise<string | undefined> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.getSecret(name);
return this.secretStorage.get(name);
}
/**
@ -2937,12 +2948,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @returns map of key name to key info the secret is encrypted
* with, or null if it is not present or not encrypted with a trusted
* key
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#isStored}.
*/
public isSecretStored(name: string): Promise<Record<string, SecretStorageKeyDescription> | null> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.isSecretStored(name);
return this.secretStorage.isStored(name);
}
/**
@ -2968,12 +2978,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* The Secure Secret Storage API is currently UNSTABLE and may change without notice.
*
* @returns The default key ID or null if no default key ID is set
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#getDefaultKeyId}.
*/
public getDefaultSecretStorageKeyId(): Promise<string | null> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.getDefaultSecretStorageKeyId();
return this.secretStorage.getDefaultKeyId();
}
/**
@ -2982,12 +2991,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* The Secure Secret Storage API is currently UNSTABLE and may change without notice.
*
* @param keyId - The new default key ID
*
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#setDefaultKeyId}.
*/
public setDefaultSecretStorageKeyId(keyId: string): Promise<void> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
return this.crypto.setDefaultSecretStorageKeyId(keyId);
return this.secretStorage.setDefaultKeyId(keyId);
}
/**
@ -3000,6 +3008,9 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @param privateKey - The private key
* @param expectedPublicKey - The public key
* @returns true if the key matches, otherwise false
*
* @deprecated The use of asymmetric keys for SSSS is deprecated.
* Use {@link SecretStorage.ServerSideSecretStorage#checkKey} for symmetric keys.
*/
public checkSecretStoragePrivateKey(privateKey: Uint8Array, expectedPublicKey: string): boolean {
if (!this.crypto) {
@ -3296,7 +3307,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
await this.crypto.backupManager.prepareKeyBackupVersion(password);
if (opts.secureSecretStorage) {
await this.storeSecret("m.megolm_backup.v1", encodeBase64(privateKey));
await this.secretStorage.store("m.megolm_backup.v1", encodeBase64(privateKey));
logger.info("Key backup private key stored in secret storage");
}
@ -3316,7 +3327,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* trusted key
*/
public isKeyBackupKeyStored(): Promise<Record<string, SecretStorageKeyDescription> | null> {
return Promise.resolve(this.isSecretStored("m.megolm_backup.v1"));
return Promise.resolve(this.secretStorage.isStored("m.megolm_backup.v1"));
}
/**
@ -3581,14 +3592,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}
const storedKey = await this.getSecret("m.megolm_backup.v1");
const storedKey = await this.secretStorage.get("m.megolm_backup.v1");
// ensure that the key is in the right format. If not, fix the key and
// store the fixed version
const fixedKey = fixBackupKey(storedKey);
if (fixedKey) {
const keys = await this.crypto.getSecretStorageKey();
await this.storeSecret("m.megolm_backup.v1", fixedKey, [keys![0]]);
const keys = await this.secretStorage.getKey();
await this.secretStorage.store("m.megolm_backup.v1", fixedKey, [keys![0]]);
}
const privKey = decodeBase64(fixedKey || storedKey!);

35
src/crypto/index.ts
View File

@ -476,15 +476,15 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
}
// try to get key from secret storage
const storedKey = await this.getSecret("m.megolm_backup.v1");
const storedKey = await this.secretStorage.get("m.megolm_backup.v1");
if (storedKey) {
// ensure that the key is in the right format. If not, fix the key and
// store the fixed version
const fixedKey = fixBackupKey(storedKey);
if (fixedKey) {
const keys = await this.getSecretStorageKey();
await this.storeSecret("m.megolm_backup.v1", fixedKey, [keys![0]]);
const keys = await this.secretStorage.getKey();
await this.secretStorage.store("m.megolm_backup.v1", fixedKey, [keys![0]]);
}
return olmlib.decodeBase64(fixedKey || storedKey);
@ -950,7 +950,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
}
};
const oldSSSSKey = await this.getSecretStorageKey();
const oldSSSSKey = await this.secretStorage.getKey();
const [oldKeyId, oldKeyInfo] = oldSSSSKey || [null, null];
const storageExists =
!setupNewSecretStorage && oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES;
@ -1100,6 +1100,9 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
logger.log("Secure Secret Storage ready");
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#addKey}.
*/
public addSecretStorageKey(
algorithm: string,
opts: AddSecretStorageKeyOpts,
@ -1108,22 +1111,37 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
return this.secretStorage.addKey(algorithm, opts, keyID);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#hasKey}.
*/
public hasSecretStorageKey(keyID?: string): Promise<boolean> {
return this.secretStorage.hasKey(keyID);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#getKey}.
*/
public getSecretStorageKey(keyID?: string): Promise<SecretStorageKeyTuple | null> {
return this.secretStorage.getKey(keyID);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#store}.
*/
public storeSecret(name: string, secret: string, keys?: string[]): Promise<void> {
return this.secretStorage.store(name, secret, keys);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#get}.
*/
public getSecret(name: string): Promise<string | undefined> {
return this.secretStorage.get(name);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#isStored}.
*/
public isSecretStored(name: string): Promise<Record<string, SecretStorageKeyDescription> | null> {
return this.secretStorage.isStored(name);
}
@ -1135,14 +1153,23 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
return this.secretStorage.request(name, devices);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#getDefaultKeyId}.
*/
public getDefaultSecretStorageKeyId(): Promise<string | null> {
return this.secretStorage.getDefaultKeyId();
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#setDefaultKeyId}.
*/
public setDefaultSecretStorageKeyId(k: string): Promise<void> {
return this.secretStorage.setDefaultKeyId(k);
}
/**
* @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#checkKey}.
*/
public checkSecretStorageKey(key: Uint8Array, info: SecretStorageKeyDescription): Promise<boolean> {
return this.secretStorage.checkKey(key, info);
}

3
src/secret-storage.ts
View File

@ -452,7 +452,8 @@ export class ServerSideSecretStorageImpl implements ServerSideSecretStorage {
* @returns Whether we have the key.
*/
public async hasKey(keyId?: string): Promise<boolean> {
return Boolean(await this.getKey(keyId));
const key = await this.getKey(keyId);
return Boolean(key);
}
/**