From 60c9c403bd1ffe8abe780e7062d3f4711285d320 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Fri, 24 Aug 2018 17:49:57 +0200
Subject: [PATCH] Improve setRoomEncryption guard against multiple
 m.room.encryption state events

we were only bailing out when receiving a non JSON-identical m.room.encryption event.
When receiving an identical event, the algorithm in _roomEncryptors would be reset,
generating a new megolm session every time this happens (there is a LL synapse bug
where this happens on every sync).

As the _roomList is backed by indexeddb you might already have a config without the algorithm being present though,
so we first check for the room encryptor algorithm being present. If so, always bail out as setRoomEncryption was
already called for the given room.

If no algorithm is present, still check if the config is not being changed.
Also setup the roomlist and room encryption synchronously before awaiting
the indexeddb operation to store the room encryption config in roomlist.
---
 src/crypto/index.js | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/crypto/index.js b/src/crypto/index.js
index 8b35bc0a4..3bc2b7843 100644
--- a/src/crypto/index.js
+++ b/src/crypto/index.js
@@ -619,20 +619,35 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) {
 Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDeviceQuery) {
     // if we already have encryption in this room, we should ignore this event
     // (for now at least. maybe we should alert the user somehow?)
-    const existingConfig = this._roomList.getRoomEncryption(roomId);
-    if (existingConfig && JSON.stringify(existingConfig) != JSON.stringify(config)) {
-        console.error("Ignoring m.room.encryption event which requests " +
-                      "a change of config in " + roomId);
+    const existingAlg = this._roomEncryptors[roomId];
+    if (existingAlg) {
         return;
     }
+    // _roomList.getRoomEncryption will not race with _roomList.setRoomEncryption
+    // because it first stores in memory. We should await the promise only
+    // after all the in-memory state (_roomEncryptors and _roomList) has been updated
+    // to avoid races when calling this method multiple times. Hence keep a hold of the promise.
+    let storeConfigPromise;
+    // if state is being replayed from storage, we might already have a configuration
+    // for this room. We just need to make sure the algorithm in
+    // _roomEncryptors and config in _roomList are in sync
+    // by making sure the existingConfig is identical to config.
+    const existingConfig = this._roomList.getRoomEncryption(roomId);
+    if (existingConfig) {
+        if (JSON.stringify(existingConfig) != JSON.stringify(config)) {
+            console.error("Ignoring m.room.encryption event which requests " +
+                          "a change of config in " + roomId);
+            return;
+        }
+    } else {
+        const storeConfigPromise = this._roomList.setRoomEncryption(roomId, config);
+    }
 
     const AlgClass = algorithms.ENCRYPTION_CLASSES[config.algorithm];
     if (!AlgClass) {
         throw new Error("Unable to encrypt with " + config.algorithm);
     }
 
-    await this._roomList.setRoomEncryption(roomId, config);
-
     const alg = new AlgClass({
         userId: this._userId,
         deviceId: this._deviceId,
@@ -644,6 +659,10 @@ Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDevic
     });
     this._roomEncryptors[roomId] = alg;
 
+    if (storeConfigPromise) {
+        await storeConfigPromise;
+    }
+
     // make sure we are tracking the device lists for all users in this room.
     console.log("Enabling encryption in " + roomId + "; " +
                 "starting to track device lists for all users therein");