From e9097e58746ce29199209d0024b1ac447e81fd3e Mon Sep 17 00:00:00 2001 From: Jonas Jelten Date: Fri, 2 Oct 2020 16:27:08 +0200 Subject: [PATCH 1/2] e2e: ensure we have both master and self-signing key it seems to be possible that only one of them ends up to be cached. when this was the case, the missing one was not fetched via federation, and clients then failed to validate cross-signed devices. Signed-off-by: Jonas Jelten --- changelog.d/8455.bugfix | 1 + synapse/handlers/e2e_keys.py | 25 ++++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 changelog.d/8455.bugfix diff --git a/changelog.d/8455.bugfix b/changelog.d/8455.bugfix new file mode 100644 index 000000000000..561e73f5e003 --- /dev/null +++ b/changelog.d/8455.bugfix @@ -0,0 +1 @@ +Fix fetching of E2E cross signing keys over federation when only one of the master key and device signing key is cached already. diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 611742ae72d5..5d1e0d2d6b96 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -129,6 +129,11 @@ async def query_devices(self, query_body, timeout, from_user_id): if user_id in local_query: results[user_id] = keys + # Get cached cross-signing keys + cross_signing_keys = await self.get_cross_signing_keys_from_cache( + device_keys_query, from_user_id + ) + # Now attempt to get any remote devices from our local cache. remote_queries_not_in_cache = {} if remote_queries: @@ -155,16 +160,26 @@ async def query_devices(self, query_body, timeout, from_user_id): 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"] + ) + + # check if only one of the cross-signing master and + # self-signing key are cached. + # for each user we want the master _and_ the self-signing key, + # so we fetch those keys from federation + if cached_cross_master ^ cached_cross_selfsigning: + 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) r = remote_queries_not_in_cache.setdefault(domain, {}) r[user_id] = remote_queries[user_id] - # Get cached cross-signing keys - cross_signing_keys = await self.get_cross_signing_keys_from_cache( - device_keys_query, from_user_id - ) - # Now fetch any devices that we don't have in our cache @trace async def do_remote_query(destination): From 198e78757de0fab18baf1cf7d9bd80a34f74c2bd Mon Sep 17 00:00:00 2001 From: Jonas Jelten Date: Mon, 26 Oct 2020 16:15:40 +0100 Subject: [PATCH 2/2] e2e: clarify the need for a federation request for e2e keys --- synapse/handlers/e2e_keys.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 5d1e0d2d6b96..929752150deb 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -167,10 +167,12 @@ async def query_devices(self, query_body, timeout, from_user_id): user_id in cross_signing_keys["self_signing_keys"] ) - # check if only one of the cross-signing master and - # self-signing key are cached. - # for each user we want the master _and_ the self-signing key, - # so we fetch those keys from federation + # 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. if cached_cross_master ^ cached_cross_selfsigning: user_ids_not_in_cache.add(user_id)