From 68d9fb23c8d8943bb3609fda33546198aeadcbed Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 13 Jun 2022 00:28:43 +0100 Subject: [PATCH 1/4] Extend the auth rule checks for `m.room.create` events ... and move them up to the top of the function. Since the no auth_events are allowed for m.room.create events, we may as well get the m.room.create event checks out of the way first. --- synapse/event_auth.py | 67 ++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 360a50cc719c..af4179fc6896 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -141,6 +141,15 @@ async def check_state_independent_auth_rules( Raises: AuthError if the checks fail """ + # Implementation of https://spec.matrix.org/v1.2/rooms/v9/#authorization-rules + + # 1. If type is m.room.create: + if event.type == EventTypes.Create: + _check_create(event) + + # 1.5 Otherwise, allow + return + # Check the auth events. auth_events = await store.get_events( event.auth_event_ids(), @@ -180,29 +189,6 @@ async def check_state_independent_auth_rules( auth_dict[(auth_event.type, auth_event.state_key)] = auth_event_id - # Implementation of https://matrix.org/docs/spec/rooms/v1#authorization-rules - # - # 1. If type is m.room.create: - if event.type == EventTypes.Create: - # 1b. If the domain of the room_id does not match the domain of the sender, - # reject. - sender_domain = get_domain_from_id(event.sender) - room_id_domain = get_domain_from_id(event.room_id) - if room_id_domain != sender_domain: - raise AuthError( - 403, "Creation event's room_id domain does not match sender's" - ) - - # 1c. If content.room_version is present and is not a recognised version, reject - room_version_prop = event.content.get("room_version", "1") - if room_version_prop not in KNOWN_ROOM_VERSIONS: - raise AuthError( - 403, - "room appears to have unsupported version %s" % (room_version_prop,), - ) - - return - # 3. If event does not have a m.room.create in its auth_events, reject. creation_event = auth_dict.get((EventTypes.Create, ""), None) if not creation_event: @@ -324,6 +310,41 @@ def _check_size_limits(event: "EventBase") -> None: raise EventSizeError("event too large") +def _check_create(event: "EventBase") -> None: + """Implementation of the auth rules for m.room.create events + + Args: + event: The event to be checked + + Raises: + AuthError if the event does not pass the auth rules + """ + assert event.type == EventTypes.Create + + # 1.1 If it has any previous events, reject. + if event.prev_event_ids(): + raise AuthError(403, "Create event has prev events") + + # 1.2 If the domain of the room_id does not match the domain of the sender, + # reject. + sender_domain = get_domain_from_id(event.sender) + room_id_domain = get_domain_from_id(event.room_id) + if room_id_domain != sender_domain: + raise AuthError(403, "Creation event's room_id domain does not match sender's") + + # 1.3 If content.room_version is present and is not a recognised version, reject + room_version_prop = event.content.get("room_version", "1") + if room_version_prop not in KNOWN_ROOM_VERSIONS: + raise AuthError( + 403, + "room appears to have unsupported version %s" % (room_version_prop,), + ) + + # 1.4 If content has no creator field, reject. + if EventContentFields.ROOM_CREATOR not in event.content: + raise AuthError(403, "Create event lacks a 'creator' property") + + def _can_federate(event: "EventBase", auth_events: StateMap["EventBase"]) -> bool: creation_event = auth_events.get((EventTypes.Create, "")) # There should always be a creation event, but if not don't federate. From 94bd9a0bd6ae2b51f1bc13dcda0a6bcb2565251f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 Jun 2022 18:45:09 +0100 Subject: [PATCH 2/4] Add a test for create events with prev_events --- tests/test_event_auth.py | 45 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index e8e458cfd3ce..ed7a3cbceefe 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -109,6 +109,47 @@ def test_rejected_auth_events(self): ) ) + def test_create_event_with_prev_events(self): + """A create event with prev_events should be rejected + + https://spec.matrix.org/v1.3/rooms/v9/#authorization-rules + 1: If type is m.room.create: + 1. If it has any previous events, reject. + """ + creator = f"@creator:{TEST_DOMAIN}" + + # we make both a good event and a bad event, to check that we are rejecting + # the bad event for the reason we think we are. + good_event = make_event_from_dict( + { + "room_id": TEST_ROOM_ID, + "type": "m.room.create", + "state_key": "", + "sender": creator, + "content": { + "creator": creator, + "room_version": RoomVersions.V9.identifier, + }, + "auth_events": [], + "prev_events": [], + }, + room_version=RoomVersions.V9, + ) + bad_event = make_event_from_dict( + {**good_event.get_dict(), "prev_events": ["$fakeevent"]}, + room_version=RoomVersions.V9, + ) + + event_store = _StubEventSourceStore() + + get_awaitable_result( + event_auth.check_state_independent_auth_rules(event_store, good_event) + ) + with self.assertRaises(AuthError): + get_awaitable_result( + event_auth.check_state_independent_auth_rules(event_store, bad_event) + ) + def test_random_users_cannot_send_state_before_first_pl(self): """ Check that, before the first PL lands, the creator is the only user @@ -564,8 +605,8 @@ def test_join_rules_msc3083_restricted(self) -> None: # helpers for making events - -TEST_ROOM_ID = "!test:room" +TEST_DOMAIN = "example.com" +TEST_ROOM_ID = f"!test_room:{TEST_DOMAIN}" def _create_event( From b09079d6f5fcd6a6dcedc1b587cb23a64d2b2362 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 Jun 2022 19:16:04 +0100 Subject: [PATCH 3/4] changelog --- changelog.d/13087.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13087.bugfix diff --git a/changelog.d/13087.bugfix b/changelog.d/13087.bugfix new file mode 100644 index 000000000000..7c69801afe6a --- /dev/null +++ b/changelog.d/13087.bugfix @@ -0,0 +1 @@ +Fix some inconsistencies in the event authentication code. From 75e5d69b87f0cc343ae6a90bcf0fb678b7a74845 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 17 Jun 2022 14:28:55 +0100 Subject: [PATCH 4/4] Update synapse/event_auth.py Co-authored-by: reivilibre --- synapse/event_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index af4179fc6896..440b1ae418ca 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -314,7 +314,7 @@ def _check_create(event: "EventBase") -> None: """Implementation of the auth rules for m.room.create events Args: - event: The event to be checked + event: The `m.room.create` event to be checked Raises: AuthError if the event does not pass the auth rules