crypto: use memberlist to derive recipient list

When we send an encrypted message with Olm, we need to know who to send it
to. Currently that is based on a user list passed to setRoomEncryption. We want
to be able to change the list as people join and leave the room; I also want to
not have to keep the member list in local storage.

So, use the member list at the point of enabling encryption to set up sessions,
and at the point of sending a message to encrypt the message.

Further work here includes:

 * updating the react-sdk not to bother setting the member list
 * monitoring for new users/devices in the room, and setting up new sessions
   with them

lib/client.js

@@ -643,90 +643,116 @@ MatrixClient.prototype.setRoomEncryption = function(roomId, config) {
         return q.reject(new Error("End-to-End encryption disabled"));
     }
 
+    var self = this;
+
     if (config.algorithm === OLM_ALGORITHM) {
-        if (!config.members) {
-            throw new Error(
-                "Config must include a 'members' list with a list of userIds"
-            );
-        }
-        var devicesWithoutSession = [];
-        var userWithoutDevices = [];
-        for (var i = 0; i < config.members.length; ++i) {
-            var userId = config.members[i];
-            var devices = this.sessionStore.getEndToEndDevicesForUser(userId);
-            if (!devices) {
-                userWithoutDevices.push(userId);
-            } else {
-                for (var deviceId in devices) {
-                    if (devices.hasOwnProperty(deviceId)) {
-                        var keys = devices[deviceId];
-                        var key = keys.keys["curve25519:" + deviceId];
-                        if (key == this._olmDevice.deviceCurve25519Key) {
-                            continue;
-                        }
-                        if (!this.sessionStore.getEndToEndSessions(key)) {
-                            devicesWithoutSession.push([userId, deviceId, key]);
-                        }
-                    }
-                }
-            }
-        }
-        var deferred = q.defer();
-        if (devicesWithoutSession.length > 0) {
-            var queries = {};
-            for (i = 0; i < devicesWithoutSession.length; ++i) {
-                var device = devicesWithoutSession[i];
-                var query = queries[device[0]] || {};
-                queries[device[0]] = query;
-                query[device[1]] = "curve25519";
-            }
-            var path = "/keys/claim";
-            var content = {one_time_keys: queries};
-            var self = this;
-            this._http.authedRequestWithPrefix(
-                undefined, "POST", path, undefined, content,
-                httpApi.PREFIX_UNSTABLE
-            ).done(function(res) {
-                var missing = {};
-                for (i = 0; i < devicesWithoutSession.length; ++i) {
-                    var device = devicesWithoutSession[i];
-                    var userRes = res.one_time_keys[device[0]] || {};
-                    var deviceRes = userRes[device[1]];
-                    var oneTimeKey;
-                    for (var keyId in deviceRes) {
-                        if (keyId.indexOf("curve25519:") === 0) {
-                            oneTimeKey = deviceRes[keyId];
-                        }
-                    }
-                    if (oneTimeKey) {
-                        var sid = self._olmDevice.createOutboundSession(
-                            device[2], oneTimeKey
-                        );
-                        console.log("Started new sessionid " + sid +
-                                    " for device " + device[2]);
-                    } else {
-                        missing[device[0]] = missing[device[0]] || [];
-                        missing[device[0]].push([device[1]]);
-                    }
-                }
-                deferred.resolve({
-                    missingUsers: userWithoutDevices,
-                    missingDevices: missing
-                });
-            });
-        } else {
-            deferred.resolve({
-                missingUsers: userWithoutDevices,
-                missingDevices: []
-            });
-        }
         this.sessionStore.storeEndToEndRoom(roomId, config);
-        return deferred.promise;
+
+        var room = this.getRoom(roomId);
+
+        if (!room) {
+            console.warn("Enabling encryption in unknown room " + roomId);
+            return q({});
+        }
+
+        var users = utils.map(room.getJoinedMembers(), function(u) {
+            return u.userId;
+        });
+
+        return self.downloadKeys(users, true).then(function(res) {
+            return self._ensureOlmSessionsForUsers(users);
+        });
     } else {
         throw new Error("Unknown algorithm: " + config.algorithm);
     }
 };
 
