Prevent federation user keys query from returning device names if disallowed #14304

anoadragon453 · 2022-10-26T13:15:13Z

Fixes #13114 using the solution described in #13114 (comment). May also fix #12750.

This commit prevents the POST /_matrix/federation/v1/user/keys/query endpoint from returning device names for users when the allow_device_name_lookup_over_federation config option was set to false.

I opted for a include_displaynames option for query_local_devices as I assume we'll eventually add in a similar option to allow_device_name_lookup_over_federation, but between users on the same homeserver (in which case setting this to False in some other places will also be useful).

Once again I'm not really sure how to test this other than manually (which doing so - looks good). Because it's homeserver-specific whether these display names will be returned, Complement isn't really the right place. But testing federation between servers is unit tests is tricky. I could try to mangle something together for this specific federation endpoint, if that seems useful.)

Failing sytests are fixed by matrix-org/sytest#1311, please review in tandem with this PR.

…allowed This commit prevents the 'POST /_matrix/federation/v1/user/keys/query' endpoint from returning device names for users when the allow_device_name_lookup_over_federation config option was set to false.

anoadragon453 · 2022-10-27T09:27:20Z

Need to fix a failing sytest ("Can query remote device keys using POST").

DMRobertson · 2022-10-27T09:51:20Z

Need to fix a failing sytest ("Can query remote device keys using POST").

matrix-org/sytest#1307 might be related.

anoadragon453 · 2022-10-27T10:01:47Z

@DMRobertson Hah, yes indeed! I've filed matrix-org/sytest#1311 with another spot that this is checked, and reused the code from your linked PR :)

DMRobertson · 2022-10-27T11:59:39Z

synapse/handlers/e2e_keys.py

+        self._allow_device_name_lookup_over_federation = (
+            hs.config.federation.allow_device_name_lookup_over_federation
+        )


As an aside: I've never understood why we copy everything off the config objects. It makes it much harder to dynamically reload config, and only saves a few getattr calls and indirections---is reading from the config really that much of a bottleneck?

It was much more of a concern before #10985. Nowadays perhaps not so. You're right in that it adds clutter and makes reloading config more difficult. I think I'll remove the class variable for now, but it'd be good to have a team discussion on whether we should stick to one way or the other.

(I don't particularly mind leaving this as written here---I'm just thinking out loud.)

Changed in e1614de. One thing it did require is a self.config = config line (or alternatively a self.hs = hs) in __init__. I wonder if this has any performance impact?

Also discussed at https://element.topicbox.com/groups/backend/T1a142202e6ba4531-M8be7fbe3edd34e232ebb168e

synapse/handlers/e2e_keys.py

changelog.d/14304.bugfix

DMRobertson

LGTM---would be nice to update the changelog and to make that docstring tweak though.

@asymmetric

