From aafb1ffdef31d54fb5eb1d0650f21dc8b23151cb Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 18 Oct 2016 13:40:13 +0100
Subject: [PATCH] Consistency checks for E2E device downloads

Check that the user_id and device_id in device query responses match those that
we expect.

This resolves an unknown-key attack whereby Eve can re-sign Bob's keys with her
own key, thus getting Alice to send her messages which she can then forward to
Bob, making Bob think that Alice sent the messages to him.
---
 lib/crypto/index.js                     | 24 +++++++--
 spec/integ/matrix-client-crypto.spec.js | 71 +++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/lib/crypto/index.js b/lib/crypto/index.js
index 5a641c09d..afa5a7c8f 100644
--- a/lib/crypto/index.js
+++ b/lib/crypto/index.js
@@ -350,9 +350,22 @@ function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore,
             continue;
         }
 
-        if (_storeDeviceKeys(
-            _olmDevice, userId, deviceId, userStore, userResult[deviceId]
-        )) {
+        var deviceResult = userResult[deviceId];
+
+        // check that the user_id and device_id in the response object are
+        // correct
+        if (deviceResult.user_id !== userId) {
+            console.warn("Mismatched user_id " + deviceResult.user_id +
+                         " in keys from " + userId + ":" + deviceId);
+            continue;
+        }
+        if (deviceResult.device_id !== deviceId) {
+            console.warn("Mismatched device_id " + deviceResult.device_id +
+                         " in keys from " + userId + ":" + deviceId);
+            continue;
+        }
+
+        if (_storeDeviceKeys(_olmDevice, userStore, deviceResult)) {
             updated = true;
         }
     }
@@ -365,12 +378,15 @@ function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore,
  *
  * returns true if a change was made, else false
  */
-function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult) {
+function _storeDeviceKeys(_olmDevice, userStore, deviceResult) {
     if (!deviceResult.keys) {
         // no keys?
         return false;
     }
 
+    var deviceId = deviceResult.device_id;
+    var userId = deviceResult.user_id;
+
     var signKeyId = "ed25519:" + deviceId;
     var signKey = deviceResult.keys[signKeyId];
     if (!signKey) {
diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js
index be121eaf4..a5b6195d6 100644
--- a/spec/integ/matrix-client-crypto.spec.js
+++ b/spec/integ/matrix-client-crypto.spec.js
@@ -529,6 +529,77 @@ describe("MatrixClient crypto", function() {
             .catch(test_utils.failTest).done(done);
     });
 
+    it("Ali gets keys with an incorrect userId", function(done) {
+        var eveUserId = "@eve:localhost";
+
+        var bobDeviceKeys = {
+            algorithms: ['m.olm.v1.curve25519-aes-sha2', 'm.megolm.v1.aes-sha2'],
+            device_id: 'bvcxz',
+            keys: {
+                'ed25519:bvcxz': 'pYuWKMCVuaDLRTM/eWuB8OlXEb61gZhfLVJ+Y54tl0Q',
+                'curve25519:bvcxz': '7Gni0loo/nzF0nFp9847RbhElGewzwUXHPrljjBGPTQ',
+            },
+            user_id: '@eve:localhost',
+            signatures: {
+                '@eve:localhost': {
+                    'ed25519:bvcxz': 'CliUPZ7dyVPBxvhSA1d+X+LYa5b2AYdjcTwG' +
+                        '0stXcIxjaJNemQqtdgwKDtBFl3pN2I13SEijRDCf1A8bYiQMDg',
+                },
+            },
+        };
+
+        var bobKeys = {};
+        bobKeys[bobDeviceId] = bobDeviceKeys;
+        aliHttpBackend.when("POST", "/keys/query").respond(200, function(path, content) {
+            var result = {};
+            result[bobUserId] = bobKeys;
+            return {device_keys: result};
+        });
+
+        q.all(
+            aliClient.downloadKeys([bobUserId, eveUserId]),
+            aliHttpBackend.flush("/keys/query", 1)
+        ).then(function() {
+            // should get an empty list
+            expect(aliClient.listDeviceKeys(bobUserId)).toEqual([]);
+            expect(aliClient.listDeviceKeys(eveUserId)).toEqual([]);
+        }).catch(test_utils.failTest).done(done);
+    });
+
+    it("Ali gets keys with an incorrect deviceId", function(done) {
+        var bobDeviceKeys = {
+            algorithms: ['m.olm.v1.curve25519-aes-sha2', 'm.megolm.v1.aes-sha2'],
+            device_id: 'bad_device',
+            keys: {
+                'ed25519:bad_device': 'e8XlY5V8x2yJcwa5xpSzeC/QVOrU+D5qBgyTK0ko+f0',
+                'curve25519:bad_device': 'YxuuLG/4L5xGeP8XPl5h0d7DzyYVcof7J7do+OXz0xc',
+            },
+            user_id: '@bob:localhost',
+            signatures: {
+                '@bob:localhost': {
+                    'ed25519:bad_device': 'fEFTq67RaSoIEVBJ8DtmRovbwUBKJ0A' +
+                        'me9m9PDzM9azPUwZ38Xvf6vv1A7W1PSafH4z3Y2ORIyEnZgHaNby3CQ',
+                },
+            },
+        };
+
+        var bobKeys = {};
+        bobKeys[bobDeviceId] = bobDeviceKeys;
+        aliHttpBackend.when("POST", "/keys/query").respond(200, function(path, content) {
+            var result = {};
+            result[bobUserId] = bobKeys;
+            return {device_keys: result};
+        });
+
+        q.all(
+            aliClient.downloadKeys([bobUserId]),
+            aliHttpBackend.flush("/keys/query", 1)
+        ).then(function() {
+            // should get an empty list
+            expect(aliClient.listDeviceKeys(bobUserId)).toEqual([]);
+        }).catch(test_utils.failTest).done(done);
+    });
+
     it("Ali enables encryption", function(done) {
         q()
             .then(bobUploadsKeys)