Merge pull request #143 from matrix-org/rav/room_encryption_state

crypto: use memberlist to derive recipient list

lib/client.js

@@ -643,16 +643,47 @@ 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) {
+        this.sessionStore.storeEndToEndRoom(roomId, config);
-            throw new Error(
+
-                "Config must include a 'members' list with a list of userIds"
+        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 < config.members.length; ++i) {
+    for (var i = 0; i < users.length; ++i) {
-            var userId = config.members[i];
+        var userId = users[i];
         var devices = this.sessionStore.getEndToEndDevicesForUser(userId);
         if (!devices) {
             userWithoutDevices.push(userId);
@@ -671,8 +702,14 @@ MatrixClient.prototype.setRoomEncryption = function(roomId, config) {
             }
         }
     }
-        var deferred = q.defer();
+
-        if (devicesWithoutSession.length > 0) {
+    if (devicesWithoutSession.length === 0) {
+        return q({
+            missingUsers: userWithoutDevices,
+            missingDevices: []
+        });
+    }
+
     var queries = {};
     for (i = 0; i < devicesWithoutSession.length; ++i) {
         var device = devicesWithoutSession[i];
@@ -683,10 +720,10 @@ MatrixClient.prototype.setRoomEncryption = function(roomId, config) {
     var path = "/keys/claim";
     var content = {one_time_keys: queries};
     var self = this;
-            this._http.authedRequestWithPrefix(
+    return this._http.authedRequestWithPrefix(
         undefined, "POST", path, undefined, content,
         httpApi.PREFIX_UNSTABLE
-            ).done(function(res) {
+    ).then(function(res) {
         var missing = {};
         for (i = 0; i < devicesWithoutSession.length; ++i) {
             var device = devicesWithoutSession[i];
@@ -709,24 +746,13 @@ MatrixClient.prototype.setRoomEncryption = function(roomId, config) {
                 missing[device[0]].push([device[1]]);
             }
         }
-                deferred.resolve({
+
+        return {
             missingUsers: userWithoutDevices,
             missingDevices: missing
-                });
-            });
-        } else {
-            deferred.resolve({
-                missingUsers: userWithoutDevices,
-                missingDevices: []
-            });
-        }
-        this.sessionStore.storeEndToEndRoom(roomId, config);
-        return deferred.promise;
-    } else {
-        throw new Error("Unknown algorithm: " + config.algorithm);
-    }
         };
-
+    });
+};
 
 /**
  * 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,29 +41,27 @@ 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),
-        aliHttpBackend.flush(uploadPath, 1),
-    ]).then(function() {
-        console.log("ali uploaded keys");
-    });
-}
 
-function bobUploadsKeys() {
+    var uploadContent;
-    var uploadPath = "/keys/upload/bvcxz";
+    httpBackend.when("POST", uploadPath).respond(200, function(path, content) {
-    bobHttpBackend.when("POST", uploadPath).respond(200, function(path, content) {
+        uploadContent = content;
-        expect(content.one_time_keys).toEqual({});
-        bobHttpBackend.when("POST", uploadPath).respond(200, function(path, content) {
         expect(content.one_time_keys).not.toEqual({});
-            bobDeviceKeys = content.device_keys;
-            bobOneTimeKeys = content.one_time_keys;
         var count = 0;
         for (var key in content.one_time_keys) {
             if (content.one_time_keys.hasOwnProperty(key)) {
@@ -73,10 +71,41 @@ function bobUploadsKeys() {
         expect(count).toEqual(5);
         return {one_time_key_counts: {curve25519: count}};
     });
-        return {one_time_key_counts: {}};
+
+    return httpBackend.flush(uploadPath, 2).then(function() {
+        return uploadContent;
     });
-    bobClient.uploadKeys(5).catch(utils.failTest);
+}
-    return bobHttpBackend.flush().then(function() {
+
+
+/**
+ * Set an expectation that ali will upload device keys and a number of one-time keys;
+ * then flush the http requests.
+ *
+ * <p>Updates <tt>aliDeviceKeys</tt>
+ *
+ * @return {promise} completes once the http requests have completed.
+ */
+function expectAliKeyUpload() {
+    return expectKeyUpload(aliDeviceId, aliHttpBackend).then(function(content) {
+        aliDeviceKeys = content.device_keys;
+    });
+}
+
+
+/**
+ * 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);