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

Do not accept pattern_type from user input in push rules #15088

Merged
merged 12 commits into from
Feb 28, 2023
Merged

Do not accept pattern_type from user input in push rules #15088

merged 12 commits into from
Feb 28, 2023

Conversation

clokep
Copy link
Member

@clokep clokep commented Feb 16, 2023 •
edited
Loading

Internally the push rules module of Synapse uses a pattern_type field for event_match (and related_event_match) if we're going to match against the current user's user ID or localpart. This works well, but it is leaky -- users are able to add their own push rules using this field. This is not allowed according to the spec.

To do this I:

  • Seperated the EventMatch enum value into EventMatch and EventMatchType, each gets an associated type with the expected fields on it.
  • A similar change is made for RelatedEventMatch.
  • We then refactor a bit to re-use code.
  • Finally, we make the user_id vs. user_localpart values into an enum because we don't have to care about serializability anymore.

My reasoning for doing this is vague gesturing at #15051 where I can't implement the same thing again because a null JSON field for exact_event_match and exact_event_property_contains actually means something.

@clokep clokep added the A-Spec-Compliance places where synapse does not conform to the spec label Feb 16, 2023
@clokep clokep force-pushed the clokep/push-rule-patterns branch 3 times, most recently from 77cdf77 to 6675e91 Compare February 17, 2023 13:14
@clokep
Copy link
Member Author

clokep commented Feb 17, 2023

Hm, this has some serialization issues with it, I might need to rethink...

@clokep
Copy link
Member Author

clokep commented Feb 21, 2023

I tried separating out the refactoring to a separate PR, but the changes don't really make sense / require a lot more churn. Sorry this got a bit big.

@clokep clokep marked this pull request as ready for review February 21, 2023 19:35
@clokep clokep requested a review from a team as a code owner February 21, 2023 19:35
@clokep clokep mentioned this pull request Feb 21, 2023
Merged
DMRobertson
DMRobertson reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

Thoughts in passing. Will stuff back in the queue for a second opinion.

rust/src/push/evaluator.rs Show resolved Hide resolved
rust/src/push/evaluator.rs Show resolved Hide resolved
rust/src/push/evaluator.rs Outdated Show resolved Hide resolved
rust/src/push/evaluator.rs Outdated Show resolved Hide resolved
@DMRobertson DMRobertson requested a review from a team February 24, 2023 18:39
erikjohnston
erikjohnston approved these changes Feb 28, 2023
View reviewed changes
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

