Merge branch 'develop' into rav/rewrite_device_query_logic

docs/warning-on-unverified-devices.txt

@@ -0,0 +1,31 @@
+Random notes from Matthew on the two possible approaches for warning users about unexpected
+unverified devices popping up in their rooms....
+
+Original idea...
+================
+
+Warn when an existing user adds an unknown device to a room.
+
+Warn when a user joins the room with unverified or unknown devices.
+
+Warn when you initial sync if the room has any unverified devices in it.
+ ^ this is good enough if we're doing local storage.
+ OR, better:
+Warn when you initial sync if the room has any new undefined devices since you were last there.
+ => This means persisting the rooms that devices are in, across initial syncs.
+
+
+Updated idea...
+===============
+
+Warn when the user tries to send a message:
+  - If the room has unverified devices which the user has not yet been told about in the context of this room
+    ...or in the context of this user?  currently all verification is per-user, not per-room.
+    ...this should be good enough.
+
+  - so track whether we have warned the user or not about unverified devices - blocked, unverified, verified, unverified_warned.
+    throw an error when trying to encrypt if there are pure unverified devices there
+    app will have to search for the devices which are pure unverified to warn about them - have to do this from MembersList anyway?
+      - or megolm could warn which devices are causing the problems.
+
+Why do we wait to establish outbound sessions?  It just makes a horrible pause when we first try to send a message... but could otherwise unnecessarily consume resources?

package.json

@@ -9,9 +9,9 @@
     "check": "npm run buildtest && jasmine-node specbuild --verbose --junitreport --captureExceptions",
     "gendoc": "jsdoc -r lib -P package.json -R README.md -d .jsdoc",
     "start": "babel -s -w -d lib src",
-    "build": "babel -s -d lib src && rimraf dist && mkdir dist && browserify browser-index.js -o dist/browser-matrix.js && uglifyjs -c -m -o dist/browser-matrix.min.js dist/browser-matrix.js",
+    "build": "babel -s -d lib src && rimraf dist && mkdir dist && browserify -d browser-index.js | exorcist dist/browser-matrix.js.map > dist/browser-matrix.js && uglifyjs -c -m -o dist/browser-matrix.min.js --source-map dist/browser-matrix.min.js.map --in-source-map dist/browser-matrix.js.map dist/browser-matrix.js",
     "dist": "npm run build",
-    "watch": "watchify browser-index.js -o dist/browser-matrix-dev.js -v",
+    "watch": "watchify -d browser-index.js -o 'exorcist dist/browser-matrix.js.map > dist/browser-matrix.js' -v",
     "lint": "eslint --max-warnings 121 src spec",
     "prepublish": "npm run build && git rev-parse HEAD > git-revision.txt"
   },
@@ -25,6 +25,7 @@
   "author": "matrix.org",
   "license": "Apache-2.0",
   "files": [
+    ".babelrc",
     ".eslintrc.js",
     "spec/.eslintrc.js",
     "CHANGELOG.md",
@@ -54,14 +55,16 @@
     "babel-cli": "^6.18.0",
     "babel-eslint": "^7.1.1",
     "babel-preset-es2015": "^6.18.0",
-    "browserify": "^10.2.3",
+    "browserify": "^14.0.0",
     "browserify-shim": "^3.8.13",
     "eslint": "^3.13.1",
     "eslint-config-google": "^0.7.1",
+    "exorcist": "^0.4.0",
     "istanbul": "^0.3.13",
     "jasmine-node": "^1.14.5",
     "jsdoc": "^3.4.0",
     "rimraf": "^2.5.4",
+    "sourceify": "^0.1.0",
     "uglifyjs": "^2.4.10",
     "watchify": "^3.2.1"
   },
@@ -69,7 +72,10 @@
     "olm": "https://matrix.org/packages/npm/olm/olm-2.2.1.tgz"
   },
   "browserify": {
-    "transform": "browserify-shim"
+    "transform": [
+      "sourceify",
+      "browserify-shim"
+    ]
   },
   "browserify-shim": {
     "olm": "global:Olm"

spec/integ/megolm.spec.js

@@ -97,7 +97,10 @@ TestClient.prototype.start = function(existingDevices) {
         }};
     });
 
