e2e: fetch cross-signing keys when device is signed but no key cached

when for a remote user we don't have cached `e2e_cross_signing_keys` but we do have cached the devices in `device_lists_remote_cache` and the user has cross-signing set up, missing keys are not fetched and cross-signing verification will fail. with this patch, the missing cross-signing keys are requested if the cached devices list does have cross-signing signatures. Signed-off-by: Jonas Jelten <[email protected]>
matrix-org · Aug 23, 2021 · f24b399 · f24b399
1 parent 4db65f9
commit f24b399
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 15 deletions.
diff --git a/changelog.d/10668.bugfix b/changelog.d/10668.bugfix
@@ -0,0 +1 @@
+Fix fetching of E2E cross signing keys over federation when devices are cached and have cross-signing-signature, but we don't have the cross-signing keys cached.
diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py
@@ -168,32 +168,63 @@ async def query_devices(
                 ) = await self.store.get_user_devices_from_cache(query_list)
                 for user_id, devices in remote_results.items():
                     user_devices = results.setdefault(user_id, {})
-                    for device_id, device in devices.items():
-                        keys = device.get("keys", None)
-                        device_display_name = device.get("device_display_name", None)
-                        if keys:
-                            result = dict(keys)
-                            unsigned = result.setdefault("unsigned", {})
-                            if device_display_name:
-                                unsigned["device_display_name"] = device_display_name
-                            user_devices[device_id] = result
 
-                # check for missing cross-signing keys.
-                for user_id in remote_queries.keys():
                     cached_cross_master = user_id in cross_signing_keys["master_keys"]
-                    cached_cross_selfsigning = (
-                        user_id in cross_signing_keys["self_signing_keys"]
+                    self_sig_key = cross_signing_keys["self_signing_keys"].get(
+                        user_id, {}
                     )
+                    cached_cross_selfsigning = bool(self_sig_key)
+
+                    have_xsign_keys = cached_cross_master and cached_cross_selfsigning
 
                     # check if we are missing only one of cross-signing master or
                     # self-signing key, but the other one is cached.
                     # as we need both, this will issue a federation request.
                     # if we don't have any of the keys, either the user doesn't have
-                    # cross-signing set up, or the cached device list
-                    # is not (yet) updated.
+                    # cross-signing set up, or we did not fetch the
+                    # cross-signing keys yet since the device list is not (yet) updated.
                     if cached_cross_master ^ cached_cross_selfsigning:
                         user_ids_not_in_cache.add(user_id)
 
+                    for device_id, device in devices.items():
+                        keys = device.get("keys", None)
+                        device_display_name = device.get("device_display_name", None)
+                        if keys:
+                            result = dict(keys)
+                            unsigned = result.setdefault("unsigned", {})
+                            if device_display_name:
+                                unsigned["device_display_name"] = device_display_name
+                            user_devices[device_id] = result
+
+                            # check if the user's device has a signature
+                            # from a cross-signing-key,
+                            # but we don't have the cross-signing keys.
+                            signatures = keys.get("signatures")
+                            if not signatures:
+                                continue
+
+                            for sig_user, sigs in signatures.items():
+                                if sig_user != user_id:
+                                    continue
+                                for sig_id in sigs.keys():
+                                    # signature is either from a user's device
+                                    # or the cross-signing self-signing key
+                                    sig_device = sig_id.split(":", maxsplit=1)[1]
+                                    if sig_device == device_id:
+                                        continue
+                                    if sig_device in devices:
+                                        continue
+
+                                    if not (
+                                        have_xsign_keys
+                                        and self_sig_key.get("keys", {}).get(sig_id)
+                                    ):
+                                        # this is a cross-signing signature,
+                                        # so we should have both master
+                                        # and selfsigning keys.
+                                        # since we do not, fetch them.
+                                        user_ids_not_in_cache.add(user_id)
+
                 # add those users to the list to fetch over federation.
                 for user_id in user_ids_not_in_cache:
                     domain = get_domain_from_id(user_id)
@@ -234,6 +265,9 @@ async def do_remote_query(destination):
                     # probably be tracking their device lists. However, we haven't
                     # done an initial sync on the device list so we do it now.
                     try:
+                        # here, we fetch the user's device list
+                        # and cross signing keys,
+                        # and update our cache with them.
                         if self._is_master:
                             user_devices = await self.device_handler.device_list_updater.user_device_resync(
                                 user_id
@@ -247,6 +281,15 @@ async def do_remote_query(destination):
                         user_results = results.setdefault(user_id, {})
                         for device in user_devices:
                             user_results[device["device_id"]] = device["keys"]
+
+                        # update the result with the devicelist's cross-signing keys
+                        cross_signing_keys["master_keys"][user_id] = user_devices.get(
+                            "master_key"
+                        )
+                        cross_signing_keys["self_signing_keys"][
+                            user_id
+                        ] = user_devices.get("self_signing_key")
+
                         user_ids_updated.append(user_id)
                     except Exception as e:
                         failures[destination] = _exception_to_failure(e)
@@ -260,6 +303,11 @@ async def do_remote_query(destination):
                 for user_id in user_ids_updated:
                     destination_query.pop(user_id)
 
+                # There's still some leftover destination queries:
+                # either we don't share a room with the user,
+                # or explicit devices were requested.
+                # Now fetch the device keys via /user/keys/query,
+                # and don't cache these results.
                 try:
                     remote_result = await self.federation.query_client_keys(
                         destination, {"device_keys": destination_query}, timeout=timeout
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fix fetching of E2E cross signing keys over federation when devices are cached and have cross-signing-signature, but we don't have the cross-signing keys cached.