Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify when state_key needs to be "" in authorization rules #1998

Open
CobaltCause opened this issue Nov 15, 2024 · 5 comments
Open

Clarify when state_key needs to be "" in authorization rules #1998

CobaltCause opened this issue Nov 15, 2024 · 5 comments
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit

Comments

@CobaltCause
Copy link

Link to problem area:

https://spec.matrix.org/v1.12/rooms/v11/#authorization-rules

Issue

It's not explicit that state events without other requirements on the state_key field must be "".

For example, there's nothing like

N. If state_key is not "", reject.

for handling m.room.create events.

@CobaltCause CobaltCause added the clarification An area where the expected behaviour is understood, but the spec could do with being more explicit label Nov 15, 2024
@richvdh
Copy link
Member

richvdh commented Nov 26, 2024

Related: #365

@dkasak
Copy link
Member

dkasak commented Dec 24, 2024

In practice, Synapse understands this to be a part of auth rule 2.2, which states:

If there are entries whose type and state_key don’t match those specified by the auth events selection algorithm described in the server specification, reject.

The actual requirements for state_key of a particular standard event type are listed in the C2S API, in the Room Events section, under each particular event type. For example, for m.room.power_levels, it is in https://spec.matrix.org/v1.12/client-server-api/#mroompower_levels (scroll down to where it says State key: A zero-length string in the first table in that section).

My conclusion is that the intention was to consider an event with a state tuple like ("m.room.power_levels", "foo") to not even be a power levels event. In other words, the definition of a "power levels event" is an event with type "m.room.power_levels" and state key "".

@CobaltCause
Copy link
Author

Firstly, aren't authorization rules 2.* referring to events that appear in the current event's auth_events and not the current event itself? Like, my read of 2.2 is that an event that has any auth_events that refer to an event with mismatching types and state keys should be rejected, but not that an event with mismatching types and state keys should be rejected.

Secondly, my understanding is that the C2S API documentation is not to be trusted for authorization rules. For an example we all know and love, the C2S API has specific requirements on the value of .content.predecessor in m.room.create events but the authorization rules don't restrict this at all. I think it'd be quite confusing and not particularly helpful if the decided conclusion of this issue is "the C2S API is authoritative for authorization rules except when it isn't".

@dkasak
Copy link
Member

dkasak commented Dec 24, 2024

Firstly, aren't authorization rules 2.* referring to events that appear in the current event's auth_events and not the current event itself? Like, my read of 2.2 is that an event that has any auth_events that refer to an event with mismatching types and state keys should be rejected, but not that an event with mismatching types and state keys should be rejected.

Your reading is correct, 2.2 is solely about auth events of events, not the events themselves. However, my interpretation of the intended effects is that events with state tuples such as ("m.room.power_levels", NON-EMPTY-STATE-KEY) should not be rejected, they are simply not to be considered power levels events. And 2.2 may be enough for that, given it prevents you from citing such an event as an auth event, forcing you instead to cite an actual power levels event (i.e. event with a ("m.room.power_levels", "") state tuple).

Secondly, my understanding is that the C2S API documentation is not to be trusted for authorization rules. For an example we all know and love, the C2S API has specific requirements on the value of .content.predecessor in m.room.create events but the authorization rules don't restrict this at all. I think it'd be quite confusing and not particularly helpful if the decided conclusion of this issue is "the C2S API is authoritative for authorization rules except when it isn't".

I completely agree that the current state of this spec in this area is subpar and it was not my intent to argue that the issue should be closed without changes to the spec. The spec should clarify the intended behaviour in a better place, likely in the room version.

I'm just trying to see whether this is a glaring omission or something more akin to a clarification, and I think it's the latter.

@turt2live
Copy link
Member

The spec's intention is indeed that non-empty state keys on auth event types are treated as any other random state event: no meaning to the operation of the room. Only state events with empty string state keys are true auth events.

This definitely sounds like clarification territory, as the current auth rules (and state res) spec appears to assume the reader knows this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit
Projects
None yet
Development

No branches or pull requests

4 participants