Skip to content

Commit

Permalink
Implement CryptoApi.isKeyBackupTrusted (#3632)
Browse files Browse the repository at this point in the history
* Implement `CryptoApi.isKeyBackupTrusted`

Fixes element-hq/crypto-internal#110

* Bump matrix-sdk-crypto-wasm to v1.2.0

* Back out some changes

These are unneeded, and break backwards compat
  • Loading branch information
richvdh authored Jul 28, 2023
1 parent 6d28154 commit 2193cd9
Show file tree
Hide file tree
Showing 11 changed files with 174 additions and 12 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
],
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/matrix-sdk-crypto-wasm": "^1.1.0",
"@matrix-org/matrix-sdk-crypto-wasm": "^1.2.0",
"another-json": "^0.2.0",
"bs58": "^5.0.0",
"content-type": "^1.0.4",
Expand Down
66 changes: 65 additions & 1 deletion spec/integ/crypto/megolm-backup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import fetchMock from "fetch-mock-jest";
import "fake-indexeddb/auto";
import { IDBFactory } from "fake-indexeddb";

import { IKeyBackupSession } from "../../../src/crypto/keybackup";
import { createClient, CryptoEvent, ICreateClientOpts, IEvent, MatrixClient } from "../../../src";
Expand All @@ -25,6 +26,7 @@ import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
import { mockInitialApiRequests } from "../../test-utils/mockEndpoints";
import { awaitDecryption, CRYPTO_BACKENDS, InitCrypto, syncPromise } from "../../test-utils/test-utils";
import * as testData from "../../test-utils/test-data";
import { KeyBackupInfo } from "../../../src/crypto-api/keybackup";

const ROOM_ID = "!ROOM:ID";

Expand Down Expand Up @@ -74,6 +76,13 @@ const CURVE25519_KEY_BACKUP_DATA: IKeyBackupSession = {
const TEST_USER_ID = "@alice:localhost";
const TEST_DEVICE_ID = "xzcvb";

afterEach(() => {
// reset fake-indexeddb after each test, to make sure we don't leak connections
// cf https://github.com/dumbmatter/fakeIndexedDB#wipingresetting-the-indexeddb-for-a-fresh-state
// eslint-disable-next-line no-global-assign
indexedDB = new IDBFactory();
});

describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backend: string, initCrypto: InitCrypto) => {
// oldBackendOnly is an alternative to `it` or `test` which will skip the test if we are running against the
// Rust backend. Once we have full support in the rust sdk, it will go away.
Expand Down Expand Up @@ -204,7 +213,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
overwriteRoutes: true,
});

