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

Element-R: Wire up room rotation #3807

Merged
merged 12 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 139 additions & 16 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
import { TestClient } from "../../TestClient";
import { logger } from "../../../src/logger";
import {
Category,
ClientEvent,
createClient,
CryptoEvent,
Expand Down Expand Up @@ -146,6 +147,27 @@ async function expectSendRoomKey(
});
}

/**
* Return the event received on rooms/{roomId}/send/m.room.encrypted endpoint.
* See https://spec.matrix.org/latest/client-server-api/#put_matrixclientv3roomsroomidsendeventtypetxnid
* @returns the content of the encrypted event
*/
function expectEncryptedSendMessage() {
return new Promise<IContent>((resolve) => {
fetchMock.putOnce(
new RegExp("/send/m.room.encrypted/"),
(url, request) => {
const content = JSON.parse(request.body as string);
resolve(content);
return { event_id: "$event_id" };
},
// append to the list of intercepts on this path (since we have some tests that call
// this function multiple times)
{ overwriteRoutes: false },
);
});
}

/**
* Expect that the client sends an encrypted event
*
Expand All @@ -159,22 +181,7 @@ async function expectSendRoomKey(
async function expectSendMegolmMessage(
inboundGroupSessionPromise: Promise<Olm.InboundGroupSession>,
): Promise<Partial<IEvent>> {
const encryptedMessageContent = await new Promise<IContent>((resolve) => {
fetchMock.putOnce(
new RegExp("/send/m.room.encrypted/"),
(url: string, opts: RequestInit): MockResponse => {
resolve(JSON.parse(opts.body as string));
return {
event_id: "$event_id",
};
},
{
// append to the list of intercepts on this path (since we have some tests that call
// this function multiple times)
overwriteRoutes: false,
},
);
});
const encryptedMessageContent = await expectEncryptedSendMessage();

// In some of the tests, the room key is sent *after* the actual event, so we may need to wait for it now.
const inboundGroupSession = await inboundGroupSessionPromise;
Expand Down Expand Up @@ -924,6 +931,122 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});
});

describe("Session should rotate according to encryption settings", () => {
/**
* Send a message to bob and get the encrypted message
* @returns {Promise<IContent>} The encrypted message
*/
async function sendEncryptedMessage(): Promise<IContent> {
const [encryptedMessage] = await Promise.all([
expectEncryptedSendMessage(),
aliceClient.sendTextMessage(ROOM_ID, "test"),
]);
return encryptedMessage;
}

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

newBackendOnly("should rotate the session after 2 messages", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount);

const syncResponse = getSyncResponse(["@bob:xyz"]);
// Every 2 messages in the room, the session should be rotated
syncResponse.rooms[Category.Join][ROOM_ID].state.events[0].content = {
algorithm: "m.megolm.v1.aes-sha2",
rotation_period_msgs: 2,
};

// Tell alice we share a room with bob
syncResponder.sendOrQueueSyncResponse(syncResponse);
await syncPromise(aliceClient);

// Force alice to download bob keys
expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz"));

// Send a message to bob and get the encrypted message
const [encryptedMessage] = await Promise.all([
sendEncryptedMessage(),
expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession),
]);

// Check that the session id exists
const sessionId = encryptedMessage.session_id;
expect(sessionId).toBeDefined();

// Send a second message to bob and get the current message
const secondEncryptedMessage = await sendEncryptedMessage();

// Check that the same session id is shared between the two messages
const secondSessionId = secondEncryptedMessage.session_id;
expect(secondSessionId).toBe(sessionId);

// The session should be rotated, we are expecting the room key to be sent
const [thirdEncryptedMessage] = await Promise.all([
sendEncryptedMessage(),
expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession),
]);

// The session is rotated every 2 messages, we should have a new session id
const thirdSessionId = thirdEncryptedMessage.session_id;
expect(thirdSessionId).not.toBe(sessionId);
});

newBackendOnly("should rotate the session after 1h", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount);

// We need to fake the timers to advance the time
jest.useFakeTimers();

const syncResponse = getSyncResponse(["@bob:xyz"]);

// The minimum rotation period is 1h
// https://github.com/matrix-org/matrix-rust-sdk/blob/f75b2cd1d0981db42751dadb08c826740af1018e/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs#L410-L415
const oneHourInMs = 60 * 60 * 1000;

// Every 1h the session should be rotated
syncResponse.rooms[Category.Join][ROOM_ID].state.events[0].content = {
algorithm: "m.megolm.v1.aes-sha2",
rotation_period_ms: oneHourInMs,
};

// Tell alice we share a room with bob
syncResponder.sendOrQueueSyncResponse(syncResponse);
await syncPromise(aliceClient);

// Force alice to download bob keys
expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz"));

// Send a message to bob and get the encrypted message
const [encryptedMessage] = await Promise.all([
sendEncryptedMessage(),
expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession),
]);

// Check that the session id exists
const sessionId = encryptedMessage.session_id;
expect(sessionId).toBeDefined();

// Advance the time by 1h
jest.advanceTimersByTime(oneHourInMs);

// Send a second message to bob and get the encrypted message
const [secondEncryptedMessage] = await Promise.all([
sendEncryptedMessage(),
expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession),
]);

// The session should be rotated
const secondSessionId = secondEncryptedMessage.session_id;
expect(secondSessionId).not.toBe(sessionId);
});
});

oldBackendOnly("We should start a new megolm session when a device is blocked", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
Expand Down
24 changes: 22 additions & 2 deletions src/rust-crypto/RoomEncryptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { EncryptionSettings, OlmMachine, RoomId, UserId } from "@matrix-org/matrix-sdk-crypto-wasm";
import {
EncryptionAlgorithm,
EncryptionSettings,
OlmMachine,
RoomId,
UserId,
} from "@matrix-org/matrix-sdk-crypto-wasm";

import { EventType } from "../@types/event";
import { IContent, MatrixEvent } from "../models/event";
Expand Down Expand Up @@ -103,7 +109,21 @@ export class RoomEncryptor {
this.prefixedLogger.debug("Sessions for users are ready; now sharing room key");

const rustEncryptionSettings = new EncryptionSettings();
/* FIXME historyVisibility, rotation, etc */
/* FIXME historyVisibility, etc */

// We only support megolm
rustEncryptionSettings.algorithm = EncryptionAlgorithm.MegolmV1AesSha2;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two solutions here:

  • Set directly the Megolm algorithm since we currently only supporting megolm and we are checking at the beginning at the function that we are using Megolm.
  • Use the value from this.encryptionSettings, it means that we need to convert the string in the field this.encryptionSettings.algoritm to the rust EncryptionAlgorithm enum

The second solution appears to me useless since we are only using Megolm


// We need to convert the rotation period from milliseconds to microseconds
// See https://spec.matrix.org/v1.8/client-server-api/#mroomencryption and
// https://matrix-org.github.io/matrix-rust-sdk-crypto-wasm/classes/EncryptionSettings.html#rotationPeriod
if (typeof this.encryptionSettings.rotation_period_ms === "number") {
rustEncryptionSettings.rotationPeriod = BigInt(this.encryptionSettings.rotation_period_ms * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a bit silly that the Rust EncryptionSettings wants a bigint rather than a number, but hohum

}

if (typeof this.encryptionSettings.rotation_period_msgs === "number") {
rustEncryptionSettings.rotationPeriodMessages = BigInt(this.encryptionSettings.rotation_period_msgs);
}

const shareMessages = await this.olmMachine.shareRoomKey(
new RoomId(this.room.roomId),
Expand Down
Loading