-    this.client.startClient();
+    this.client.startClient({
+        // set this so that we can get hold of failed events
+        pendingEventOrdering: 'detached',
+    });
 
     return this.httpBackend.flush();
 };
@@ -533,11 +536,24 @@ describe("megolm", function() {
             aliceTestClient.httpBackend.when('GET', '/sync').respond(200, syncResponse);
             return aliceTestClient.httpBackend.flush('/sync', 1);
         }).then(function() {
-            let inboundGroupSession;
+            // start out with the device unknown - the send should be rejected.
             aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
                 200, getTestKeysQueryResponse('@bob:xyz'),
             );
 
+            return q.all([
+                aliceTestClient.client.sendTextMessage(ROOM_ID, 'test').then(() => {
+                    throw new Error("sendTextMessage failed on an unknown device");
+                }, (e) => {
+                    expect(e.name).toEqual("UnknownDeviceError");
+                }),
+                aliceTestClient.httpBackend.flush(),
+            ]);
+        }).then(function() {
+            // mark the device as known, and resend.
+            aliceTestClient.client.setDeviceKnown('@bob:xyz', 'DEVICE_ID');
+
+            let inboundGroupSession;
             aliceTestClient.httpBackend.when(
                 'PUT', '/sendToDevice/m.room.encrypted/',
             ).respond(200, function(path, content) {
@@ -568,8 +584,11 @@ describe("megolm", function() {
                 };
             });
 
+            const room = aliceTestClient.client.getRoom(ROOM_ID);
+            const pendingMsg = room.getPendingEvents()[0];
+
             return q.all([
-                aliceTestClient.client.sendTextMessage(ROOM_ID, 'test'),
+                aliceTestClient.client.resendEvent(pendingMsg, room),
                 aliceTestClient.httpBackend.flush(),
             ]);
         }).nodeify(done);
@@ -678,12 +697,21 @@ describe("megolm", function() {
 
             return aliceTestClient.httpBackend.flush('/sync', 1);
         }).then(function() {
-            console.log('Telling alice to send a megolm message');
+            console.log("Fetching bob's devices and marking known");
 
             aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
                 200, getTestKeysQueryResponse('@bob:xyz'),
             );
 
+            return q.all([
+                aliceTestClient.client.downloadKeys(['@bob:xyz']),
+                aliceTestClient.httpBackend.flush(),
+            ]).then((keys) => {
+                aliceTestClient.client.setDeviceKnown('@bob:xyz', 'DEVICE_ID');
+            });
+        }).then(function() {
+            console.log('Telling alice to send a megolm message');
+
             aliceTestClient.httpBackend.when(
                 'PUT', '/sendToDevice/m.room.encrypted/',
             ).respond(200, function(path, content) {
@@ -737,7 +765,7 @@ describe("megolm", function() {
     // https://github.com/vector-im/riot-web/issues/2676
     it("Alice should send to her other devices", function(done) {
         // for this test, we make the testOlmAccount be another of Alice's devices.
-        // it ought to get include in messages Alice sends.
+        // it ought to get included in messages Alice sends.
 
         let p2pSession;
         let inboundGroupSession;
@@ -774,6 +802,7 @@ describe("megolm", function() {
 
             return aliceTestClient.httpBackend.flush();
         }).then(function() {
+            aliceTestClient.client.setDeviceKnown(aliceTestClient.userId, 'DEVICE_ID');
             aliceTestClient.httpBackend.when('POST', '/keys/claim').respond(
                 200, function(path, content) {
                 expect(content.one_time_keys[aliceTestClient.userId].DEVICE_ID)
@@ -882,10 +911,15 @@ describe("megolm", function() {
 
             // this will block
             downloadPromise = aliceTestClient.client.downloadKeys(['@bob:xyz']);
-        }).then(function() {
+
             // so will this.
-            sendPromise = aliceTestClient.client.sendTextMessage(ROOM_ID, 'test');
-        }).then(function() {
+            sendPromise = aliceTestClient.client.sendTextMessage(ROOM_ID, 'test')
+            .then(() => {
+                throw new Error("sendTextMessage failed on an unknown device");
+            }, (e) => {
+                expect(e.name).toEqual("UnknownDeviceError");
+            });
+
             aliceTestClient.httpBackend.when('POST', '/keys/query').respond(
                 200, getTestKeysQueryResponse('@bob:xyz'),
             );

src/client.js

@@ -395,11 +395,29 @@ MatrixClient.prototype.setDeviceBlocked = function(userId, deviceId, blocked) {
     _setDeviceVerification(this, userId, deviceId, null, blocked);
 };
 
-function _setDeviceVerification(client, userId, deviceId, verified, blocked) {
+/**
+ * Mark the given device as known/unknown
+ *
+ * @param {string} userId owner of the device
+ * @param {string} deviceId unique identifier for the device
+ *
+ * @param {boolean=} known whether to mark the device as known. defaults
+ *   to 'true'.
+ *
+ * @fires module:client~event:MatrixClient"deviceVerificationChanged"
+ */
+MatrixClient.prototype.setDeviceKnown = function(userId, deviceId, known) {
+    if (known === undefined) {
+        known = true;
+    }
+    _setDeviceVerification(this, userId, deviceId, null, null, known);
+};
+
+function _setDeviceVerification(client, userId, deviceId, verified, blocked, known) {
     if (!client._crypto) {
         throw new Error("End-to-End encryption disabled");
     }
-    client._crypto.setDeviceVerification(userId, deviceId, verified, blocked);
+    client._crypto.setDeviceVerification(userId, deviceId, verified, blocked, known);
     client.emit("deviceVerificationChanged", userId, deviceId);
 }
 

src/crypto/DeviceList.js

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 OpenMarket Ltd
+Copyright 2017 Vector Creations Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

src/crypto/algorithms/base.js

@@ -157,6 +157,23 @@ module.exports.DecryptionError = function(msg) {
 };
 utils.inherits(module.exports.DecryptionError, Error);
 
+/**
+ * Exception thrown specifically when we want to warn the user to consider
+ * the security of their conversation before continuing
+ *
+ * @constructor
+ * @param {string} msg message describing the problem
+ * @param {Object} devices userId -> {deviceId -> object}
+ *      set of unknown devices per user we're warning about
+ * @extends Error
+ */
+module.exports.UnknownDeviceError = function(msg, devices) {
+    this.name = "UnknownDeviceError";
+    this.message = msg;
+    this.devices = devices;
+};
+utils.inherits(module.exports.UnknownDeviceError, Error);
+
 /**
  * Registers an encryption/decryption class for a particular algorithm
  *

src/crypto/algorithms/megolm.js

@@ -388,6 +388,10 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session, devicesByUse
 MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) {
     const self = this;
     return this._getDevicesInRoom(room).then(function(devicesInRoom) {
+        // check if any of these devices are not yet known to the user.
+        // if so, warn the user so they can verify or ignore.
+        self._checkForUnknownDevices(devicesInRoom);
+
         return self._ensureOutboundSession(devicesInRoom);
     }).then(function(session) {
         const payloadJson = {
@@ -415,6 +419,37 @@ MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) {
     });
 };
 
+/**
+ * Checks the devices we're about to send to and see if any are entirely
+ * unknown to the user.  If so, warn the user, and mark them as known to
+ * give the user a chance to go verify them before re-sending this message.
+ *
+ * @param {Object} devicesInRoom userId -> {deviceId -> object}
+ *   devices we should shared the session with.
+ */
+MegolmEncryption.prototype._checkForUnknownDevices = function(devicesInRoom) {
+    const unknownDevices = {};
+
+    Object.keys(devicesInRoom).forEach((userId)=>{
+        Object.keys(devicesInRoom[userId]).forEach((deviceId)=>{
+            const device = devicesInRoom[userId][deviceId];
+            if (device.isUnverified() && !device.isKnown()) {
+                if (!unknownDevices[userId]) {
+                    unknownDevices[userId] = {};
+                }
+                unknownDevices[userId][deviceId] = device;
+            }
+        });
+    });
+
+    if (Object.keys(unknownDevices).length) {
+        // it'd be kind to pass unknownDevices up to the user in this error
+        throw new base.UnknownDeviceError(
+            "This room contains unknown devices which have not been verified. " +
+            "We strongly recommend you verify them before continuing.", unknownDevices);
+    }
+};
+
 /**
  * Get the list of unblocked devices for all users in the room
  *
@@ -433,6 +468,11 @@ MegolmEncryption.prototype._getDevicesInRoom = function(room) {
     // have a list of the user's devices, then we already share an e2e room
     // with them, which means that they will have announced any new devices via
     // an m.new_device.
+    //
+    // XXX: what if the cache is stale, and the user left the room we had in
+    // common and then added new devices before joining this one? --Matthew
+    //
+    // yup, see https://github.com/vector-im/riot-web/issues/2305 --richvdh
     return this._crypto.downloadKeys(roomMembers, false).then(function(devices) {
         // remove any blocked devices
         for (const userId in devices) {

src/crypto/deviceinfo.js

@@ -34,7 +34,11 @@ limitations under the License.
   *      <key type>:<id> -> <base64-encoded key>>
   *
   * @property {module:crypto/deviceinfo.DeviceVerification} verified
-  *     whether the device has been verified by the user
+  *     whether the device has been verified/blocked by the user
+  *
+  * @property {boolean} known
+  *     whether the user knows of this device's existence (useful when warning
+  *     the user that a user has added new devices)
   *
   * @property {Object} unsigned  additional data from the homeserver
   *
@@ -50,6 +54,7 @@ function DeviceInfo(deviceId) {
     this.algorithms = [];
     this.keys = {};
     this.verified = DeviceVerification.UNVERIFIED;
+    this.known = false;
     this.unsigned = {};
 }
 
@@ -81,6 +86,7 @@ DeviceInfo.prototype.toStorage = function() {
         algorithms: this.algorithms,
         keys: this.keys,
         verified: this.verified,
+        known: this.known,
         unsigned: this.unsigned,
     };
 };
@@ -130,6 +136,24 @@ DeviceInfo.prototype.isVerified = function() {
     return this.verified == DeviceVerification.VERIFIED;
 };
 
+/**
+ * Returns true if this device is unverified
+ *
+ * @return {Boolean} true if unverified
+ */
+DeviceInfo.prototype.isUnverified = function() {
+    return this.verified == DeviceVerification.UNVERIFIED;
+};
+
+/**
+ * Returns true if the user knows about this device's existence
+ *
+ * @return {Boolean} true if known
+ */
+DeviceInfo.prototype.isKnown = function() {
+    return this.known == true;
+};
+
 /**
  * @enum
  */

src/crypto/index.js

@@ -94,6 +94,7 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId) {
             keys: this._deviceKeys,
             algorithms: this._supportedAlgorithms,
             verified: DeviceVerification.VERIFIED,
+            known: true,
         };
 
         myDevices[this._deviceId] = deviceInfo;
@@ -362,8 +363,12 @@ Crypto.prototype.listDeviceKeys = function(userId) {
  *
  * @param {?boolean} blocked whether to mark the device as blocked. Null to
  *      leave unchanged.
+ *
+ * @param {?boolean} known whether to mark that the user has been made aware of
+ *      the existence of this device. Null to leave unchanged
  */
-Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified, blocked) {
+Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified,
+                                                  blocked, known) {
     const devices = this._sessionStore.getEndToEndDevicesForUser(userId);
     if (!devices || !devices[deviceId]) {
         throw new Error("Unknown device " + userId + ":" + deviceId);
@@ -384,10 +389,16 @@ Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified, bl
         verificationStatus = DeviceVerification.UNVERIFIED;
     }
 
-    if (dev.verified === verificationStatus) {
+    let knownStatus = dev.known;
+    if (known !== null && known !== undefined) {
+        knownStatus = known;
+    }
+
+    if (dev.verified === verificationStatus && dev.known === knownStatus) {
         return;
     }
     dev.verified = verificationStatus;
+    dev.known = knownStatus;
     this._sessionStore.storeEndToEndDevicesForUser(userId, devices);
 };