Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10668.bugfix
View file
Open in desktop
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.
78 changes: 63 additions & 15 deletions synapse/handlers/e2e_keys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that this is pulled over federation from a potentially untrusted remote server, which may itself not have validated the content of the key object, what guarantee do we have that signatures is actually a dict? We should handle invalid responses more gracefully than throwing no attribute 'items' exceptions). Likewise we should not make assumptions about the content of the dict.

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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down