Skip to content

Commit

Permalink
Crypto: use a new error code for UTDs from device-relative historical…
Browse files Browse the repository at this point in the history
… events (#4139)

* Add `PerSessionKeyBackupDownloader.isKeyBackupDownloadConfigured()`

* Add new `RustBackupManager.getServerBackupInfo`

... and a convenience method in PerSessionKeyBackupDownloader to access it.

* Crypto.spec: move `useRealTimers` to global `afterEach`

... so that we don't need to remember to do it everywhere.

* Use fake timers for UTD error code tests

This doesn't have any effect on the tests, but *does* stop jest from hanging
when you run the tests in in-band mode. It shouldn't *really* be needed, but
using fake timers gives more reproducible tests, and I don't have the
time/patience to debug why it is needed.

* Use new error codes for UTDs from historical events
  • Loading branch information
richvdh authored Apr 17, 2024
1 parent 8240bf0 commit c30e498
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 32 deletions.
120 changes: 101 additions & 19 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ afterEach(() => {
// cf https://github.com/dumbmatter/fakeIndexedDB#wipingresetting-the-indexeddb-for-a-fresh-state
// eslint-disable-next-line no-global-assign
indexedDB = new IDBFactory();

jest.useRealTimers();
});

/**
Expand Down Expand Up @@ -467,18 +469,28 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});

describe("Unable to decrypt error codes", function () {
beforeEach(() => {
jest.useFakeTimers({ doNotFake: ["queueMicrotask"] });
});

it("Decryption fails with UISI error", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();

// A promise which resolves, with the MatrixEvent which wraps the event, once the decryption fails.
const awaitDecryption = emitPromise(aliceClient, MatrixEventEvent.Decrypted);

// Ensure that the timestamp post-dates the creation of our device
const encryptedEvent = {
...testData.ENCRYPTED_EVENT,
origin_server_ts: Date.now(),
};

const syncResponse = {
next_batch: 1,
rooms: {
join: {
[testData.TEST_ROOM_ID]: { timeline: { events: [testData.ENCRYPTED_EVENT] } },
[testData.TEST_ROOM_ID]: { timeline: { events: [encryptedEvent] } },
},
},
};
Expand All @@ -498,12 +510,17 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,

await aliceClient.getCrypto()!.importRoomKeys([testData.RATCHTED_MEGOLM_SESSION_DATA]);

// Alice gets both the events in a single sync
// Ensure that the timestamp post-dates the creation of our device
const encryptedEvent = {
...testData.ENCRYPTED_EVENT,
origin_server_ts: Date.now(),
};

const syncResponse = {
next_batch: 1,
rooms: {
join: {
[testData.TEST_ROOM_ID]: { timeline: { events: [testData.ENCRYPTED_EVENT] } },
[testData.TEST_ROOM_ID]: { timeline: { events: [encryptedEvent] } },
},
},
};
Expand All @@ -515,6 +532,87 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.OLM_UNKNOWN_MESSAGE_INDEX);
});

describe("Historical events", () => {
async function sendEventAndAwaitDecryption(): Promise<MatrixEvent> {
// A promise which resolves, with the MatrixEvent which wraps the event, once the decryption fails.
const awaitDecryption = emitPromise(aliceClient, MatrixEventEvent.Decrypted);

// Ensure that the timestamp pre-dates the creation of our device: set it to 24 hours ago
const encryptedEvent = {
...testData.ENCRYPTED_EVENT,
origin_server_ts: Date.now() - 24 * 3600 * 1000,
};

const syncResponse = {
next_batch: 1,
rooms: {
join: {
[testData.TEST_ROOM_ID]: { timeline: { events: [encryptedEvent] } },
},
},
};

syncResponder.sendOrQueueSyncResponse(syncResponse);
return await awaitDecryption;
}

newBackendOnly("fails with HISTORICAL_MESSAGE_BACKUP_NO_BACKUP when there is no backup", async () => {
fetchMock.get("path:/_matrix/client/v3/room_keys/version", {
status: 404,
body: { errcode: "M_NOT_FOUND", error: "No current backup version." },
});
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();

const ev = await sendEventAndAwaitDecryption();
expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP);
});

newBackendOnly("fails with HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED when the backup is broken", async () => {
fetchMock.get("path:/_matrix/client/v3/room_keys/version", {});
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();

const ev = await sendEventAndAwaitDecryption();
expect(ev.decryptionFailureReason).toEqual(
DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED,
);
});

newBackendOnly("fails with HISTORICAL_MESSAGE_WORKING_BACKUP when backup is working", async () => {
// The test backup data is signed by a dummy device. We'll need to tell Alice about the device, and
// later, tell her to trust it, so that she trusts the backup.
const e2eResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl());
e2eResponder.addDeviceKeys(testData.SIGNED_TEST_DEVICE_DATA);
fetchMock.get("path:/_matrix/client/v3/room_keys/version", testData.SIGNED_BACKUP_DATA);
await startClientAndAwaitFirstSync();

await aliceClient
.getCrypto()!
.storeSessionBackupPrivateKey(
Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
testData.SIGNED_BACKUP_DATA.version!,
);

// Tell Alice to trust the dummy device that signed the backup
const devices = await aliceClient.getCrypto()!.getUserDeviceInfo([TEST_USER_ID]);
expect(devices.get(TEST_USER_ID)!.keys()).toContain(testData.TEST_DEVICE_ID);
await aliceClient.getCrypto()!.setDeviceVerified(testData.TEST_USER_ID, testData.TEST_DEVICE_ID);

// Tell Alice to check and enable backup
await aliceClient.getCrypto()!.checkKeyBackupAndEnable();

// Sanity: Alice should now have working backup.
expect(await aliceClient.getCrypto()!.getActiveSessionBackupVersion()).toEqual(
testData.SIGNED_BACKUP_DATA.version,
);

// Finally! we can check what happens when we get an event.
const ev = await sendEventAndAwaitDecryption();
expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP);
});
});

it("Decryption fails with Unable to decrypt for other errors", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
Expand Down Expand Up @@ -997,10 +1095,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
return encryptedMessage;
}

afterEach(() => {
jest.useRealTimers();
});

newBackendOnly("should rotate the session after 2 messages", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
Expand Down Expand Up @@ -2184,10 +2278,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
jest.useFakeTimers({ doNotFake: ["queueMicrotask"] });
});

afterEach(() => {
jest.useRealTimers();
});

function awaitKeyUploadRequest(): Promise<{ keysCount: number; fallbackKeysCount: number }> {
return new Promise((resolve) => {
const listener = (url: string, options: RequestInit) => {
Expand Down Expand Up @@ -2250,10 +2340,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});

describe("getUserDeviceInfo", () => {
afterEach(() => {
jest.useRealTimers();
});

// From https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3keysquery
// Using extracted response from matrix.org, it needs to have real keys etc to pass old crypto verification
const queryResponseBody = {
Expand Down Expand Up @@ -2742,10 +2828,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
jest.useFakeTimers({ doNotFake: ["queueMicrotask"] });
});

afterEach(() => {
jest.useRealTimers();
});

it("Should be able to restore from 4S after bootstrap", async () => {
const backupVersion = "1";
await bootstrapSecurity(backupVersion);
Expand Down
12 changes: 11 additions & 1 deletion spec/integ/crypto/megolm-backup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { KeyBackupInfo, KeyBackupSession } from "../../../src/crypto-api/keyback
import { IKeyBackup } from "../../../src/crypto/backup";
import { flushPromises } from "../../test-utils/flushPromises";
import { defer, IDeferred } from "../../../src/utils";
import { DecryptionFailureCode } from "../../../src/crypto-api";

const ROOM_ID = testData.TEST_ROOM_ID;

Expand Down Expand Up @@ -242,8 +243,17 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe

const room = aliceClient.getRoom(ROOM_ID)!;
const event = room.getLiveTimeline().getEvents()[0];
await advanceTimersUntil(awaitDecryption(event, { waitOnDecryptionFailure: true }));

// On the first decryption attempt, decryption fails.
await awaitDecryption(event);
expect(event.decryptionFailureReason).toEqual(
backend === "libolm"
? DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID
: DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP,
);

// Eventually, decryption succeeds.
await awaitDecryption(event, { waitOnDecryptionFailure: true });
expect(event.getContent()).toEqual(testData.CLEAR_EVENT.content);
});

Expand Down
28 changes: 24 additions & 4 deletions spec/unit/rust-crypto/PerSessionKeyBackupDownloader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe("PerSessionKeyBackupDownloader", () => {

mockRustBackupManager = {
getActiveBackupVersion: jest.fn(),
requestKeyBackupVersion: jest.fn(),
getServerBackupInfo: jest.fn(),
importBackedUpRoomKeys: jest.fn(),
createBackupDecryptor: jest.fn().mockReturnValue(mockBackupDecryptor),
on: jest.fn().mockImplementation((event, listener) => {
Expand Down Expand Up @@ -135,7 +135,7 @@ describe("PerSessionKeyBackupDownloader", () => {
decryptionKey: RustSdkCryptoJs.BackupDecryptionKey.fromBase64(TestData.BACKUP_DECRYPTION_KEY_BASE64),
} as unknown as RustSdkCryptoJs.BackupKeys);

mockRustBackupManager.requestKeyBackupVersion.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);
mockRustBackupManager.getServerBackupInfo.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);
});

it("Should download and import a missing key from backup", async () => {
Expand All @@ -155,8 +155,11 @@ describe("PerSessionKeyBackupDownloader", () => {

downloader.onDecryptionKeyMissingError(roomId, sessionId);

// `isKeyBackupDownloadConfigured` is false until the config is proven.
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
await expectAPICall;
await awaitKeyImported.promise;
expect(downloader.isKeyBackupDownloadConfigured()).toBe(true);
expect(mockRustBackupManager.createBackupDecryptor).toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -313,6 +316,9 @@ describe("PerSessionKeyBackupDownloader", () => {

expect(getConfigSpy).toHaveBeenCalledTimes(1);
expect(getConfigSpy).toHaveReturnedWith(Promise.resolve(null));

// isKeyBackupDownloadConfigured remains false
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
});

it("Should not query server if backup not active", async () => {
Expand All @@ -328,6 +334,9 @@ describe("PerSessionKeyBackupDownloader", () => {

expect(getConfigSpy).toHaveBeenCalledTimes(1);
expect(getConfigSpy).toHaveReturnedWith(Promise.resolve(null));

// isKeyBackupDownloadConfigured remains false
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
});

it("Should stop if backup key is not cached", async () => {
Expand All @@ -344,6 +353,9 @@ describe("PerSessionKeyBackupDownloader", () => {

expect(getConfigSpy).toHaveBeenCalledTimes(1);
expect(getConfigSpy).toHaveReturnedWith(Promise.resolve(null));

// isKeyBackupDownloadConfigured remains false
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
});

it("Should stop if backup key cached as wrong version", async () => {
Expand All @@ -363,6 +375,9 @@ describe("PerSessionKeyBackupDownloader", () => {

expect(getConfigSpy).toHaveBeenCalledTimes(1);
expect(getConfigSpy).toHaveReturnedWith(Promise.resolve(null));

// isKeyBackupDownloadConfigured remains false
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
});

it("Should stop if backup key version does not match the active one", async () => {
Expand All @@ -382,13 +397,16 @@ describe("PerSessionKeyBackupDownloader", () => {

expect(getConfigSpy).toHaveBeenCalledTimes(1);
expect(getConfigSpy).toHaveReturnedWith(Promise.resolve(null));

// isKeyBackupDownloadConfigured remains false
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);
});
});

describe("Given Backup state update", () => {
it("After initial sync, when backup becomes trusted it should request keys for past requests", async () => {
// there is a backup
mockRustBackupManager.requestKeyBackupVersion.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);
mockRustBackupManager.getServerBackupInfo.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);

// but at this point it's not trusted and we don't have the key
mockRustBackupManager.getActiveBackupVersion.mockResolvedValue(null);
Expand All @@ -410,6 +428,7 @@ describe("PerSessionKeyBackupDownloader", () => {

// @ts-ignore access to private property
expect(downloader.hasConfigurationProblem).toEqual(true);
expect(downloader.isKeyBackupDownloadConfigured()).toBe(false);

// Now the backup becomes trusted
mockRustBackupManager.getActiveBackupVersion.mockResolvedValue(TestData.SIGNED_BACKUP_DATA.version!);
Expand All @@ -423,6 +442,7 @@ describe("PerSessionKeyBackupDownloader", () => {
mockEmitter.emit(CryptoEvent.KeyBackupStatus, true);

await jest.runAllTimersAsync();
expect(downloader.isKeyBackupDownloadConfigured()).toBe(true);

await a0Imported;
await a1Imported;
Expand All @@ -434,7 +454,7 @@ describe("PerSessionKeyBackupDownloader", () => {
describe("Error cases", () => {
beforeEach(async () => {
// there is a backup
mockRustBackupManager.requestKeyBackupVersion.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);
mockRustBackupManager.getServerBackupInfo.mockResolvedValue(TestData.SIGNED_BACKUP_DATA);
// It's trusted
mockRustBackupManager.getActiveBackupVersion.mockResolvedValue(TestData.SIGNED_BACKUP_DATA.version!);
// And we have the key in cache
Expand Down
18 changes: 18 additions & 0 deletions src/crypto-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,24 @@ export enum DecryptionFailureCode {
/** Message was encrypted with a Megolm session which has been shared with us, but in a later ratchet state. */
OLM_UNKNOWN_MESSAGE_INDEX = "OLM_UNKNOWN_MESSAGE_INDEX",

/**
* Message was sent before the current device was created; there is no key backup on the server, so this
* decryption failure is expected.
*/
HISTORICAL_MESSAGE_NO_KEY_BACKUP = "HISTORICAL_MESSAGE_NO_KEY_BACKUP",

/**
* Message was sent before the current device was created; there was a key backup on the server, but we don't
* seem to have access to the backup. (Probably we don't have the right key.)
*/
HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED = "HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED",

/**
* Message was sent before the current device was created; there was a (usable) key backup on the server, but we
* still can't decrypt. (Either the session isn't in the backup, or we just haven't gotten around to checking yet.)
*/
HISTORICAL_MESSAGE_WORKING_BACKUP = "HISTORICAL_MESSAGE_WORKING_BACKUP",

/** Unknown or unclassified error. */
UNKNOWN_ERROR = "UNKNOWN_ERROR",

Expand Down
Loading

0 comments on commit c30e498

Please sign in to comment.