Sign one-time keys, and verify their signatures

We have decided that signing one-time keys is the lesser of two evils;
accordingly, use a new key algorithm type (`signed_curve25519`), sign the
one-time keys that we upload to the server, and verify the signatures on those
we download.

This will mean that develop won't be able to talk to master, but hey, we're in
beta.

lib/base-apis.js

@@ -976,24 +976,28 @@ MatrixBaseApis.prototype.downloadKeysForUsers = function(userIds, callback) {
  *
  * @param {string[][]} devices  a list of [userId, deviceId] pairs
  *
- * @param {module:client.callback=} callback
+ * @param {string} [key_algorithm = signed_curve25519]  desired key type
  *
  * @return {module:client.Promise} Resolves: result object. Rejects: with
  *     an error response ({@link module:http-api.MatrixError}).
  */
-MatrixBaseApis.prototype.claimOneTimeKeys = function(devices, callback) {
+MatrixBaseApis.prototype.claimOneTimeKeys = function(devices, key_algorithm) {
     var queries = {};
 
+    if (key_algorithm === undefined) {
+        key_algorithm = "signed_curve25519";
+    }
+
     for (var i = 0; i < devices.length; ++i) {
         var userId = devices[i][0];
         var deviceId = devices[i][1];
         var query = queries[userId] || {};
         queries[userId] = query;
-        query[deviceId] = "curve25519";
+        query[deviceId] = key_algorithm;
     }
     var content = {one_time_keys: queries};
     return this._http.authedRequestWithPrefix(
-        callback, "POST", "/keys/claim", undefined, content,
+        undefined, "POST", "/keys/claim", undefined, content,
         httpApi.PREFIX_UNSTABLE
     );
 };

lib/crypto/index.js

@@ -174,7 +174,7 @@ Crypto.prototype.uploadKeys = function(maxKeys) {
         // these factors.
 
         // We first find how many keys the server has for us.
-        var keyCount = res.one_time_key_counts.curve25519 || 0;
+        var keyCount = res.one_time_key_counts.signed_curve25519 || 0;
         // We then check how many keys we can store in the Account object.
         var maxOneTimeKeys = self._olmDevice.maxNumberOfOneTimeKeys();
         // Try to keep at most half that number on the server. This leaves the
@@ -217,11 +217,7 @@ function _uploadDeviceKeys(crypto) {
         keys: crypto._deviceKeys,
         user_id: userId,
     };
-
+    crypto._signObject(deviceKeys);
-    var sig = crypto._olmDevice.sign(anotherjson.stringify(deviceKeys));
-    deviceKeys.signatures = {};
-    deviceKeys.signatures[userId] = {};
-    deviceKeys.signatures[userId]["ed25519:" + deviceId] = sig;
 
     return crypto._baseApis.uploadKeysRequest({
         device_keys: deviceKeys,
@@ -239,9 +235,14 @@ function _uploadOneTimeKeys(crypto) {
 
     for (var keyId in oneTimeKeys.curve25519) {
         if (oneTimeKeys.curve25519.hasOwnProperty(keyId)) {
-            oneTimeJson["curve25519:" + keyId] = oneTimeKeys.curve25519[keyId];
+            var k = {
+                key: oneTimeKeys.curve25519[keyId],
+            };
+            crypto._signObject(k);
+            oneTimeJson["signed_curve25519:" + keyId] = k;
         }
     }
+
     return crypto._baseApis.uploadKeysRequest({
         one_time_keys: oneTimeJson
     }, {
@@ -396,23 +397,9 @@ function _storeDeviceKeys(_olmDevice, userStore, deviceResult) {
     }
 
     var unsigned = deviceResult.unsigned || {};
-    var signatures = deviceResult.signatures || {};
-    var userSigs = signatures[userId] || {};
-    var signature = userSigs[signKeyId];
-    if (!signature) {
-        console.log("Device " + userId + ":" + deviceId +
-                    " is not signed");
-        return false;
-    }
-
-    // prepare the canonical json: remove 'unsigned' and signatures, and
-    // stringify with anotherjson
-    delete deviceResult.unsigned;
-    delete deviceResult.signatures;
-    var json = anotherjson.stringify(deviceResult);
 
     try {
-        _olmDevice.verifySignature(signKey, json, signature);
+        _verifySignature(_olmDevice, deviceResult, userId, deviceId, signKey);
     } catch (e) {
         console.log("Unable to verify signature on device " +
                     userId + ":" + deviceId + ":", e);
@@ -785,8 +772,9 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) {
     // That should eventually resolve itself, but it's poor form.
 
     var self = this;
+    var oneTimeKeyAlgorithm = "signed_curve25519";
     return this._baseApis.claimOneTimeKeys(
-        devicesWithoutSession
+        devicesWithoutSession, oneTimeKeyAlgorithm
     ).then(function(res) {
         for (var i = 0; i < devicesWithoutSession.length; ++i) {
             var device = devicesWithoutSession[i];
@@ -798,22 +786,48 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) {
             var deviceRes = userRes[deviceId];
             var oneTimeKey = null;
             for (var keyId in deviceRes) {
-                if (keyId.indexOf("curve25519:") === 0) {
+                if (keyId.indexOf(oneTimeKeyAlgorithm + ":") === 0) {
                     oneTimeKey = deviceRes[keyId];
                 }
             }
-            if (oneTimeKey) {
+
-                var sid = self._olmDevice.createOutboundSession(
+            if (!oneTimeKey) {
-                    deviceInfo.getIdentityKey(), oneTimeKey
+                console.warn(
+                    "No one-time keys (alg=" + oneTimeKeyAlgorithm +
+                        ") for device " + userId + ":" + deviceId
                 );
+                continue;
+            }
+
+            try {
+                _verifySignature(
+                    self._olmDevice, oneTimeKey, userId, deviceId,
+                    deviceInfo.getFingerprint()
+                );
+            } catch (e) {
+                console.log(
+                    "Unable to verify signature on one-time key for device " +
+                        userId + ":" + deviceId + ":", e
+                );
+                continue;
+            }
+
+            var sid;
+            try {
+                sid = self._olmDevice.createOutboundSession(
+                    deviceInfo.getIdentityKey(), oneTimeKey.key
+                );
+            } catch (e) {
+                // possibly a bad key
+                console.error("Error starting session with device " +
+                              userId + ":" + deviceId + ": " + e);
+                continue;
+            }
+
             console.log("Started new sessionid " + sid +
                         " for device " + userId + ":" + deviceId);
 
             result[userId][deviceId].sessionId = sid;
-            } else {
-                console.warn("No one-time keys for device " +
-                             userId + ":" + deviceId);
-            }
         }
         return result;
     });
@@ -1181,6 +1195,54 @@ Crypto.prototype._onNewDeviceEvent = function(event) {
     }).done();
 };
 
+
+/**
+ * sign the given object with our ed25519 key
+ *
+ * @param {Object} obj  Object to which we will add a 'signatures' property
+ */
+Crypto.prototype._signObject = function(obj) {
+    var sigs = {};
+    sigs[this._userId] = {};
+    sigs[this._userId]["ed25519:" + this._deviceId] =
+        this._olmDevice.sign(anotherjson.stringify(obj));
+    obj.signatures = sigs;
+};
+
+/**
+ * Verify the signature on an object
+ *
+ * @param {module:crypto/OlmDevice} olmDevice olm wrapper to use for verify op
+ *
+ * @param {Object} obj object to check signature on. Note that this will be
+ * stripped of its 'signatures' and 'unsigned' properties.
+ *
+ * @param {string} signingUserId  ID of the user whose signature should be checked
+ *
+ * @param {string} signingDeviceId  ID of the device whose signature should be checked
+ *
+ * @param {string} signingKey   base64-ed ed25519 public key
+ */
+function _verifySignature(olmDevice, obj, signingUserId, signingDeviceId, signingKey) {
+    var signKeyId = "ed25519:" + signingDeviceId;
+    var signatures = obj.signatures || {};
+    var userSigs = signatures[signingUserId] || {};
+    var signature = userSigs[signKeyId];
+    if (!signature) {
+        throw Error("No signature");
+    }
+
+    // prepare the canonical json: remove unsigned and signatures, and stringify with
+    // anotherjson
+    delete obj.unsigned;
+    delete obj.signatures;
+    var json = anotherjson.stringify(obj);
+
+    olmDevice.verifySignature(
+        signingKey, json, signature
+    );
+}
+
 /**
  * @see module:crypto/algorithms/base.DecryptionError
  */

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

@@ -61,7 +61,7 @@ function expectKeyUpload(deviceId, httpBackend) {
         expect(content.one_time_keys).not.toBeDefined();
         expect(content.device_keys).toBeDefined();
         keys.device_keys = content.device_keys;
-        return {one_time_key_counts: {curve25519: 0}};
+        return {one_time_key_counts: {signed_curve25519: 0}};
     });
 
     httpBackend.when("POST", uploadPath).respond(200, function(path, content) {
@@ -76,7 +76,7 @@ function expectKeyUpload(deviceId, httpBackend) {
         }
         expect(count).toEqual(5);
         keys.one_time_keys = content.one_time_keys;
-        return {one_time_key_counts: {curve25519: count}};
+        return {one_time_key_counts: {signed_curve25519: count}};
     });
 
     return httpBackend.flush(uploadPath, 2).then(function() {
@@ -176,10 +176,11 @@ function expectAliClaimKeys() {
     expect(bobOneTimeKeys).toBeDefined();
 
     aliHttpBackend.when("POST", "/keys/claim").respond(200, function(path, content) {
-        expect(content.one_time_keys[bobUserId][bobDeviceId]).toEqual("curve25519");
+        var claimType = content.one_time_keys[bobUserId][bobDeviceId];
+        expect(claimType).toEqual("signed_curve25519");
         for (var keyId in bobOneTimeKeys) {
             if (bobOneTimeKeys.hasOwnProperty(keyId)) {
-                if (keyId.indexOf("curve25519:") === 0) {
+                if (keyId.indexOf(claimType + ":") === 0) {
                     break;
                 }
             }