diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 3574213e..c995e3e2 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -47,10 +47,8 @@ function timelineIsEmpty(roomResponse) { * const changes = await room.writeSync(roomResponse, isInitialSync, preparation, syncTxn); * // applies and emits changes once syncTxn is committed * room.afterSync(changes); - * if (room.needsAfterSyncCompleted(changes)) { - * // can do network requests - * await room.afterSyncCompleted(changes); - * } + * // can do network requests + * await room.afterSyncCompleted(changes); * ``` */ export class Sync { @@ -163,13 +161,9 @@ export class Sync { await log.wrap("session", log => this._session.afterSyncCompleted(sessionChanges, isCatchupSync, log), log.level.Detail); } catch (err) {} // error is logged, but don't fail sessionPromise })(); - - const roomsNeedingAfterSyncCompleted = roomStates.filter(rs => { - return rs.room.needsAfterSyncCompleted(rs.changes); - }); - const roomsPromises = roomsNeedingAfterSyncCompleted.map(async rs => { + const roomsPromises = roomStates.map(async rs => { try { - await log.wrap("room", log => rs.room.afterSyncCompleted(rs.changes, log), log.level.Detail); + await rs.room.afterSyncCompleted(rs.changes, log); } catch (err) {} // error is logged, but don't fail roomsPromises }); // run everything in parallel, diff --git a/src/matrix/e2ee/DecryptionResult.ts b/src/matrix/e2ee/DecryptionResult.ts index 7735856a..8846616c 100644 --- a/src/matrix/e2ee/DecryptionResult.ts +++ b/src/matrix/e2ee/DecryptionResult.ts @@ -27,6 +27,7 @@ limitations under the License. */ import type {DeviceIdentity} from "../storage/idb/stores/DeviceIdentityStore"; +import type {TimelineEvent} from "../storage/types"; type DecryptedEvent = { type?: string, @@ -35,22 +36,18 @@ type DecryptedEvent = { export class DecryptionResult { private device?: DeviceIdentity; - private roomTracked: boolean = true; constructor( public readonly event: DecryptedEvent, public readonly senderCurve25519Key: string, - public readonly claimedEd25519Key: string + public readonly claimedEd25519Key: string, + public readonly encryptedEvent?: TimelineEvent ) {} setDevice(device: DeviceIdentity): void { this.device = device; } - setRoomNotTrackedYet(): void { - this.roomTracked = false; - } - get isVerified(): boolean { if (this.device) { const comesFromDevice = this.device.ed25519Key === this.claimedEd25519Key; @@ -62,15 +59,12 @@ export class DecryptionResult { get isUnverified(): boolean { if (this.device) { return !this.isVerified; - } else if (this.isVerificationUnknown) { - return false; } else { return true; } } get isVerificationUnknown(): boolean { - // verification is unknown if we haven't yet fetched the devices for the room - return !this.device && !this.roomTracked; + return !this.device; } } diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 36424b02..4dd6cd0b 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -208,17 +208,43 @@ export class RoomEncryption { }); } - async _verifyDecryptionResult(result, txn) { - let device = this._senderDeviceCache.get(result.senderCurve25519Key); - if (!device) { - device = await this._deviceTracker.getDeviceByCurve25519Key(result.senderCurve25519Key, txn); - this._senderDeviceCache.set(result.senderCurve25519Key, device); - } - if (device) { - result.setDevice(device); - } else if (!this._room.isTrackingMembers) { - result.setRoomNotTrackedYet(); + async _verifyDecryptionResults(results, txn) { + await Promise.all(results.map(async result => { + let device = this._senderDeviceCache.get(result.senderCurve25519Key); + if (!device) { + device = await this._deviceTracker.getDeviceByCurve25519Key(result.senderCurve25519Key, txn); + this._senderDeviceCache.set(result.senderCurve25519Key, device); + } + if (device) { + result.setDevice(device); + } + })); + } + + /** fetches the devices that are not yet known locally from the homeserver to verify the sender of this message. */ + async _fetchKeyAndVerifyDecryptionResults(results, hsApi, log) { + const resultsWithoutDevice = results.filter(r => r.isVerificationUnknown); + if (resultsWithoutDevice.length) { + return log.wrap("fetch unverified senders", async log => { + const sendersWithoutDevice = Array.from(resultsWithoutDevice.reduce((senders, r) => { + return senders.add(r.encryptedEvent.sender); + }, new Set())); + log.set("senders", sendersWithoutDevice); + // fetch the devices, ignore return value, + // and just reuse _verifyDecryptionResults method so we only have one impl how to verify + await this._deviceTracker.devicesForRoomMembers(this._room.id, sendersWithoutDevice, hsApi, log); + // now that we've fetched the missing devices, try verifying the results again + const txn = await this._storage.readTxn([this._storage.storeNames.deviceIdentities]); + await this._verifyDecryptionResults(resultsWithoutDevice, txn); + const resultsWithFoundDevice = resultsWithoutDevice.filter(r => !r.isVerificationUnknown); + const resultsToEventIdMap = resultsWithFoundDevice.reduce((map, r) => { + map.set(r.encryptedEvent.event_id, r); + return map; + }, new Map()); + return new BatchDecryptionResult(resultsToEventIdMap, new Map(), this); + }); } + return new BatchDecryptionResult(new Map(), new Map(), this); } async _requestMissingSessionFromBackup(senderKey, sessionId, log) { @@ -531,24 +557,42 @@ class BatchDecryptionResult { this._roomEncryption = roomEncryption; } - applyToEntries(entries) { + applyToEntries(entries, callback = undefined) { for (const entry of entries) { const result = this.results.get(entry.id); if (result) { entry.setDecryptionResult(result); + callback?.(entry); } else { const error = this.errors.get(entry.id); if (error) { entry.setDecryptionError(error); + callback?.(entry); } } } } - verifySenders(txn) { - return Promise.all(Array.from(this.results.values()).map(result => { - return this._roomEncryption._verifyDecryptionResult(result, txn); - })); + /** Verify the decryption results by looking for the corresponding device in local persistance + * @returns {BatchDecryptionResult} a new batch result with the results for which we now found a device */ + verifyKnownSenders(txn) { + return this._roomEncryption._verifyDecryptionResults(Array.from(this.results.values()), txn); + } + + get hasUnverifiedSenders() { + for (const r of this.results.values()) { + if (r.isVerificationUnknown) { + return true; + } + } + return false; + } + + /** Verify any decryption results for which we could not find a device when + * calling `verifyKnownSenders` prior, by fetching them from the homeserver. + * @returns {Promise} the results for which we found a device */ + fetchAndVerifyRemainingSenders(hsApi, log) { + return this._roomEncryption._fetchKeyAndVerifyDecryptionResults(Array.from(this.results.values()), hsApi, log); } } @@ -556,7 +600,6 @@ 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() { @@ -665,7 +708,7 @@ export function tests() { const storage = await createMockStorage(); let isMemberChangesCalled = false; const deviceTracker = { - async writeMemberChanges(room, memberChanges, historyVisibility, txn) { + async writeMemberChanges(room, memberChanges, historyVisibility) { assert.equal(historyVisibility, "invited"); isMemberChangesCalled = true; return {removed: [], added: []}; diff --git a/src/matrix/e2ee/megolm/decryption/SessionDecryption.ts b/src/matrix/e2ee/megolm/decryption/SessionDecryption.ts index 57ef9a96..ca294460 100644 --- a/src/matrix/e2ee/megolm/decryption/SessionDecryption.ts +++ b/src/matrix/e2ee/megolm/decryption/SessionDecryption.ts @@ -31,17 +31,14 @@ interface DecryptAllResult { * Does the actual decryption of all events for a given megolm session in a batch */ export class SessionDecryption { - private key: RoomKey; - private events: TimelineEvent[]; - private keyLoader: KeyLoader; - private olmWorker?: OlmWorker; private decryptionRequests?: any[]; - constructor(key: RoomKey, events: TimelineEvent[], olmWorker: OlmWorker | undefined, keyLoader: KeyLoader) { - this.key = key; - this.events = events; - this.olmWorker = olmWorker; - this.keyLoader = keyLoader; + constructor( + private readonly key: RoomKey, + private readonly events: TimelineEvent[], + private readonly olmWorker: OlmWorker | undefined, + private readonly keyLoader: KeyLoader + ) { this.decryptionRequests = olmWorker ? [] : undefined; } @@ -75,7 +72,7 @@ export class SessionDecryption { {encryptedRoomId: payload.room_id, eventRoomId: this.key.roomId}); } replayEntries.push(new ReplayDetectionEntry(this.key.sessionId, decryptionResult!.message_index, event)); - const result = new DecryptionResult(payload, this.key.senderKey, this.key.claimedEd25519Key); + const result = new DecryptionResult(payload, this.key.senderKey, this.key.claimedEd25519Key, event); results.set(event.event_id, result); } catch (err) { // ignore AbortError from cancelling decryption requests in dispose method diff --git a/src/matrix/room/BaseRoom.js b/src/matrix/room/BaseRoom.js index 57d2a7b2..9e12f257 100644 --- a/src/matrix/room/BaseRoom.js +++ b/src/matrix/room/BaseRoom.js @@ -154,7 +154,7 @@ export class BaseRoom extends EventEmitter { try { decryption = await changes.write(writeTxn, log); if (isTimelineOpen) { - await decryption.verifySenders(writeTxn); + await decryption.verifyKnownSenders(writeTxn); } } catch (err) { writeTxn.abort(); @@ -166,6 +166,16 @@ export class BaseRoom extends EventEmitter { if (this._observedEvents) { this._observedEvents.updateEvents(entries); } + if (isTimelineOpen && decryption.hasUnverifiedSenders) { + // verify missing senders async and update timeline once done so we don't delay rendering with network requests + log.wrapDetached("fetch unknown senders keys", async log => { + const newlyVerifiedDecryption = await decryption.fetchAndVerifyRemainingSenders(this._hsApi, log); + const verifiedEntries = []; + newlyVerifiedDecryption.applyToEntries(entries, entry => verifiedEntries.push(entry)); + this._timeline?.replaceEntries(verifiedEntries); + this._observedEvents?.updateEvents(verifiedEntries); + }); + } }, ensureLogItem(log)); return request; } diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 8cc87845..d3d1a3b0 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -124,12 +124,13 @@ export class Room extends BaseRoom { const {entries: newEntries, updatedEntries, newLiveKey, memberChanges} = await log.wrap("syncWriter", log => this._syncWriter.writeSync( roomResponse, isRejoin, summaryChanges.hasFetchedMembers, txn, log), log.level.Detail); + let decryption; if (decryptChanges) { - const decryption = await log.wrap("decryptChanges", log => decryptChanges.write(txn, log)); + decryption = await log.wrap("decryptChanges", log => decryptChanges.write(txn, log)); log.set("decryptionResults", decryption.results.size); log.set("decryptionErrors", decryption.errors.size); if (this._isTimelineOpen) { - await decryption.verifySenders(txn); + await decryption.verifyKnownSenders(txn); } decryption.applyToEntries(newEntries); if (retryEntries?.length) { @@ -189,6 +190,7 @@ export class Room extends BaseRoom { heroChanges, powerLevelsEvent, encryptionChanges, + decryption }; } @@ -291,19 +293,36 @@ export class Room extends BaseRoom { } } - needsAfterSyncCompleted({encryptionChanges}) { - return encryptionChanges?.shouldFlush; - } - /** * Only called if the result of writeSync had `needsAfterSyncCompleted` set. * Can be used to do longer running operations that resulted from the last sync, * like network operations. */ - async afterSyncCompleted(changes, log) { - log.set("id", this.id); - if (this._roomEncryption) { - await this._roomEncryption.flushPendingRoomKeyShares(this._hsApi, null, log); + async afterSyncCompleted({encryptionChanges, decryption, newEntries, updatedEntries}, log) { + const shouldFlushKeys = encryptionChanges?.shouldFlush; + const shouldFetchUnverifiedSenders = this._isTimelineOpen && decryption?.hasUnverifiedSenders; + // only log rooms where we actually do something + if (shouldFlushKeys || shouldFetchUnverifiedSenders) { + await log.wrap({l: "room", id: this.id}, async log => { + const promises = []; + if (shouldFlushKeys) { + promises.push(this._roomEncryption.flushPendingRoomKeyShares(this._hsApi, null, log)); + } + if (shouldFetchUnverifiedSenders) { + const promise = log.wrap("verify senders", (async log => { + const newlyVerifiedDecryption = await decryption.fetchAndVerifyRemainingSenders(this._hsApi, log); + const verifiedEntries = []; + const updateCallback = entry => verifiedEntries.push(entry); + newlyVerifiedDecryption.applyToEntries(newEntries, updateCallback); + newlyVerifiedDecryption.applyToEntries(updatedEntries, updateCallback); + log.set("verifiedEntries", verifiedEntries.length); + this._timeline?.replaceEntries(verifiedEntries); + this._observedEvents?.updateEvents(verifiedEntries); + })); + promises.push(promise); + } + await Promise.all(promises); + }); } } diff --git a/src/matrix/room/timeline/entries/EventEntry.js b/src/matrix/room/timeline/entries/EventEntry.js index d218a598..cf56cbf9 100644 --- a/src/matrix/room/timeline/entries/EventEntry.js +++ b/src/matrix/room/timeline/entries/EventEntry.js @@ -33,10 +33,11 @@ export class EventEntry extends BaseEventEntry { } updateFrom(other) { - if (other._decryptionResult && !this._decryptionResult) { + // only update these when we attempted decryption, as some updates (like reactions) don't. + if (other._decryptionResult) { this._decryptionResult = other._decryptionResult; } - if (other._decryptionError && !this._decryptionError) { + if (other._decryptionError) { this._decryptionError = other._decryptionError; } this._contextForEntries = other.contextForEntries; diff --git a/src/platform/web/ui/session/room/timeline/BaseMessageView.js b/src/platform/web/ui/session/room/timeline/BaseMessageView.js index 74b96ecf..ee0a37db 100644 --- a/src/platform/web/ui/session/room/timeline/BaseMessageView.js +++ b/src/platform/web/ui/session/room/timeline/BaseMessageView.js @@ -46,7 +46,7 @@ export class BaseMessageView extends TemplateView { "Timeline_message": true, own: vm.isOwn, unsent: vm.isUnsent, - unverified: vm.isUnverified, + unverified: vm => vm.isUnverified, disabled: !this._interactive, continuation: vm => vm.isContinuation, },