Synapse 1.71.0 (2022-11-08) =========================== Please note that, as announced in the release notes for Synapse 1.69.0, legacy Prometheus metric names are now disabled by default. They will be removed altogether in Synapse 1.73.0. If not already done, server administrators should update their dashboards and alerting rules to avoid using the deprecated metric names. See the [upgrade notes](https://matrix-org.github.io/synapse/v1.71/upgrade.html#upgrading-to-v1710) for more details. **Note:** in line with our [deprecation policy](https://matrix-org.github.io/synapse/latest/deprecation_policy.html) for platform dependencies, this will be the last release to support PostgreSQL 10, which reaches upstream end-of-life on November 10th, 2022. Future releases of Synapse will require PostgreSQL 11+. No significant changes since 1.71.0rc2. Synapse 1.71.0rc2 (2022-11-04) ============================== Improved Documentation ---------------------- - Document the changes to monthly active user metrics due to deprecation of legacy Prometheus metric names. ([\matrix-org#14358](matrix-org#14358), [\matrix-org#14360](matrix-org#14360)) Deprecations and Removals ------------------------- - Disable legacy Prometheus metric names by default. They can still be re-enabled for now, but they will be removed altogether in Synapse 1.73.0. ([\matrix-org#14353](matrix-org#14353)) Internal Changes ---------------- - Run unit tests against Python 3.11. ([\matrix-org#13812](matrix-org#13812)) Synapse 1.71.0rc1 (2022-11-01) ============================== Features -------- - Support back-channel logouts from OpenID Connect providers. ([\matrix-org#11414](matrix-org#11414)) - Allow use of Postgres and SQLlite full-text search operators in search queries. ([\matrix-org#11635](matrix-org#11635), [\matrix-org#14310](matrix-org#14310), [\matrix-org#14311](matrix-org#14311)) - Implement [MSC3664](matrix-org/matrix-spec-proposals#3664), Pushrules for relations. Contributed by Nico. ([\matrix-org#11804](matrix-org#11804)) - Improve aesthetics of HTML templates. Note that these changes do not retroactively apply to templates which have been [customised](https://matrix-org.github.io/synapse/latest/templates.html#templates) by server admins. ([\matrix-org#13652](matrix-org#13652)) - Enable write-ahead logging for SQLite installations. Contributed by [@asymmetric](https://github.com/asymmetric). ([\matrix-org#13897](matrix-org#13897)) - Show erasure status when [listing users](https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html#query-user-account) in the Admin API. ([\matrix-org#14205](matrix-org#14205)) - Provide a specific error code when a `/sync` request provides a filter which doesn't represent a JSON object. ([\matrix-org#14262](matrix-org#14262)) Bugfixes -------- - Fix a long-standing bug where the `update_synapse_database` script could not be run with multiple databases. Contributed by @thefinn93 @ Beeper. ([\matrix-org#13422](matrix-org#13422)) - Fix a bug which prevented setting an avatar on homeservers which have an explicit port in their `server_name` and have `max_avatar_size` and/or `allowed_avatar_mimetypes` configuration. Contributed by @ashfame. ([\matrix-org#13927](matrix-org#13927)) - Check appservice user interest against the local users instead of all users in the room to align with [MSC3905](matrix-org/matrix-spec-proposals#3905). ([\matrix-org#13958](matrix-org#13958)) - Fix a long-standing bug where Synapse would accidentally include extra information in the response to [`PUT /_matrix/federation/v2/invite/{roomId}/{eventId}`](https://spec.matrix.org/v1.4/server-server-api/#put_matrixfederationv2inviteroomideventid). ([\matrix-org#14064](matrix-org#14064)) - Fix a bug introduced in Synapse 1.64.0 where presence updates could be missing from `/sync` responses. ([\matrix-org#14243](matrix-org#14243)) - Fix a bug introduced in Synapse 1.60.0 which caused an error to be logged when Synapse received a SIGHUP signal if debug logging was enabled. ([\matrix-org#14258](matrix-org#14258)) - Prevent history insertion ([MSC2716](matrix-org/matrix-spec-proposals#2716)) during an partial join ([MSC3706](matrix-org/matrix-spec-proposals#3706)). ([\matrix-org#14291](matrix-org#14291)) - Fix a bug introduced in Synapse 1.34.0 where device names would be returned via a federation user key query request when `allow_device_name_lookup_over_federation` was set to `false`. ([\matrix-org#14304](matrix-org#14304)) - Fix a bug introduced in Synapse 0.34.0 where logs could include error spam when background processes are measured as taking a negative amount of time. ([\matrix-org#14323](matrix-org#14323)) - Fix a bug introduced in Synapse 1.70.0 where clients were unable to PUT new [dehydrated devices](matrix-org/matrix-spec-proposals#2697). ([\matrix-org#14336](matrix-org#14336)) Improved Documentation ---------------------- - Explain how to disable the use of [`trusted_key_servers`](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#trusted_key_servers). ([\matrix-org#13999](matrix-org#13999)) - Add workers settings to [configuration manual](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#individual-worker-configuration). ([\matrix-org#14086](matrix-org#14086)) - Correct the name of the config option [`encryption_enabled_by_default_for_room_type`](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#encryption_enabled_by_default_for_room_type). ([\matrix-org#14110](matrix-org#14110)) - Update docstrings of `SynapseError` and `FederationError` to bettter describe what they are used for and the effects of using them are. ([\matrix-org#14191](matrix-org#14191)) Internal Changes ---------------- - Remove unused `@lru_cache` decorator. ([\matrix-org#13595](matrix-org#13595)) - Save login tokens in database and prevent login token reuse. ([\matrix-org#13844](matrix-org#13844)) - Refactor OIDC tests to better mimic an actual OIDC provider. ([\matrix-org#13910](matrix-org#13910)) - Fix type annotation causing import time error in the Complement forking launcher. ([\matrix-org#14084](matrix-org#14084)) - Refactor [MSC3030](matrix-org/matrix-spec-proposals#3030) `/timestamp_to_event` endpoint to loop over federation destinations with standard pattern and error handling. ([\matrix-org#14096](matrix-org#14096)) - Add initial power level event to batch of bulk persisted events when creating a new room. ([\matrix-org#14228](matrix-org#14228)) - Refactor `/key/` endpoints to use `RestServlet` classes. ([\matrix-org#14229](matrix-org#14229)) - Switch to using the `matrix-org/backend-meta` version of `triage-incoming` for new issues in CI. ([\matrix-org#14230](matrix-org#14230)) - Build wheels on macos 11, not 10.15. ([\matrix-org#14249](matrix-org#14249)) - Add debugging to help diagnose lost device list updates. ([\matrix-org#14268](matrix-org#14268)) - Add Rust cache to CI for `trial` runs. ([\matrix-org#14287](matrix-org#14287)) - Improve type hinting of `RawHeaders`. ([\matrix-org#14303](matrix-org#14303)) - Use Poetry 1.2.0 in the Twisted Trunk CI job. ([\matrix-org#14305](matrix-org#14305)) <details> <summary>Dependency updates</summary> Runtime: - Bump anyhow from 1.0.65 to 1.0.66. ([\matrix-org#14278](matrix-org#14278)) - Bump jinja2 from 3.0.3 to 3.1.2. ([\matrix-org#14271](matrix-org#14271)) - Bump prometheus-client from 0.14.0 to 0.15.0. ([\matrix-org#14274](matrix-org#14274)) - Bump psycopg2 from 2.9.4 to 2.9.5. ([\matrix-org#14331](matrix-org#14331)) - Bump pysaml2 from 7.1.2 to 7.2.1. ([\matrix-org#14270](matrix-org#14270)) - Bump sentry-sdk from 1.5.11 to 1.10.1. ([\matrix-org#14330](matrix-org#14330)) - Bump serde from 1.0.145 to 1.0.147. ([\matrix-org#14277](matrix-org#14277)) - Bump serde_json from 1.0.86 to 1.0.87. ([\matrix-org#14279](matrix-org#14279)) Tooling and CI: - Bump black from 22.3.0 to 22.10.0. ([\matrix-org#14328](matrix-org#14328)) - Bump flake8-bugbear from 21.3.2 to 22.9.23. ([\matrix-org#14042](matrix-org#14042)) - Bump peaceiris/actions-gh-pages from 3.8.0 to 3.9.0. ([\matrix-org#14276](matrix-org#14276)) - Bump peaceiris/actions-mdbook from 1.1.14 to 1.2.0. ([\matrix-org#14275](matrix-org#14275)) - Bump setuptools-rust from 1.5.1 to 1.5.2. ([\matrix-org#14273](matrix-org#14273)) - Bump twine from 3.8.0 to 4.0.1. ([\matrix-org#14332](matrix-org#14332)) - Bump types-opentracing from 2.4.7 to 2.4.10. ([\matrix-org#14133](matrix-org#14133)) - Bump types-requests from 2.28.11 to 2.28.11.2. ([\matrix-org#14272](matrix-org#14272)) </details>

anoadragon453 added 2 commits October 26, 2022 14:06

Prevent federation user keys query from returning device names if dis…

e5c2ad5

…allowed This commit prevents the 'POST /_matrix/federation/v1/user/keys/query' endpoint from returning device names for users when the allow_device_name_lookup_over_federation config option was set to false.

changelog

129f333

anoadragon453 marked this pull request as ready for review October 26, 2022 13:22

anoadragon453 requested a review from a team as a code owner October 26, 2022 13:22

anoadragon453 removed the request for review from a team October 27, 2022 09:26

anoadragon453 requested a review from a team October 27, 2022 10:14

DMRobertson reviewed Oct 27, 2022

View reviewed changes

synapse/handlers/e2e_keys.py Outdated Show resolved Hide resolved

DMRobertson reviewed Oct 27, 2022

View reviewed changes

changelog.d/14304.bugfix Outdated Show resolved Hide resolved

DMRobertson approved these changes Oct 27, 2022

View reviewed changes

anoadragon453 added 2 commits October 27, 2022 14:51

Update changelog, docstring

d68d86c

Access config option directly

e1614de

anoadragon453 merged commit 7911e28 into develop Oct 28, 2022

anoadragon453 deleted the anoa/device_name_federation branch October 28, 2022 17:06

anoadragon453 mentioned this pull request Oct 28, 2022

allow_device_name_lookup_over_federation config option may be leaky #12750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent federation user keys query from returning device names if disallowed #14304

Prevent federation user keys query from returning device names if disallowed #14304

anoadragon453 commented Oct 26, 2022 •

edited

Loading

anoadragon453 commented Oct 27, 2022

DMRobertson commented Oct 27, 2022

anoadragon453 commented Oct 27, 2022

DMRobertson Oct 27, 2022

anoadragon453 Oct 27, 2022

DMRobertson Oct 27, 2022 •

edited

Loading

anoadragon453 Oct 27, 2022

MadLittleMods Oct 27, 2022

DMRobertson left a comment

Prevent federation user keys query from returning device names if disallowed #14304

Prevent federation user keys query from returning device names if disallowed #14304

Conversation

anoadragon453 commented Oct 26, 2022 • edited Loading

anoadragon453 commented Oct 27, 2022

DMRobertson commented Oct 27, 2022

anoadragon453 commented Oct 27, 2022

DMRobertson Oct 27, 2022

Choose a reason for hiding this comment

anoadragon453 Oct 27, 2022

Choose a reason for hiding this comment

DMRobertson Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

anoadragon453 Oct 27, 2022

Choose a reason for hiding this comment

MadLittleMods Oct 27, 2022

Choose a reason for hiding this comment

DMRobertson left a comment

Choose a reason for hiding this comment

anoadragon453 commented Oct 26, 2022 •

edited

Loading

DMRobertson Oct 27, 2022 •

edited

Loading