From 7457da80e93263b056b6dacea45d87cf7e1d8c1b Mon Sep 17 00:00:00 2001
From: "J. Ryan Stinnett" <jryans@gmail.com>
Date: Thu, 12 Dec 2019 13:18:32 +0000
Subject: [PATCH] Clean up backup trust checks

There were several inaccurate comments and redundant code paths around backup
trust checks.
---
 src/client.js        |  8 ++++----
 src/crypto/index.js  | 13 ++++++-------
 src/crypto/olmlib.js |  3 +--
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/client.js b/src/client.js
index aebf18ec6..4ba045118 100644
--- a/src/client.js
+++ b/src/client.js
@@ -1492,13 +1492,13 @@ MatrixClient.prototype.createKeyBackupVersion = async function(info) {
         auth_data: info.auth_data,
     };
 
-    // Now sign the backup auth data. Do it as this device first because crypto._signObject
-    // is dumb and bluntly replaces the whole signatures block...
-    // this can probably go away very soon in favour of just signing with the SSK.
+    // Sign the backup auth data with the device key for backwards compat with
+    // older devices with cross-signing. This can probably go away very soon in
+    // favour of just signing with the cross-singing master key.
     await this._crypto._signObject(data.auth_data);
 
     if (this._crypto._crossSigningInfo.getId()) {
-        // now also sign the auth data with the master key
+        // now also sign the auth data with the cross-signing master key
         await this._crypto._crossSigningInfo.signObject(data.auth_data, "master");
     }
 
diff --git a/src/crypto/index.js b/src/crypto/index.js
index e38af407b..918f3cfb4 100644
--- a/src/crypto/index.js
+++ b/src/crypto/index.js
@@ -999,12 +999,13 @@ Crypto.prototype.isKeyBackupTrusted = async function(backupInfo) {
             logger.log("Ignoring unknown signature type: " + keyIdParts[0]);
             continue;
         }
-        // Could be an SSK but just say this is the device ID for backwards compat
-        const sigInfo = { deviceId: keyIdParts[1] }; // XXX: is this how we're supposed to get the device ID?
+        // Could be a cross-signing master key, but just say this is the device
+        // ID for backwards compat
+        const sigInfo = { deviceId: keyIdParts[1] };
 
         // first check to see if it's from our cross-signing key
         const crossSigningId = this._crossSigningInfo.getId();
-        if (crossSigningId === keyId) {
+        if (crossSigningId === sigInfo.deviceId) {
             sigInfo.cross_signing_key = crossSigningId;
             try {
                 await olmlib.verifySignature(
@@ -1027,7 +1028,7 @@ Crypto.prototype.isKeyBackupTrusted = async function(backupInfo) {
 
         // Now look for a sig from a device
         // At some point this can probably go away and we'll just support
-        // it being signed by the SSK
+        // it being signed by the cross-signing master key
         const device = this._deviceList.getStoredDevice(
             this._userId, sigInfo.deviceId,
         );
@@ -1036,9 +1037,7 @@ Crypto.prototype.isKeyBackupTrusted = async function(backupInfo) {
             try {
                 await olmlib.verifySignature(
                     this._olmDevice,
-                    // verifySignature modifies the object so we need to copy
-                    // if we verify more than one sig
-                    Object.assign({}, backupInfo.auth_data),
+                    backupInfo.auth_data,
                     this._userId,
                     device.deviceId,
                     device.getFingerprint(),
diff --git a/src/crypto/olmlib.js b/src/crypto/olmlib.js
index ab1b13e96..e00431d93 100644
--- a/src/crypto/olmlib.js
+++ b/src/crypto/olmlib.js
@@ -303,8 +303,7 @@ async function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceIn
  *
  * @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 {Object} obj object to check signature on.
  *
  * @param {string} signingUserId  ID of the user whose signature should be checked
  *