Query missing cross-signing keys on local sig upload

changelog.d/7289.bugfix

@@ -1 +1 @@
-Fix a bug with cross-signing devices with remote users when they did not share a room with any user on the local homeserver.
+Fix a bug with cross-signing devices belonging to remote users who did not share a room with any user on the local homeserver.

synapse/federation/transport/client.py

@@ -407,12 +407,12 @@ def query_client_keys(self, destination, query_content, timeout):
                 "<user_id>": {
                   "<device_id>": {...}
               } }
-              "master_keys": {
+              "master_key": {
                 "<user_id>": {...}
               } }
-              "self_signing_keys": {
+              "self_signing_key": {
                 "<user_id>": {...}
-            } } }
+            } }
 
         Args:
             destination(str): The server to query.
@@ -436,9 +436,22 @@ def query_user_devices(self, destination, user_id, timeout):
             {
               "stream_id": "...",
               "devices": [ { ... } ],
-              "master_key": { ... },
-              "self_signing_key: { ... }
-            }
+              "master_key": {
+                "user_id": "<user_id>",
+                "usage": [...],
+                "keys": {...},
+                "signatures": {
+                  "<user_id>": {...}
+                }
+              },
+              "self_signing_key": {
+                "user_id": "<user_id>",
+                "usage": [...],
+                "keys": {...},
+                "signatures": {
+                  "<user_id>": {...}
+                }
+            } }
 
         Args:
             destination(str): The server to query.

synapse/handlers/e2e_keys.py

@@ -989,40 +989,33 @@ def _get_e2e_cross_signing_verify_key(
             user_id, key_type, from_user_id
         )
 
+        if key:
+            # We found a copy of this key in our database. Decode and return it
+            key_id, verify_key = get_verify_key_from_cross_signing_key(key)
+            return key, key_id, verify_key
+
         # If we couldn't find the key locally, and we're looking for keys of
         # another user then attempt to fetch the missing key from the remote
         # user's server.
         #
         # We may run into this in possible edge cases where a user tries to
         # cross-sign a remote user, but does not share any rooms with them yet.
-        # Thus, we would not have their key list yet. We fetch the key here,
+        # Thus, we would not have their key list yet. We instead fetch the key,
         # store it and notify clients of new, associated device IDs.
-        if (
-            key is None
-            and not self.is_mine(user)
-            # We only get "master" and "self_signing" keys from remote servers
-            and key_type in ["master", "self_signing"]
-        ):
-            (
-                key,
-                key_id,
-                verify_key,
-            ) = yield self._retrieve_cross_signing_keys_for_remote_user(user, key_type)
+        if self.is_mine(user) or key_type not in ["master", "self_signing"]:
+            # Note that master and self_signing keys are the only cross-signing keys we
+            # can request over federation
+            return
+
+        (
+            key,
+            key_id,
+            verify_key,
+        ) = yield self._retrieve_cross_signing_keys_for_remote_user(user, key_type)
 
         if key is None:
-            logger.warning("No %s key found for %s", key_type, user_id)
             raise NotFoundError("No %s key found for %s" % (key_type, user_id))
 
-        try:
-            key_id, verify_key = get_verify_key_from_cross_signing_key(key)
-        except ValueError as e:
-            logger.warning(
-                "Invalid %s key retrieved: %s - %s %s", key_type, key, type(e), e,
-            )
-            raise SynapseError(
-                502, "Invalid %s key retrieved from remote server" % (key_type,)
-            )
-
         return key, key_id, verify_key
 
     @defer.inlineCallbacks
@@ -1058,22 +1051,33 @@ def _retrieve_cross_signing_keys_for_remote_user(
             return None, None, None
 
         # Process each of the retrieved cross-signing keys
-        final_key = None
-        final_key_id = None
-        final_verify_key = None
-        device_ids = []
+        desired_key = None
+        desired_key_id = None
+        desired_verify_key = None
+        retrieved_device_ids = []
         for key_type in ["master", "self_signing"]:
             key_content = remote_result.get(key_type + "_key")
             if not key_content:
                 continue
 
-            # At the same time, store this key in the db for
-            # subsequent queries
-            yield self.store.set_e2e_cross_signing_key(
-                user.to_string(), key_type, key_content
-            )
+            # Ensure these keys belong to the correct user
+            if "user_id" not in key_content:
+                logger.warning(
+                    "Invalid %s key retrieved, missing user_id field: %s",
+                    key_type,
+                    key_content,
+                )
+                continue
+            if user.to_string() != key_content["user_id"]:
+                logger.warning(
+                    "Found %s key of user %s when querying for keys of user %s",
+                    key_type,
+                    key_content["user_id"],
+                    user.to_string(),
+                )
+                continue
 
-            # Note down the device ID attached to this key
+            # Validate the key contents
             try:
                 # verify_key is a VerifyKey from signedjson, which uses
                 # .version to denote the portion of the key ID after the
@@ -1088,19 +1092,28 @@ def _retrieve_cross_signing_keys_for_remote_user(
                     e,
                 )
                 continue
-            device_ids.append(verify_key.version)
+
+            # Note down the device ID attached to this key
+            retrieved_device_ids.append(verify_key.version)
 
             # If this is the desired key type, save it and its ID/VerifyKey
             if key_type == desired_key_type:
-                final_key = key_content
-                final_verify_key = verify_key
-                final_key_id = key_id
+                desired_key = key_content
+                desired_verify_key = verify_key
+                desired_key_id = key_id
+
+            # At the same time, store this key in the db for subsequent queries
+            yield self.store.set_e2e_cross_signing_key(
+                user.to_string(), key_type, key_content
+            )
 
         # Notify clients that new devices for this user have been discovered
-        if device_ids:
-            yield self.device_handler.notify_device_update(user.to_string(), device_ids)
+        if retrieved_device_ids:
+            yield self.device_handler.notify_device_update(
+                user.to_string(), retrieved_device_ids
+            )
 
-        return final_key, final_key_id, final_verify_key
+        return desired_key, desired_key_id, desired_verify_key
 
 
 def _check_cross_signing_key(key, user_id, key_type, signing_key=None):