Skip to content

Commit

Permalink
Remove unused CryptoCallbacks implementations (#12919)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@richvdh
richvdh authored Aug 23, 2024
1 parent 69da175 commit 5a9d7ba
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 188 deletions.
6 changes: 0 additions & 6 deletions src/MatrixClientPeg.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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));

Expand Down
106 changes: 1 addition & 105 deletions src/SecurityManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -39,11 +39,6 @@ let secretStorageKeys: Record<string, Uint8Array> = {};
let secretStorageKeyInfo: Record<string, SecretStorage.SecretStorageKeyDescription> = {};
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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -171,56 +158,6 @@ async function getSecretStorageKey({
return [keyId, key];
}

export async function getDehydrationKey(
keyInfo: SecretStorage.SecretStorageKeyDescription,
checkFunc: (data: Uint8Array) => void,
): Promise<Uint8Array> {
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<boolean> => {
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<boolean> => {
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,
Expand All @@ -232,50 +169,9 @@ function cacheSecretStorageKey(
}
}

async function onSecretRequested(
userId: string,
deviceId: string,
requestId: string,
name: string,
deviceTrust: Crypto.DeviceVerificationStatus,
): Promise<string | undefined> {
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,
};

/**
Expand Down
77 changes: 0 additions & 77 deletions test/MatrixClientPeg-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 5a9d7ba

Please sign in to comment.