Hide device displaynames from other users github/anoa/msc3480 anoa/msc3480

Some argument finagling was needed as query_local_devices can be called
from requests of both local and remote users, and in the case of remote
users, without a user ID.

In the end, we have an option 'from_local_user_id' which tells
`query_local_devices` both a) whether the request is from a local or
remote user and b) if a local user, which one.

3 files changed, 37 insertions, 14 deletions

diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index 5c06073901..563a17f888 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -1086,7 +1086,7 @@ class DeviceListUpdater(DeviceListWorkerUpdater):
 
     @measure_func("_incoming_device_list_update")
     async def _handle_device_updates(self, user_id: str) -> None:
-        "Actually handle pending updates."
+        """Actually handle pending updates."""
 
         async with self._remote_edu_linearizer.queue(user_id):
             pending_updates = self._pending_updates.pop(user_id, [])
diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py
index 8eca6b1381..12a2c09618 100644
--- a/synapse/handlers/e2e_keys.py
+++ b/synapse/handlers/e2e_keys.py
@@ -118,7 +118,8 @@ class E2eKeysHandler:
         Args:
             from_user_id: the user making the query.  This is used when
                 adding cross-signing signatures to limit what signatures users
-                can see.
+                can see, and to prevent leaking the displayname of devices of
+                one user to another when experimental MSC3480 support is enabled.
             from_device_id: the device making the query. This is used to limit
                 the number of in-flight queries at a time.
         """
@@ -145,7 +146,7 @@ class E2eKeysHandler:
             failures: Dict[str, JsonDict] = {}
             results = {}
             if local_query:
-                local_result = await self.query_local_devices(local_query)
+                local_result = await self.query_local_devices(local_query, from_user_id)
                 for user_id, keys in local_result.items():
                     if user_id in local_query:
                         results[user_id] = keys
@@ -453,15 +454,15 @@ class E2eKeysHandler:
     async def query_local_devices(
         self,
         query: Mapping[str, Optional[List[str]]],
-        include_displaynames: bool = True,
+        from_local_user_id: Optional[str],
     ) -> Dict[str, Dict[str, dict]]:
         """Get E2E device keys for local users
 
         Args:
             query: map from user_id to a list
                  of devices to query (None for all devices)
-            include_displaynames: Whether to include device displaynames in the returned
-                device details.
+            from_local_user_id: If the request originates from a local user, their
+                User ID should be specified here. Otherwise, this should be None.
 
         Returns:
             A map from user_id -> device_id -> device details
@@ -494,7 +495,7 @@ class E2eKeysHandler:
             result_dict[user_id] = {}
 
         results = await self.store.get_e2e_device_keys_for_cs_api(
-            local_query, include_displaynames
+            local_query, from_local_user_id
         )
 
         # Build the result structure
@@ -531,9 +532,8 @@ class E2eKeysHandler:
         )
         res = await self.query_local_devices(
             device_keys_query,
-            include_displaynames=(
-                self.config.federation.allow_device_name_lookup_over_federation
-            ),
+            # This is a request originating from a remote user.
+            from_local_user_id=None,
         )
         ret = {"device_keys": res}
 
@@ -935,7 +935,9 @@ class E2eKeysHandler:
             # fetch our stored devices.  This is used to 1. verify
             # signatures on the master key, and 2. to compare with what
             # was sent if the device was signed
-            devices = await self.store.get_e2e_device_keys_for_cs_api([(user_id, None)])
+            devices = await self.store.get_e2e_device_keys_for_cs_api(
+                [(user_id, None)], user_id
+            )
 
             if user_id not in devices:
                 raise NotFoundError("No device keys found")
diff --git a/synapse/storage/databases/main/end_to_end_keys.py b/synapse/storage/databases/main/end_to_end_keys.py
index c4ac6c33ba..22460819a4 100644
--- a/synapse/storage/databases/main/end_to_end_keys.py
+++ b/synapse/storage/databases/main/end_to_end_keys.py
@@ -141,13 +141,15 @@ class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorker
     async def get_e2e_device_keys_for_cs_api(
         self,
         query_list: Collection[Tuple[str, Optional[str]]],
-        include_displaynames: bool = True,
+        from_local_user_id: Optional[str],
     ) -> Dict[str, Dict[str, JsonDict]]:
         """Fetch a list of device keys, formatted suitably for the C/S API.
+
         Args:
             query_list: List of pairs of user_ids and device_ids.
-            include_displaynames: Whether to include the displayname of returned devices
-                (if one exists).
+            from_local_user_id: If the request originates from a local user, their
+                User ID should be specified here. Otherwise, this should be None.
+
         Returns:
             Dict mapping from user-id to dict mapping from device_id to
             key data.  The key data will be a dict in the same format as the
@@ -169,6 +171,25 @@ class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorker
                 if r is None:
                     continue
 
+                # Determine whether the displayname of this device should be shared with
+                # the user making the request.
+                include_displaynames = True
+
+                if (
+                    from_local_user_id is not None
+                    and user_id != from_local_user_id
+                    and self.hs.config.experimental.msc3480_enabled is True
+                ):
+                    include_displaynames = False
+
+                # If this is a request from a remote user, and we've disallowed sharing
+                # local user device names over federation, strip the device's displayname.
+                elif (
+                    from_local_user_id is None
+                    and not self._allow_device_name_lookup_over_federation
+                ):
+                    include_displaynames = False
+
                 r["unsigned"] = {}
                 if include_displaynames:
                     # Include the device's display name in the "unsigned" dictionary