// check that signaling is working
// check that signalling is working
const backupPromise = new Promise<void>((resolve, reject) => {
aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => {
if (enabled) {
Expand All @@ -222,6 +231,61 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
expect(backupStatus).toStrictEqual(testData.SIGNED_BACKUP_DATA.version);
});

describe("isKeyBackupTrusted", () => {
it("does not trust a backup signed by an untrusted device", async () => {
aliceClient = await initTestClient();
const aliceCrypto = aliceClient.getCrypto()!;

// download the device list, to match the trusted case
await aliceClient.startClient();
await waitForDeviceList();

const result = await aliceCrypto.isKeyBackupTrusted(testData.SIGNED_BACKUP_DATA);
expect(result).toEqual({ trusted: false, matchesDecryptionKey: false });
});

it("trusts a backup signed by a trusted device", async () => {
aliceClient = await initTestClient();
const aliceCrypto = aliceClient.getCrypto()!;

// tell Alice to trust the dummy device that signed the backup
await aliceClient.startClient();
await waitForDeviceList();
await aliceCrypto.setDeviceVerified(testData.TEST_USER_ID, testData.TEST_DEVICE_ID);

const result = await aliceCrypto.isKeyBackupTrusted(testData.SIGNED_BACKUP_DATA);
expect(result).toEqual({ trusted: true, matchesDecryptionKey: false });
});

it("recognises a backup which matches the decryption key", async () => {
aliceClient = await initTestClient();
const aliceCrypto = aliceClient.getCrypto()!;

await aliceClient.startClient();
await aliceCrypto.storeSessionBackupPrivateKey(
Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
);

const result = await aliceCrypto.isKeyBackupTrusted(testData.SIGNED_BACKUP_DATA);
expect(result).toEqual({ trusted: false, matchesDecryptionKey: true });
});

it("is not fooled by a backup which matches the decryption key but uses a different algorithm", async () => {
aliceClient = await initTestClient();
const aliceCrypto = aliceClient.getCrypto()!;

await aliceClient.startClient();
await aliceCrypto.storeSessionBackupPrivateKey(
Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
);

const backup: KeyBackupInfo = JSON.parse(JSON.stringify(testData.SIGNED_BACKUP_DATA));
backup.algorithm = "m.megolm_backup.v1.aes-hmac-sha2";
const result = await aliceCrypto.isKeyBackupTrusted(backup);
expect(result).toEqual({ trusted: false, matchesDecryptionKey: false });
});
});

/** make sure that the client knows about the dummy device */
async function waitForDeviceList(): Promise<void> {
// Completing the initial sync will make the device list download outdated device lists (of which our own
Expand Down
2 changes: 2 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3282,6 +3282,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

/**
* @param info - key backup info dict from getKeyBackupVersion()
*
* @deprecated Prefer {@link CryptoApi.isKeyBackupTrusted | `CryptoApi.isKeyBackupTrusted`}.
*/
public isKeyBackupTrusted(info: IKeyBackupInfo): Promise<TrustInfo> {
if (!this.crypto) {
Expand Down
9 changes: 8 additions & 1 deletion src/crypto-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { DeviceMap } from "./models/device";
import { UIAuthCallback } from "./interactive-auth";
import { AddSecretStorageKeyOpts, SecretStorageCallbacks, SecretStorageKeyDescription } from "./secret-storage";
import { VerificationRequest } from "./crypto-api/verification";
import { KeyBackupInfo } from "./crypto-api/keybackup";
import { BackupTrustInfo, KeyBackupInfo } from "./crypto-api/keybackup";
import { ISignatures } from "./@types/signed";

/**
Expand Down Expand Up @@ -332,6 +332,13 @@ export interface CryptoApi {
* @returns If automatic key backups are enabled, the `version` of the active backup. Otherwise, `null`.
*/
getActiveSessionBackupVersion(): Promise<string | null>;

/**
* Determine if a key backup can be trusted.
*
* @param info - key backup info dict from {@link MatrixClient#getKeyBackupVersion}.
*/
isKeyBackupTrusted(info: KeyBackupInfo): Promise<BackupTrustInfo>;
}

/**
Expand Down
22 changes: 21 additions & 1 deletion src/crypto-api/keybackup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ export interface Aes256AuthData {
}

/**
* Extra info of a recovery key
* Information about a server-side key backup.
*
* Returned by [`GET /_matrix/client/v3/room_keys/version`](https://spec.matrix.org/v1.7/client-server-api/#get_matrixclientv3room_keysversion)
* and hence {@link MatrixClient#getKeyBackupVersion}.
*/
export interface KeyBackupInfo {
algorithm: string;
Expand All @@ -40,3 +43,20 @@ export interface KeyBackupInfo {
etag?: string;
version?: string; // number contained within
}

/**
* Information on whether a given server-side backup is trusted.
*/
export interface BackupTrustInfo {
/**
* Is this backup trusted?
*
* True if, and only if, there is a valid signature on the backup from a trusted device.
*/
readonly trusted: boolean;

/**
* True if this backup matches the stored decryption key.
*/
readonly matchesDecryptionKey: boolean;
}
14 changes: 14 additions & 0 deletions src/crypto/backup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { UnstableValue } from "../NamespacedValue";
import { CryptoEvent } from "./index";
import { crypto } from "./crypto";
import { HTTPError, MatrixError } from "../http-api";
import { BackupTrustInfo } from "../crypto-api/keybackup";

const KEY_BACKUP_KEYS_PER_REQUEST = 200;
const KEY_BACKUP_CHECK_RATE_LIMIT = 5000; // ms
Expand All @@ -54,6 +55,7 @@ type SigInfo = {
deviceTrust?: DeviceTrustLevel;
};

/** @deprecated Prefer {@link BackupTrustInfo} */
export type TrustInfo = {
usable: boolean; // is the backup trusted, true iff there is a sig that is valid & from a trusted device
sigs: SigInfo[];
Expand Down Expand Up @@ -829,3 +831,15 @@ export const algorithmsByName: Record<string, BackupAlgorithmClass> = {
};

export const DefaultAlgorithm: BackupAlgorithmClass = Curve25519;

/**
* Map a legacy {@link TrustInfo} into a new-style {@link BackupTrustInfo}.
*
* @param trustInfo - trustInfo to convert
*/
export function backupTrustInfoFromLegacyTrustInfo(trustInfo: TrustInfo): BackupTrustInfo {
return {
trusted: trustInfo.usable,
matchesDecryptionKey: trustInfo.trusted_locally ?? false,
};
}
2 changes: 2 additions & 0 deletions src/crypto/deviceinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export interface IDevice {

/**
* Information about a user's device
*
* Superceded by {@link Device}.
*/
export class DeviceInfo {
/**
Expand Down
14 changes: 13 additions & 1 deletion src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { IllegalMethod } from "./verification/IllegalMethod";
import { KeySignatureUploadError } from "../errors";
import { calculateKeyCheck, decryptAES, encryptAES, IEncryptedPayload } from "./aes";
import { DehydrationManager } from "./dehydration";
import { BackupManager } from "./backup";
import { BackupManager, backupTrustInfoFromLegacyTrustInfo } from "./backup";
import { IStore } from "../store";
import { Room, RoomEvent } from "../models/room";
import { RoomMember, RoomMemberEvent } from "../models/room-member";
Expand Down Expand Up @@ -87,10 +87,12 @@ import {
} from "../secret-storage";
import { ISecretRequest } from "./SecretSharing";
import {
BackupTrustInfo,
BootstrapCrossSigningOpts,
CrossSigningStatus,
DeviceVerificationStatus,
ImportRoomKeysOpts,
KeyBackupInfo,
VerificationRequest as CryptoApiVerificationRequest,
} from "../crypto-api";
import { Device, DeviceMap } from "../models/device";
Expand Down Expand Up @@ -1292,6 +1294,16 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
return null;
}

/**
* Determine if a key backup can be trusted.
*
* Implementation of {@link Crypto.CryptoApi.isKeyBackupTrusted}.
*/
public async isKeyBackupTrusted(info: KeyBackupInfo): Promise<BackupTrustInfo> {
const trustInfo = await this.backupManager.isKeyBackupTrusted(info);
return backupTrustInfoFromLegacyTrustInfo(trustInfo);
}

/**
* Checks that a given cross-signing private key matches a given public key.
* This can be used by the getCrossSigningKey callback to verify that the
Expand Down
30 changes: 30 additions & 0 deletions src/rust-crypto/backup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,42 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { OlmMachine, SignatureVerification } from "@matrix-org/matrix-sdk-crypto-wasm";
import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-wasm";

import { BackupTrustInfo, Curve25519AuthData, KeyBackupInfo } from "../crypto-api/keybackup";

/**
* @internal
*/
export class RustBackupManager {
public constructor(private readonly olmMachine: OlmMachine) {}

/**
* Get the backup version we are currently backing up to, if any
*/
public async getActiveBackupVersion(): Promise<string | null> {
// TODO stub
return null;
}

/**
* Determine if a key backup can be trusted.
*
* @param info - key backup info dict from {@link MatrixClient#getKeyBackupVersion}.
*/
public async isKeyBackupTrusted(info: KeyBackupInfo): Promise<BackupTrustInfo> {
const signatureVerification: SignatureVerification = await this.olmMachine.verifyBackup(info);

const backupKeys: RustSdkCryptoJs.BackupKeys = await this.olmMachine.getBackupKeys();
const pubKeyForSavedPrivateKey = backupKeys?.decryptionKey?.megolmV1PublicKey;
const backupMatchesSavedPrivateKey =
info.algorithm === pubKeyForSavedPrivateKey?.algorithm &&
(info.auth_data as Curve25519AuthData)?.public_key === pubKeyForSavedPrivateKey.publicKeyBase64;

return {
matchesDecryptionKey: backupMatchesSavedPrivateKey,
trusted: signatureVerification.trusted(),
};
}
}
17 changes: 14 additions & 3 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProc
import { KeyClaimManager } from "./KeyClaimManager";
import { MapWithDefault } from "../utils";
import {
BackupTrustInfo,
BootstrapCrossSigningOpts,
CreateSecretStorageOpts,
CrossSigningKey,
Expand All @@ -40,6 +41,7 @@ import {
GeneratedSecretStorageKey,
ImportRoomKeyProgressData,
ImportRoomKeysOpts,
KeyBackupInfo,
VerificationRequest,
CrossSigningKeyInfo,
} from "../crypto-api";
Expand Down Expand Up @@ -112,7 +114,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
this.outgoingRequestProcessor = new OutgoingRequestProcessor(olmMachine, http);
this.keyClaimManager = new KeyClaimManager(olmMachine, this.outgoingRequestProcessor);
this.eventDecryptor = new EventDecryptor(olmMachine);
this.backupManager = new RustBackupManager();
this.backupManager = new RustBackupManager(olmMachine);

// Fire if the cross signing keys are imported from the secret storage
const onCrossSigningKeysImport = (): void => {
Expand Down Expand Up @@ -766,8 +768,8 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
*/
public async getSessionBackupPrivateKey(): Promise<Uint8Array | null> {
const backupKeys: RustSdkCryptoJs.BackupKeys = await this.olmMachine.getBackupKeys();
if (!backupKeys.decryptionKeyBase64) return null;
return Buffer.from(backupKeys.decryptionKeyBase64, "base64");
if (!backupKeys.decryptionKey) return null;
return Buffer.from(backupKeys.decryptionKey.toBase64(), "base64");
}

/**
Expand All @@ -793,6 +795,15 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
return await this.backupManager.getActiveBackupVersion();
}

/**
* Determine if a key backup can be trusted.
*
* Implementation of {@link Crypto.CryptoApi.isKeyBackupTrusted}.
*/
public async isKeyBackupTrusted(info: KeyBackupInfo): Promise<BackupTrustInfo> {
return await this.backupManager.isKeyBackupTrusted(info);
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
// SyncCryptoCallbacks implementation
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1482,10 +1482,10 @@
"@jridgewell/resolve-uri" "3.1.0"
"@jridgewell/sourcemap-codec" "1.4.14"

"@matrix-org/matrix-sdk-crypto-wasm@^1.1.0":
version "1.1.0"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-1.1.0.tgz#43996a2c5fc8786999eeaaf6df51007244f6b3c4"
integrity sha512-BSMYqXRgQOHG3N18z8b05x3UQcdLL3XDrxjtjjA88t9PadZ7RwNowLm1Sx3ESzdzRX+r1SEVAWs2JnTTs0rv3Q==
"@matrix-org/matrix-sdk-crypto-wasm@^1.2.0":
version "1.2.0"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-1.2.0.tgz#115cd21cb2bba3c8166cf09e7d61da0902aa8973"
integrity sha512-vmpbtXYFzfBSFjeAx/PNRjy7zyH+Xx2HVXNKdApgo3+hSALewcXwdOTJy5pKq+poumM2TjjKDhG2s6/zSDNUYg==

"@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz":
version "3.2.14"
Expand Down

0 comments on commit 2193cd9

Please sign in to comment.