Spec leaves wiggle room in the rules surrounding `m.room.create` events

[DMRobertson]

Link to problem area: https://spec.matrix.org/v1.2/rooms/v9/#authorization-rules

Issue

The auth rules begin:

1. If type is m.room.create:
   1. If it has any previous events, reject.
   2. If the domain of the room_id does not match the domain of the sender, reject.
   3. If content.room_version is present and is not a recognised version, reject.
   4. If content has no creator field, reject.
   5. Otherwise, allow.

I understand this to mean that m.room.create events:

• MUST NOT have any prev_events.
• (implicitly) MUST have a sender, a room_id and a content field, each with appropriate types.
  • this is vaguely guaranteed by the S-S API's checks on PDUs, which asserts that each incoming event "is a valid event". (See also Better specify how to validate events #365.)
  • This presumably means that it compiles to the PDU format of the given room version, and additionally complies to the room-agnostic specification of m.room.create's content, defined in the C-S API spec.
• (implicitly): MUST have a room_id and sender which are matrix IDs.
  • Again, arguably covered by "the event is valid".
• MUST have matching domains for the room_id and sender.
• MUST have a content.room_version field which represents a recognised room version.
• MUST have a content.creator field.

I can see the following holes and problems n this definition.

1. There is no requirement that content.creator is an MXID, nor that its domain matches that of the room_id or sender. Come to think of it, why is the content.creator required at all, when the event already contains a sender? Should servers consider the event invalid if the creator and sender disagree?
2. This doesn't mention auth_events at all. Should servers ignore any auth events on m.room.create, or reject such events?
   • I don't think it makes sense to include any auth_events here at all. Maybe you could make an argument for room upgrades, but there's a "Previous Room" field for that in the event's content.
3. Suppose @alice:server creates a room !room:server. What is there to stop @bob:server from trying to create a new room with the same ID?
   • If so, how should homeservers processing these two events decide who wins? (origin_server_ts, tiebreaking lexicographically on the creator's MXID?)
   • The client-server API is supposed to forbid this with M_ROOM_IN_USE, but a buggy or malicious homeserver could send whatever PDUs it likes.

Additionally:

1. A cross-reference to the definition of m.room.create would be useful here (though note my confusion and complaints in Confusing cycle: S-S API -> room version auth rules -> S-S api auth events selection #1046).

[turt2live]

creator is removed by matrix-org/matrix-spec-proposals#2175 - needs implementation.

The rules don't mention auth_events, but there is an implicit requirement that they can't have auth events. I believe it'll be noted in a sentence or two somewhere in the server-server spec rather than in the auth rules (we should move it).

If server creates two rooms with the same ID, that's its problem to deal with. Room IDs are globally unique, hence the domain component for namespacing.

[turt2live]

for the third point: if a malicious server were to interject a conflicting room ID (ie: create event originating from a different server, using the victim server's namespace), then Rule 1.2 of the authorization rules should prevent any other server from accepting that event, and thus the falsified room.

If a server creates a room ID within its own namespace though (due to implementation bugs or otherwise), then it's just a DAG fork and the sending server now has a unique problem on its hands.

[DMRobertson]

The rules don't mention auth_events, but there is an implicit requirement that they can't have auth events. I believe it'll be noted in a sentence or two somewhere in the server-server spec rather than in the auth rules (we should move it).

Indeed. Auth event selection reads:

The auth_events for the m.room.create event in a room is empty; for other events, it should be the following subset of the room state: [...]

[DMRobertson]

I'm happy to add that to the auth rules, but as I understand it that would (strictly speaking) require a new room version?

[turt2live]

The auth rules aspect should almost certainly get its own issue so we can investigate whether it needs a room version or not without drowning out other concerns here.

[DMRobertson]

The auth rules aspect should almost certainly get its own issue so we can investigate whether it needs a room version or not without drowning out other concerns here.

-> #1061

[richvdh]

There is no requirement that content.creator is an MXID, nor that its domain matches that of the room_id or sender.

In theory, the two are supposed to match exactly. This is a bit of a security hazard, since it would be easy for implementations to assume that they are the same when they are not (and hence grant permissions to the sender of the m.room.create event that are not justified).