+/**
+ * Try to make sure we have established olm sessions for the given users.
+ *
+ * @param {string[]} users list of user ids
+ *
+ * @return {module:client.Promise} resolves once the sessions are complete, to
+ *  an object with keys <tt>missingUsers</tt> (a list of users with no known
+ *  olm devices), and <tt>missingDevices</tt> a list of olm devices with no
+ *  known one-time keys.
+ *
+ * @private
+ */
+MatrixClient.prototype._ensureOlmSessionsForUsers = function(users) {
+    var devicesWithoutSession = [];
+    var userWithoutDevices = [];
+    for (var i = 0; i < users.length; ++i) {
+        var userId = users[i];
+        var devices = this.sessionStore.getEndToEndDevicesForUser(userId);
+        if (!devices) {
+            userWithoutDevices.push(userId);
+        } else {
+            for (var deviceId in devices) {
+                if (devices.hasOwnProperty(deviceId)) {
+                    var keys = devices[deviceId];
+                    var key = keys.keys["curve25519:" + deviceId];
+                    if (key == this._olmDevice.deviceCurve25519Key) {
+                        continue;
+                    }
+                    if (!this.sessionStore.getEndToEndSessions(key)) {
+                        devicesWithoutSession.push([userId, deviceId, key]);
+                    }
+                }
+            }
+        }
+    }
+
+    if (devicesWithoutSession.length === 0) {
+        return q({
+            missingUsers: userWithoutDevices,
+            missingDevices: []
+        });
+    }
+
+    var queries = {};
+    for (i = 0; i < devicesWithoutSession.length; ++i) {
+        var device = devicesWithoutSession[i];
+        var query = queries[device[0]] || {};
+        queries[device[0]] = query;
+        query[device[1]] = "curve25519";
+    }
+    var path = "/keys/claim";
+    var content = {one_time_keys: queries};
+    var self = this;
+    return this._http.authedRequestWithPrefix(
+        undefined, "POST", path, undefined, content,
+        httpApi.PREFIX_UNSTABLE
+    ).then(function(res) {
+        var missing = {};
+        for (i = 0; i < devicesWithoutSession.length; ++i) {
+            var device = devicesWithoutSession[i];
+            var userRes = res.one_time_keys[device[0]] || {};
+            var deviceRes = userRes[device[1]];
+            var oneTimeKey;
+            for (var keyId in deviceRes) {
+                if (keyId.indexOf("curve25519:") === 0) {
+                    oneTimeKey = deviceRes[keyId];
+                }
+            }
+            if (oneTimeKey) {
+                var sid = self._olmDevice.createOutboundSession(
+                    device[2], oneTimeKey
+                );
+                console.log("Started new sessionid " + sid +
+                            " for device " + device[2]);
+            } else {
+                missing[device[0]] = missing[device[0]] || [];
+                missing[device[0]].push([device[1]]);
+            }
+        }
+
+        return {
+            missingUsers: userWithoutDevices,
+            missingDevices: missing
+        };
+    });
+};
 
 /**
  * Disable encryption for a room.
@@ -1177,9 +1203,23 @@ function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content) {
     }
 
     if (e2eRoomInfo.algorithm === OLM_ALGORITHM) {
+        var room = client.getRoom(roomId);
+        if (!room) {
+            throw new Error("Cannot send encrypted messages in unknown rooms");
+        }
+
+        // pick the list of recipients based on the membership list.
+        //
+        // TODO: there is a race condition here! What if a new user turns up
+        // just as you are sending a secret message?
+
+        var users = utils.map(room.getJoinedMembers(), function(u) {
+            return u.userId;
+        });
+
         var participantKeys = [];
-        for (var i = 0; i < e2eRoomInfo.members.length; ++i) {
+        for (var i = 0; i < users.length; ++i) {
-            var userId = e2eRoomInfo.members[i];
+            var userId = users[i];
             var devices = client.sessionStore.getEndToEndDevicesForUser(userId);
             for (var deviceId in devices) {
                 if (devices.hasOwnProperty(deviceId)) {

spec/integ/matrix-client-crypto.spec.js

@@ -41,42 +41,71 @@ var aliMessages;
 var bobMessages;
 
 
-function aliUploadsKeys() {
+/**
-    var uploadPath = "/keys/upload/" + aliDeviceId;
+ * Set an expectation that the client will upload device keys and a number of
-    aliHttpBackend.when("POST", uploadPath).respond(200, function(path, content) {
+ * one-time keys; then flush the http requests.
+ *
+ * @param {string} deviceId expected device id in upload request
+ * @param {object} httpBackend
+ *
+ * @return {promise} completes once the http requests have completed, returning the
+ * content of the upload request.
+ */
+function expectKeyUpload(deviceId, httpBackend) {
+    var uploadPath = "/keys/upload/" + deviceId;
+    httpBackend.when("POST", uploadPath).respond(200, function(path, content) {
         expect(content.one_time_keys).toEqual({});
-        aliDeviceKeys = content.device_keys;
         return {one_time_key_counts: {curve25519: 0}};
     });
-    return q.all([
+
-        aliClient.uploadKeys(0),
+    var uploadContent;
-        aliHttpBackend.flush(uploadPath, 1),
+    httpBackend.when("POST", uploadPath).respond(200, function(path, content) {
-    ]).then(function() {
+        uploadContent = content;
-        console.log("ali uploaded keys");
+        expect(content.one_time_keys).not.toEqual({});
+        var count = 0;
+        for (var key in content.one_time_keys) {
+            if (content.one_time_keys.hasOwnProperty(key)) {
+                count++;
+            }
+        }
+        expect(count).toEqual(5);
+        return {one_time_key_counts: {curve25519: count}};
+    });
+
+    return httpBackend.flush(uploadPath, 2).then(function() {
+        return uploadContent;
     });
 }
 
-function bobUploadsKeys() {
+
-    var uploadPath = "/keys/upload/bvcxz";
+/**
-    bobHttpBackend.when("POST", uploadPath).respond(200, function(path, content) {
+ * Set an expectation that ali will upload device keys and a number of one-time keys;
-        expect(content.one_time_keys).toEqual({});
+ * then flush the http requests.
-        bobHttpBackend.when("POST", uploadPath).respond(200, function(path, content) {
+ *
-            expect(content.one_time_keys).not.toEqual({});
+ * <p>Updates <tt>aliDeviceKeys</tt>
-            bobDeviceKeys = content.device_keys;
+ *
-            bobOneTimeKeys = content.one_time_keys;
+ * @return {promise} completes once the http requests have completed.
-            var count = 0;
+ */
-            for (var key in content.one_time_keys) {
+function expectAliKeyUpload() {
-                if (content.one_time_keys.hasOwnProperty(key)) {
+    return expectKeyUpload(aliDeviceId, aliHttpBackend).then(function(content) {
-                    count++;
+        aliDeviceKeys = content.device_keys;
-                }
-            }
-            expect(count).toEqual(5);
-            return {one_time_key_counts: {curve25519: count}};
-        });
-        return {one_time_key_counts: {}};
     });
-    bobClient.uploadKeys(5).catch(utils.failTest);
+}
-    return bobHttpBackend.flush().then(function() {
+
+
+/**
+ * Set an expectation that bob will upload device keys and a number of one-time keys;
+ * then flush the http requests.
+ *
+ * <p>Updates <tt>bobDeviceKeys</tt>, <tt>bobOneTimeKeys</tt>,
+ * <tt>bobDeviceCurve25519Key</tt>, <tt>bobDeviceEd25519Key</tt>
+ *
+ * @return {promise} completes once the http requests have completed.
+ */
+function expectBobKeyUpload() {
+    return expectKeyUpload(bobDeviceId, bobHttpBackend).then(function(content) {
+        bobDeviceKeys = content.device_keys;
+        bobOneTimeKeys = content.one_time_keys;
         expect(bobDeviceKeys).toBeDefined();
         expect(bobOneTimeKeys).toBeDefined();
         bobDeviceCurve25519Key = bobDeviceKeys.keys["curve25519:bvcxz"];
@@ -84,7 +113,21 @@ function bobUploadsKeys() {
     });
 }
 
-function aliDownloadsKeys() {
+function bobUploadsKeys() {
+    bobClient.uploadKeys(5).catch(utils.failTest);
+    return expectBobKeyUpload();
+}
+
+
+/**
+ * Set an expectation that ali will query bobs keys; then flush the http request.
+ *
+ * @return {promise} resolves once the http request has completed.
+ */
+function aliQueryKeys() {
+    // can't query keys before bob has uploaded them
+    expect(bobDeviceKeys).toBeDefined();
+
     var bobKeys = {};
     bobKeys[bobDeviceId] = bobDeviceKeys;
     aliHttpBackend.when("POST", "/keys/query").respond(200, function(path, content) {
@@ -93,23 +136,18 @@ function aliDownloadsKeys() {
         result[bobUserId] = bobKeys;
         return {device_keys: result};
     });
-    var p1 = aliClient.downloadKeys([bobUserId]).then(function() {
+    return aliHttpBackend.flush("/keys/query", 1);
-        expect(aliClient.listDeviceKeys(bobUserId)).toEqual([{
-            id: "bvcxz",
-            key: bobDeviceEd25519Key,
-            verified: false,
-        }]);
-    });
-    var p2 = aliHttpBackend.flush();
-
-    return q.all([p1, p2]).then(function() {
-        var devices = aliStorage.getEndToEndDevicesForUser(bobUserId);
-        expect(devices[bobDeviceId].keys).toEqual(bobDeviceKeys.keys);
-        expect(devices[bobDeviceId].verified).toBe(false);
-    });
 }
 
-function bobDownloadsKeys() {
+/**
+ * Set an expectation that bob will query alis keys; then flush the http request.
+ *
+ * @return {promise} which resolves once the http request has completed.
+ */
+function bobQueryKeys() {
+    // can't query keys before ali has uploaded them
+    expect(aliDeviceKeys).toBeDefined();
+
     var aliKeys = {};
     aliKeys[aliDeviceId] = aliDeviceKeys;
     bobHttpBackend.when("POST", "/keys/query").respond(200, function(path, content) {
@@ -118,13 +156,35 @@ function bobDownloadsKeys() {
         result[aliUserId] = aliKeys;
         return {device_keys: result};
     });
-    return q.all([
+    return bobHttpBackend.flush("/keys/query", 1);
-        bobClient.downloadKeys([aliUserId]),
+}
-        bobHttpBackend.flush(),
+
-    ]);
+
+function aliDownloadsKeys() {
+    // can't query keys before bob has uploaded them
+    expect(bobDeviceEd25519Key).toBeDefined();
+
+    var p1 = aliClient.downloadKeys([bobUserId]).then(function() {
+        expect(aliClient.listDeviceKeys(bobUserId)).toEqual([{
+            id: "bvcxz",
+            key: bobDeviceEd25519Key,
+            verified: false,
+        }]);
+    });
+    var p2 = aliQueryKeys();
+
+    return q.all([p1, p2]).then(function() {
+        var devices = aliStorage.getEndToEndDevicesForUser(bobUserId);
+        expect(devices[bobDeviceId].keys).toEqual(bobDeviceKeys.keys);
+        expect(devices[bobDeviceId].verified).toBe(false);
+    });
 }
 
 function aliEnablesEncryption() {
+    // can't query keys before bob has uploaded them
+    expect(bobOneTimeKeys).toBeDefined();
+
+    aliQueryKeys().catch(utils.failTest);
     aliHttpBackend.when("POST", "/keys/claim").respond(200, function(path, content) {
         expect(content.one_time_keys[bobUserId][bobDeviceId]).toEqual("curve25519");
         for (var keyId in bobOneTimeKeys) {
@@ -142,7 +202,6 @@ function aliEnablesEncryption() {
     });
     var p = aliClient.setRoomEncryption(roomId, {
         algorithm: "m.olm.v1.curve25519-aes-sha2",
-        members: [aliUserId, bobUserId]
     }).then(function(res) {
         expect(res.missingUsers).toEqual([]);
         expect(res.missingDevices).toEqual({});
@@ -153,11 +212,10 @@ function aliEnablesEncryption() {
 }
 
 function bobEnablesEncryption() {
+    bobQueryKeys().catch(utils.failTest);
     return bobClient.setRoomEncryption(roomId, {
         algorithm: "m.olm.v1.curve25519-aes-sha2",
-        members: [aliUserId, bobUserId]
     }).then(function(res) {
-        console.log("bob enabled encryption");
         expect(res.missingUsers).toEqual([]);
         expect(res.missingDevices).toEqual({});
         expect(bobClient.isRoomEncrypted(roomId)).toBeTruthy();
@@ -233,24 +291,88 @@ function recvMessage(httpBackend, client, message) {
     };
     httpBackend.when("GET", "/sync").respond(200, syncData);
     var deferred = q.defer();
-    client.on("event", function(event) {
+    var onEvent = function(event) {
+        console.log(client.credentials.userId + " received event",
+                    event);
+
+        // ignore the m.room.member events
+        if (event.getType() == "m.room.member") {
+            return;
+        }
+
         expect(event.getType()).toEqual("m.room.message");
         expect(event.getContent()).toEqual({
             msgtype: "m.text",
             body: "Hello, World"
         });
         expect(event.isEncrypted()).toBeTruthy();
+
+        client.removeListener("event", onEvent);
         deferred.resolve();
-    });
+    };
-    startClient(httpBackend, client);
+
+    client.on("event", onEvent);
+
     httpBackend.flush();
     return deferred.promise;
 }
 
+
+function aliStartClient() {
+    expectAliKeyUpload().catch(utils.failTest);
+    startClient(aliHttpBackend, aliClient);
+    return aliHttpBackend.flush().then(function() {
+        console.log("Ali client started");
+    });
+}
+
+function bobStartClient() {
+    expectBobKeyUpload().catch(utils.failTest);
+    startClient(bobHttpBackend, bobClient);
+    return bobHttpBackend.flush().then(function() {
+        console.log("Bob client started");
+    });
+}
+
+
+/**
+ * Set http responses for the requests which are made when a client starts, and
+ * start the client.
+ *
+ * @param {object} httpBackend
+ * @param {MatrixClient} client
+ */
 function startClient(httpBackend, client) {
-    client.startClient();
     httpBackend.when("GET", "/pushrules").respond(200, {});
     httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" });
+
+    // send a sync response including our test room.
+    var syncData = {
+        next_batch: "x",
+        rooms: {
+            join: { }
+        }
+    };
+    syncData.rooms.join[roomId] = {
+        state: {
+            events: [
+                utils.mkMembership({
+                    mship: "join",
+                    user: aliUserId,
+                }),
+                utils.mkMembership({
+                    mship: "join",
+                    user: bobUserId,
+                }),
+            ]
+        },
+        timeline: {
+            events: []
+        }
+    };
+    httpBackend.when("GET", "/sync").respond(200, syncData);
+
+    client.startClient();
 }
 
 
@@ -285,6 +407,11 @@ describe("MatrixClient crypto", function() {
             request: bobHttpBackend.requestFn,
         });
 
+        bobOneTimeKeys = undefined;
+        aliDeviceKeys = undefined;
+        bobDeviceKeys = undefined;
+        bobDeviceCurve25519Key = undefined;
+        bobDeviceEd25519Key = undefined;
         aliMessages = [];
         bobMessages = [];
     });
@@ -319,7 +446,7 @@ describe("MatrixClient crypto", function() {
     it("Ali enables encryption", function(done) {
         q()
             .then(bobUploadsKeys)
-            .then(aliDownloadsKeys)
+            .then(aliStartClient)
             .then(aliEnablesEncryption)
             .catch(utils.failTest).done(done);
     });
@@ -327,7 +454,7 @@ describe("MatrixClient crypto", function() {
     it("Ali sends a message", function(done) {
         q()
             .then(bobUploadsKeys)
-            .then(aliDownloadsKeys)
+            .then(aliStartClient)
             .then(aliEnablesEncryption)
             .then(aliSendsMessage)
             .catch(utils.failTest).done(done);
@@ -336,9 +463,10 @@ describe("MatrixClient crypto", function() {
     it("Bob receives a message", function(done) {
         q()
             .then(bobUploadsKeys)
-            .then(aliDownloadsKeys)
+            .then(aliStartClient)
             .then(aliEnablesEncryption)
             .then(aliSendsMessage)
+            .then(bobStartClient)
             .then(bobRecvMessage)
             .catch(utils.failTest).done(done);
     });
@@ -346,9 +474,10 @@ describe("MatrixClient crypto", function() {
     it("Bob receives two pre-key messages", function(done) {
         q()
             .then(bobUploadsKeys)
-            .then(aliDownloadsKeys)
+            .then(aliStartClient)
             .then(aliEnablesEncryption)
             .then(aliSendsMessage)
+            .then(bobStartClient)
             .then(bobRecvMessage)
             .then(aliSendsMessage)
             .then(bobRecvMessage)
@@ -358,12 +487,11 @@ describe("MatrixClient crypto", function() {
     it("Bob replies to the message", function(done) {
         q()
             .then(bobUploadsKeys)
-            .then(aliDownloadsKeys)
+            .then(aliStartClient)
             .then(aliEnablesEncryption)
             .then(aliSendsMessage)
+            .then(bobStartClient)
             .then(bobRecvMessage)
-            .then(aliUploadsKeys)
-            .then(bobDownloadsKeys)
             .then(bobEnablesEncryption)
             .then(bobSendsMessage).then(function(ciphertext) {
                 expect(ciphertext.type).toEqual(1);