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

Device names are returned over federation by /keys/query even if allow_device_name_lookup_over_federation is false #13114

Closed
RUzOfuz5m opened this issue Jun 26, 2022 · 3 comments · Fixed by #14304
Labels
S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@RUzOfuz5m
Copy link

Description

allow_device_name_lookup_over_federation: false 

This will still show device names if someone invites you to a room and you have not joined yet.

Steps to reproduce

  • invite someone to a 1:1 chat on a different homeserver with allow_device_name_lookup_over_federation: set to false
  • Before they join, click on their Bubble to bring up the side menu of their profile.
  • expand N sessions
  • Look at the name of their sessions.
  • Have the user join the 1:1 chat.
  • bring up their profile on the side menu again.
  • expand N sessions
  • The names are no longer visible, you now see the ID.

Homeserver

matrix.org + another home server

Synapse Version

{"server_version":"1.61.0","python_version":"3.9.13"}

Installation Method

Docker (matrixdotorg/synapse)

Platform

docker pull matrixdotorg/synapse

Relevant log output

n/a

Anything else that would be useful to know?

n/a

@anoadragon453
Copy link
Member

anoadragon453 commented Jun 26, 2022

Related #10015. However, this is attempting to fix a different leak.

@anoadragon453
Copy link
Member

I can confirm this. Upon having a user invited to a room, Synapse looks to be returning device names to remote homeservers over federation in response to POST /_matrix/client/r0/keys/query requests from the inviter.

Request:

POST | http://localhost:8080/_matrix/client/r0/keys/query

{
  "device_keys": {
    "@person:localhost:8481": []
  },
  "token": "s10_22_0_1_5_1_1_7_0"
}

Response:

{
  "device_keys": {
    "@person:localhost:8481": {
      "HQHKSKPIQI": {
        "algorithms": [
          "m.olm.v1.curve25519-aes-sha2",
          "m.megolm.v1.aes-sha2"
        ],
        "device_id": "HQHKSKPIQI",
        "keys": {
          "curve25519:HQHKSKPIQI": "O3D1GbEKopw5dAcuc2E6xYMMRaQvwCkf2D+4xeXeCSo",
          "ed25519:HQHKSKPIQI": "X53nYxuEQd1usY8387uJfUDtazIRwhjS6Pkzg3uEuyg"
        },
        "signatures": {
          "@person:localhost:8481": {
            "ed25519:HQHKSKPIQI": "7dCg4kjsi+oETLhFv/quZmPyHQgE4vwJxwwqetH4L0I2AU/Cf6c+2zd2I8Of8kZ2FNk+wrk1P2Thldl20xbLCA",
            "ed25519:yIWhEMxxzCDEhfeqRn103sQJY9ehnDfhXFC3qyhr3nE": "ivXpqC7L2I0I+Xh6lMTdjvswLvVNB2aTedACDOEGT5XeB0Qip9hzX0UtRWwzRao5VHvegn+/tLdqLEZu0ga0Dg"
          }
        },
        "user_id": "@person:localhost:8481",
        "unsigned": {
          "device_display_name": "develop.element.io (Firefox, Linux)"  // Uh oh!
        }
      }
    }
  },
  "failures": {},
  "master_keys": {
    "@person:localhost:8481": {
      "user_id": "@person:localhost:8481",
      "usage": [
        "master"
      ],
      "keys": {
        "ed25519:RN7JygS0riSCBVD2uwiXei4UuRIV4ovtsRXtT8tK9W0": "RN7JygS0riSCBVD2uwiXei4UuRIV4ovtsRXtT8tK9W0"
      },
      "signatures": {
        "@person:localhost:8481": {
          "ed25519:HQHKSKPIQI": "zxz9UFUqbTpYIq0uiq9wrzmVlDYge1Xzqll9vSez/QJIMKxYEhoqQcOLH5Dkaalr/b3Kx2zb0+mZQCFwYPiNAg"
        }
      }
    }
  },
  "self_signing_keys": {
    "@person:localhost:8481": {
      "user_id": "@person:localhost:8481",
      "usage": [
        "self_signing"
      ],
      "keys": {
        "ed25519:yIWhEMxxzCDEhfeqRn103sQJY9ehnDfhXFC3qyhr3nE": "yIWhEMxxzCDEhfeqRn103sQJY9ehnDfhXFC3qyhr3nE"
      },
      "signatures": {
        "@person:localhost:8481": {
          "ed25519:RN7JygS0riSCBVD2uwiXei4UuRIV4ovtsRXtT8tK9W0": "QcEq98iYp427e9zDmxixXfwgx7sYWq+HH7kgOlSVYVhWI6i2QxyOfPEkVO6m+kPdehGgiGva4BW2GhAje13/Ag"
        }
      }
    }
  },
  "user_signing_keys": {}
}

@anoadragon453 anoadragon453 added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Jun 30, 2022
@anoadragon453
Copy link
Member

Looks like this is a result of on_federation_query_client_keys making use of query_local_devices, which happily returns device names:

async def on_federation_query_client_keys(
self, query_body: Dict[str, Dict[str, Optional[List[str]]]]
) -> JsonDict:
"""Handle a device key query from a federated server"""
device_keys_query: Dict[str, Optional[List[str]]] = query_body.get(
"device_keys", {}
)
res = await self.query_local_devices(device_keys_query)
ret = {"device_keys": res}
# add in the cross-signing keys
cross_signing_keys = await self.get_cross_signing_keys_from_cache(
device_keys_query, None
)
ret.update(cross_signing_keys)
return ret

results = await self.get_e2e_device_keys_and_signatures(query_list)
# Build the result structure, un-jsonify the results, and add the
# "unsigned" section
rv: Dict[str, Dict[str, JsonDict]] = {}
for user_id, device_keys in results.items():
rv[user_id] = {}
for device_id, device_info in device_keys.items():
r = device_info.keys
r["unsigned"] = {}
display_name = device_info.display_name
if display_name is not None:
r["unsigned"]["device_display_name"] = display_name
rv[user_id][device_id] = r
return rv

One solution would be to add a return_device_names boolean argument to query_local_devices, and get_e2e_device_keys_for_cs_api, which would control whether device names were returned. on_federation_query_client_keys could then check the homeserver config and set this option appropriately.

@anoadragon453 anoadragon453 changed the title device name over federation Device names are returned over federation by /keys/query even if allow_device_name_lookup_over_federation is false Jun 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants