Fix inconsistencies in event validation for m.room.create events (#…

…13087)

* 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.

* Add a test for create events with prev_events

changelog.d/13087.bugfix

@@ -0,0 +1 @@
+Fix some inconsistencies in the event authentication code.

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 `m.room.create` 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.

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(