MSC3761: State event change control #3761

turt2live · 2022-03-31T19:12:24Z

Draft because of TODOs, but general concept should be there.

turt2live

documenting comments from internal review (anonymized/summarized where context is sensitive)

turt2live · 2022-03-31T19:20:09Z

proposals/3761-state-event-change-control.md

+Additionally, anyone who can normally redact a (state) event can *always* redact the ACL'd state event and the ACL
+event itself.
+
+Only events which are **not** included in `auth_events` can be ACL'd. This means that ACL events cannot be ACL'd,


I don't quite understand what this line is trying to say. Which event are we talking about when we reference auth_events here?

It's trying to say that you can't apply ACLs to membership events, power level events, etc: the things that are required for inclusion in auth_events on any event.

All other state, like topic changes, beacons, and other custom stuff would be ACL-able.

(Gah, I accidentally deleted my own review comment due to Github's sucky UX, so reposting.)

Suggested wording:

ACLs can be set on all state events **except** [authorization events](https://spec.matrix.org/v1.2/rooms/v9/#authorization-rules).

The m.event.acl event is included in authorization events though, so should already be covered.

(also, why v2?)

Ah, good point. And the v2 was just an accident, edited accordingly.

turt2live · 2022-03-31T19:21:31Z

proposals/3761-state-event-change-control.md

+target state event, the event cannot be sent. Similarly, if the ACL event ID is not able to be located, the event
+cannot be sent.
+
+***TODO: Should the state event be rejected if the ACL event ID cannot be resolved, or just treat it as an ACL of "no one"?***


This may splitbrain the room if another homeserver does end up resolving the event ID?

it'd be similar to the server missing the power level event during auth, I think.

turt2live · 2022-03-31T19:28:09Z

proposals/3761-state-event-change-control.md

+
+To apply this ACL to a state event, the sender would include an `?acl=$event_id_of_acl` query parameter to
+[`PUT /state/:type/:key`](https://spec.matrix.org/v1.2/client-server-api/#put_matrixclientv3roomsroomidstateeventtypestatekey)
+which denotes the ACL event to apply to the to-be-sent state event. If the sender fails the existing ACL on the


There was discussion around race conditions and someone being able to apply ACLs before you get a chance to. Particularly if someone maliciously wants to take over the event type + state key tuple they could send their event at the same time or earlier than you, making the ACL questionable and potentially not apply.

I think this would be fixed by state resolution which will pick a "correct" event in the case of conflict, and if someone is racing to claim a state key before you then it's a bit of a social/room moderation problem moreso than tech. If someone were to be maliciously trying to claim location beacons (for example) then that might not be a room you want to be a member of.

That said, we should consider a case where a state event can be sent by PL0 people and thus lead to further (social) conflict.

ShadowJonathan · 2022-03-31T19:39:19Z

proposals/3761-state-event-change-control.md

+
+## Proposal
+
+We introduce a new state event called `m.event.acl` with a `content` which looks as follows:


To me, the way that this event is defined as a state event (as opposed to a regular event) is "weird".

Down below the state event is specifically mentioned by ID, as opposed to relied on when defining the event. That's understandable, considering that you'd essentially "pick the rules" when posting an event like this.

However, via this method, one would spew a bunch of nonsensical m.event.acl events as state in a room, while they may not be useful as a state event in that room thereafter.

This to me looks a lot more reasonable as a normal event. However, I can imagine why it wasn't chosen as thus, due to retention policies potentially evicting and removing this event, as it is no state event.

This way, this detail just strikes me as "weird", though not "wrong". I'd like to know if my suspicions and musings regarding this are correct.

Reading the other comments, I get the feeling that my initial assessment of "picking and choosing" ACLs wasn't initially correct, though to me it doesn't make much sense to have it any other way; If i were to have 3 different events, and all three of them would have different ACL rules, why would they all adhere to one specific ACL event?

It was picked as a state event so the server is reasonably assured to get it as part of event authorization: authorizing an event against a non-state event means taking a harder path to locate the event.

Plus, with it being a state event we inherit the permissions structure of most rooms where regular members can't send them, preventing spam. A notable exception is in the MSC where someone can specify their own ACL event, but this is a future-proof optimization for if/when we want to apply ACLs to non-state too. Worst case is you end up with double the state events (1 for each member, 1 for each member's personal ACL).

dkasak · 2022-04-01T13:43:35Z

proposals/3761-state-event-change-control.md

+Additionally, anyone who can normally redact a (state) event can *always* redact the ACL'd state event and the ACL
+event itself.


What is the intended effect of such a redaction?

Moderation and anti-spam.

Sorry, I just realized I'm only talking about part of that sentence and was not explicit enough.

I'm only asking about the redaction of the ACL event itself. The intent is not that the redaction should neutralize the ACL's rules, right?

potentially. It's not really been worked out what will be protected from redaction and what won't.

It sounds like redactions should not have the ability to neutralize the ACL. Otherwise the example for with_power_for is useless since that power is always available to people who can redact things given they could always just redact (and therefore neutralize) the ACL.

dkasak · 2022-04-01T13:45:23Z

proposals/3761-state-event-change-control.md

+To apply this ACL to a state event, the sender would include an `?acl=$event_id_of_acl` query parameter to
+[`PUT /state/:type/:key`](https://spec.matrix.org/v1.2/client-server-api/#put_matrixclientv3roomsroomidstateeventtypestatekey)
+which denotes the ACL event to apply to the to-be-sent state event. If the sender fails the existing ACL on the
+target state event, the event cannot be sent. Similarly, if the ACL event ID is not able to be located, the event
+cannot be sent.
+
+***TODO: Should the state event be rejected if the ACL event ID cannot be resolved, or just treat it as an ACL of "no one"?***
+
+The ACL event ID is then appended to the top level of the state event as `acl`:
+
+```json5
+{
+    "type": "org.example.event",
+    "state_key": "whatever",
+    "acl": "$abcdef",
+    "content": {
+        // event-specific detail
+    },
+    // etc
+}
+```
+
+Changes to that state event, including the removal of the ACL event ID from the event (by not sending an `?acl` on
+subsequent `PUT`s), must include the ACL event which allows the change in its `auth_events`.


The mechanism of use and rules relating to this new event are quite convoluted. I may be missing something, but why does this need to be a separate event at all, as opposed to embedding the ACL content directly under the acl key of the actual ACL-ed state event?

I guess the primary problem is that we have no nice way of passing an additional JSON object to the current API, besides the content. Still, all of this feels very race condition-y to me and I would encourage us to try finding a way of embedding the ACL content directly when sending a state event instead of making it a separate state event.

We lose the ability to have reusable ACLs though, which leads to data duplication :(

That's a valid concern, though the finickiness (and potential for unforeseen problems) with the current approach seem worse. It should be noted that state events are relatively rare compared to non-state events and we're already faring quite a bit worse there with respect to redundancy.

I'm also having trouble imagining how this ACL reuse would work from a client's POV. Let's say I decide to place an ACL on a new event and I have previously already sent an equivalent ACL to the room. How would I learn that this ACL is already available in the state?

I guess I would have to fetch all of my m.event.acl events in the room, then iterate through them in search for an equivalent one to the one I'm planning on setting right now. That seems rather error-prone.

It would be nice, as a client author, to be able to do something like m.peer_unwritable or something under acl to handle a common usecase - basically have a few basic ACLS baked in.

I'm with @dkasak here. This seems finnicky, and having to add new stuff to auth_events is really scary.

MTRNord · 2022-04-02T19:32:38Z

proposals/3761-state-event-change-control.md

@@ -0,0 +1,97 @@
+# MSC3761: State event change control


This comment exists to point out an additional usecase where this would help (as I assume not many will be aware of this?):

Matrix-Art could highly benefit from this.

The usecase is that MatrixArt is using a Folder structure based upon MSC3089 for the user directory per instance.

The thing is that any user needs to be able to add the child spaces to it to reflect their own profiles. However, as of today, there is no way to protect against User A removing the room from User B from that space. This MSC most likely will be able to protect against this. Explicitly, I would need it to restrict the event edits to their own Rooms instead of modifying the space child states of every user.

kegsay · 2022-04-04T10:56:01Z

proposals/3761-state-event-change-control.md

@@ -0,0 +1,97 @@
+# MSC3761: State event change control
+


I don't really like this proposal. It feels like this functionality should exist in the power levels event, and the fact that it doesn't complicates things. Any kind of server-enforced check means modification to auth rules. This can't live in the PL event currently because this MSC is allowing random users to assign arbitrary ACLs to random sets of state events, which seem to just be given on a first-come first-serve basis. This also has problems.

richvdh

generally this feels like it is going to be tricky to get right, and I'm yet to be convinced it solves the problems at hand.

richvdh · 2022-04-05T14:47:10Z

proposals/3761-state-event-change-control.md

+The ability to send this ACL event would still be subject to power levels, except in the special case where the
+sender uses a `state_key` of their own user ID: this bypasses the `events["m.event.acl"]` power level check,
+allowing the user to define a single ACL for their state events. If they want multiple ACLs, they will need
+permission to do so.


this "exception" (allowing @alice:example.com to send an event withstate_key: @alice:example.com) would be specific to events of type: m.event.acl, correct?

it would be good to clarify how this interacts with events_default and state_default, but note that this implies there is no way to stop people sending such events in a room, even if that room is supposedly locked down with a high events_default.

richvdh · 2022-04-05T15:04:15Z

proposals/3761-state-event-change-control.md

+To apply this ACL to a state event, the sender would include an `?acl=$event_id_of_acl` query parameter to
+[`PUT /state/:type/:key`](https://spec.matrix.org/v1.2/client-server-api/#put_matrixclientv3roomsroomidstateeventtypestatekey)
+which denotes the ACL event to apply to the to-be-sent state event. If the sender fails the existing ACL on the
+target state event, the event cannot be sent. Similarly, if the ACL event ID is not able to be located, the event
+cannot be sent.
+
+***TODO: Should the state event be rejected if the ACL event ID cannot be resolved, or just treat it as an ACL of "no one"?***
+
+The ACL event ID is then appended to the top level of the state event as `acl`:
+
+```json5
+{
+    "type": "org.example.event",
+    "state_key": "whatever",
+    "acl": "$abcdef",
+    "content": {
+        // event-specific detail
+    },
+    // etc
+}
+```
+
+Changes to that state event, including the removal of the ACL event ID from the event (by not sending an `?acl` on
+subsequent `PUT`s), must include the ACL event which allows the change in its `auth_events`.


I'm with @dkasak here. This seems finnicky, and having to add new stuff to auth_events is really scary.

richvdh · 2022-04-05T15:06:34Z

proposals/3761-state-event-change-control.md

+{
+    "type": "org.example.event",
+    "state_key": "whatever",
+    "acl": "$abcdef",


to be explicit: is this key redacted when the event is redacted? If so, I think that leaves us with an event that may not be valid (because there is no longer an acl allowing its change)?

richvdh · 2022-04-05T15:09:09Z

proposals/3761-state-event-change-control.md

+Changes to that state event, including the removal of the ACL event ID from the event (by not sending an `?acl` on
+subsequent `PUT`s), must include the ACL event which allows the change in its `auth_events`.


to be clear: the original event doesn't need an m.event.acl in its auth_events (just a reference to it in the event body), but subsequent changes do require it in the auth events?

er, why?

turt2live · 2022-04-05T16:01:06Z

This MSC and any updates to it are blocked on MSC3672 defining the actual problems in more detail - this MSC appears to be premature for what was intended.

turt2live added 2 commits March 31, 2022 13:10

MSC: State event change control

6eecb72

Reference some MSCs

b62218a

turt2live changed the title ~~MSC: State event change control~~ MSC3761: State event change control Mar 31, 2022

turt2live commented Mar 31, 2022

View reviewed changes

ShadowJonathan reviewed Mar 31, 2022

View reviewed changes

dkasak reviewed Apr 1, 2022

View reviewed changes

MTRNord reviewed Apr 2, 2022

View reviewed changes

kegsay reviewed Apr 4, 2022

View reviewed changes

richvdh reviewed Apr 5, 2022

View reviewed changes

turt2live added the blocked Something needs to be done before action can be taken on this PR/issue. label Apr 5, 2022

andybalaam mentioned this pull request May 5, 2022

MSC3757: Restricting who can overwrite a state event #3757

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSC3761: State event change control #3761

MSC3761: State event change control #3761

turt2live commented Mar 31, 2022

turt2live left a comment

turt2live Mar 31, 2022

turt2live Mar 31, 2022

dkasak Apr 1, 2022 •

edited

Loading

turt2live Apr 1, 2022

dkasak Apr 1, 2022

turt2live Mar 31, 2022

turt2live Mar 31, 2022

turt2live Mar 31, 2022

ShadowJonathan Mar 31, 2022

ShadowJonathan Mar 31, 2022 •

edited

Loading

turt2live Mar 31, 2022

dkasak Apr 1, 2022

turt2live Apr 1, 2022

dkasak Apr 1, 2022

turt2live Apr 1, 2022

dkasak Apr 1, 2022

dkasak Apr 1, 2022

turt2live Apr 1, 2022

dkasak Apr 1, 2022

gleachkr Apr 3, 2022 •

edited

Loading

richvdh Apr 5, 2022

MTRNord Apr 2, 2022 •

edited

Loading

kegsay Apr 4, 2022

richvdh left a comment

richvdh Apr 5, 2022

richvdh Apr 5, 2022

richvdh Apr 5, 2022

richvdh Apr 5, 2022

richvdh Apr 5, 2022

turt2live commented Apr 5, 2022


		## Proposal

		We introduce a new state event called `m.event.acl` with a `content` which looks as follows:

		Additionally, anyone who can normally redact a (state) event can always redact the ACL'd state event and the ACL
		event itself.

		Changes to that state event, including the removal of the ACL event ID from the event (by not sending an `?acl` on
		subsequent `PUT`s), must include the ACL event which allows the change in its `auth_events`.

MSC3761: State event change control #3761

Are you sure you want to change the base?

MSC3761: State event change control #3761

Conversation

turt2live commented Mar 31, 2022

turt2live left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkasak Apr 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShadowJonathan Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gleachkr Apr 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MTRNord Apr 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented Apr 5, 2022

dkasak Apr 1, 2022 •

edited

Loading

ShadowJonathan Mar 31, 2022 •

edited

Loading

gleachkr Apr 3, 2022 •

edited

Loading

MTRNord Apr 2, 2022 •

edited

Loading