From a4f192bc88a4aee195e3274afee235fee5db8b0f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Oct 2016 12:17:34 +0100 Subject: [PATCH] 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 | 12 ++- lib/crypto/index.js | 130 +++++++++++++++++------- spec/integ/matrix-client-crypto.spec.js | 9 +- 3 files changed, 109 insertions(+), 42 deletions(-) diff --git a/lib/base-apis.js b/lib/base-apis.js index ed6ce9188..bfed201a7 100644 --- a/lib/base-apis.js +++ b/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 ); }; diff --git a/lib/crypto/index.js b/lib/crypto/index.js index b4e599035..0e7dafefa 100644 --- a/lib/crypto/index.js +++ b/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, }; - - var sig = crypto._olmDevice.sign(anotherjson.stringify(deviceKeys)); - deviceKeys.signatures = {}; - deviceKeys.signatures[userId] = {}; - deviceKeys.signatures[userId]["ed25519:" + deviceId] = sig; + crypto._signObject(deviceKeys); 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; - } else { - console.warn("No one-time keys for device " + - userId + ":" + deviceId); + if (!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; } 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 */ diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index 8afabee92..14207eafe 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/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; } }