Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't back up keys that we got from backup #3934

Merged
merged 10 commits into from
Dec 7, 2023
46 changes: 46 additions & 0 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ import {
EventShieldReason,
ImportRoomKeysOpts,
KeyBackupCheck,
RoomKeySource,
VerificationRequest,
} from "../../../src/crypto-api";
import * as testData from "../../test-utils/test-data";
import { defer } from "../../../src/utils";
import { logger } from "../../../src/logger";
import { OutgoingRequestsManager } from "../../../src/rust-crypto/OutgoingRequestsManager";
import { Curve25519AuthData } from "../../../src/crypto-api/keybackup";

const TEST_USER = "@alice:example.com";
const TEST_DEVICE_ID = "TEST_DEVICE";
Expand Down Expand Up @@ -931,6 +933,50 @@ describe("RustCrypto", () => {
await rustCrypto.onUserIdentityUpdated(new RustSdkCryptoJs.UserId(testData.TEST_USER_ID));
expect(await keyBackupStatusPromise).toBe(true);
});

it("does not back up keys that came from backup", async () => {
const rustCrypto = await makeTestRustCrypto();
const olmMachine: OlmMachine = rustCrypto["olmMachine"];

await olmMachine.enableBackupV1(
(testData.SIGNED_BACKUP_DATA.auth_data as Curve25519AuthData).public_key,
testData.SIGNED_BACKUP_DATA.version!,
);

// we import two keys: one "from backup", and one "from export"
const [backedUpRoomKey, exportedRoomKey] = testData.MEGOLM_SESSION_DATA_ARRAY;
await rustCrypto.importRoomKeys([backedUpRoomKey], {
source: RoomKeySource.Backup,
});
await rustCrypto.importRoomKeys([exportedRoomKey]);

// we ask for the keys that should be backed up
const roomKeysRequest = await olmMachine.backupRoomKeys();
expect(roomKeysRequest).toBeTruthy();
const roomKeys = JSON.parse(roomKeysRequest!.body);

// we expect that the key "from export" is present
expect(roomKeys).toMatchObject({
rooms: {
[exportedRoomKey.room_id]: {
sessions: {
[exportedRoomKey.session_id]: {},
},
},
},
});

// we expect that the key "from backup" is not present
expect(roomKeys).not.toMatchObject({
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
rooms: {
[backedUpRoomKey.room_id]: {
sessions: {
[backedUpRoomKey.session_id]: {},
},
},
},
});
});
});
});

Expand Down
10 changes: 8 additions & 2 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,13 @@ import { LocalNotificationSettings } from "./@types/local_notifications";
import { buildFeatureSupportMap, Feature, ServerSupport } from "./feature";
import { CryptoBackend } from "./common-crypto/CryptoBackend";
import { RUST_SDK_STORE_PREFIX } from "./rust-crypto/constants";
import { BootstrapCrossSigningOpts, CrossSigningKeyInfo, CryptoApi, ImportRoomKeysOpts } from "./crypto-api";
import {
BootstrapCrossSigningOpts,
CrossSigningKeyInfo,
CryptoApi,
ImportRoomKeysOpts,
RoomKeySource,
} from "./crypto-api";
import { DeviceInfoMap } from "./crypto/DeviceList";
import {
AddSecretStorageKeyOpts,
Expand Down Expand Up @@ -3980,7 +3986,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
await this.cryptoBackend.importRoomKeys(keys, {
progressCallback,
untrusted,
source: "backup",
source: RoomKeySource.Backup,
});

/// in case entering the passphrase would add a new signature?
Expand Down
17 changes: 16 additions & 1 deletion src/crypto-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,20 @@ export interface ImportRoomKeyProgressData {
total: number;
}

/**
* Where a room key came from
*/
export enum RoomKeySource {
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
/** from key backup */
Backup = "backup",
/** from a key export */
Export = "export",
/** via a key forward */
Forward = "forward",
/** directly from the sender */
Direct = "direct",
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Options object for {@link CryptoApi#importRoomKeys}.
*/
Expand All @@ -599,7 +613,8 @@ export interface ImportRoomKeysOpts {
progressCallback?: (stage: ImportRoomKeyProgressData) => void;
// TODO, the rust SDK will always such imported keys as untrusted
untrusted?: boolean;
source?: String; // TODO: Enum (backup, file, ??)
/** Where the room key came from */
source?: RoomKeySource;
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/algorithms/megolm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { EventType, MsgType, ToDeviceMessageId } from "../../@types/event";
import { IMegolmEncryptedContent, IncomingRoomKeyRequest, IEncryptedContent } from "../index";
import { RoomKeyRequestState } from "../OutgoingRoomKeyRequestManager";
import { OlmGroupSessionExtraData } from "../../@types/crypto";
import { RoomKeySource } from "../../crypto-api";
import { MatrixError } from "../../http-api";
import { immediate, MapWithDefault } from "../../utils";

Expand Down Expand Up @@ -2046,7 +2047,7 @@ export class MegolmDecryption extends DecryptionAlgorithm {
extraSessionData,
)
.then(() => {
if (source !== "backup") {
if (source !== RoomKeySource.Backup) {
// don't wait for it to complete
this.crypto.backupManager.backupGroupSession(session.sender_key, session.session_id).catch((e) => {
// This throws if the upload failed, but this is fine
Expand Down
22 changes: 18 additions & 4 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
KeyBackupInfo,
KeyBackupSession,
OwnDeviceKeys,
RoomKeySource,
UserVerificationStatus,
VerificationRequest,
} from "../crypto-api";
Expand Down Expand Up @@ -225,7 +226,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
for (const k of keys) {
k.room_id = targetRoomId;
}
await this.importRoomKeys(keys);
await this.importRoomKeys(keys, { source: RoomKeySource.Backup });
}

/**
Expand Down Expand Up @@ -410,16 +411,29 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv

public async importRoomKeys(keys: IMegolmSessionData[], opts?: ImportRoomKeysOpts): Promise<void> {
// TODO when backup support will be added we would need to expose the `from_backup` flag in the bindings
const jsonKeys = JSON.stringify(keys);
await this.olmMachine.importRoomKeys(jsonKeys, (progress: BigInt, total: BigInt) => {
const callback = (progress: BigInt, total: BigInt): void => {
const importOpt: ImportRoomKeyProgressData = {
total: Number(total),
successes: Number(progress),
stage: "load_keys",
failures: 0,
};
opts?.progressCallback?.(importOpt);
});
};
if (opts?.source === RoomKeySource.Backup) {
const keysByRoom: Map<RustSdkCryptoJs.RoomId, Map<string, IMegolmSessionData>> = new Map();
for (const key of keys) {
const roomId = new RustSdkCryptoJs.RoomId(key.room_id);
if (!keysByRoom.has(roomId)) {
keysByRoom.set(roomId, new Map());
}
keysByRoom.get(roomId)!.set(key.session_id, key);
}
await this.olmMachine.importBackedUpRoomKeys(keysByRoom, callback);
} else {
const jsonKeys = JSON.stringify(keys);
await this.olmMachine.importExportedRoomKeys(jsonKeys, callback);
}
}

/**
Expand Down