Merge pull request #243 from matrix-org/rav/sign_one_time_keys

Sign one-time keys, and verify their signatures

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(
-                    deviceInfo.getIdentityKey(), oneTimeKey
-                );
-                console.log("Started new sessionid " + sid +
-                            " for device " + userId + ":" + deviceId);
 
-                result[userId][deviceId].sessionId = sid;
+            if (!oneTimeKey) {
-            } else {
+                console.warn(
-                console.warn("No one-time keys for device " +
+                    "No one-time keys (alg=" + oneTimeKeyAlgorithm +
-                             userId + ":" + deviceId);
+                        ") 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;
         }
         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;
                 }
             }