From 3fa2d220154e89e2c75319b2948140ccc47d65ed Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 1 Mar 2021 23:14:14 +0100 Subject: [PATCH 01/16] remove isTimelineOpen flag and rather do the check to verify in Room flags are ugly, let's avoid them where we can --- src/matrix/e2ee/RoomEncryption.js | 32 ++++++++++++++++--------------- src/matrix/room/Room.js | 12 +++++++++--- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 13b00f6f..62521a4f 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -93,7 +93,7 @@ export class RoomEncryption { // this happens before entries exists, as they are created by the syncwriter // but we want to be able to map it back to something in the timeline easily // when retrying decryption. - async prepareDecryptAll(events, newKeys, source, isTimelineOpen, txn) { + async prepareDecryptAll(events, newKeys, source, txn) { const errors = new Map(); const validEvents = []; for (const event of events) { @@ -126,7 +126,7 @@ export class RoomEncryption { if (customCache) { customCache.dispose(); } - return new DecryptionPreparation(preparation, errors, {isTimelineOpen, source}, this, events); + return new DecryptionPreparation(preparation, errors, source, this, events); } async _processDecryptionResults(events, results, errors, flags, txn) { @@ -139,11 +139,6 @@ export class RoomEncryption { this._missingSessionCandidates.removeEvent(event); } } - if (flags.isTimelineOpen) { - for (const result of results.values()) { - await this._verifyDecryptionResult(result, txn); - } - } } async _verifyDecryptionResult(result, txn) { @@ -424,10 +419,10 @@ export class RoomEncryption { * the decryption results before turning them */ class DecryptionPreparation { - constructor(megolmDecryptionPreparation, extraErrors, flags, roomEncryption, events) { + constructor(megolmDecryptionPreparation, extraErrors, source, roomEncryption, events) { this._megolmDecryptionPreparation = megolmDecryptionPreparation; this._extraErrors = extraErrors; - this._flags = flags; + this._source = source; this._roomEncryption = roomEncryption; this._events = events; } @@ -436,7 +431,7 @@ class DecryptionPreparation { return new DecryptionChanges( await this._megolmDecryptionPreparation.decrypt(), this._extraErrors, - this._flags, + this._source, this._roomEncryption, this._events); } @@ -447,10 +442,10 @@ class DecryptionPreparation { } class DecryptionChanges { - constructor(megolmDecryptionChanges, extraErrors, flags, roomEncryption, events) { + constructor(megolmDecryptionChanges, extraErrors, source, roomEncryption, events) { this._megolmDecryptionChanges = megolmDecryptionChanges; this._extraErrors = extraErrors; - this._flags = flags; + this._source = source; this._roomEncryption = roomEncryption; this._events = events; } @@ -458,15 +453,16 @@ class DecryptionChanges { async write(txn) { const {results, errors} = await this._megolmDecryptionChanges.write(txn); mergeMap(this._extraErrors, errors); - await this._roomEncryption._processDecryptionResults(this._events, results, errors, this._flags, txn); - return new BatchDecryptionResult(results, errors); + await this._roomEncryption._processDecryptionResults(this._events, results, errors, this._source, txn); + return new BatchDecryptionResult(results, errors, this._roomEncryption); } } class BatchDecryptionResult { - constructor(results, errors) { + constructor(results, errors, roomEncryption) { this.results = results; this.errors = errors; + this._roomEncryption = roomEncryption; } applyToEntries(entries) { @@ -482,6 +478,12 @@ class BatchDecryptionResult { } } } + + verifySenders(txn) { + return Promise.all(Array.from(this.results.values()).map(result => { + return this._roomEncryption._verifyDecryptionResult(result, txn); + })); + } } class SessionToEventIdsMap { diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 5883da1a..442da4fb 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -147,13 +147,13 @@ export class Room extends EventEmitter { const events = entries.filter(entry => { return entry.eventType === EVENT_ENCRYPTED_TYPE; }).map(entry => entry.event); - const isTimelineOpen = this._isTimelineOpen; - r.preparation = await this._roomEncryption.prepareDecryptAll(events, null, source, isTimelineOpen, inboundSessionTxn); + r.preparation = await this._roomEncryption.prepareDecryptAll(events, null, source, inboundSessionTxn); if (r.cancelled) return; const changes = await r.preparation.decrypt(); r.preparation = null; if (r.cancelled) return; const stores = [this._storage.storeNames.groupSessionDecryptions]; + const isTimelineOpen = this._isTimelineOpen; if (isTimelineOpen) { // read to fetch devices if timeline is open stores.push(this._storage.storeNames.deviceIdentities); @@ -162,6 +162,9 @@ export class Room extends EventEmitter { let decryption; try { decryption = await changes.write(writeTxn); + if (isTimelineOpen) { + await decryption.verifySenders(writeTxn); + } } catch (err) { writeTxn.abort(); throw err; @@ -210,7 +213,7 @@ export class Room extends EventEmitter { return event?.type === EVENT_ENCRYPTED_TYPE; }); decryptPreparation = await roomEncryption.prepareDecryptAll( - eventsToDecrypt, newKeys, DecryptionSource.Sync, this._isTimelineOpen, txn); + eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); } } @@ -240,6 +243,9 @@ export class Room extends EventEmitter { await log.wrap("syncWriter", log => this._syncWriter.writeSync(roomResponse, txn, log), log.level.Detail); if (decryptChanges) { const decryption = await decryptChanges.write(txn); + if (this._isTimelineOpen) { + await decryption.verifySenders(txn); + } if (retryEntries?.length) { // TODO: this will modify existing timeline entries (which we should not do in writeSync), // but it is a temporary way of reattempting decryption while timeline is open From c6db23bcfbf7bd42333421a33c61fe9b92e9433e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 15:29:35 +0100 Subject: [PATCH 02/16] WIP to store missing session event ids --- src/matrix/Sync.js | 1 + src/matrix/e2ee/README.md | 33 +-- src/matrix/e2ee/RoomEncryption.js | 201 +++++++------------ src/matrix/e2ee/megolm/Decryption.js | 69 ++++--- src/matrix/e2ee/megolm/decryption/RoomKey.js | 2 +- src/matrix/e2ee/megolm/decryption/utils.js | 57 ++++++ src/matrix/room/Room.js | 85 +++----- src/matrix/room/RoomSummary.js | 33 --- 8 files changed, 217 insertions(+), 264 deletions(-) create mode 100644 src/matrix/e2ee/megolm/decryption/utils.js diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 05c1d138..4154f959 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -248,6 +248,7 @@ export class Sync { return this._storage.readTxn([ storeNames.olmSessions, storeNames.inboundGroupSessions, + storeNames.timelineEvents // to read events that can now be decrypted ]); } diff --git a/src/matrix/e2ee/README.md b/src/matrix/e2ee/README.md index 46f4e95f..fab53880 100644 --- a/src/matrix/e2ee/README.md +++ b/src/matrix/e2ee/README.md @@ -1,40 +1,41 @@ ## Integratation within the sync lifetime cycle -### prepareSync +### session.prepareSync + +Decrypt any device messages, and turn them into RoomKey instances. +Any rooms that are not in the sync response but for which we receive keys will be included in the rooms to sync. + +Runs before any room.prepareSync, so the new room keys can be passed to each room prepareSync to use in decryption. + +### room.prepareSync The session can start its own read/write transactions here, rooms only read from a shared transaction - - session - - device handler - - txn - - write pending encrypted - - txn - - olm decryption read - - olm async decryption - - dispatch to worker - - txn - - olm decryption write / remove pending encrypted - rooms (with shared read txn) - - megolm decryption read + - megolm decryption read using any new keys decrypted by the session. -### afterPrepareSync +### room.afterPrepareSync - rooms - megolm async decryption - dispatch to worker -### writeSync +### room.writeSync - rooms (with shared readwrite txn) - megolm decryption write, yielding decrypted events - use decrypted events to write room summary -### afterSync +### session.writeSync + + - writes any room keys that were received + +### room.afterSync - rooms - emit changes -### afterSyncCompleted +### room.afterSyncCompleted - session - e2ee account diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 62521a4f..dbfa19de 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -15,7 +15,7 @@ limitations under the License. */ import {MEGOLM_ALGORITHM, DecryptionSource} from "./common.js"; -import {groupBy} from "../../utils/groupBy.js"; +import {groupEventsBySession} from "./megolm/decryption/utils.js"; import {mergeMap} from "../../utils/mergeMap.js"; import {makeTxnId} from "../common.js"; @@ -25,15 +25,6 @@ const ENCRYPTED_TYPE = "m.room.encrypted"; // note that encrypt could still create a new session const MIN_PRESHARE_INTERVAL = 60 * 1000; // 1min -function encodeMissingSessionKey(senderKey, sessionId) { - return `${senderKey}|${sessionId}`; -} - -function decodeMissingSessionKey(key) { - const [senderKey, sessionId] = key.split("|"); - return {senderKey, sessionId}; -} - export class RoomEncryption { constructor({room, deviceTracker, olmEncryption, megolmEncryption, megolmDecryption, encryptionParams, storage, sessionBackup, notifyMissingMegolmSession, clock}) { this._room = room; @@ -43,32 +34,40 @@ export class RoomEncryption { this._megolmDecryption = megolmDecryption; // content of the m.room.encryption event this._encryptionParams = encryptionParams; - this._megolmBackfillCache = this._megolmDecryption.createSessionCache(); this._megolmSyncCache = this._megolmDecryption.createSessionCache(1); - // session => event ids of messages we tried to decrypt and the session was missing - this._missingSessions = new SessionToEventIdsMap(); - // sessions that may or may not be missing, but that while - // looking for a particular session came up as a candidate and were - // added to the cache to prevent further lookups from storage - this._missingSessionCandidates = new SessionToEventIdsMap(); + // caches devices to verify events this._senderDeviceCache = new Map(); this._storage = storage; this._sessionBackup = sessionBackup; this._notifyMissingMegolmSession = notifyMissingMegolmSession; this._clock = clock; - this._disposed = false; this._isFlushingRoomKeyShares = false; this._lastKeyPreShareTime = null; + this._disposed = false; } - async enableSessionBackup(sessionBackup) { + enableSessionBackup(sessionBackup) { if (this._sessionBackup) { return; } this._sessionBackup = sessionBackup; - for(const {senderKey, sessionId} of this._missingSessions.getSessions()) { - await this._requestMissingSessionFromBackup(senderKey, sessionId, null); + } + + async restoreMissingSessionsFromBackup(events) { + const eventsBySession = groupEventsBySession(events); + const groups = Array.from(eventsBySession.values()); + const txn = this._storage.readTxn([this._storage.storeNames.inboundGroupSessions]); + const hasSessions = await Promise.all(groups.map(async group => { + return this._megolmDecryption.hasSession(this._room.id, group.senderKey, group.sessionId, txn); + })); + const missingSessions = groups.filter((_, i) => !hasSessions[i]); + if (missingSessions.length) { + // start with last sessions which should be for the last items in the timeline + for (var i = missingSessions.length - 1; i >= 0; i--) { + const session = missingSessions[i]; + await this._requestMissingSessionFromBackup(session.senderKey, session.sessionId); + } } } @@ -129,16 +128,48 @@ export class RoomEncryption { return new DecryptionPreparation(preparation, errors, source, this, events); } - async _processDecryptionResults(events, results, errors, flags, txn) { - for (const event of events) { + async _processDecryptionResults(events, results, errors, source, txn) { + const missingSessionEvents = events.filter(event => { const error = errors.get(event.event_id); - if (error?.code === "MEGOLM_NO_SESSION") { - this._addMissingSessionEvent(event, flags.source); - } else { - this._missingSessions.removeEvent(event); - this._missingSessionCandidates.removeEvent(event); - } + return error?.code === "MEGOLM_NO_SESSION"; + }); + if (!missingSessionEvents.length) { + return; } + const eventsBySession = groupEventsBySession(events); + if (source === DecryptionSource.Sync) { + await Promise.all(Array.from(eventsBySession.values()).map(async group => { + const eventIds = group.events.map(e => e.event_id); + return this._megolmDecryption.addMissingKeyEventIds( + this._room.id, group.senderKey, group.sessionId, eventIds, txn); + })); + } + + // TODO: do proper logging here + // run detached + Promise.resolve().then(async () => { + // if the message came from sync, wait 10s to see if the room key arrives late, + // and only after that proceed to request from backup + if (source === DecryptionSource.Sync) { + await this._clock.createTimeout(10000).elapsed(); + if (this._disposed) { + return; + } + // now check which sessions have been received already + const txn = this._storage.readTxn([this._storage.storeNames.inboundGroupSessions]); + await Promise.all(Array.from(eventsBySession).map(async ([key, group]) => { + if (await this._megolmDecryption.hasSession(this._room.id, group.senderKey, group.sessionId, txn)) { + eventsBySession.delete(key); + } + })); + } + await Promise.all(Array.from(eventsBySession.values()).map(group => { + return this._requestMissingSessionFromBackup(group.senderKey, group.sessionId); + })); + }).catch(err => { + console.log("failed to fetch missing session from key backup"); + console.error(err); + }); } async _verifyDecryptionResult(result, txn) { @@ -154,24 +185,7 @@ export class RoomEncryption { } } - _addMissingSessionEvent(event, source) { - const isNewSession = this._missingSessions.addEvent(event); - if (isNewSession) { - const senderKey = event.content?.["sender_key"]; - const sessionId = event.content?.["session_id"]; - this._requestMissingSessionFromBackup(senderKey, sessionId, source); - } - } - - async _requestMissingSessionFromBackup(senderKey, sessionId, source) { - // if the message came from sync, wait 10s to see if the room key arrives, - // and only after that proceed to request from backup - if (source === DecryptionSource.Sync) { - await this._clock.createTimeout(10000).elapsed(); - if (this._disposed || !this._missingSessions.hasSession(senderKey, sessionId)) { - return; - } - } + async _requestMissingSessionFromBackup(senderKey, sessionId) { // show prompt to enable secret storage if (!this._sessionBackup) { this._notifyMissingMegolmSession(); @@ -211,43 +225,19 @@ export class RoomEncryption { console.info(`Backed-up session of unknown algorithm: ${session.algorithm}`); } } catch (err) { - console.error(`Could not get session ${sessionId} from backup`, err); + if (!(err.name === "HomeServerError" && err.errcode === "M_NOT_FOUND")) { + console.error(`Could not get session ${sessionId} from backup`, err); + } } } /** * @param {RoomKey} roomKeys - * @return {Array} the event ids that should be retried to decrypt + * @param {Transaction} txn + * @return {Promise>} the event ids that should be retried to decrypt */ - getEventIdsForRoomKey(roomKey) { - // TODO: we could concat both results here, and only put stuff in - // candidates if it is not in missing sessions to use a bit less memory - let eventIds = this._missingSessions.getEventIds(roomKey.senderKey, roomKey.sessionId); - if (!eventIds) { - eventIds = this._missingSessionCandidates.getEventIds(roomKey.senderKey, roomKey.sessionId); - } - return eventIds; - } - - /** - * caches mapping of session to event id of all encrypted candidates - * and filters to return only the candidates for the given room key - */ - findAndCacheEntriesForRoomKey(roomKey, candidateEntries) { - const matches = []; - - for (const entry of candidateEntries) { - if (entry.eventType === ENCRYPTED_TYPE) { - this._missingSessionCandidates.addEvent(entry.event); - const senderKey = entry.event?.content?.["sender_key"]; - const sessionId = entry.event?.content?.["session_id"]; - if (senderKey === roomKey.senderKey && sessionId === roomKey.sessionId) { - matches.push(entry); - } - } - } - - return matches; + getEventIdsForMissingKey(roomKey, txn) { + return this._megolmDecryption.getEventIdsForMissingKey(this._room.id, roomKey.senderKey, roomKey.sessionId, txn); } /** shares the encryption key for the next message if needed */ @@ -485,58 +475,3 @@ class BatchDecryptionResult { })); } } - -class SessionToEventIdsMap { - constructor() { - this._eventIdsBySession = new Map(); - } - - addEvent(event) { - let isNewSession = false; - const senderKey = event.content?.["sender_key"]; - const sessionId = event.content?.["session_id"]; - const key = encodeMissingSessionKey(senderKey, sessionId); - let eventIds = this._eventIdsBySession.get(key); - // new missing session - if (!eventIds) { - eventIds = new Set(); - this._eventIdsBySession.set(key, eventIds); - isNewSession = true; - } - eventIds.add(event.event_id); - return isNewSession; - } - - getEventIds(senderKey, sessionId) { - const key = encodeMissingSessionKey(senderKey, sessionId); - const entriesForSession = this._eventIdsBySession.get(key); - if (entriesForSession) { - return [...entriesForSession]; - } - } - - getSessions() { - return Array.from(this._eventIdsBySession.keys()).map(decodeMissingSessionKey); - } - - hasSession(senderKey, sessionId) { - return this._eventIdsBySession.has(encodeMissingSessionKey(senderKey, sessionId)); - } - - removeEvent(event) { - let hasRemovedSession = false; - const senderKey = event.content?.["sender_key"]; - const sessionId = event.content?.["session_id"]; - const key = encodeMissingSessionKey(senderKey, sessionId); - let eventIds = this._eventIdsBySession.get(key); - if (eventIds) { - if (eventIds.delete(event.event_id)) { - if (!eventIds.length) { - this._eventIdsBySession.delete(key); - hasRemovedSession = true; - } - } - } - return hasRemovedSession; - } -} diff --git a/src/matrix/e2ee/megolm/Decryption.js b/src/matrix/e2ee/megolm/Decryption.js index 80a55961..8f4714ea 100644 --- a/src/matrix/e2ee/megolm/Decryption.js +++ b/src/matrix/e2ee/megolm/Decryption.js @@ -15,25 +15,13 @@ limitations under the License. */ import {DecryptionError} from "../common.js"; -import {groupBy} from "../../../utils/groupBy.js"; import * as RoomKey from "./decryption/RoomKey.js"; import {SessionInfo} from "./decryption/SessionInfo.js"; import {DecryptionPreparation} from "./decryption/DecryptionPreparation.js"; import {SessionDecryption} from "./decryption/SessionDecryption.js"; import {SessionCache} from "./decryption/SessionCache.js"; import {MEGOLM_ALGORITHM} from "../common.js"; - -function getSenderKey(event) { - return event.content?.["sender_key"]; -} - -function getSessionId(event) { - return event.content?.["session_id"]; -} - -function getCiphertext(event) { - return event.content?.ciphertext; -} +import {validateEvent, groupEventsBySession} from "./decryption/utils.js"; export class Decryption { constructor({pickleKey, olm, olmWorker}) { @@ -46,6 +34,37 @@ export class Decryption { return new SessionCache(size); } + async addMissingKeyEventIds(roomId, senderKey, sessionId, eventIds, txn) { + let sessionEntry = await txn.inboundGroupSessions.get(roomId, senderKey, sessionId); + // we never want to overwrite an existing key + if (sessionEntry?.session) { + return; + } + if (sessionEntry) { + const uniqueEventIds = new Set(sessionEntry.eventIds); + for (const id of eventIds) { + uniqueEventIds.add(id); + } + sessionEntry.eventIds = Array.from(uniqueEventIds); + } else { + sessionEntry = {roomId, senderKey, sessionId, eventIds}; + } + txn.inboundGroupSessions.set(sessionEntry); + } + + async getEventIdsForMissingKey(roomId, senderKey, sessionId, txn) { + const sessionEntry = await txn.inboundGroupSessions.get(roomId, senderKey, sessionId); + if (sessionEntry && !sessionEntry.session) { + return sessionEntry.eventIds; + } + } + + async hasSession(roomId, senderKey, sessionId, txn) { + const sessionEntry = await txn.inboundGroupSessions.get(roomId, senderKey, sessionId); + const isValidSession = typeof sessionEntry?.session === "string"; + return isValidSession; + } + /** * Reads all the state from storage to be able to decrypt the given events. * Decryption can then happen outside of a storage transaction. @@ -61,28 +80,22 @@ export class Decryption { const validEvents = []; for (const event of events) { - const isValid = typeof getSenderKey(event) === "string" && - typeof getSessionId(event) === "string" && - typeof getCiphertext(event) === "string"; - if (isValid) { + if (validateEvent(event)) { validEvents.push(event); } else { errors.set(event.event_id, new DecryptionError("MEGOLM_INVALID_EVENT", event)) } } - const eventsBySession = groupBy(validEvents, event => { - return `${getSenderKey(event)}|${getSessionId(event)}`; - }); + const eventsBySession = groupEventsBySession(validEvents); const sessionDecryptions = []; - await Promise.all(Array.from(eventsBySession.values()).map(async eventsForSession => { - const firstEvent = eventsForSession[0]; - const sessionInfo = await this._getSessionInfoForEvent(roomId, firstEvent, newKeys, sessionCache, txn); + await Promise.all(Array.from(eventsBySession.values()).map(async group => { + const sessionInfo = await this._getSessionInfo(roomId, group.senderKey, group.sessionId, newKeys, sessionCache, txn); if (sessionInfo) { - sessionDecryptions.push(new SessionDecryption(sessionInfo, eventsForSession, this._olmWorker)); + sessionDecryptions.push(new SessionDecryption(sessionInfo, group.events, this._olmWorker)); } else { - for (const event of eventsForSession) { + for (const event of group.events) { errors.set(event.event_id, new DecryptionError("MEGOLM_NO_SESSION", event)); } } @@ -91,9 +104,7 @@ export class Decryption { return new DecryptionPreparation(roomId, sessionDecryptions, errors); } - async _getSessionInfoForEvent(roomId, event, newKeys, sessionCache, txn) { - const senderKey = getSenderKey(event); - const sessionId = getSessionId(event); + async _getSessionInfo(roomId, senderKey, sessionId, newKeys, sessionCache, txn) { let sessionInfo; if (newKeys) { const key = newKeys.find(k => k.roomId === roomId && k.senderKey === senderKey && k.sessionId === sessionId); @@ -110,7 +121,7 @@ export class Decryption { } if (!sessionInfo) { const sessionEntry = await txn.inboundGroupSessions.get(roomId, senderKey, sessionId); - if (sessionEntry) { + if (sessionEntry && sessionEntry.session) { let session = new this._olm.InboundGroupSession(); try { session.unpickle(this._pickleKey, sessionEntry.session); diff --git a/src/matrix/e2ee/megolm/decryption/RoomKey.js b/src/matrix/e2ee/megolm/decryption/RoomKey.js index e206003a..1880961b 100644 --- a/src/matrix/e2ee/megolm/decryption/RoomKey.js +++ b/src/matrix/e2ee/megolm/decryption/RoomKey.js @@ -33,7 +33,7 @@ export class BaseRoomKey { async _isBetterThanKnown(session, olm, pickleKey, txn) { let isBetter = true; const existingSessionEntry = await txn.inboundGroupSessions.get(this.roomId, this.senderKey, this.sessionId); - if (existingSessionEntry) { + if (existingSessionEntry?.session) { const existingSession = new olm.InboundGroupSession(); try { existingSession.unpickle(pickleKey, existingSessionEntry.session); diff --git a/src/matrix/e2ee/megolm/decryption/utils.js b/src/matrix/e2ee/megolm/decryption/utils.js new file mode 100644 index 00000000..c38b1416 --- /dev/null +++ b/src/matrix/e2ee/megolm/decryption/utils.js @@ -0,0 +1,57 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {groupByWithCreator} from "../../../../utils/groupBy.js"; + +function getSenderKey(event) { + return event.content?.["sender_key"]; +} + +function getSessionId(event) { + return event.content?.["session_id"]; +} + +function getCiphertext(event) { + return event.content?.ciphertext; +} + +export function validateEvent(event) { + return typeof getSenderKey(event) === "string" && + typeof getSessionId(event) === "string" && + typeof getCiphertext(event) === "string"; +} + +class SessionKeyGroup { + constructor() { + this.events = []; + } + + get senderKey() { + return getSenderKey(this.events[0]); + } + + get sessionId() { + return getSessionId(this.events[0]); + } +} + +export function groupEventsBySession(events) { + return groupByWithCreator(events, + event => `${getSenderKey(event)}|${getSessionId(event)}`, + () => new SessionKeyGroup(), + (group, event) => group.events.push(event) + ); +} diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 442da4fb..27fd32f9 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -58,56 +58,36 @@ export class Room extends EventEmitter { this._observedEvents = null; } - _readRetryDecryptCandidateEntries(sinceEventKey, txn) { - if (sinceEventKey) { - return readRawTimelineEntriesWithTxn(this._roomId, sinceEventKey, - Direction.Forward, Number.MAX_SAFE_INTEGER, this._fragmentIdComparer, txn); - } else { - // all messages for room ... - // if you haven't decrypted any message in a room yet, - // it's unlikely you will have tons of them. - // so this should be fine as a last resort - return readRawTimelineEntriesWithTxn(this._roomId, this._syncWriter.lastMessageKey, - Direction.Backward, Number.MAX_SAFE_INTEGER, this._fragmentIdComparer, txn); - } - } - - async notifyRoomKey(roomKey) { - if (!this._roomEncryption) { - return; - } - const retryEventIds = this._roomEncryption.getEventIdsForRoomKey(roomKey); - const stores = [ - this._storage.storeNames.timelineEvents, - this._storage.storeNames.inboundGroupSessions, - ]; - let txn; - let retryEntries; + async _getRetryDecryptEntriesForKey(roomKey, txn) { + const retryEventIds = await this._roomEncryption.getEventIdsForMissingKey(roomKey, txn); + const retryEntries = []; if (retryEventIds) { - retryEntries = []; - txn = this._storage.readTxn(stores); for (const eventId of retryEventIds) { const storageEntry = await txn.timelineEvents.getByEventId(this._roomId, eventId); if (storageEntry) { retryEntries.push(new EventEntry(storageEntry, this._fragmentIdComparer)); } } - } else { - // we only look for messages since lastDecryptedEventKey because - // the timeline must be closed (otherwise getEventIdsForRoomKey would have found the event ids) - // and to update the summary we only care about events since lastDecryptedEventKey - const key = this._summary.data.lastDecryptedEventKey; - // key might be missing if we haven't decrypted any events in this room - const sinceEventKey = key && new EventKey(key.fragmentId, key.entryIndex); - // check we have not already decrypted the most recent event in the room - // otherwise we know that the messages for this room key will not update the room summary - if (!sinceEventKey || !sinceEventKey.equals(this._syncWriter.lastMessageKey)) { - txn = this._storage.readTxn(stores.concat(this._storage.storeNames.timelineFragments)); - const candidateEntries = await this._readRetryDecryptCandidateEntries(sinceEventKey, txn); - retryEntries = this._roomEncryption.findAndCacheEntriesForRoomKey(roomKey, candidateEntries); - } } - if (retryEntries?.length) { + return retryEntries; + } + + /** + * Used for keys received from other sources than sync, like key backup. + * @internal + * @param {RoomKey} roomKey + * @return {Promise} + */ + async notifyRoomKey(roomKey) { + if (!this._roomEncryption) { + return; + } + const txn = this._storage.readTxn([ + this._storage.storeNames.timelineEvents, + this._storage.storeNames.inboundGroupSessions, + ]); + const retryEntries = this._getRetryDecryptEntriesForKey(roomKey, txn); + if (retryEntries.length) { const decryptRequest = this._decryptEntries(DecryptionSource.Retry, retryEntries, txn); // this will close txn while awaiting decryption await decryptRequest.complete(); @@ -197,11 +177,10 @@ export class Room extends EventEmitter { if (roomEncryption) { // also look for events in timeline here let events = roomResponse?.timeline?.events || []; - // when new keys arrive, also see if any events currently loaded in the timeline - // can now be retried to decrypt - if (this._timeline && newKeys) { - retryEntries = roomEncryption.filterEventEntriesForKeys( - this._timeline.remoteEntries, newKeys); + // when new keys arrive, also see if any events that can now be retried to decrypt + if (newKeys) { + const nestedEntries = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); + const retryEntries = nestedEntries.reduce((allEntries, entries) => allEntries.concat(entries), []); if (retryEntries.length) { log.set("retry", retryEntries.length); events = events.concat(retryEntries.map(entry => entry.event)); @@ -228,7 +207,7 @@ export class Room extends EventEmitter { async afterPrepareSync(preparation, parentLog) { if (preparation.decryptPreparation) { - await parentLog.wrap("afterPrepareSync decrypt", async log => { + await parentLog.wrap("decrypt", async log => { log.set("id", this.id); preparation.decryptChanges = await preparation.decryptPreparation.decrypt(); preparation.decryptPreparation = null; @@ -247,10 +226,8 @@ export class Room extends EventEmitter { await decryption.verifySenders(txn); } if (retryEntries?.length) { - // TODO: this will modify existing timeline entries (which we should not do in writeSync), - // but it is a temporary way of reattempting decryption while timeline is open - // won't need copies when tracking missing sessions properly - // prepend the retried entries, as we know they are older (not that it should matter much for the summary) + // prepend the retried entries, as we know they are older + // (not that it should matter much for the summary) entries.unshift(...retryEntries); } decryption.applyToEntries(entries); @@ -554,6 +531,10 @@ export class Room extends EventEmitter { enableSessionBackup(sessionBackup) { this._roomEncryption?.enableSessionBackup(sessionBackup); + if (this._timeline) { + const timelineEvents = this._timeline.remoteEntries.filter(e => e.event).map(e => e.event); + this._roomEncryption.restoreMissingSessionsFromBackup(timelineEvents); + } } get isTrackingMembers() { diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 39b1d1b3..2f1e7703 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -122,36 +122,6 @@ function processTimelineEvent(data, eventEntry, isInitialSync, canMarkUnread, ow data = data.cloneIfNeeded(); data.isUnread = true; } - const {content} = eventEntry; - const body = content?.body; - const msgtype = content?.msgtype; - if (msgtype === "m.text" && !eventEntry.isEncrypted) { - data = data.cloneIfNeeded(); - data.lastMessageBody = body; - } - } - // store the event key of the last decrypted event so when decryption does succeed, - // we can attempt to re-decrypt from this point to update the room summary - if (!!data.encryption && eventEntry.isEncrypted && eventEntry.isDecrypted) { - let hasLargerEventKey = true; - if (data.lastDecryptedEventKey) { - try { - hasLargerEventKey = eventEntry.compare(data.lastDecryptedEventKey) > 0; - } catch (err) { - // TODO: load the fragments in between here? - // this could happen if an earlier event gets decrypted that - // is in a fragment different from the live one and the timeline is not open. - // In this case, we will just read too many events once per app load - // and then keep the mapping in memory. When eventually an event is decrypted in - // the live fragment, this should stop failing and the event key will be written. - hasLargerEventKey = false; - } - } - if (hasLargerEventKey) { - data = data.cloneIfNeeded(); - const {fragmentId, entryIndex} = eventEntry; - data.lastDecryptedEventKey = {fragmentId, entryIndex}; - } } return data; } @@ -182,12 +152,9 @@ class SummaryData { constructor(copy, roomId) { this.roomId = copy ? copy.roomId : roomId; this.name = copy ? copy.name : null; - this.lastMessageBody = copy ? copy.lastMessageBody : null; this.lastMessageTimestamp = copy ? copy.lastMessageTimestamp : null; this.isUnread = copy ? copy.isUnread : false; this.encryption = copy ? copy.encryption : null; - this.lastDecryptedEventKey = copy ? copy.lastDecryptedEventKey : null; - this.isDirectMessage = copy ? copy.isDirectMessage : false; this.membership = copy ? copy.membership : null; this.inviteCount = copy ? copy.inviteCount : 0; this.joinCount = copy ? copy.joinCount : 0; From e29bc6710ae533dcb695f655848ddedabadd9245 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 19:13:52 +0100 Subject: [PATCH 03/16] bring back missing import --- src/matrix/e2ee/RoomEncryption.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index dbfa19de..07e62749 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -17,6 +17,7 @@ limitations under the License. import {MEGOLM_ALGORITHM, DecryptionSource} from "./common.js"; import {groupEventsBySession} from "./megolm/decryption/utils.js"; import {mergeMap} from "../../utils/mergeMap.js"; +import {groupBy} from "../../utils/groupBy.js"; import {makeTxnId} from "../common.js"; const ENCRYPTED_TYPE = "m.room.encrypted"; From 56db210763bfd5e6ce33d9b6bfa4a92fa0829b1e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 19:14:29 +0100 Subject: [PATCH 04/16] attempt at fixing https://github.com/vector-im/hydrogen-web/issues/245 --- src/matrix/e2ee/RoomEncryption.js | 6 +++++- src/matrix/room/Room.js | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 07e62749..3cf1310d 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -80,14 +80,16 @@ export class RoomEncryption { } async writeMemberChanges(memberChanges, txn) { + let shouldFlush; const memberChangesArray = Array.from(memberChanges.values()); if (memberChangesArray.some(m => m.hasLeft)) { this._megolmEncryption.discardOutboundSession(this._room.id, txn); } if (memberChangesArray.some(m => m.hasJoined)) { - await this._addShareRoomKeyOperationForNewMembers(memberChangesArray, txn); + shouldFlush = await this._addShareRoomKeyOperationForNewMembers(memberChangesArray, txn); } await this._deviceTracker.writeMemberChanges(this._room, memberChanges, txn); + return shouldFlush; } // this happens before entries exists, as they are created by the syncwriter @@ -314,7 +316,9 @@ export class RoomEncryption { this._room.id, txn); if (roomKeyMessage) { this._writeRoomKeyShareOperation(roomKeyMessage, userIds, txn); + return true; } + return false; } _writeRoomKeyShareOperation(roomKeyMessage, userIds, txn) { diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 27fd32f9..198652c7 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -232,9 +232,11 @@ export class Room extends EventEmitter { } decryption.applyToEntries(entries); } + let shouldFlushKeyShares = false; // pass member changes to device tracker if (roomEncryption && this.isTrackingMembers && memberChanges?.size) { - await roomEncryption.writeMemberChanges(memberChanges, txn); + shouldFlushKeyShares = await roomEncryption.writeMemberChanges(memberChanges, txn); + log.set("shouldFlushKeyShares", shouldFlushKeyShares); } // also apply (decrypted) timeline entries to the summary changes summaryChanges = summaryChanges.applyTimelineEntries( @@ -263,6 +265,7 @@ export class Room extends EventEmitter { removedPendingEvents, memberChanges, heroChanges, + shouldFlushKeyShares, }; } @@ -314,8 +317,8 @@ export class Room extends EventEmitter { } } - needsAfterSyncCompleted({memberChanges}) { - return this._roomEncryption?.needsToShareKeys(memberChanges); + needsAfterSyncCompleted({shouldFlushKeyShares}) { + return shouldFlushKeyShares; } /** From e85844f482c786c9a284638005de65f459cb8f3c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 19:16:39 +0100 Subject: [PATCH 05/16] don't redeclare retryEntries, or they won't get passed to writeSync --- src/matrix/room/Room.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 198652c7..dc6549a9 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -180,7 +180,7 @@ export class Room extends EventEmitter { // when new keys arrive, also see if any events that can now be retried to decrypt if (newKeys) { const nestedEntries = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); - const retryEntries = nestedEntries.reduce((allEntries, entries) => allEntries.concat(entries), []); + retryEntries = nestedEntries.reduce((allEntries, entries) => allEntries.concat(entries), []); if (retryEntries.length) { log.set("retry", retryEntries.length); events = events.concat(retryEntries.map(entry => entry.event)); From 8d7cb2a39a42df0defb1e8d7ccf79dfcc2094de3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 21:28:55 +0100 Subject: [PATCH 06/16] remove unused imports --- src/matrix/room/Room.js | 3 --- src/matrix/room/timeline/persistence/TimelineReader.js | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index dc6549a9..0f4d80ab 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -18,7 +18,6 @@ import {EventEmitter} from "../../utils/EventEmitter.js"; import {RoomSummary} from "./RoomSummary.js"; import {SyncWriter} from "./timeline/persistence/SyncWriter.js"; import {GapWriter} from "./timeline/persistence/GapWriter.js"; -import {readRawTimelineEntriesWithTxn} from "./timeline/persistence/TimelineReader.js"; import {Timeline} from "./timeline/Timeline.js"; import {FragmentIdComparer} from "./timeline/FragmentIdComparer.js"; import {SendQueue} from "./sending/SendQueue.js"; @@ -27,8 +26,6 @@ import {fetchOrLoadMembers} from "./members/load.js"; import {MemberList} from "./members/MemberList.js"; import {Heroes} from "./members/Heroes.js"; import {EventEntry} from "./timeline/entries/EventEntry.js"; -import {EventKey} from "./timeline/EventKey.js"; -import {Direction} from "./timeline/Direction.js"; import {ObservedEventMap} from "./ObservedEventMap.js"; import {AttachmentUpload} from "./AttachmentUpload.js"; import {DecryptionSource} from "../e2ee/common.js"; diff --git a/src/matrix/room/timeline/persistence/TimelineReader.js b/src/matrix/room/timeline/persistence/TimelineReader.js index 24ad4127..7493727c 100644 --- a/src/matrix/room/timeline/persistence/TimelineReader.js +++ b/src/matrix/room/timeline/persistence/TimelineReader.js @@ -41,7 +41,7 @@ class ReaderRequest { * Raw because it doesn't do decryption and in the future it should not read relations either. * It is just about reading entries and following fragment links */ -export async function readRawTimelineEntriesWithTxn(roomId, eventKey, direction, amount, fragmentIdComparer, txn) { +async function readRawTimelineEntriesWithTxn(roomId, eventKey, direction, amount, fragmentIdComparer, txn) { let entries = []; const timelineStore = txn.timelineEvents; const fragmentStore = txn.timelineFragments; From 9702c4fd64768e8c4fe0c8b8b7bb5dd6198d7f8c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 21:29:32 +0100 Subject: [PATCH 07/16] more logging in sync write --- src/matrix/room/Room.js | 6 ++++++ src/matrix/room/RoomSummary.js | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 0f4d80ab..392ef264 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -219,6 +219,8 @@ export class Room extends EventEmitter { await log.wrap("syncWriter", log => this._syncWriter.writeSync(roomResponse, txn, log), log.level.Detail); if (decryptChanges) { const decryption = await decryptChanges.write(txn); + log.set("decryptionResults", decryption.results.size); + log.set("decryptionErrors", decryption.errors.size); if (this._isTimelineOpen) { await decryption.verifySenders(txn); } @@ -229,6 +231,7 @@ export class Room extends EventEmitter { } decryption.applyToEntries(entries); } + log.set("entries", entries.length); let shouldFlushKeyShares = false; // pass member changes to device tracker if (roomEncryption && this.isTrackingMembers && memberChanges?.size) { @@ -240,6 +243,9 @@ export class Room extends EventEmitter { entries, isInitialSync, !this._isTimelineOpen, this._user.id); // write summary changes, and unset if nothing was actually changed summaryChanges = this._summary.writeData(summaryChanges, txn); + if (summaryChanges) { + log.set("summaryChanges", summaryChanges.diff(this._summary.data)); + } // fetch new members while we have txn open, // but don't make any in-memory changes yet let heroChanges; diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 2f1e7703..8c7c8576 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -169,6 +169,18 @@ class SummaryData { this.cloned = copy ? true : false; } + diff(other) { + const props = Object.getOwnPropertyNames(this); + return props.reduce((diff, prop) => { + if (prop !== "cloned") { + if (this[prop] !== other[prop]) { + diff[prop] = this[prop]; + } + } + return diff; + }, {}); + } + cloneIfNeeded() { if (this.cloned) { return this; From 3bfe52b1dcb2d4e66209064da76429d7dd54f382 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 21:29:43 +0100 Subject: [PATCH 08/16] filter encrypted events before deciding to decrypt or not --- src/matrix/room/Room.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 392ef264..4ab04314 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -183,11 +183,10 @@ export class Room extends EventEmitter { events = events.concat(retryEntries.map(entry => entry.event)); } } - - if (events.length) { - const eventsToDecrypt = events.filter(event => { - return event?.type === EVENT_ENCRYPTED_TYPE; - }); + const eventsToDecrypt = events.filter(event => { + return event?.type === EVENT_ENCRYPTED_TYPE; + }); + if (eventsToDecrypt.length) { decryptPreparation = await roomEncryption.prepareDecryptAll( eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); } From 367157454799b50187625779119bd5b90fcb810f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 21:30:00 +0100 Subject: [PATCH 09/16] comment about session backup --- src/matrix/room/Room.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 4ab04314..a5781de0 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -536,6 +536,7 @@ export class Room extends EventEmitter { enableSessionBackup(sessionBackup) { this._roomEncryption?.enableSessionBackup(sessionBackup); + // TODO: do we really want to do this every time you open the app? if (this._timeline) { const timelineEvents = this._timeline.remoteEntries.filter(e => e.event).map(e => e.event); this._roomEncryption.restoreMissingSessionsFromBackup(timelineEvents); From 7f1cdf68419c5f6c74ad778b1065d0b5c42a06d3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 2 Mar 2021 23:20:33 +0100 Subject: [PATCH 10/16] also decrypt all matching timeline entries when new key --- src/matrix/room/Room.js | 16 ++++++++++++++-- src/matrix/room/timeline/entries/EventEntry.js | 7 +++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index a5781de0..e8fda52e 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -176,8 +176,20 @@ export class Room extends EventEmitter { let events = roomResponse?.timeline?.events || []; // when new keys arrive, also see if any events that can now be retried to decrypt if (newKeys) { - const nestedEntries = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); - retryEntries = nestedEntries.reduce((allEntries, entries) => allEntries.concat(entries), []); + const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); + retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); + // If we have the timeline open, see if there are more entries for the new keys + // as we only store missing session for synced and not backfilled events. + // We want to decrypt all events we can though if the user is looking + // at them given the timeline is open + if (this._timeline) { + let retryTimelineEntries = this._roomEncryption.filterEventEntriesForKeys(this._timeline.remoteEntries, newKeys); + // filter out any entries already in retryEntries so we don't decrypt them twice + const existingIds = new Set(retryEntries.map(e => e.id)); + retryTimelineEntries = retryTimelineEntries.filter(e => !existingIds.has(e.id)) + // make copies so we don't modify the original entry before the afterSync stage + retryEntries = retryTimelineEntries.map(e => e.clone()); + } if (retryEntries.length) { log.set("retry", retryEntries.length); events = events.concat(retryEntries.map(entry => entry.event)); diff --git a/src/matrix/room/timeline/entries/EventEntry.js b/src/matrix/room/timeline/entries/EventEntry.js index 410c3d63..88a0aa5e 100644 --- a/src/matrix/room/timeline/entries/EventEntry.js +++ b/src/matrix/room/timeline/entries/EventEntry.js @@ -25,6 +25,13 @@ export class EventEntry extends BaseEntry { this._decryptionResult = null; } + clone() { + const clone = new EventEntry(this._eventEntry, this._fragmentIdComparer); + clone._decryptionResult = this._decryptionResult; + clone._decryptionError = this._decryptionError; + return clone; + } + get event() { return this._eventEntry.event; } From 43547e0901e1c7e94e83c2ac54370e4b7e19309b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 11:27:18 +0100 Subject: [PATCH 11/16] don't add retried entries to the timeline if they are not already there --- src/matrix/room/Room.js | 33 ++++++++++++++++++---------- src/matrix/room/timeline/Timeline.js | 10 +-------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index e8fda52e..83dd5bf9 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -226,8 +226,9 @@ export class Room extends EventEmitter { /** @package */ async writeSync(roomResponse, isInitialSync, {summaryChanges, decryptChanges, roomEncryption, retryEntries}, txn, log) { log.set("id", this.id); - const {entries, newLiveKey, memberChanges} = + const {entries: newEntries, newLiveKey, memberChanges} = await log.wrap("syncWriter", log => this._syncWriter.writeSync(roomResponse, txn, log), log.level.Detail); + let allEntries = newEntries; if (decryptChanges) { const decryption = await decryptChanges.write(txn); log.set("decryptionResults", decryption.results.size); @@ -235,14 +236,13 @@ export class Room extends EventEmitter { if (this._isTimelineOpen) { await decryption.verifySenders(txn); } + decryption.applyToEntries(newEntries); if (retryEntries?.length) { - // prepend the retried entries, as we know they are older - // (not that it should matter much for the summary) - entries.unshift(...retryEntries); + decryption.applyToEntries(retryEntries); + allEntries = retryEntries.concat(allEntries); } - decryption.applyToEntries(entries); } - log.set("entries", entries.length); + log.set("allEntries", allEntries.length); let shouldFlushKeyShares = false; // pass member changes to device tracker if (roomEncryption && this.isTrackingMembers && memberChanges?.size) { @@ -251,7 +251,7 @@ export class Room extends EventEmitter { } // also apply (decrypted) timeline entries to the summary changes summaryChanges = summaryChanges.applyTimelineEntries( - entries, isInitialSync, !this._isTimelineOpen, this._user.id); + allEntries, isInitialSync, !this._isTimelineOpen, this._user.id); // write summary changes, and unset if nothing was actually changed summaryChanges = this._summary.writeData(summaryChanges, txn); if (summaryChanges) { @@ -274,7 +274,8 @@ export class Room extends EventEmitter { return { summaryChanges, roomEncryption, - newAndUpdatedEntries: entries, + newEntries, + updatedEntries: retryEntries || [], newLiveKey, removedPendingEvents, memberChanges, @@ -288,7 +289,12 @@ export class Room extends EventEmitter { * Called with the changes returned from `writeSync` to apply them and emit changes. * No storage or network operations should be done here. */ - afterSync({summaryChanges, newAndUpdatedEntries, newLiveKey, removedPendingEvents, memberChanges, heroChanges, roomEncryption}, log) { + afterSync(changes, log) { + const { + summaryChanges, newEntries, updatedEntries, newLiveKey, + removedPendingEvents, memberChanges, + heroChanges, roomEncryption + } = changes; log.set("id", this.id); this._syncWriter.afterSync(newLiveKey); this._setEncryption(roomEncryption); @@ -321,10 +327,13 @@ export class Room extends EventEmitter { this._emitUpdate(); } if (this._timeline) { - this._timeline.appendLiveEntries(newAndUpdatedEntries); + // these should not be added if not already there + this._timeline.replaceEntries(updatedEntries); + this._timeline.addOrReplaceEntries(newEntries); } if (this._observedEvents) { - this._observedEvents.updateEvents(newAndUpdatedEntries); + this._observedEvents.updateEvents(updatedEntries); + this._observedEvents.updateEvents(newEntries); } if (removedPendingEvents) { this._sendQueue.emitRemovals(removedPendingEvents); @@ -483,7 +492,7 @@ export class Room extends EventEmitter { this._sendQueue.emitRemovals(removedPendingEvents); } if (this._timeline) { - this._timeline.addGapEntries(gapResult.entries); + this._timeline.addOrReplaceEntries(gapResult.entries); } }); } diff --git a/src/matrix/room/timeline/Timeline.js b/src/matrix/room/timeline/Timeline.js index e5592526..7e2d2110 100644 --- a/src/matrix/room/timeline/Timeline.js +++ b/src/matrix/room/timeline/Timeline.js @@ -60,16 +60,8 @@ export class Timeline { } } - // TODO: should we rather have generic methods for - // - adding new entries - // - updating existing entries (redaction, relations) /** @package */ - appendLiveEntries(newEntries) { - this._remoteEntries.setManySorted(newEntries); - } - - /** @package */ - addGapEntries(newEntries) { + addOrReplaceEntries(newEntries) { this._remoteEntries.setManySorted(newEntries); } From f3c49e51f2081ac6e8bb4074014149fd707a7fa2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 11:27:55 +0100 Subject: [PATCH 12/16] add, don't replace timeline retry entries also, filter out any that have been decrypted already --- src/matrix/e2ee/RoomEncryption.js | 14 ++++--- src/matrix/room/Room.js | 70 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 3cf1310d..9a6a5fc3 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -390,13 +390,15 @@ export class RoomEncryption { await hsApi.sendToDevice(type, payload, txnId, {log}).response(); } - filterEventEntriesForKeys(entries, keys) { + filterUndecryptedEventEntriesForKeys(entries, keys) { return entries.filter(entry => { - const {event} = entry; - if (event) { - const senderKey = event.content?.["sender_key"]; - const sessionId = event.content?.["session_id"]; - return keys.some(key => senderKey === key.senderKey && sessionId === key.sessionId); + if (entry.isEncrypted && !entry.isDecrypted) { + const {event} = entry; + if (event) { + const senderKey = event.content?.["sender_key"]; + const sessionId = event.content?.["session_id"]; + return keys.some(key => senderKey === key.senderKey && sessionId === key.sessionId); + } } return false; }); diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 83dd5bf9..6f8c7530 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -156,6 +156,42 @@ export class Room extends EventEmitter { return request; } + async _prepareSyncDecryption(events, newKeys, roomEncryption, txn, log) { + let retryEntries; + let decryptPreparation; + // when new keys arrive, also see if any events that can now be retried to decrypt + if (newKeys) { + const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); + retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); + // If we have the timeline open, see if there are more entries for the new keys + // as we only store missing session information for synced events, not backfilled. + // We want to decrypt all events we can though if the user is looking + // at them when the timeline is open + if (this._timeline) { + let retryTimelineEntries = this._roomEncryption.filterUndecryptedEventEntriesForKeys(this._timeline.remoteEntries, newKeys); + // filter out any entries already in retryEntries so we don't decrypt them twice + const existingIds = retryEntries.reduce((ids, e) => {ids.add(e.id); return ids;}, new Set()); + retryTimelineEntries = retryTimelineEntries.filter(e => !existingIds.has(e.id)); + // make copies so we don't modify the original entry in writeSync, before the afterSync stage + const retryTimelineEntriesCopies = retryTimelineEntries.map(e => e.clone()); + // add to other retry entries + retryEntries = retryEntries.concat(retryTimelineEntriesCopies); + } + if (retryEntries.length) { + log.set("retry", retryEntries.length); + events = events.concat(retryEntries.map(entry => entry.event)); + } + } + const eventsToDecrypt = events.filter(event => { + return event?.type === EVENT_ENCRYPTED_TYPE; + }); + if (eventsToDecrypt.length) { + decryptPreparation = await roomEncryption.prepareDecryptAll( + eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); + } + return {retryEntries, decryptPreparation}; + } + async prepareSync(roomResponse, membership, newKeys, txn, log) { log.set("id", this.id); if (newKeys) { @@ -172,36 +208,10 @@ export class Room extends EventEmitter { let retryEntries; let decryptPreparation; if (roomEncryption) { - // also look for events in timeline here - let events = roomResponse?.timeline?.events || []; - // when new keys arrive, also see if any events that can now be retried to decrypt - if (newKeys) { - const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); - retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); - // If we have the timeline open, see if there are more entries for the new keys - // as we only store missing session for synced and not backfilled events. - // We want to decrypt all events we can though if the user is looking - // at them given the timeline is open - if (this._timeline) { - let retryTimelineEntries = this._roomEncryption.filterEventEntriesForKeys(this._timeline.remoteEntries, newKeys); - // filter out any entries already in retryEntries so we don't decrypt them twice - const existingIds = new Set(retryEntries.map(e => e.id)); - retryTimelineEntries = retryTimelineEntries.filter(e => !existingIds.has(e.id)) - // make copies so we don't modify the original entry before the afterSync stage - retryEntries = retryTimelineEntries.map(e => e.clone()); - } - if (retryEntries.length) { - log.set("retry", retryEntries.length); - events = events.concat(retryEntries.map(entry => entry.event)); - } - } - const eventsToDecrypt = events.filter(event => { - return event?.type === EVENT_ENCRYPTED_TYPE; - }); - if (eventsToDecrypt.length) { - decryptPreparation = await roomEncryption.prepareDecryptAll( - eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); - } + const events = roomResponse?.timeline?.events || []; + const result = await this._prepareSyncDecryption(events, newKeys, roomEncryption, txn, log); + retryEntries = result.retryEntries; + decryptPreparation = result.decryptPreparation; } return { From 30481a5a9e3e42cde1cb9d999d4038da646404b4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 11:51:50 +0100 Subject: [PATCH 13/16] add logging to key sharing and discarding in reaction to member changes --- src/matrix/e2ee/RoomEncryption.js | 15 ++++++++++++--- src/matrix/room/Room.js | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 9a6a5fc3..59994c0f 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -79,14 +79,18 @@ export class RoomEncryption { this._senderDeviceCache = new Map(); // purge the sender device cache } - async writeMemberChanges(memberChanges, txn) { + async writeMemberChanges(memberChanges, txn, log) { let shouldFlush; const memberChangesArray = Array.from(memberChanges.values()); if (memberChangesArray.some(m => m.hasLeft)) { + log.log({ + l: "discardOutboundSession", + leftUsers: memberChangesArray.filter(m => m.hasLeft).map(m => m.userId), + }); this._megolmEncryption.discardOutboundSession(this._room.id, txn); } if (memberChangesArray.some(m => m.hasJoined)) { - shouldFlush = await this._addShareRoomKeyOperationForNewMembers(memberChangesArray, txn); + shouldFlush = await this._addShareRoomKeyOperationForNewMembers(memberChangesArray, txn, log); } await this._deviceTracker.writeMemberChanges(this._room, memberChanges, txn); return shouldFlush; @@ -310,11 +314,16 @@ export class RoomEncryption { await removeOpTxn.complete(); } - async _addShareRoomKeyOperationForNewMembers(memberChangesArray, txn) { + async _addShareRoomKeyOperationForNewMembers(memberChangesArray, txn, log) { const userIds = memberChangesArray.filter(m => m.hasJoined).map(m => m.userId); const roomKeyMessage = await this._megolmEncryption.createRoomKeyMessage( this._room.id, txn); if (roomKeyMessage) { + log.log({ + l: "share key for new members", userIds, + id: roomKeyMessage.session_id, + chain_index: roomKeyMessage.chain_index + }); this._writeRoomKeyShareOperation(roomKeyMessage, userIds, txn); return true; } diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 6f8c7530..cc325aa4 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -256,7 +256,7 @@ export class Room extends EventEmitter { let shouldFlushKeyShares = false; // pass member changes to device tracker if (roomEncryption && this.isTrackingMembers && memberChanges?.size) { - shouldFlushKeyShares = await roomEncryption.writeMemberChanges(memberChanges, txn); + shouldFlushKeyShares = await roomEncryption.writeMemberChanges(memberChanges, txn, log); log.set("shouldFlushKeyShares", shouldFlushKeyShares); } // also apply (decrypted) timeline entries to the summary changes From e7598b9c762d7e316c10fa0f5cc7b1571e990d8f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 12:52:43 +0100 Subject: [PATCH 14/16] only request key backup for events that are UTD --- src/matrix/e2ee/RoomEncryption.js | 3 ++- src/matrix/room/Room.js | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 59994c0f..f7a1920e 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -55,7 +55,8 @@ export class RoomEncryption { this._sessionBackup = sessionBackup; } - async restoreMissingSessionsFromBackup(events) { + async restoreMissingSessionsFromBackup(entries) { + const events = entries.filter(e => e.isEncrypted && !e.isDecrypted && e.event).map(e => e.event); const eventsBySession = groupEventsBySession(events); const groups = Array.from(eventsBySession.values()); const txn = this._storage.readTxn([this._storage.storeNames.inboundGroupSessions]); diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index cc325aa4..c9f577cb 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -569,8 +569,7 @@ export class Room extends EventEmitter { this._roomEncryption?.enableSessionBackup(sessionBackup); // TODO: do we really want to do this every time you open the app? if (this._timeline) { - const timelineEvents = this._timeline.remoteEntries.filter(e => e.event).map(e => e.event); - this._roomEncryption.restoreMissingSessionsFromBackup(timelineEvents); + this._roomEncryption.restoreMissingSessionsFromBackup(this._timeline.remoteEntries); } } From 404dbcd0656ba8c974e764621326f89a46837d10 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 13:31:42 +0100 Subject: [PATCH 15/16] english --- src/matrix/room/Room.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index c9f577cb..454b935e 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -159,7 +159,7 @@ export class Room extends EventEmitter { async _prepareSyncDecryption(events, newKeys, roomEncryption, txn, log) { let retryEntries; let decryptPreparation; - // when new keys arrive, also see if any events that can now be retried to decrypt + // when new keys arrive, also see if any events can now be retried to decrypt if (newKeys) { const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); From 6771303086cc65f35cdc9c476906a960ee09fffe Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 3 Mar 2021 13:53:52 +0100 Subject: [PATCH 16/16] make a method to determine only the retry entries rather --- src/matrix/room/Room.js | 70 +++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 454b935e..89653b62 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -156,40 +156,24 @@ export class Room extends EventEmitter { return request; } - async _prepareSyncDecryption(events, newKeys, roomEncryption, txn, log) { - let retryEntries; - let decryptPreparation; - // when new keys arrive, also see if any events can now be retried to decrypt - if (newKeys) { - const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); - retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); - // If we have the timeline open, see if there are more entries for the new keys - // as we only store missing session information for synced events, not backfilled. - // We want to decrypt all events we can though if the user is looking - // at them when the timeline is open - if (this._timeline) { - let retryTimelineEntries = this._roomEncryption.filterUndecryptedEventEntriesForKeys(this._timeline.remoteEntries, newKeys); - // filter out any entries already in retryEntries so we don't decrypt them twice - const existingIds = retryEntries.reduce((ids, e) => {ids.add(e.id); return ids;}, new Set()); - retryTimelineEntries = retryTimelineEntries.filter(e => !existingIds.has(e.id)); - // make copies so we don't modify the original entry in writeSync, before the afterSync stage - const retryTimelineEntriesCopies = retryTimelineEntries.map(e => e.clone()); - // add to other retry entries - retryEntries = retryEntries.concat(retryTimelineEntriesCopies); - } - if (retryEntries.length) { - log.set("retry", retryEntries.length); - events = events.concat(retryEntries.map(entry => entry.event)); - } + async _getSyncRetryDecryptEntries(newKeys, txn) { + const entriesPerKey = await Promise.all(newKeys.map(key => this._getRetryDecryptEntriesForKey(key, txn))); + let retryEntries = entriesPerKey.reduce((allEntries, entries) => allEntries.concat(entries), []); + // If we have the timeline open, see if there are more entries for the new keys + // as we only store missing session information for synced events, not backfilled. + // We want to decrypt all events we can though if the user is looking + // at them when the timeline is open + if (this._timeline) { + let retryTimelineEntries = this._roomEncryption.filterUndecryptedEventEntriesForKeys(this._timeline.remoteEntries, newKeys); + // filter out any entries already in retryEntries so we don't decrypt them twice + const existingIds = retryEntries.reduce((ids, e) => {ids.add(e.id); return ids;}, new Set()); + retryTimelineEntries = retryTimelineEntries.filter(e => !existingIds.has(e.id)); + // make copies so we don't modify the original entry in writeSync, before the afterSync stage + const retryTimelineEntriesCopies = retryTimelineEntries.map(e => e.clone()); + // add to other retry entries + retryEntries = retryEntries.concat(retryTimelineEntriesCopies); } - const eventsToDecrypt = events.filter(event => { - return event?.type === EVENT_ENCRYPTED_TYPE; - }); - if (eventsToDecrypt.length) { - decryptPreparation = await roomEncryption.prepareDecryptAll( - eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); - } - return {retryEntries, decryptPreparation}; + return retryEntries; } async prepareSync(roomResponse, membership, newKeys, txn, log) { @@ -208,10 +192,22 @@ export class Room extends EventEmitter { let retryEntries; let decryptPreparation; if (roomEncryption) { - const events = roomResponse?.timeline?.events || []; - const result = await this._prepareSyncDecryption(events, newKeys, roomEncryption, txn, log); - retryEntries = result.retryEntries; - decryptPreparation = result.decryptPreparation; + let eventsToDecrypt = roomResponse?.timeline?.events || []; + // when new keys arrive, also see if any older events can now be retried to decrypt + if (newKeys) { + retryEntries = await this._getSyncRetryDecryptEntries(newKeys, txn); + if (retryEntries.length) { + log.set("retry", retryEntries.length); + eventsToDecrypt = eventsToDecrypt.concat(retryEntries.map(entry => entry.event)); + } + } + eventsToDecrypt = eventsToDecrypt.filter(event => { + return event?.type === EVENT_ENCRYPTED_TYPE; + }); + if (eventsToDecrypt.length) { + decryptPreparation = await roomEncryption.prepareDecryptAll( + eventsToDecrypt, newKeys, DecryptionSource.Sync, txn); + } } return {