From 5a9d7ba2d756f635f23e1aa058aa79a59c55fa26 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 23 Aug 2024 15:00:18 +0100 Subject: [PATCH] Remove unused CryptoCallbacks implementations (#12919) * Remove unused `onSecretRequested` callback This thing is unused with the rust crypto stack (which is lucky, because it uses methods that only work with the legacy stack). * Remove unused `getDehydrationKey` method This callback is no longer used, so there is no need for an implementation. * Remove unused `dehydrationCache` This is no longer written to, so is redundant. * Remove another write to `CryptoCallbacks.getDehydrationKey` As before: this hook is no longer used by the js-sdk, so writing to it is pointless. --- src/MatrixClientPeg.ts | 6 -- src/SecurityManager.ts | 106 +---------------------------------- test/MatrixClientPeg-test.ts | 77 ------------------------- 3 files changed, 1 insertion(+), 188 deletions(-) diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 646f9eec62..775897bb1c 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -41,7 +41,6 @@ import MatrixClientBackedSettingsHandler from "./settings/handlers/MatrixClientB import * as StorageManager from "./utils/StorageManager"; import IdentityAuthClient from "./IdentityAuthClient"; import { crossSigningCallbacks } from "./SecurityManager"; -import { ModuleRunner } from "./modules/ModuleRunner"; import { SlidingSyncManager } from "./SlidingSyncManager"; import { _t, UserFriendlyError } from "./languageHandler"; import { SettingLevel } from "./settings/SettingLevel"; @@ -452,11 +451,6 @@ class MatrixClientPegClass implements IMatrixClientPeg { }, }; - const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup.getDehydrationKeyCallback(); - if (dehydrationKeyCallback) { - opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback; - } - this.matrixClient = createMatrixClient(opts); this.matrixClient.setGuest(Boolean(creds.guest)); diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index c1c4e7a72b..faf915e3c8 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Crypto, ICryptoCallbacks, encodeBase64, SecretStorage } from "matrix-js-sdk/src/matrix"; +import { ICryptoCallbacks, SecretStorage } from "matrix-js-sdk/src/matrix"; import { deriveKey } from "matrix-js-sdk/src/crypto/key_passphrase"; import { decodeRecoveryKey } from "matrix-js-sdk/src/crypto/recoverykey"; import { logger } from "matrix-js-sdk/src/logger"; @@ -39,11 +39,6 @@ let secretStorageKeys: Record = {}; let secretStorageKeyInfo: Record = {}; let secretStorageBeingAccessed = false; -let dehydrationCache: { - key?: Uint8Array; - keyInfo?: SecretStorage.SecretStorageKeyDescription; -} = {}; - /** * This can be used by other components to check if secret storage access is in * progress, so that we can e.g. avoid intermittently showing toasts during @@ -119,14 +114,6 @@ async function getSecretStorageKey({ return [keyId, secretStorageKeys[keyId]]; } - if (dehydrationCache.key) { - if (await MatrixClientPeg.safeGet().checkSecretStorageKey(dehydrationCache.key, keyInfo)) { - logger.debug("getSecretStorageKey: returning key from dehydration cache"); - cacheSecretStorageKey(keyId, keyInfo, dehydrationCache.key); - return [keyId, dehydrationCache.key]; - } - } - const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); if (keyFromCustomisations) { logger.log("getSecretStorageKey: Using secret storage key from CryptoSetupExtension"); @@ -171,56 +158,6 @@ async function getSecretStorageKey({ return [keyId, key]; } -export async function getDehydrationKey( - keyInfo: SecretStorage.SecretStorageKeyDescription, - checkFunc: (data: Uint8Array) => void, -): Promise { - const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); - if (keyFromCustomisations) { - logger.log("CryptoSetupExtension: Using key from extension (dehydration)"); - return keyFromCustomisations; - } - - const inputToKey = makeInputToKey(keyInfo); - const { finished } = Modal.createDialog( - AccessSecretStorageDialog, - /* props= */ - { - keyInfo, - checkPrivateKey: async (input: KeyParams): Promise => { - const key = await inputToKey(input); - try { - checkFunc(key); - return true; - } catch (e) { - return false; - } - }, - }, - /* className= */ undefined, - /* isPriorityModal= */ false, - /* isStaticModal= */ false, - /* options= */ { - onBeforeClose: async (reason): Promise => { - if (reason === "backgroundClick") { - return confirmToDismiss(); - } - return true; - }, - }, - ); - const [input] = await finished; - if (!input) { - throw new AccessCancelledError(); - } - const key = await inputToKey(input); - - // need to copy the key because rehydration (unpickling) will clobber it - dehydrationCache = { key: new Uint8Array(key), keyInfo }; - - return key; -} - function cacheSecretStorageKey( keyId: string, keyInfo: SecretStorage.SecretStorageKeyDescription, @@ -232,50 +169,9 @@ function cacheSecretStorageKey( } } -async function onSecretRequested( - userId: string, - deviceId: string, - requestId: string, - name: string, - deviceTrust: Crypto.DeviceVerificationStatus, -): Promise { - logger.log("onSecretRequested", userId, deviceId, requestId, name, deviceTrust); - const client = MatrixClientPeg.safeGet(); - if (userId !== client.getUserId()) { - return; - } - if (!deviceTrust?.isVerified()) { - logger.log(`Ignoring secret request from untrusted device ${deviceId}`); - return; - } - if ( - name === "m.cross_signing.master" || - name === "m.cross_signing.self_signing" || - name === "m.cross_signing.user_signing" - ) { - const callbacks = client.getCrossSigningCacheCallbacks(); - if (!callbacks?.getCrossSigningKeyCache) return; - const keyId = name.replace("m.cross_signing.", ""); - const key = await callbacks.getCrossSigningKeyCache(keyId); - if (!key) { - logger.log(`${keyId} requested by ${deviceId}, but not found in cache`); - } - return key ? encodeBase64(key) : undefined; - } else if (name === "m.megolm_backup.v1") { - const key = await client.crypto?.getSessionBackupPrivateKey(); - if (!key) { - logger.log(`session backup key requested by ${deviceId}, but not found in cache`); - } - return key ? encodeBase64(key) : undefined; - } - logger.warn("onSecretRequested didn't recognise the secret named ", name); -} - export const crossSigningCallbacks: ICryptoCallbacks = { getSecretStorageKey, cacheSecretStorageKey, - onSecretRequested, - getDehydrationKey, }; /** diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 121da2a154..99e9195422 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -16,16 +16,11 @@ limitations under the License. import { logger } from "matrix-js-sdk/src/logger"; import fetchMockJest from "fetch-mock-jest"; -import { - ProvideCryptoSetupExtensions, - SecretStorageKeyDescription, -} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { advanceDateAndTime, stubClient } from "./test-utils"; import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg"; import SettingsStore from "../src/settings/SettingsStore"; import { SettingLevel } from "../src/settings/SettingLevel"; -import { ModuleRunner } from "../src/modules/ModuleRunner"; jest.useFakeTimers(); @@ -78,78 +73,6 @@ describe("MatrixClientPeg", () => { expect(peg.userRegisteredWithinLastHours(24)).toBe(false); }); - describe(".start extensions", () => { - let testPeg: IMatrixClientPeg; - - beforeEach(() => { - // instantiate a MatrixClientPegClass instance, with a new MatrixClient - testPeg = new PegClass(); - fetchMockJest.get("http://example.com/_matrix/client/versions", {}); - }); - - describe("cryptoSetup extension", () => { - it("should call default cryptoSetup.getDehydrationKeyCallback", async () => { - const mockCryptoSetup = { - SHOW_ENCRYPTION_SETUP_UI: true, - examineLoginResponse: jest.fn(), - persistCredentials: jest.fn(), - getSecretStorageKey: jest.fn(), - createSecretStorageKey: jest.fn(), - catchAccessSecretStorageError: jest.fn(), - setupEncryptionNeeded: jest.fn(), - getDehydrationKeyCallback: jest.fn().mockReturnValue(null), - } as ProvideCryptoSetupExtensions; - - // Ensure we have an instance before we set up spies - const instance = ModuleRunner.instance; - jest.spyOn(instance.extensions, "cryptoSetup", "get").mockReturnValue(mockCryptoSetup); - - testPeg.replaceUsingCreds({ - accessToken: "SEKRET", - homeserverUrl: "http://example.com", - userId: "@user:example.com", - deviceId: "TEST_DEVICE_ID", - }); - - expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); - }); - - it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { - const mockDehydrationKeyCallback = () => Uint8Array.from([0x11, 0x22, 0x33]); - - const mockCryptoSetup = { - SHOW_ENCRYPTION_SETUP_UI: true, - examineLoginResponse: jest.fn(), - persistCredentials: jest.fn(), - getSecretStorageKey: jest.fn(), - createSecretStorageKey: jest.fn(), - catchAccessSecretStorageError: jest.fn(), - setupEncryptionNeeded: jest.fn(), - getDehydrationKeyCallback: jest.fn().mockReturnValue(mockDehydrationKeyCallback), - } as ProvideCryptoSetupExtensions; - - // Ensure we have an instance before we set up spies - const instance = ModuleRunner.instance; - jest.spyOn(instance.extensions, "cryptoSetup", "get").mockReturnValue(mockCryptoSetup); - - testPeg.replaceUsingCreds({ - accessToken: "SEKRET", - homeserverUrl: "http://example.com", - userId: "@user:example.com", - deviceId: "TEST_DEVICE_ID", - }); - expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); - - const client = testPeg.get(); - const dehydrationKey = await client?.cryptoCallbacks.getDehydrationKey!( - {} as SecretStorageKeyDescription, - (key: Uint8Array) => true, - ); - expect(dehydrationKey).toEqual(Uint8Array.from([0x11, 0x22, 0x33])); - }); - }); - }); - describe(".start", () => { let testPeg: IMatrixClientPeg;