From 169fcbc40e076f24ec9376eb291dad5ca911c0f2 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 7 Jul 2022 16:29:28 -0700 Subject: [PATCH 1/9] add check for non-int powerlevels if supported --- changelog.d/13219.feature | 0 synapse/api/room_versions.py | 33 +++++++++++++++++++++++++++++++++ synapse/event_auth.py | 25 +++++++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 changelog.d/13219.feature diff --git a/changelog.d/13219.feature b/changelog.d/13219.feature new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index 3f85d61b4633..00e81b3afc5e 100644 --- a/synapse/api/room_versions.py +++ b/synapse/api/room_versions.py @@ -84,6 +84,8 @@ class RoomVersion: # MSC3787: Adds support for a `knock_restricted` join rule, mixing concepts of # knocks and restricted join rules into the same join condition. msc3787_knock_restricted_join_rule: bool + # MSC3667: Enforce integer power levels + msc3667_int_only_power_levels: bool class RoomVersions: @@ -103,6 +105,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V2 = RoomVersion( "2", @@ -120,6 +123,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V3 = RoomVersion( "3", @@ -137,6 +141,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V4 = RoomVersion( "4", @@ -154,6 +159,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V5 = RoomVersion( "5", @@ -171,6 +177,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V6 = RoomVersion( "6", @@ -188,6 +195,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) MSC2176 = RoomVersion( "org.matrix.msc2176", @@ -205,6 +213,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V7 = RoomVersion( "7", @@ -222,6 +231,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V8 = RoomVersion( "8", @@ -239,6 +249,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) V9 = RoomVersion( "9", @@ -256,6 +267,7 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) MSC2716v3 = RoomVersion( "org.matrix.msc2716v3", @@ -273,6 +285,7 @@ class RoomVersions: msc2716_historical=True, msc2716_redactions=True, msc3787_knock_restricted_join_rule=False, + msc3667_int_only_power_levels=False, ) MSC3787 = RoomVersion( "org.matrix.msc3787", @@ -290,6 +303,25 @@ class RoomVersions: msc2716_historical=False, msc2716_redactions=False, msc3787_knock_restricted_join_rule=True, + msc3667_int_only_power_levels=False, + ) + V10 = RoomVersion( + "10", + RoomDisposition.STABLE, + EventFormatVersions.V3, + StateResolutionVersions.V2, + enforce_key_validity=True, + special_case_aliases_auth=False, + strict_canonicaljson=True, + limit_notifications_power_levels=True, + msc2176_redaction_rules=False, + msc3083_join_rules=True, + msc3375_redaction_rules=True, + msc2403_knocking=True, + msc2716_historical=False, + msc2716_redactions=False, + msc3787_knock_restricted_join_rule=True, + msc3667_int_only_power_levels=True, ) @@ -308,6 +340,7 @@ class RoomVersions: RoomVersions.V9, RoomVersions.MSC2716v3, RoomVersions.MSC3787, + RoomVersions.V10, ) } diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 0fc2c4b27efe..535057737952 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -80,6 +80,31 @@ def validate_event_for_room_version(event: "EventBase") -> None: Raises: SynapseError if there is a problem with the event """ + # Reject events with stringy power levels if required by room version + if ( + event.type == EventTypes.PowerLevels + and event.room_version.msc3667_int_only_power_levels + ): + for key, value in event.content.items(): + if key in [ + "users_default", + "events_default", + "state_default", + "ban", + "reject", + "kick", + "invite", + ]: + if not isinstance(value, int): + raise AuthError(403, f"{value} must be an integer.") + if key in ["events", "notifications"]: + if not isinstance(value, dict) or not all( + isinstance(v, int) for v in value.values() + ): + raise AuthError( + 403, f"{value} must be a dict with integers as values." + ) + _check_size_limits(event) if not hasattr(event, "room_id"): From 3e7242d509fd210eac4b0f04e627ea52d4467c35 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 7 Jul 2022 16:30:00 -0700 Subject: [PATCH 2/9] add tests to see that events in room v10 fail with stringy powerlevels --- tests/test_event_auth.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index 371cd201af12..e75ffc6ae9d5 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -689,6 +689,41 @@ def test_join_rules_msc3083_restricted(self) -> None: auth_events.values(), ) + def test_room_v10_rejects_string_power_levels(self) -> None: + pl_event_content = {"users_default": "42"} + pl_event = make_event_from_dict( + { + "room_id": TEST_ROOM_ID, + **_maybe_get_event_id_dict_for_room_version(RoomVersions.V10), + "type": "m.room.power_levels", + "sender": "@test:test.com", + "state_key": "", + "content": pl_event_content, + "signatures": {"test.com": {"ed25519:0": "some9signature"}}, + }, + room_version=RoomVersions.V10, + ) + + with self.assertRaises(AuthError): + event_auth.validate_event_for_room_version(pl_event) + + pl_event2_content = {"events": {"m.room.name": "42", "m.room.power_levels": 42}} + pl_event2 = make_event_from_dict( + { + "room_id": TEST_ROOM_ID, + **_maybe_get_event_id_dict_for_room_version(RoomVersions.V10), + "type": "m.room.power_levels", + "sender": "@test:test.com", + "state_key": "", + "content": pl_event2_content, + "signatures": {"test.com": {"ed25519:0": "some9signature"}}, + }, + room_version=RoomVersions.V10, + ) + + with self.assertRaises(AuthError): + event_auth.validate_event_for_room_version(pl_event2) + # helpers for making events TEST_DOMAIN = "example.com" From e2f3113b93dbef45a501bcf2964d2d8875132fe9 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 7 Jul 2022 16:30:08 -0700 Subject: [PATCH 3/9] newsfragement --- changelog.d/13219.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.d/13219.feature b/changelog.d/13219.feature index e69de29bb2d1..9b0240fdc8c5 100644 --- a/changelog.d/13219.feature +++ b/changelog.d/13219.feature @@ -0,0 +1 @@ +Add support for room version 10. From 017cf51db4004bd943b9ca455d45b90b95166779 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 7 Jul 2022 16:38:55 -0700 Subject: [PATCH 4/9] fix newsfragment number --- changelog.d/{13219.feature => 13220.feature} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{13219.feature => 13220.feature} (100%) diff --git a/changelog.d/13219.feature b/changelog.d/13220.feature similarity index 100% rename from changelog.d/13219.feature rename to changelog.d/13220.feature From 808dd94083ea779f8d1bc0c48600d336c2ddae76 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 12 Jul 2022 12:43:18 -0700 Subject: [PATCH 5/9] requested changes --- synapse/event_auth.py | 54 ++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 535057737952..63ebff5f4f17 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -80,30 +80,6 @@ def validate_event_for_room_version(event: "EventBase") -> None: Raises: SynapseError if there is a problem with the event """ - # Reject events with stringy power levels if required by room version - if ( - event.type == EventTypes.PowerLevels - and event.room_version.msc3667_int_only_power_levels - ): - for key, value in event.content.items(): - if key in [ - "users_default", - "events_default", - "state_default", - "ban", - "reject", - "kick", - "invite", - ]: - if not isinstance(value, int): - raise AuthError(403, f"{value} must be an integer.") - if key in ["events", "notifications"]: - if not isinstance(value, dict) or not all( - isinstance(v, int) for v in value.values() - ): - raise AuthError( - 403, f"{value} must be a dict with integers as values." - ) _check_size_limits(event) @@ -754,16 +730,36 @@ def _check_power_levels( ) -> None: user_list = event.content.get("users", {}) # Validate users - for k, v in user_list.items(): + for k in user_list.keys(): try: UserID.from_string(k) except Exception: raise SynapseError(400, "Not a valid user_id: %s" % (k,)) - try: - int(v) - except Exception: - raise SynapseError(400, "Not a valid power level: %s" % (v,)) + # Reject events with stringy power levels if required by room version + if ( + event.type == EventTypes.PowerLevels + and room_version_obj.msc3667_int_only_power_levels + ): + for k, v in event.content.items(): + if k in { + "users_default", + "events_default", + "state_default", + "ban", + "redact", + "kick", + "invite", + }: + if not isinstance(v, int): + raise AuthError(403, f"{v} must be an integer.") + if k in {"events", "notifications", "users"}: + if not isinstance(v, dict) or not all( + isinstance(v, int) for v in v.values() + ): + raise AuthError( + 403, f"{v} must be a dict wherein all the values are integers." + ) key = (event.type, event.state_key) current_state = auth_events.get(key) From b0718420b627a61ebfaad1897165c812363c6a9e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 12 Jul 2022 12:43:43 -0700 Subject: [PATCH 6/9] tweaks to test to accomodate suggested changes --- tests/test_event_auth.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index e75ffc6ae9d5..bf8f5a871d6c 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -704,9 +704,6 @@ def test_room_v10_rejects_string_power_levels(self) -> None: room_version=RoomVersions.V10, ) - with self.assertRaises(AuthError): - event_auth.validate_event_for_room_version(pl_event) - pl_event2_content = {"events": {"m.room.name": "42", "m.room.power_levels": 42}} pl_event2 = make_event_from_dict( { @@ -722,7 +719,14 @@ def test_room_v10_rejects_string_power_levels(self) -> None: ) with self.assertRaises(AuthError): - event_auth.validate_event_for_room_version(pl_event2) + event_auth._check_power_levels( + pl_event.room_version, pl_event, {("fake_type", "fake_key"): pl_event2} + ) + + with self.assertRaises(AuthError): + event_auth._check_power_levels( + pl_event.room_version, pl_event2, {("fake_type", "fake_key"): pl_event} + ) # helpers for making events From da2d7a2e90f2106d4e88f0a374b1f5f304e97cbb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 13 Jul 2022 09:14:48 -0700 Subject: [PATCH 7/9] requested changes --- synapse/event_auth.py | 12 ++++++++---- tests/test_event_auth.py | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 63ebff5f4f17..ea7e52d98148 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -730,11 +730,15 @@ def _check_power_levels( ) -> None: user_list = event.content.get("users", {}) # Validate users - for k in user_list.keys(): + for k, v in user_list.items(): try: UserID.from_string(k) except Exception: raise SynapseError(400, "Not a valid user_id: %s" % (k,)) + try: + int(v) + except Exception: + raise SynapseError(400, "Not a valid power level: %s" % (v,)) # Reject events with stringy power levels if required by room version if ( @@ -752,13 +756,13 @@ def _check_power_levels( "invite", }: if not isinstance(v, int): - raise AuthError(403, f"{v} must be an integer.") + raise SynapseError(400, f"{v} must be an integer.") if k in {"events", "notifications", "users"}: if not isinstance(v, dict) or not all( isinstance(v, int) for v in v.values() ): - raise AuthError( - 403, f"{v} must be a dict wherein all the values are integers." + raise SynapseError( + 400, f"{v} must be a dict wherein all the values are integers." ) key = (event.type, event.state_key) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index bf8f5a871d6c..e42d7b9ba080 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -19,7 +19,7 @@ from synapse import event_auth from synapse.api.constants import EventContentFields -from synapse.api.errors import AuthError +from synapse.api.errors import AuthError, SynapseError from synapse.api.room_versions import EventFormatVersions, RoomVersion, RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.storage.databases.main.events_worker import EventRedactBehaviour @@ -718,12 +718,12 @@ def test_room_v10_rejects_string_power_levels(self) -> None: room_version=RoomVersions.V10, ) - with self.assertRaises(AuthError): + with self.assertRaises(SynapseError): event_auth._check_power_levels( pl_event.room_version, pl_event, {("fake_type", "fake_key"): pl_event2} ) - with self.assertRaises(AuthError): + with self.assertRaises(SynapseError): event_auth._check_power_levels( pl_event.room_version, pl_event2, {("fake_type", "fake_key"): pl_event} ) From 65fd49ebe306f56a279ef882099cbd6fe333f6ca Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 13 Jul 2022 10:14:06 -0700 Subject: [PATCH 8/9] suggested changes --- synapse/event_auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index ea7e52d98148..e5ca3d557448 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -80,7 +80,6 @@ def validate_event_for_room_version(event: "EventBase") -> None: Raises: SynapseError if there is a problem with the event """ - _check_size_limits(event) if not hasattr(event, "room_id"): @@ -735,6 +734,7 @@ def _check_power_levels( UserID.from_string(k) except Exception: raise SynapseError(400, "Not a valid user_id: %s" % (k,)) + try: int(v) except Exception: @@ -756,13 +756,13 @@ def _check_power_levels( "invite", }: if not isinstance(v, int): - raise SynapseError(400, f"{v} must be an integer.") + raise SynapseError(400, f"{v!r} must be an integer.") if k in {"events", "notifications", "users"}: if not isinstance(v, dict) or not all( isinstance(v, int) for v in v.values() ): raise SynapseError( - 400, f"{v} must be a dict wherein all the values are integers." + 400, f"{v!r} must be a dict wherein all the values are integers." ) key = (event.type, event.state_key) From 77a1a57dff1650f811942a5d387bfef772a2d14c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 13 Jul 2022 10:21:27 -0700 Subject: [PATCH 9/9] lint --- synapse/event_auth.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index e5ca3d557448..965cb265daf8 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -762,7 +762,8 @@ def _check_power_levels( isinstance(v, int) for v in v.values() ): raise SynapseError( - 400, f"{v!r} must be a dict wherein all the values are integers." + 400, + f"{v!r} must be a dict wherein all the values are integers.", ) key = (event.type, event.state_key)