From d79e5f7806a2b90c352ce947915dd85fa0597fb0 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Wed, 20 Jul 2022 15:20:23 +0200 Subject: [PATCH 01/19] create key share operations for invitees when history visibility=invited --- src/matrix/e2ee/RoomEncryption.js | 51 +++++++++++++++++-- src/matrix/room/Room.js | 19 ++++--- src/matrix/room/common.ts | 71 +++++++++++++++++++++++++++ src/matrix/room/members/RoomMember.js | 4 ++ 4 files changed, 132 insertions(+), 13 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 80f57507..0ec7212e 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -19,13 +19,23 @@ import {groupEventsBySession} from "./megolm/decryption/utils"; import {mergeMap} from "../../utils/mergeMap"; import {groupBy} from "../../utils/groupBy"; import {makeTxnId} from "../common.js"; +import {iterateResponseStateEvents} from "../room/common"; const ENCRYPTED_TYPE = "m.room.encrypted"; +const ROOM_HISTORY_VISIBILITY_TYPE = "m.room.history_visibility"; // how often ensureMessageKeyIsShared can check if it needs to // create a new outbound session // note that encrypt could still create a new session const MIN_PRESHARE_INTERVAL = 60 * 1000; // 1min +// Use enum when converting to TS +const HistoryVisibility = Object.freeze({ + Joined: "joined", + Invited: "invited", + WorldReadable: "world_readable", + Shared: "shared", +}); + // TODO: this class is a good candidate for splitting up into encryption and decryption, there doesn't seem to be much overlap export class RoomEncryption { constructor({room, deviceTracker, olmEncryption, megolmEncryption, megolmDecryption, encryptionParams, storage, keyBackup, notifyMissingMegolmSession, clock}) { @@ -45,6 +55,7 @@ export class RoomEncryption { this._isFlushingRoomKeyShares = false; this._lastKeyPreShareTime = null; this._keySharePromise = null; + this._historyVisibility = undefined; this._disposed = false; } @@ -77,7 +88,13 @@ export class RoomEncryption { this._senderDeviceCache = new Map(); // purge the sender device cache } - async writeMemberChanges(memberChanges, txn, log) { + async writeSync(roomResponse, memberChanges, txn, log) { + let historyVisibility = this._historyVisibility; + iterateResponseStateEvents(roomResponse, event => { + if(event.state_key === "" && event.type === ROOM_HISTORY_VISIBILITY_TYPE) { + historyVisibility = event?.content?.history_visibility; + } + }); let shouldFlush = false; const memberChangesArray = Array.from(memberChanges.values()); // this also clears our session if we leave the room ourselves @@ -89,10 +106,35 @@ export class RoomEncryption { this._megolmEncryption.discardOutboundSession(this._room.id, txn); } if (memberChangesArray.some(m => m.hasJoined)) { - shouldFlush = await this._addShareRoomKeyOperationForNewMembers(memberChangesArray, txn, log); + const userIds = memberChangesArray.filter(m => m.hasJoined).map(m => m.userId); + shouldFlush = await this._addShareRoomKeyOperationForMembers(userIds, txn, log) + || shouldFlush; } + if (memberChangesArray.some(m => m.wasInvited)) { + historyVisibility = await this._loadHistoryVisibilityIfNeeded(historyVisibility, txn); + if (historyVisibility === HistoryVisibility.Invited) { + const userIds = memberChangesArray.filter(m => m.wasInvited).map(m => m.userId); + shouldFlush = await this._addShareRoomKeyOperationForMembers(userIds, txn, log) + || shouldFlush; + } + } + await this._deviceTracker.writeMemberChanges(this._room, memberChanges, txn); - return shouldFlush; + return {shouldFlush, historyVisibility}; + } + + afterSync({historyVisibility}) { + this._historyVisibility = historyVisibility; + } + + async _loadHistoryVisibilityIfNeeded(historyVisibility, txn) { + if (!historyVisibility) { + const visibilityEntry = await txn.roomState.get(this.id, ROOM_HISTORY_VISIBILITY_TYPE, ""); + if (visibilityEntry) { + return event?.content?.history_visibility; + } + } + return historyVisibility; } async prepareDecryptAll(events, newKeys, source, txn) { @@ -288,8 +330,7 @@ export class RoomEncryption { await this._processShareRoomKeyOperation(operation, hsApi, log); } - async _addShareRoomKeyOperationForNewMembers(memberChangesArray, txn, log) { - const userIds = memberChangesArray.filter(m => m.hasJoined).map(m => m.userId); + async _addShareRoomKeyOperationForMembers(userIds, txn, log) { const roomKeyMessage = await this._megolmEncryption.createRoomKeyMessage( this._room.id, txn); if (roomKeyMessage) { diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 12c17580..8cc87845 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -139,11 +139,11 @@ export class Room extends BaseRoom { } log.set("newEntries", newEntries.length); log.set("updatedEntries", updatedEntries.length); - let shouldFlushKeyShares = false; + let encryptionChanges; // pass member changes to device tracker - if (roomEncryption && this.isTrackingMembers && memberChanges?.size) { - shouldFlushKeyShares = await roomEncryption.writeMemberChanges(memberChanges, txn, log); - log.set("shouldFlushKeyShares", shouldFlushKeyShares); + if (roomEncryption) { + encryptionChanges = await roomEncryption.writeSync(roomResponse, memberChanges, txn, log); + log.set("shouldFlushKeyShares", encryptionChanges.shouldFlush); } const allEntries = newEntries.concat(updatedEntries); // also apply (decrypted) timeline entries to the summary changes @@ -188,7 +188,7 @@ export class Room extends BaseRoom { memberChanges, heroChanges, powerLevelsEvent, - shouldFlushKeyShares, + encryptionChanges, }; } @@ -201,11 +201,14 @@ export class Room extends BaseRoom { const { summaryChanges, newEntries, updatedEntries, newLiveKey, removedPendingEvents, memberChanges, powerLevelsEvent, - heroChanges, roomEncryption + heroChanges, roomEncryption, encryptionChanges } = changes; log.set("id", this.id); this._syncWriter.afterSync(newLiveKey); this._setEncryption(roomEncryption); + if (this._roomEncryption) { + this._roomEncryption.afterSync(encryptionChanges); + } if (memberChanges.size) { if (this._changedMembersDuringSync) { for (const [userId, memberChange] of memberChanges.entries()) { @@ -288,8 +291,8 @@ export class Room extends BaseRoom { } } - needsAfterSyncCompleted({shouldFlushKeyShares}) { - return shouldFlushKeyShares; + needsAfterSyncCompleted({encryptionChanges}) { + return encryptionChanges?.shouldFlush; } /** diff --git a/src/matrix/room/common.ts b/src/matrix/room/common.ts index 57ab7023..a03703a4 100644 --- a/src/matrix/room/common.ts +++ b/src/matrix/room/common.ts @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import type {StateEvent} from "../storage/types"; + export function getPrevContentFromStateEvent(event) { // where to look for prev_content is a bit of a mess, // see https://matrix.to/#/!NasysSDfxKxZBzJJoE:matrix.org/$DvrAbZJiILkOmOIuRsNoHmh2v7UO5CWp_rYhlGk34fQ?via=matrix.org&via=pixie.town&via=amorgan.xyz @@ -40,3 +42,72 @@ export enum RoomType { Private, Public } + +type RoomResponse = { + state?: { + events?: Array + }, + timeline?: { + events?: Array + } +} + +/** iterates over any state events in a sync room response, in the order that they should be applied (from older to younger events) */ +export function iterateResponseStateEvents(roomResponse: RoomResponse, callback: (StateEvent) => void) { + // first iterate over state events, they precede the timeline + const stateEvents = roomResponse.state?.events; + if (stateEvents) { + for (let i = 0; i < stateEvents.length; i++) { + callback(stateEvents[i]); + } + } + // now see if there are any state events within the timeline + let timelineEvents = roomResponse.timeline?.events; + if (timelineEvents) { + for (let i = 0; i < timelineEvents.length; i++) { + const event = timelineEvents[i]; + if (typeof event.state_key === "string") { + callback(event); + } + } + } +} + +export function tests() { + return { + "test iterateResponseStateEvents with both state and timeline sections": assert => { + const roomResponse = { + state: { + events: [ + {type: "m.room.member", state_key: "1"}, + {type: "m.room.member", state_key: "2", content: {a: 1}}, + ] + }, + timeline: { + events: [ + {type: "m.room.message"}, + {type: "m.room.member", state_key: "3"}, + {type: "m.room.message"}, + {type: "m.room.member", state_key: "2", content: {a: 2}}, + ] + } + } as unknown as RoomResponse; + const expectedStateKeys = ["1", "2", "3", "2"]; + const expectedAForMember2 = [1, 2]; + iterateResponseStateEvents(roomResponse, event => { + assert.strictEqual(event.type, "m.room.member"); + assert.strictEqual(expectedStateKeys.shift(), event.state_key); + if (event.state_key === "2") { + assert.strictEqual(expectedAForMember2.shift(), event.content.a); + } + }); + assert.strictEqual(expectedStateKeys.length, 0); + assert.strictEqual(expectedAForMember2.length, 0); + }, + "test iterateResponseStateEvents with empty response": assert => { + iterateResponseStateEvents({}, () => { + assert.fail("no events expected"); + }); + } + } +} diff --git a/src/matrix/room/members/RoomMember.js b/src/matrix/room/members/RoomMember.js index dabff972..8e00f5de 100644 --- a/src/matrix/room/members/RoomMember.js +++ b/src/matrix/room/members/RoomMember.js @@ -137,6 +137,10 @@ export class MemberChange { return this.member.membership; } + get wasInvited() { + return this.previousMembership === "invite" && this.membership !== "invite"; + } + get hasLeft() { return this.previousMembership === "join" && this.membership !== "join"; } From c8a8eb10b5aa34467e7894ec3eb525ce7d7ca6d1 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Wed, 20 Jul 2022 15:21:33 +0200 Subject: [PATCH 02/19] get user ids for sharing a new key when the message is sent rather than when the key happens to get sent --- src/matrix/e2ee/RoomEncryption.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 0ec7212e..8839556f 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -316,10 +316,13 @@ export class RoomEncryption { } async _shareNewRoomKey(roomKeyMessage, hsApi, log) { + const devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, this._historyVisibility, hsApi, log); + const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); + let writeOpTxn = await this._storage.readWriteTxn([this._storage.storeNames.operations]); let operation; try { - operation = this._writeRoomKeyShareOperation(roomKeyMessage, null, writeOpTxn); + operation = this._writeRoomKeyShareOperation(roomKeyMessage, userIds, writeOpTxn); } catch (err) { writeOpTxn.abort(); throw err; @@ -385,16 +388,7 @@ export class RoomEncryption { log.set("id", operation.id); await this._deviceTracker.trackRoom(this._room, log); - let devices; - if (operation.userIds === null) { - devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, hsApi, log); - const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); - operation.userIds = userIds; - await this._updateOperationsStore(operations => operations.update(operation)); - } else { - devices = await this._deviceTracker.devicesForRoomMembers(this._room.id, operation.userIds, hsApi, log); - } - + const devices = await this._deviceTracker.devicesForRoomMembers(this._room.id, operation.userIds, hsApi, log); const messages = await log.wrap("olm encrypt", log => this._olmEncryption.encrypt( "m.room_key", operation.roomKeyMessage, devices, hsApi, log)); const missingDevices = devices.filter(d => !messages.some(m => m.device === d)); From 22831e710ca456009eb2f819c86e08b0ed0875bd Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 14:15:26 +0200 Subject: [PATCH 03/19] support async callback in iterateResponseStateEvents --- src/matrix/room/common.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/matrix/room/common.ts b/src/matrix/room/common.ts index a03703a4..5a7fc98e 100644 --- a/src/matrix/room/common.ts +++ b/src/matrix/room/common.ts @@ -53,12 +53,20 @@ type RoomResponse = { } /** iterates over any state events in a sync room response, in the order that they should be applied (from older to younger events) */ -export function iterateResponseStateEvents(roomResponse: RoomResponse, callback: (StateEvent) => void) { +export function iterateResponseStateEvents(roomResponse: RoomResponse, callback: (StateEvent) => Promise | void): Promise | void { + let promises: Promise[] | undefined = undefined; + const callCallback = stateEvent => { + const result = callback(stateEvent); + if (result instanceof Promise) { + promises = promises ?? []; + promises.push(result); + } + }; // first iterate over state events, they precede the timeline const stateEvents = roomResponse.state?.events; if (stateEvents) { for (let i = 0; i < stateEvents.length; i++) { - callback(stateEvents[i]); + callCallback(stateEvents[i]); } } // now see if there are any state events within the timeline @@ -67,10 +75,13 @@ export function iterateResponseStateEvents(roomResponse: RoomResponse, callback: for (let i = 0; i < timelineEvents.length; i++) { const event = timelineEvents[i]; if (typeof event.state_key === "string") { - callback(event); + callCallback(event); } } } + if (promises) { + return Promise.all(promises); + } } export function tests() { From f3379402027a13ff41943aec4d62b77a8490b5eb Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 17:46:29 +0200 Subject: [PATCH 04/19] this migration shouldn't be needed anymore and undoes the export of addRoomToIdentity, which is somewhat internal --- src/matrix/e2ee/DeviceTracker.js | 2 +- src/matrix/storage/idb/schema.ts | 52 ++++---------------------------- 2 files changed, 7 insertions(+), 47 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index f8c3bca8..207aee05 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -19,7 +19,7 @@ import {verifyEd25519Signature, SIGNATURE_ALGORITHM} from "./common.js"; const TRACKING_STATUS_OUTDATED = 0; const TRACKING_STATUS_UPTODATE = 1; -export function addRoomToIdentity(identity, userId, roomId) { +function addRoomToIdentity(identity, userId, roomId) { if (!identity) { identity = { userId: userId, diff --git a/src/matrix/storage/idb/schema.ts b/src/matrix/storage/idb/schema.ts index 7819130e..9b875f9c 100644 --- a/src/matrix/storage/idb/schema.ts +++ b/src/matrix/storage/idb/schema.ts @@ -2,7 +2,6 @@ import {IDOMStorage} from "./types"; import {ITransaction} from "./QueryTarget"; import {iterateCursor, NOT_DONE, reqAsPromise} from "./utils"; import {RoomMember, EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../room/members/RoomMember.js"; -import {addRoomToIdentity} from "../../e2ee/DeviceTracker.js"; import {SESSION_E2EE_KEY_PREFIX} from "../../e2ee/common.js"; import {SummaryData} from "../../room/RoomSummary"; import {RoomMemberStore, MemberData} from "./stores/RoomMemberStore"; @@ -183,51 +182,12 @@ function createTimelineRelationsStore(db: IDBDatabase) : void { db.createObjectStore("timelineRelations", {keyPath: "key"}); } -//v11 doesn't change the schema, but ensures all userIdentities have all the roomIds they should (see #470) -async function fixMissingRoomsInUserIdentities(db: IDBDatabase, txn: IDBTransaction, localStorage: IDOMStorage, log: ILogItem) { - const roomSummaryStore = txn.objectStore("roomSummary"); - const trackedRoomIds: string[] = []; - await iterateCursor(roomSummaryStore.openCursor(), roomSummary => { - if (roomSummary.isTrackingMembers) { - trackedRoomIds.push(roomSummary.roomId); - } - return NOT_DONE; - }); - const outboundGroupSessionsStore = txn.objectStore("outboundGroupSessions"); - const userIdentitiesStore: IDBObjectStore = txn.objectStore("userIdentities"); - const roomMemberStore = txn.objectStore("roomMembers"); - for (const roomId of trackedRoomIds) { - let foundMissing = false; - const joinedUserIds: string[] = []; - const memberRange = IDBKeyRange.bound(roomId, `${roomId}|${MAX_UNICODE}`, true, true); - await log.wrap({l: "room", id: roomId}, async log => { - await iterateCursor(roomMemberStore.openCursor(memberRange), member => { - if (member.membership === "join") { - joinedUserIds.push(member.userId); - } - return NOT_DONE; - }); - log.set("joinedUserIds", joinedUserIds.length); - for (const userId of joinedUserIds) { - const identity = await reqAsPromise(userIdentitiesStore.get(userId)); - const originalRoomCount = identity?.roomIds?.length; - const updatedIdentity = addRoomToIdentity(identity, userId, roomId); - if (updatedIdentity) { - log.log({l: `fixing up`, id: userId, - roomsBefore: originalRoomCount, roomsAfter: updatedIdentity.roomIds.length}); - userIdentitiesStore.put(updatedIdentity); - foundMissing = true; - } - } - log.set("foundMissing", foundMissing); - if (foundMissing) { - // clear outbound megolm session, - // so we'll create a new one on the next message that will be properly shared - outboundGroupSessionsStore.delete(roomId); - } - }); - } -} +//v11 doesn't change the schema, +// but ensured all userIdentities have all the roomIds they should (see #470) + +// 2022-07-20: The fix dated from August 2021, and have removed it now because of a +// refactoring needed in the device tracker, which made it inconvenient to expose addRoomToIdentity +function fixMissingRoomsInUserIdentities() {} // v12 move ssssKey to e2ee:ssssKey so it will get backed up in the next step async function changeSSSSKeyPrefix(db: IDBDatabase, txn: IDBTransaction) { From 86c0e9e66936d3c18aca88fa4c9aab53acfd8d3d Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 17:46:53 +0200 Subject: [PATCH 05/19] logic for whether a key should be shared by membership and h. visibility --- src/matrix/e2ee/common.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/matrix/e2ee/common.js b/src/matrix/e2ee/common.js index 2b9d46b9..cc3bfff5 100644 --- a/src/matrix/e2ee/common.js +++ b/src/matrix/e2ee/common.js @@ -69,3 +69,28 @@ export function createRoomEncryptionEvent() { } } } + + +// Use enum when converting to TS +export const HistoryVisibility = Object.freeze({ + Joined: "joined", + Invited: "invited", + WorldReadable: "world_readable", + Shared: "shared", +}); + +export function shouldShareKey(membership, historyVisibility) { + switch (historyVisibility) { + case HistoryVisibility.WorldReadable: + return true; + case HistoryVisibility.Shared: + // was part of room at some time + return membership !== undefined; + case HistoryVisibility.Joined: + return membership === "join"; + case HistoryVisibility.Invited: + return membership === "invite" || membership === "join"; + default: + return false; + } +} From f6011f3f348215d0829af9e1f8f7e6553bc4a87b Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 17:48:26 +0200 Subject: [PATCH 06/19] take history visibility into account in device tracker and return added and removed userids to their userIdentity for the given room, so room encryption can share and discard the keys for them --- src/matrix/e2ee/DeviceTracker.js | 71 +++++++++++++++++++------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 207aee05..7ce601d2 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -15,6 +15,7 @@ limitations under the License. */ import {verifyEd25519Signature, SIGNATURE_ALGORITHM} from "./common.js"; +import {shouldShareKey} from "./common.js"; const TRACKING_STATUS_OUTDATED = 0; const TRACKING_STATUS_UPTODATE = 1; @@ -79,13 +80,38 @@ export class DeviceTracker { })); } - writeMemberChanges(room, memberChanges, txn) { - return Promise.all(Array.from(memberChanges.values()).map(async memberChange => { - return this._applyMemberChange(memberChange, txn); + /** @return Promise<{added: string[], removed: string[]}> the user ids for who the room was added or removed to the userIdentity, + * and with who a key should be now be shared + **/ + async writeMemberChanges(room, memberChanges, historyVisibility, txn) { + const added = []; + const removed = []; + await Promise.all(Array.from(memberChanges.values()).map(async memberChange => { + // keys should now be shared with this member? + // add the room to the userIdentity if so + if (shouldShareKey(memberChange.membership, historyVisibility)) { + if (await this._addRoomToUserIdentity(memberChange.roomId, memberChange.userId, txn)) { + added.push(memberChange.userId); + } + } else if (memberChange.hasLeft) { + // remove room + const {roomId} = memberChange; + // if we left the room, remove room from all user identities in the room + if (memberChange.userId === this._ownUserId) { + const userIds = await txn.roomMembers.getAllUserIds(roomId); + await Promise.all(userIds.map(userId => { + return this._removeRoomFromUserIdentity(roomId, userId, txn); + })); + } else { + await this._removeRoomFromUserIdentity(roomId, memberChange.userId, txn); + } + removed.push(memberChange.userId); + } })); + return {added, removed}; } - async trackRoom(room, log) { + async trackRoom(room, historyVisibility, log) { if (room.isTrackingMembers || !room.isEncrypted) { return; } @@ -100,7 +126,11 @@ export class DeviceTracker { isTrackingChanges = room.writeIsTrackingMembers(true, txn); const members = Array.from(memberList.members.values()); log.set("members", members.length); - await this._writeJoinedMembers(members, txn); + await Promise.all(members.map(async member => { + if (shouldShareKey(member.membership, historyVisibility)) { + await this._addRoomToUserIdentity(member.roomId, member.userId, txn); + } + })); } catch (err) { txn.abort(); throw err; @@ -120,13 +150,15 @@ export class DeviceTracker { })); } - async _writeMember(member, txn) { + async _addRoomToUserIdentity(roomId, userId, txn) { const {userIdentities} = txn; - const identity = await userIdentities.get(member.userId); - const updatedIdentity = addRoomToIdentity(identity, member.userId, member.roomId); + const identity = await userIdentities.get(userId); + const updatedIdentity = addRoomToIdentity(identity, userId, roomId); if (updatedIdentity) { userIdentities.set(updatedIdentity); + return true; } + return false; } async _removeRoomFromUserIdentity(roomId, userId, txn) { @@ -141,28 +173,9 @@ export class DeviceTracker { } else { userIdentities.set(identity); } + return true; } - } - - async _applyMemberChange(memberChange, txn) { - // TODO: depends whether we encrypt for invited users?? - // add room - if (memberChange.hasJoined) { - await this._writeMember(memberChange.member, txn); - } - // remove room - else if (memberChange.hasLeft) { - const {roomId} = memberChange; - // if we left the room, remove room from all user identities in the room - if (memberChange.userId === this._ownUserId) { - const userIds = await txn.roomMembers.getAllUserIds(roomId); - await Promise.all(userIds.map(userId => { - return this._removeRoomFromUserIdentity(roomId, userId, txn); - })); - } else { - await this._removeRoomFromUserIdentity(roomId, memberChange.userId, txn); - } - } + return false; } async _queryKeys(userIds, hsApi, log) { From 17f42f523a66afa0351ed15c51b62119c97794c8 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 17:49:26 +0200 Subject: [PATCH 07/19] add write method for when history visibility changes also returning added and removed user ids --- src/matrix/e2ee/DeviceTracker.js | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 7ce601d2..8bed487d 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -16,6 +16,7 @@ limitations under the License. import {verifyEd25519Signature, SIGNATURE_ALGORITHM} from "./common.js"; import {shouldShareKey} from "./common.js"; +import {RoomMember} from "../room/members/RoomMember.js"; const TRACKING_STATUS_OUTDATED = 0; const TRACKING_STATUS_UPTODATE = 1; @@ -142,12 +143,29 @@ export class DeviceTracker { } } - async _writeJoinedMembers(members, txn) { - await Promise.all(members.map(async member => { - if (member.membership === "join") { - await this._writeMember(member, txn); - } - })); + async writeHistoryVisibility(room, historyVisibility, syncTxn, log) { + const added = []; + const removed = []; + if (room.isTrackingMembers && room.isEncrypted) { + await log.wrap("rewriting userIdentities", async log => { + // don't use room.loadMemberList here because we want to use the syncTxn to load the members + const memberDatas = await syncTxn.roomMembers.getAll(room.id); + const members = memberDatas.map(d => new RoomMember(d)); + log.set("members", members.length); + await Promise.all(members.map(async member => { + if (shouldShareKey(member.membership, historyVisibility)) { + if (await this._addRoomToUserIdentity(member.roomId, member.userId, syncTxn)) { + added.push(member.userId); + } + } else { + if (await this._removeRoomFromUserIdentity(member.roomId, member.userId, syncTxn)) { + removed.push(member.userId); + } + } + })); + }); + } + return {added, removed}; } async _addRoomToUserIdentity(roomId, userId, txn) { From a23df8a5457e63cd0f0c10941f88f1631c504624 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 22 Jul 2022 17:49:59 +0200 Subject: [PATCH 08/19] pass history visibility to device tracker and delegate adding and removing members to share keys with to it --- src/matrix/e2ee/RoomEncryption.js | 74 +++++++++++++++++-------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 8839556f..a841a6bd 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -28,14 +28,6 @@ const ROOM_HISTORY_VISIBILITY_TYPE = "m.room.history_visibility"; // note that encrypt could still create a new session const MIN_PRESHARE_INTERVAL = 60 * 1000; // 1min -// Use enum when converting to TS -const HistoryVisibility = Object.freeze({ - Joined: "joined", - Invited: "invited", - WorldReadable: "world_readable", - Shared: "shared", -}); - // TODO: this class is a good candidate for splitting up into encryption and decryption, there doesn't seem to be much overlap export class RoomEncryption { constructor({room, deviceTracker, olmEncryption, megolmEncryption, megolmDecryption, encryptionParams, storage, keyBackup, notifyMissingMegolmSession, clock}) { @@ -89,37 +81,49 @@ export class RoomEncryption { } async writeSync(roomResponse, memberChanges, txn, log) { - let historyVisibility = this._historyVisibility; - iterateResponseStateEvents(roomResponse, event => { + let historyVisibility = await this._loadHistoryVisibilityIfNeeded(this._historyVisibility, txn); + const addedMembers = []; + const removedMembers = []; + // update the historyVisibility if needed + await iterateResponseStateEvents(roomResponse, event => { + // TODO: can the same state event appear twice? Hence we would be rewriting the useridentities twice... + // we'll see in the logs if(event.state_key === "" && event.type === ROOM_HISTORY_VISIBILITY_TYPE) { - historyVisibility = event?.content?.history_visibility; + const newHistoryVisibility = event?.content?.history_visibility; + if (newHistoryVisibility !== historyVisibility) { + return log.wrap({ + l: "history_visibility changed", + from: historyVisibility, + to: newHistoryVisibility + }, async log => { + historyVisibility = newHistoryVisibility; + const result = await this._deviceTracker.writeHistoryVisibility(this._room, historyVisibility, txn, log); + addedMembers.push(...result.added); + removedMembers.push(...result.removed); + }); + } } }); - let shouldFlush = false; - const memberChangesArray = Array.from(memberChanges.values()); - // this also clears our session if we leave the room ourselves - if (memberChangesArray.some(m => m.hasLeft)) { + // process member changes + if (memberChanges.size) { + const result = await this._deviceTracker.writeMemberChanges( + this._room, memberChanges, historyVisibility, txn); + addedMembers.push(...result.added); + removedMembers.push(...result.removed); + } + // discard key if somebody (including ourselves) left + if (removedMembers.length) { log.log({ l: "discardOutboundSession", - leftUsers: memberChangesArray.filter(m => m.hasLeft).map(m => m.userId), + leftUsers: removedMembers, }); this._megolmEncryption.discardOutboundSession(this._room.id, txn); } - if (memberChangesArray.some(m => m.hasJoined)) { - const userIds = memberChangesArray.filter(m => m.hasJoined).map(m => m.userId); - shouldFlush = await this._addShareRoomKeyOperationForMembers(userIds, txn, log) - || shouldFlush; + let shouldFlush = false; + // add room to userIdentities if needed, and share the current key with them + if (addedMembers.length) { + shouldFlush = await this._addShareRoomKeyOperationForMembers(addedMembers, txn, log); } - if (memberChangesArray.some(m => m.wasInvited)) { - historyVisibility = await this._loadHistoryVisibilityIfNeeded(historyVisibility, txn); - if (historyVisibility === HistoryVisibility.Invited) { - const userIds = memberChangesArray.filter(m => m.wasInvited).map(m => m.userId); - shouldFlush = await this._addShareRoomKeyOperationForMembers(userIds, txn, log) - || shouldFlush; - } - } - - await this._deviceTracker.writeMemberChanges(this._room, memberChanges, txn); return {shouldFlush, historyVisibility}; } @@ -127,8 +131,11 @@ export class RoomEncryption { this._historyVisibility = historyVisibility; } - async _loadHistoryVisibilityIfNeeded(historyVisibility, txn) { + async _loadHistoryVisibilityIfNeeded(historyVisibility, txn = undefined) { if (!historyVisibility) { + if (!txn) { + txn = await this._storage.readTxn([this._storage.storeNames.roomState]); + } const visibilityEntry = await txn.roomState.get(this.id, ROOM_HISTORY_VISIBILITY_TYPE, ""); if (visibilityEntry) { return event?.content?.history_visibility; @@ -316,6 +323,7 @@ export class RoomEncryption { } async _shareNewRoomKey(roomKeyMessage, hsApi, log) { + this._historyVisibility = await this._loadHistoryVisibilityIfNeeded(this._historyVisibility); const devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, this._historyVisibility, hsApi, log); const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); @@ -386,8 +394,8 @@ export class RoomEncryption { async _processShareRoomKeyOperation(operation, hsApi, log) { log.set("id", operation.id); - - await this._deviceTracker.trackRoom(this._room, log); + this._historyVisibility = await this._loadHistoryVisibilityIfNeeded(this._historyVisibility); + await this._deviceTracker.trackRoom(this._room, this._historyVisibility, log); const devices = await this._deviceTracker.devicesForRoomMembers(this._room.id, operation.userIds, hsApi, log); const messages = await log.wrap("olm encrypt", log => this._olmEncryption.encrypt( "m.room_key", operation.roomKeyMessage, devices, hsApi, log)); From 4c17612b0588143a84d35a2f24928a646a117f70 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 26 Jul 2022 16:53:02 +0200 Subject: [PATCH 09/19] allow passing txn to loadMembers so we can do it as part of sync txn to rewrite useridentities upon receiving new history visibility --- src/matrix/e2ee/DeviceTracker.js | 13 +++++++------ src/matrix/room/BaseRoom.js | 5 ++++- src/matrix/room/members/load.js | 10 ++++++---- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 8bed487d..1ba59d0a 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -116,12 +116,13 @@ export class DeviceTracker { if (room.isTrackingMembers || !room.isEncrypted) { return; } - const memberList = await room.loadMemberList(log); + const txn = await this._storage.readWriteTxn([ + this._storage.storeNames.roomSummary, + this._storage.storeNames.userIdentities, + ]); + let memberList; try { - const txn = await this._storage.readWriteTxn([ - this._storage.storeNames.roomSummary, - this._storage.storeNames.userIdentities, - ]); + memberList = await room.loadMemberList(txn, log); let isTrackingChanges; try { isTrackingChanges = room.writeIsTrackingMembers(true, txn); @@ -139,7 +140,7 @@ export class DeviceTracker { await txn.complete(); room.applyIsTrackingMembersChanges(isTrackingChanges); } finally { - memberList.release(); + memberList?.release(); } } diff --git a/src/matrix/room/BaseRoom.js b/src/matrix/room/BaseRoom.js index dda3e2e5..57d2a7b2 100644 --- a/src/matrix/room/BaseRoom.js +++ b/src/matrix/room/BaseRoom.js @@ -243,7 +243,7 @@ export class BaseRoom extends EventEmitter { /** @public */ - async loadMemberList(log = null) { + async loadMemberList(txn = undefined, log = null) { if (this._memberList) { // TODO: also await fetchOrLoadMembers promise here this._memberList.retain(); @@ -254,6 +254,9 @@ export class BaseRoom extends EventEmitter { roomId: this._roomId, hsApi: this._hsApi, storage: this._storage, + // pass in a transaction if we know we won't need to fetch (which would abort the transaction) + // and we want to make this operation part of the larger transaction + txn, syncToken: this._getSyncToken(), // to handle race between /members and /sync setChangedMembersMap: map => this._changedMembersDuringSync = map, diff --git a/src/matrix/room/members/load.js b/src/matrix/room/members/load.js index 5077d793..3d0556fc 100644 --- a/src/matrix/room/members/load.js +++ b/src/matrix/room/members/load.js @@ -17,10 +17,12 @@ limitations under the License. import {RoomMember} from "./RoomMember.js"; -async function loadMembers({roomId, storage}) { - const txn = await storage.readTxn([ - storage.storeNames.roomMembers, - ]); +async function loadMembers({roomId, storage, txn}) { + if (!txn) { + txn = await storage.readTxn([ + storage.storeNames.roomMembers, + ]); + } const memberDatas = await txn.roomMembers.getAll(roomId); return memberDatas.map(d => new RoomMember(d)); } From dea3852425eda1ec668b89f9afaa0df7843d3b26 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 26 Jul 2022 16:57:28 +0200 Subject: [PATCH 10/19] add some tests for sharing keys with invitees --- src/matrix/e2ee/DeviceTracker.js | 165 +++++++++++++++++++++++++++---- 1 file changed, 144 insertions(+), 21 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 1ba59d0a..582a0e31 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -15,7 +15,7 @@ limitations under the License. */ import {verifyEd25519Signature, SIGNATURE_ALGORITHM} from "./common.js"; -import {shouldShareKey} from "./common.js"; +import {HistoryVisibility, shouldShareKey} from "./common.js"; import {RoomMember} from "../room/members/RoomMember.js"; const TRACKING_STATUS_OUTDATED = 0; @@ -150,20 +150,24 @@ export class DeviceTracker { if (room.isTrackingMembers && room.isEncrypted) { await log.wrap("rewriting userIdentities", async log => { // don't use room.loadMemberList here because we want to use the syncTxn to load the members - const memberDatas = await syncTxn.roomMembers.getAll(room.id); - const members = memberDatas.map(d => new RoomMember(d)); - log.set("members", members.length); - await Promise.all(members.map(async member => { - if (shouldShareKey(member.membership, historyVisibility)) { - if (await this._addRoomToUserIdentity(member.roomId, member.userId, syncTxn)) { - added.push(member.userId); + const memberList = await room.loadMemberList(syncTxn, log); + try { + const members = Array.from(memberList.members.values()); + log.set("members", members.length); + await Promise.all(members.map(async member => { + if (shouldShareKey(member.membership, historyVisibility)) { + if (await this._addRoomToUserIdentity(member.roomId, member.userId, syncTxn)) { + added.push(member.userId); + } + } else { + if (await this._removeRoomFromUserIdentity(member.roomId, member.userId, syncTxn)) { + removed.push(member.userId); + } } - } else { - if (await this._removeRoomFromUserIdentity(member.roomId, member.userId, syncTxn)) { - removed.push(member.userId); - } - } - })); + })); + } finally { + memberList.release(); + } }); } return {added, removed}; @@ -399,6 +403,7 @@ export class DeviceTracker { import {createMockStorage} from "../../mocks/Storage"; import {Instance as NullLoggerInstance} from "../../logging/NullLogger"; +import {MemberChange} from "../room/members/RoomMember"; export function tests() { @@ -407,8 +412,8 @@ export function tests() { isTrackingMembers: false, isEncrypted: true, loadMemberList: () => { - const joinedMembers = joinedUserIds.map(userId => {return {membership: "join", roomId, userId};}); - const invitedMembers = invitedUserIds.map(userId => {return {membership: "invite", roomId, userId};}); + const joinedMembers = joinedUserIds.map(userId => {return RoomMember.fromUserId(roomId, userId, "join");}); + const invitedMembers = invitedUserIds.map(userId => {return RoomMember.fromUserId(roomId, userId, "invite");}); const members = joinedMembers.concat(invitedMembers); const memberMap = members.reduce((map, member) => { map.set(member.userId, member); @@ -472,10 +477,29 @@ export function tests() { } }; } + + async function writeMemberListToStorage(room, storage) { + const txn = await storage.readWriteTxn([ + storage.storeNames.roomMembers, + ]); + const memberList = await room.loadMemberList(txn); + try { + for (const member of memberList.members.values()) { + txn.roomMembers.set(member.serialize()); + } + } catch (err) { + txn.abort(); + throw err; + } finally { + memberList.release(); + } + await txn.complete(); + } + const roomId = "!abc:hs.tld"; return { - "trackRoom only writes joined members": async assert => { + "trackRoom only writes joined members with history visibility of joined": async assert => { const storage = await createMockStorage(); const tracker = new DeviceTracker({ storage, @@ -485,7 +509,7 @@ export function tests() { ownDeviceId: "ABCD", }); const room = createUntrackedRoomMock(roomId, ["@alice:hs.tld", "@bob:hs.tld"], ["@charly:hs.tld"]); - await tracker.trackRoom(room, NullLoggerInstance.item); + await tracker.trackRoom(room, HistoryVisibility.Joined, NullLoggerInstance.item); const txn = await storage.readTxn([storage.storeNames.userIdentities]); assert.deepEqual(await txn.userIdentities.get("@alice:hs.tld"), { userId: "@alice:hs.tld", @@ -509,7 +533,7 @@ export function tests() { ownDeviceId: "ABCD", }); const room = createUntrackedRoomMock(roomId, ["@alice:hs.tld", "@bob:hs.tld"]); - await tracker.trackRoom(room, NullLoggerInstance.item); + await tracker.trackRoom(room, HistoryVisibility.Joined, NullLoggerInstance.item); const hsApi = createQueryKeysHSApiMock(); const devices = await tracker.devicesForRoomMembers(roomId, ["@alice:hs.tld", "@bob:hs.tld"], hsApi, NullLoggerInstance.item); assert.equal(devices.length, 2); @@ -526,7 +550,7 @@ export function tests() { ownDeviceId: "ABCD", }); const room = createUntrackedRoomMock(roomId, ["@alice:hs.tld", "@bob:hs.tld"]); - await tracker.trackRoom(room, NullLoggerInstance.item); + await tracker.trackRoom(room, HistoryVisibility.Joined, NullLoggerInstance.item); const hsApi = createQueryKeysHSApiMock(); // query devices first time await tracker.devicesForRoomMembers(roomId, ["@alice:hs.tld", "@bob:hs.tld"], hsApi, NullLoggerInstance.item); @@ -544,6 +568,105 @@ export function tests() { const txn2 = await storage.readTxn([storage.storeNames.deviceIdentities]); // also check the modified key was not stored assert.equal((await txn2.deviceIdentities.get("@alice:hs.tld", "device1")).ed25519Key, "ed25519:@alice:hs.tld:device1:key"); - } + }, + "change history visibility from joined to invited adds invitees": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + // alice is joined, bob is invited + const room = await createUntrackedRoomMock(roomId, + ["@alice:hs.tld"], ["@bob:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Joined, NullLoggerInstance.item); + const txn = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + assert.equal(await txn.userIdentities.get("@bob:hs.tld"), undefined); + const {added, removed} = await tracker.writeHistoryVisibility(room, HistoryVisibility.Invited, txn, NullLoggerInstance.item); + assert.equal((await txn.userIdentities.get("@bob:hs.tld")).userId, "@bob:hs.tld"); + assert.deepEqual(added, ["@bob:hs.tld"]); + assert.deepEqual(removed, []); + }, + "change history visibility from invited to joined removes invitees": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + // alice is joined, bob is invited + const room = await createUntrackedRoomMock(roomId, + ["@alice:hs.tld"], ["@bob:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Invited, NullLoggerInstance.item); + const txn = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + assert.equal((await txn.userIdentities.get("@bob:hs.tld")).userId, "@bob:hs.tld"); + const {added, removed} = await tracker.writeHistoryVisibility(room, HistoryVisibility.Joined, txn, NullLoggerInstance.item); + assert.equal(await txn.userIdentities.get("@bob:hs.tld"), undefined); + assert.deepEqual(added, []); + assert.deepEqual(removed, ["@bob:hs.tld"]); + }, + "adding invitee with history visibility of invited adds room to userIdentities": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + const room = await createUntrackedRoomMock(roomId, ["@alice:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Invited, NullLoggerInstance.item); + const txn = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + // inviting a new member + const inviteChange = new MemberChange(RoomMember.fromUserId(roomId, "@bob:hs.tld", "invite")); + const {added, removed} = await tracker.writeMemberChanges(room, [inviteChange], HistoryVisibility.Invited, txn); + assert.deepEqual(added, ["@bob:hs.tld"]); + assert.deepEqual(removed, []); + assert.equal((await txn.userIdentities.get("@bob:hs.tld")).userId, "@bob:hs.tld"); + }, + "adding invitee with history visibility of joined doesn't add room": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + const room = await createUntrackedRoomMock(roomId, ["@alice:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Joined, NullLoggerInstance.item); + const txn = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + // inviting a new member + const inviteChange = new MemberChange(RoomMember.fromUserId(roomId, "@bob:hs.tld", "invite")); + const memberChanges = new Map([[inviteChange.userId, inviteChange]]); + const {added, removed} = await tracker.writeMemberChanges(room, memberChanges, HistoryVisibility.Joined, txn); + assert.deepEqual(added, []); + assert.deepEqual(removed, []); + assert.equal(await txn.userIdentities.get("@bob:hs.tld"), undefined); + }, + "getting all devices after changing history visibility now includes invitees": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + const room = createUntrackedRoomMock(roomId, ["@alice:hs.tld"], ["@bob:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Invited, NullLoggerInstance.item); + const hsApi = createQueryKeysHSApiMock(); + // write memberlist from room mock to mock storage, + // as devicesForTrackedRoom reads directly from roomMembers store. + await writeMemberListToStorage(room, storage); + const devices = await tracker.devicesForTrackedRoom(roomId, hsApi, NullLoggerInstance.item); + assert.equal(devices.length, 2); + assert.equal(devices.find(d => d.userId === "@alice:hs.tld").ed25519Key, "ed25519:@alice:hs.tld:device1:key"); + assert.equal(devices.find(d => d.userId === "@bob:hs.tld").ed25519Key, "ed25519:@bob:hs.tld:device1:key"); + }, } } From dd878bb8d6e7ef36994d7e8a53b0983112ea98b7 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 26 Jul 2022 16:58:07 +0200 Subject: [PATCH 11/19] also take rejecting invites into account to remove user identity --- src/matrix/e2ee/DeviceTracker.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 582a0e31..3fe88357 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -94,8 +94,8 @@ export class DeviceTracker { if (await this._addRoomToUserIdentity(memberChange.roomId, memberChange.userId, txn)) { added.push(memberChange.userId); } - } else if (memberChange.hasLeft) { - // remove room + } else if (shouldShareKey(memberChange.previousMembership, historyVisibility)) { + // try to remove room we were previously sharing the key with the member but not anymore const {roomId} = memberChange; // if we left the room, remove room from all user identities in the room if (memberChange.userId === this._ownUserId) { @@ -668,5 +668,26 @@ export function tests() { assert.equal(devices.find(d => d.userId === "@alice:hs.tld").ed25519Key, "ed25519:@alice:hs.tld:device1:key"); assert.equal(devices.find(d => d.userId === "@bob:hs.tld").ed25519Key, "ed25519:@bob:hs.tld:device1:key"); }, + "rejecting invite with history visibility of invited removes room from user identity": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + // alice is joined, bob is invited + const room = await createUntrackedRoomMock(roomId, ["@alice:hs.tld"], ["@bob:hs.tld"]); + await tracker.trackRoom(room, HistoryVisibility.Invited, NullLoggerInstance.item); + const txn = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + // reject invite + const inviteChange = new MemberChange(RoomMember.fromUserId(roomId, "@bob:hs.tld", "leave"), "invite"); + const memberChanges = new Map([[inviteChange.userId, inviteChange]]); + const {added, removed} = await tracker.writeMemberChanges(room, memberChanges, HistoryVisibility.Invited, txn); + assert.deepEqual(added, []); + assert.deepEqual(removed, ["@bob:hs.tld"]); + assert.equal(await txn.userIdentities.get("@bob:hs.tld"), undefined); + }, } } From 544afef90249c851849f24c405ab8e4b6208ab8f Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 26 Jul 2022 17:41:26 +0200 Subject: [PATCH 12/19] test adding and removing when tracking multiple rooms --- src/matrix/e2ee/DeviceTracker.js | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 3fe88357..edf3c133 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -409,6 +409,7 @@ export function tests() { function createUntrackedRoomMock(roomId, joinedUserIds, invitedUserIds = []) { return { + id: roomId, isTrackingMembers: false, isEncrypted: true, loadMemberList: () => { @@ -689,5 +690,48 @@ export function tests() { assert.deepEqual(removed, ["@bob:hs.tld"]); assert.equal(await txn.userIdentities.get("@bob:hs.tld"), undefined); }, + "remove room from user identity sharing multiple rooms with us preserves other room": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + // alice is joined, bob is invited + const room1 = await createUntrackedRoomMock("!abc:hs.tld", ["@alice:hs.tld", "@bob:hs.tld"]); + const room2 = await createUntrackedRoomMock("!def:hs.tld", ["@alice:hs.tld", "@bob:hs.tld"]); + await tracker.trackRoom(room1, HistoryVisibility.Joined, NullLoggerInstance.item); + await tracker.trackRoom(room2, HistoryVisibility.Joined, NullLoggerInstance.item); + const txn1 = await storage.readTxn([storage.storeNames.userIdentities]); + assert.deepEqual((await txn1.userIdentities.get("@bob:hs.tld")).roomIds, ["!abc:hs.tld", "!def:hs.tld"]); + const leaveChange = new MemberChange(RoomMember.fromUserId(room2.id, "@bob:hs.tld", "leave"), "join"); + const memberChanges = new Map([[leaveChange.userId, leaveChange]]); + const txn2 = await storage.readWriteTxn([storage.storeNames.userIdentities, storage.storeNames.deviceIdentities]); + await tracker.writeMemberChanges(room2, memberChanges, HistoryVisibility.Joined, txn2); + await txn2.complete(); + const txn3 = await storage.readTxn([storage.storeNames.userIdentities]); + assert.deepEqual((await txn3.userIdentities.get("@bob:hs.tld")).roomIds, ["!abc:hs.tld"]); + }, + "add room to user identity sharing multiple rooms with us preserves other room": async assert => { + const storage = await createMockStorage(); + const tracker = new DeviceTracker({ + storage, + getSyncToken: () => "token", + olmUtil: {ed25519_verify: () => {}}, // valid if it does not throw + ownUserId: "@alice:hs.tld", + ownDeviceId: "ABCD", + }); + // alice is joined, bob is invited + const room1 = await createUntrackedRoomMock("!abc:hs.tld", ["@alice:hs.tld", "@bob:hs.tld"]); + const room2 = await createUntrackedRoomMock("!def:hs.tld", ["@alice:hs.tld", "@bob:hs.tld"]); + await tracker.trackRoom(room1, HistoryVisibility.Joined, NullLoggerInstance.item); + const txn1 = await storage.readTxn([storage.storeNames.userIdentities]); + assert.deepEqual((await txn1.userIdentities.get("@bob:hs.tld")).roomIds, ["!abc:hs.tld"]); + await tracker.trackRoom(room2, HistoryVisibility.Joined, NullLoggerInstance.item); + const txn2 = await storage.readTxn([storage.storeNames.userIdentities]); + assert.deepEqual((await txn2.userIdentities.get("@bob:hs.tld")).roomIds, ["!abc:hs.tld", "!def:hs.tld"]); + }, } } From bfaba63f473c84e65413806752b712b87fbc035b Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 26 Jul 2022 17:55:21 +0200 Subject: [PATCH 13/19] fix ts error --- src/matrix/room/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/room/common.ts b/src/matrix/room/common.ts index 5a7fc98e..4556302f 100644 --- a/src/matrix/room/common.ts +++ b/src/matrix/room/common.ts @@ -80,7 +80,7 @@ export function iterateResponseStateEvents(roomResponse: RoomResponse, callback: } } if (promises) { - return Promise.all(promises); + return Promise.all(promises).then(() => undefined); } } From 50b6ee91d72d69e45cd97d1f480fa6d1127113f7 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Wed, 27 Jul 2022 11:39:36 +0200 Subject: [PATCH 14/19] don't need history visibility here --- src/matrix/e2ee/RoomEncryption.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index a841a6bd..f49d5c97 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -323,8 +323,7 @@ export class RoomEncryption { } async _shareNewRoomKey(roomKeyMessage, hsApi, log) { - this._historyVisibility = await this._loadHistoryVisibilityIfNeeded(this._historyVisibility); - const devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, this._historyVisibility, hsApi, log); + const devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, hsApi, log); const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); let writeOpTxn = await this._storage.readWriteTxn([this._storage.storeNames.operations]); From f18520a2fe2da30a3c4028be00d98c60467cec1a Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Wed, 27 Jul 2022 11:39:50 +0200 Subject: [PATCH 15/19] let loadMembers use own txn in case members haven't been fetched yet if they haven't, it will need a network request, meaning that the txn will get closed, so we can't reuse it afterwards --- src/matrix/e2ee/DeviceTracker.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index edf3c133..55051994 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -116,13 +116,12 @@ export class DeviceTracker { if (room.isTrackingMembers || !room.isEncrypted) { return; } + const memberList = await room.loadMemberList(undefined, log); const txn = await this._storage.readWriteTxn([ this._storage.storeNames.roomSummary, this._storage.storeNames.userIdentities, ]); - let memberList; try { - memberList = await room.loadMemberList(txn, log); let isTrackingChanges; try { isTrackingChanges = room.writeIsTrackingMembers(true, txn); @@ -140,7 +139,7 @@ export class DeviceTracker { await txn.complete(); room.applyIsTrackingMembersChanges(isTrackingChanges); } finally { - memberList?.release(); + memberList.release(); } } From 0df66b5aea6b448ea7ec03c3c26f65ca63c45f93 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Wed, 27 Jul 2022 12:06:55 +0200 Subject: [PATCH 16/19] track room before listing user ids when sharing key --- src/matrix/e2ee/RoomEncryption.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index f49d5c97..49ca1a2f 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -323,6 +323,8 @@ export class RoomEncryption { } async _shareNewRoomKey(roomKeyMessage, hsApi, log) { + this._historyVisibility = await this._loadHistoryVisibilityIfNeeded(this._historyVisibility); + await this._deviceTracker.trackRoom(this._room, this._historyVisibility, log); const devices = await this._deviceTracker.devicesForTrackedRoom(this._room.id, hsApi, log); const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); From 319ec37864017a38abda67c76af0901dcc3bc4d6 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Thu, 28 Jul 2022 11:44:50 +0200 Subject: [PATCH 17/19] fix typos preventing to load the history visibility --- src/matrix/e2ee/RoomEncryption.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 49ca1a2f..fa1ecf6e 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -136,9 +136,9 @@ export class RoomEncryption { if (!txn) { txn = await this._storage.readTxn([this._storage.storeNames.roomState]); } - const visibilityEntry = await txn.roomState.get(this.id, ROOM_HISTORY_VISIBILITY_TYPE, ""); + const visibilityEntry = await txn.roomState.get(this._room.id, ROOM_HISTORY_VISIBILITY_TYPE, ""); if (visibilityEntry) { - return event?.content?.history_visibility; + return visibilityEntry.event?.content?.history_visibility; } } return historyVisibility; From 62b3a67e33c9d53446ac55c79e155728f7dc5255 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Thu, 28 Jul 2022 17:09:41 +0200 Subject: [PATCH 18/19] write unit tests for correctly reading history visibility when needed --- src/matrix/e2ee/RoomEncryption.js | 140 ++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index fa1ecf6e..36424b02 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -551,3 +551,143 @@ class BatchDecryptionResult { })); } } + +import {createMockStorage} from "../../mocks/Storage"; +import {Clock as MockClock} from "../../mocks/Clock"; +import {poll} from "../../mocks/poll"; +import {Instance as NullLoggerInstance} from "../../logging/NullLogger"; +import {ConsoleLogger} from "../../logging/ConsoleLogger"; +import {HomeServer as MockHomeServer} from "../../mocks/HomeServer.js"; + +export function tests() { + const roomId = "!abc:hs.tld"; + return { + "ensureMessageKeyIsShared tracks room and passes correct history visibility to deviceTracker": async assert => { + const storage = await createMockStorage(); + const megolmMock = { + async ensureOutboundSession() { return { }; } + }; + const olmMock = { + async encrypt() { return []; } + } + let isRoomTracked = false; + let isDevicesRequested = false; + const deviceTracker = { + async trackRoom(room, historyVisibility) { + // only assert on first call + if (isRoomTracked) { return; } + assert(!isDevicesRequested); + assert.equal(room.id, roomId); + assert.equal(historyVisibility, "invited"); + isRoomTracked = true; + }, + async devicesForTrackedRoom() { + assert(isRoomTracked); + isDevicesRequested = true; + return []; + }, + async devicesForRoomMembers() { + return []; + } + } + const writeTxn = await storage.readWriteTxn([storage.storeNames.roomState]); + writeTxn.roomState.set(roomId, {state_key: "", type: ROOM_HISTORY_VISIBILITY_TYPE, content: { + history_visibility: "invited" + }}); + await writeTxn.complete(); + const roomEncryption = new RoomEncryption({ + room: {id: roomId}, + megolmEncryption: megolmMock, + olmEncryption: olmMock, + storage, + deviceTracker, + clock: new MockClock() + }); + const homeServer = new MockHomeServer(); + const promise = roomEncryption.ensureMessageKeyIsShared(homeServer.api, NullLoggerInstance.item); + // need to poll because sendToDevice isn't first async step + const request = await poll(() => homeServer.requests.sendToDevice?.[0]); + request.respond({}); + await promise; + assert(isRoomTracked); + assert(isDevicesRequested); + }, + "encrypt tracks room and passes correct history visibility to deviceTracker": async assert => { + const storage = await createMockStorage(); + const megolmMock = { + async encrypt() { return { roomKeyMessage: {} }; } + }; + const olmMock = { + async encrypt() { return []; } + } + let isRoomTracked = false; + let isDevicesRequested = false; + const deviceTracker = { + async trackRoom(room, historyVisibility) { + // only assert on first call + if (isRoomTracked) { return; } + assert(!isDevicesRequested); + assert.equal(room.id, roomId); + assert.equal(historyVisibility, "invited"); + isRoomTracked = true; + }, + async devicesForTrackedRoom() { + assert(isRoomTracked); + isDevicesRequested = true; + return []; + }, + async devicesForRoomMembers() { + return []; + } + } + const writeTxn = await storage.readWriteTxn([storage.storeNames.roomState]); + writeTxn.roomState.set(roomId, {state_key: "", type: ROOM_HISTORY_VISIBILITY_TYPE, content: { + history_visibility: "invited" + }}); + await writeTxn.complete(); + const roomEncryption = new RoomEncryption({ + room: {id: roomId}, + megolmEncryption: megolmMock, + olmEncryption: olmMock, + storage, + deviceTracker + }); + const homeServer = new MockHomeServer(); + const promise = roomEncryption.encrypt("m.room.message", {body: "hello"}, homeServer.api, NullLoggerInstance.item); + // need to poll because sendToDevice isn't first async step + const request = await poll(() => homeServer.requests.sendToDevice?.[0]); + request.respond({}); + await promise; + assert(isRoomTracked); + assert(isDevicesRequested); + }, + "writeSync passes correct history visibility to deviceTracker": async assert => { + const storage = await createMockStorage(); + let isMemberChangesCalled = false; + const deviceTracker = { + async writeMemberChanges(room, memberChanges, historyVisibility, txn) { + assert.equal(historyVisibility, "invited"); + isMemberChangesCalled = true; + return {removed: [], added: []}; + }, + async devicesForRoomMembers() { + return []; + } + } + const writeTxn = await storage.readWriteTxn([storage.storeNames.roomState]); + writeTxn.roomState.set(roomId, {state_key: "", type: ROOM_HISTORY_VISIBILITY_TYPE, content: { + history_visibility: "invited" + }}); + const memberChanges = new Map([["@alice:hs.tld", {}]]); + const roomEncryption = new RoomEncryption({ + room: {id: roomId}, + storage, + deviceTracker + }); + const roomResponse = {}; + const txn = await storage.readWriteTxn([storage.storeNames.roomState]); + await roomEncryption.writeSync(roomResponse, memberChanges, txn, NullLoggerInstance.item); + assert(isMemberChangesCalled); + }, + } +} From cb0ac846c7bf9691dd1dd8951da33511135e0399 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Fri, 29 Jul 2022 16:22:01 +0200 Subject: [PATCH 19/19] remove obsolete comment --- src/matrix/e2ee/DeviceTracker.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index 55051994..484a6d0b 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -148,7 +148,6 @@ export class DeviceTracker { const removed = []; if (room.isTrackingMembers && room.isEncrypted) { await log.wrap("rewriting userIdentities", async log => { - // don't use room.loadMemberList here because we want to use the syncTxn to load the members const memberList = await room.loadMemberList(syncTxn, log); try { const members = Array.from(memberList.members.values());