synapse/push/clientformat.py Outdated Show resolved Hide resolved
rust/src/push/mod.rs
@@ -328,10 +328,16 @@ pub enum Condition {
#[serde(tag = "kind")]
pub enum KnownCondition {
EventMatch(EventMatchCondition),
// Identical to event_match but gives predefined patterns. Cannot be added by users.
#[serde(skip_deserializing, rename = "event_match")]
EventMatchType(EventMatchTypeCondition),
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a test that this serializes correctly, for paranoia.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! I added deserialization ones too.

@clokep clokep merged commit e746f80 into develop Feb 28, 2023
@clokep clokep deleted the clokep/push-rule-patterns branch February 28, 2023 15:11
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 9, 2023
@Midar
Update chat/matrix-synapse to 1.18.0
f524488 
Synapse 1.80.0 (2023-03-28)
===========================

No significant changes since 1.80.0rc2.


Synapse 1.80.0rc2 (2023-03-22)
==============================

Bugfixes
--------

- Fix a bug in which the [`POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3roomsroomidreporteventid) endpoint would return the wrong error if the user did not have permission to view the event. This aligns Synapse's implementation with [MSC2249](matrix-org/matrix-spec-proposals#2249). ([\#15298](matrix-org/synapse#15298), [\#15300](matrix-org/synapse#15300))
- Fix a bug introduced in Synapse 1.75.0rc1 where the [SQLite port_db script](https://matrix-org.github.io/synapse/latest/postgres.html#porting-from-sqlite)
  would fail to open the SQLite database. ([\#15301](matrix-org/synapse#15301))


Synapse 1.80.0rc1 (2023-03-21)
==============================

Features
--------

- Stabilise support for [MSC3966](matrix-org/matrix-spec-proposals#3966): `event_property_contains` push condition. ([\#15187](matrix-org/synapse#15187))
- Implement [MSC2659](matrix-org/matrix-spec-proposals#2659): application service ping endpoint. Contributed by Tulir @ Beeper. ([\#15249](matrix-org/synapse#15249))
- Allow loading `/register/available` endpoint on workers. ([\#15268](matrix-org/synapse#15268))
- Improve performance of creating and authenticating events. ([\#15195](matrix-org/synapse#15195))
- Add topic and name events to group of events that are batch persisted when creating a room. ([\#15229](matrix-org/synapse#15229))


Bugfixes
--------

- Fix a long-standing bug in which the user directory would assume any remote membership state events represent a profile change. ([\#14755](matrix-org/synapse#14755), [\#14756](matrix-org/synapse#14756))
- Implement [MSC3873](matrix-org/matrix-spec-proposals#3873) to fix a long-standing bug where properties with dots were handled ambiguously in push rules. ([\#15190](matrix-org/synapse#15190))
- Faster joins: Fix a bug introduced in Synapse 1.66 where spurious "Failed to find memberships ..." errors would be logged. ([\#15232](matrix-org/synapse#15232))
- Fix a long-standing error when sending message into deleted room. ([\#15235](matrix-org/synapse#15235))


Updates to the Docker image
---------------------------

- Ensure the Dockerfile builds on platforms that don't have a `cryptography` wheel. ([\#15239](matrix-org/synapse#15239))
- Mirror images to the GitHub Container Registry (`ghcr.io/matrix-org/synapse`). ([\#15281](matrix-org/synapse#15281), [\#15282](matrix-org/synapse#15282))


Improved Documentation
----------------------

- Add a missing endpoint to the workers documentation. ([\#15223](matrix-org/synapse#15223))


Internal Changes
----------------

- Add additional functionality to declaring worker types when starting Complement in worker mode. ([\#14921](matrix-org/synapse#14921))
- Add `Synapse-Trace-Id` to `access-control-expose-headers` header. ([\#14974](matrix-org/synapse#14974))
- Make the `HttpTransactionCache` use the `Requester` in addition of the just the `Request` to build the transaction key. ([\#15200](matrix-org/synapse#15200))
- Improve log lines when purging rooms. ([\#15222](matrix-org/synapse#15222))
- Improve type hints. ([\#15230](matrix-org/synapse#15230), [\#15231](matrix-org/synapse#15231), [\#15238](matrix-org/synapse#15238))
- Move various module API callback registration methods to a dedicated class. ([\#15237](matrix-org/synapse#15237))
- Configure GitHub Actions for merge queues. ([\#15244](matrix-org/synapse#15244))
- Add schema comments about the `destinations` and `destination_rooms` tables. ([\#15247](matrix-org/synapse#15247))
- Skip processing of auto-join room behaviour if there are no auto-join rooms configured. ([\#15262](matrix-org/synapse#15262))
- Remove unused store method `_set_destination_retry_timings_emulated`. ([\#15266](matrix-org/synapse#15266))
- Reorganize URL preview code. ([\#15269](matrix-org/synapse#15269))
- Clean-up direct TCP replication code. ([\#15272](matrix-org/synapse#15272), [\#15274](matrix-org/synapse#15274))
- Make `configure_workers_and_start` script used in Complement tests compatible with older versions of Python. ([\#15275](matrix-org/synapse#15275))
- Add a `/versions` flag for [MSC3952](matrix-org/matrix-spec-proposals#3952). ([\#15293](matrix-org/synapse#15293))
- Bump hiredis from 2.2.1 to 2.2.2. ([\#15252](matrix-org/synapse#15252))
- Bump serde from 1.0.152 to 1.0.155. ([\#15253](matrix-org/synapse#15253))
- Bump pysaml2 from 7.2.1 to 7.3.1. ([\#15254](matrix-org/synapse#15254))
- Bump msgpack from 1.0.4 to 1.0.5. ([\#15255](matrix-org/synapse#15255))
- Bump gitpython from 3.1.30 to 3.1.31. ([\#15256](matrix-org/synapse#15256))
- Bump cryptography from 39.0.1 to 39.0.2. ([\#15257](matrix-org/synapse#15257))
- Bump pydantic from 1.10.4 to 1.10.6. ([\#15286](matrix-org/synapse#15286))
- Bump serde from 1.0.155 to 1.0.157. ([\#15287](matrix-org/synapse#15287))
- Bump anyhow from 1.0.69 to 1.0.70. ([\#15288](matrix-org/synapse#15288))
- Bump txredisapi from 1.4.7 to 1.4.9. ([\#15289](matrix-org/synapse#15289))
- Bump pygithub from 1.57 to 1.58.1. ([\#15290](matrix-org/synapse#15290))
- Bump types-requests from 2.28.11.12 to 2.28.11.15. ([\#15291](matrix-org/synapse#15291))



Synapse 1.79.0 (2023-03-14)
===========================

No significant changes since 1.79.0rc2.


Synapse 1.79.0rc2 (2023-03-13)
==============================

Bugfixes
--------

- Fix a bug introduced in Synapse 1.79.0rc1 where attempting to register a `on_remove_user_third_party_identifier` module API callback would be a no-op. ([\#15227](matrix-org/synapse#15227))
- Fix a rare bug introduced in Synapse 1.73 where events could remain unsent to other homeservers after a faster-join to a room. ([\#15248](matrix-org/synapse#15248))


Internal Changes
----------------

- Refactor `filter_events_for_server`. ([\#15240](matrix-org/synapse#15240))


Synapse 1.79.0rc1 (2023-03-07)
==============================

Features
--------

- Add two new Third Party Rules module API callbacks: [`on_add_user_third_party_identifier`](https://matrix-org.github.io/synapse/v1.79/modules/third_party_rules_callbacks.html#on_add_user_third_party_identifier) and [`on_remove_user_third_party_identifier`](https://matrix-org.github.io/synapse/v1.79/modules/third_party_rules_callbacks.html#on_remove_user_third_party_identifier). ([\#15044](matrix-org/synapse#15044))
- Experimental support for [MSC3967](matrix-org/matrix-spec-proposals#3967) to not require UIA for setting up cross-signing on first use. ([\#15077](matrix-org/synapse#15077))
- Add media information to the command line [user data export tool](https://matrix-org.github.io/synapse/v1.79/usage/administration/admin_faq.html#how-can-i-export-user-data). ([\#15107](matrix-org/synapse#15107))
- Add an [admin API](https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html) to delete a [specific event report](https://spec.matrix.org/v1.6/client-server-api/#reporting-content). ([\#15116](matrix-org/synapse#15116))
- Add support for knocking to workers. ([\#15133](matrix-org/synapse#15133))
- Allow use of the `/filter` Client-Server APIs on workers. ([\#15134](matrix-org/synapse#15134))
- Update support for [MSC2677](matrix-org/matrix-spec-proposals#2677): remove support for server-side aggregation of reactions. ([\#15172](matrix-org/synapse#15172))
- Stabilise support for [MSC3758](matrix-org/matrix-spec-proposals#3758): `event_property_is` push condition. ([\#15185](matrix-org/synapse#15185))


Bugfixes
--------

- Fix a bug introduced in Synapse 1.75 that caused experimental support for deleting account data to raise an internal server error while using an account data writer worker. ([\#14869](matrix-org/synapse#14869))
- Fix a long-standing bug where Synapse handled an unspecced field on push rules. ([\#15088](matrix-org/synapse#15088))
- Fix a long-standing bug where a URL preview would break if the discovered oEmbed failed to download. ([\#15092](matrix-org/synapse#15092))
- Fix a long-standing bug where an initial sync would not respond to changes to the list of ignored users if there was an initial sync cached. ([\#15163](matrix-org/synapse#15163))
- Add the `transaction_id` in the events included in many endpoints' responses. ([\#15174](matrix-org/synapse#15174))
- Fix a bug introduced in Synapse 1.78.0 where requests to claim dehydrated devices would fail with a `405` error. ([\#15180](matrix-org/synapse#15180))
- Stop applying edits when bundling aggregations, per [MSC3925](matrix-org/matrix-spec-proposals#3925). ([\#15193](matrix-org/synapse#15193))
- Fix a long-standing bug where the user directory search was not case-insensitive for accented characters. ([\#15143](matrix-org/synapse#15143))


Updates to the Docker image
---------------------------

- Improve startup logging in the with-workers Docker image. ([\#15186](matrix-org/synapse#15186))


Improved Documentation
----------------------

- Document how to use caches in a module. ([\#14026](matrix-org/synapse#14026))
- Clarify which worker processes the ThirdPartyRules' [`on_new_event`](https://matrix-org.github.io/synapse/v1.78/modules/third_party_rules_callbacks.html#on_new_event) module API callback runs on. ([\#15071](matrix-org/synapse#15071))
- Document using [Shibboleth](https://www.shibboleth.net/) as an OpenID Provider. ([\#15112](matrix-org/synapse#15112))
- Correct reference to `federation_verify_certificates` in configuration documentation. ([\#15139](matrix-org/synapse#15139))
- Correct small documentation errors in some `MatrixFederationHttpClient` methods. ([\#15148](matrix-org/synapse#15148))
- Correct the description of the behavior of `registration_shared_secret_path` on startup. ([\#15168](matrix-org/synapse#15168))


Deprecations and Removals
-------------------------

- Deprecate the `on_threepid_bind` module callback, to be replaced by [`on_add_user_third_party_identifier`](https://matrix-org.github.io/synapse/v1.79/modules/third_party_rules_callbacks.html#on_add_user_third_party_identifier). See [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.79/docs/upgrade.md#upgrading-to-v1790). ([\#15044](matrix-org/synapse#15044))
- Remove the unspecced `room_alias` field from the [`/createRoom`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3createroom) response. ([\#15093](matrix-org/synapse#15093))
- Remove the unspecced `PUT` on the `/knock/{roomIdOrAlias}` endpoint. ([\#15189](matrix-org/synapse#15189))
- Remove the undocumented and unspecced `type` parameter to the `/thumbnail` endpoint. ([\#15137](matrix-org/synapse#15137))
- Remove unspecced and buggy `PUT` method on the unstable `/rooms/<room_id>/batch_send` endpoint. ([\#15199](matrix-org/synapse#15199))


Internal Changes
----------------

- Run the integration test suites with the asyncio reactor enabled in CI. ([\#14101](matrix-org/synapse#14101))
- Batch up storing state groups when creating a new room. ([\#14918](matrix-org/synapse#14918))
- Update [MSC3952](matrix-org/matrix-spec-proposals#3952) support based on changes to the MSC. ([\#15051](matrix-org/synapse#15051))
- Refactor writing json data in `FileExfiltrationWriter`. ([\#15095](matrix-org/synapse#15095))
- Tighten the login ratelimit defaults. ([\#15135](matrix-org/synapse#15135))
- Fix a typo in an experimental config setting. ([\#15138](matrix-org/synapse#15138))
- Refactor the media modules. ([\#15146](matrix-org/synapse#15146), [\#15175](matrix-org/synapse#15175))
- Improve type hints. ([\#15164](matrix-org/synapse#15164))
- Move `get_event_report` and `get_event_reports_paginate` from `RoomStore` to `RoomWorkerStore`. ([\#15165](matrix-org/synapse#15165))
- Remove dangling reference to being a reference implementation in docstring. ([\#15167](matrix-org/synapse#15167))
- Add an option to force a rebuild of the "editable" complement image. ([\#15184](matrix-org/synapse#15184))
- Use nightly rustfmt in CI. ([\#15188](matrix-org/synapse#15188))
- Add a `get_next_txn` method to `StreamIdGenerator` to match `MultiWriterIdGenerator`. ([\#15191](matrix-org/synapse#15191))
- Combine `AbstractStreamIdTracker` and `AbstractStreamIdGenerator`. ([\#15192](matrix-org/synapse#15192))
- Automatically fix errors with `ruff`. ([\#15194](matrix-org/synapse#15194))
- Refactor database transaction for query users' devices to reduce database pool contention. ([\#15215](matrix-org/synapse#15215))
- Correct `test_icu_word_boundary_punctuation` so that it passes with the ICU versions available in Alpine and macOS. ([\#15177](matrix-org/synapse#15177))

<details><summary>Locked dependency updates</summary>

  - Bump actions/checkout from 2 to 3. ([\#15155](matrix-org/synapse#15155))
  - Bump black from 22.12.0 to 23.1.0. ([\#15103](matrix-org/synapse#15103))
  - Bump dawidd6/action-download-artifact from 2.25.0 to 2.26.0. ([\#15152](matrix-org/synapse#15152))
  - Bump docker/login-action from 1 to 2. ([\#15154](matrix-org/synapse#15154))
  - Bump matrix-org/backend-meta from 1 to 2. ([\#15156](matrix-org/synapse#15156))
  - Bump ruff from 0.0.237 to 0.0.252. ([\#15159](matrix-org/synapse#15159))
  - Bump serde_json from 1.0.93 to 1.0.94. ([\#15214](matrix-org/synapse#15214))
  - Bump types-commonmark from 0.9.2.1 to 0.9.2.2. ([\#15209](matrix-org/synapse#15209))
  - Bump types-opentracing from 2.4.10.1 to 2.4.10.3. ([\#15158](matrix-org/synapse#15158))
  - Bump types-pillow from 9.4.0.13 to 9.4.0.17. ([\#15211](matrix-org/synapse#15211))
  - Bump types-psycopg2 from 2.9.21.4 to 2.9.21.8. ([\#15210](matrix-org/synapse#15210))
  - Bump types-pyopenssl from 22.1.0.2 to 23.0.0.4. ([\#15213](matrix-org/synapse#15213))
  - Bump types-setuptools from 67.3.0.1 to 67.4.0.3. ([\#15160](matrix-org/synapse#15160))
  - Bump types-setuptools from 67.4.0.3 to 67.5.0.0. ([\#15212](matrix-org/synapse#15212))
  - Bump typing-extensions from 4.4.0 to 4.5.0. ([\#15157](matrix-org/synapse#15157))
</details>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DMRobertson DMRobertson DMRobertson left review comments

@erikjohnston erikjohnston erikjohnston approved these changes

Assignees
No one assigned
Labels
A-Spec-Compliance places where synapse does not conform to the spec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clokep @erikjohnston @DMRobertson