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

MSC3757: Restricting who can overwrite a state event #3757

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Changes from 31 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
ff5fd48
Add 'Restricting who can overwrite a state event'
andybalaam Mar 25, 2022
610f244
Rename MSC3757 to its proper number
andybalaam Mar 25, 2022
bfde329
Clarify wording of comment
andybalaam Mar 31, 2022
344e876
Add unstable room version
andybalaam Mar 31, 2022
ccb7e52
Note that this requires a new room version
andybalaam Mar 31, 2022
1ce7e0e
Refer to MSC3760 as an alternative
andybalaam Mar 31, 2022
6df6109
Re-word link to MSC3760
andybalaam Mar 31, 2022
6e108b3
Update to m.self to match latest thinking on MSC3672
andybalaam Apr 26, 2022
bd4176f
Remove the user of 'unprivileged'
andybalaam May 11, 2022
e352a1d
Add a note about allowed characters
andybalaam May 11, 2022
5e95ff3
Reword proposed auth rule
AndrewFerr Sep 4, 2024
68dc97f
Nitpick: always use formatted text for state_key
AndrewFerr Sep 11, 2024
17890fd
Nitpick: remove trailing whitespace
AndrewFerr Sep 11, 2024
f962bf3
Change recommended room versions to apply on
AndrewFerr Sep 11, 2024
dd9b33e
Mention that _ can't be in any form of server name
AndrewFerr Sep 24, 2024
eb0eed6
Add issue of incompatibility with long MXIDs
AndrewFerr Sep 24, 2024
ac24510
Add issue of underscores in domain names
AndrewFerr Sep 24, 2024
9490cbd
Fix typo
AndrewFerr Sep 24, 2024
486b0cd
Use device ID suffix in location beacon example
AndrewFerr Sep 26, 2024
590ff96
Increase state key size limit & set suffix limit
AndrewFerr Sep 26, 2024
d9b149d
Keep original size limit on unprefixed state keys
AndrewFerr Sep 27, 2024
ae17437
Move paragraph to alternative section
AndrewFerr Sep 27, 2024
8222738
Add alternative of state key arrays
AndrewFerr Sep 27, 2024
63955d7
Add alternative of field for non-state events
AndrewFerr Sep 27, 2024
07d784a
Clarify state subkey/array relevance to user IDs
AndrewFerr Sep 27, 2024
99698ef
Fix formatting of auth rule's numeric list
AndrewFerr Sep 27, 2024
9f4f31a
Rephrase the current restrictions on state events
AndrewFerr Oct 7, 2024
75f03da
Better explain limitations of current restrictions
AndrewFerr Oct 7, 2024
e833e8a
Reword and reformat
AndrewFerr Oct 7, 2024
a4b40b5
Remove redundant explanation for separating with _
AndrewFerr Oct 7, 2024
a0da59b
Scope proposal to a future room version
AndrewFerr Oct 7, 2024
5855a7f
Elaborate on multi-component state key alternative
AndrewFerr Oct 8, 2024
deba3b8
Add sub-headers to alternatives section
AndrewFerr Oct 8, 2024
8090f69
Propose sender-scoped state with ownership flag
AndrewFerr Oct 8, 2024
3a0d095
Fix typo
AndrewFerr Oct 8, 2024
e16482a
Mention impact of sender-scoped state on servers
AndrewFerr Oct 8, 2024
1ddddb6
Say how the ownership flag impacts administration
AndrewFerr Oct 9, 2024
fd87b8a
Fix contradictions for flag alternative
AndrewFerr Oct 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 182 additions & 0 deletions proposals/3757-restricting-who-can-overwrite-a-state-event.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
# MSC3757: Restricting who can overwrite a state event.
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is actually derestricting who can overwrite a state event.

Suggested change
# MSC3757: Restricting who can overwrite a state event.
# MSC3757: Relaxing the restrictions on who can overwrite a state event.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true for MSC3779, but not as much for this one; the former bypasses PL restrictions for setting state that belongs to the sender (where "belongs to" = the state key starts with the sender's MXID), but this MSC does not.

The restriction proposed by this MSC is to prevent state that belongs to a particular user from being overwritten by other, equally-powerful users.

The only PL restriction that's relaxed by this MSC is for allowing more powerful users to overwrite state that doesn't belong to them, for the sake of having a tool against state graffiti.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Still, I find the title a bit confusing. How about:

Suggested change
# MSC3757: Restricting who can overwrite a state event.
# MSC3757: Extending the set of write-protected state keys

or something

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I informally refer to this MSC as "the owned state MSC" despite that being the title of MSC3779 😉

Since one of the distinguishing differences of this MSC over 3779 is the ability for admins to manage others' state, maybe we could call it

Suggested change
# MSC3757: Restricting who can overwrite a state event.
# MSC3757: State event ownership (with admin management)

?


andybalaam marked this conversation as resolved.
Show resolved Hide resolved
## Problem

Currently, there are two main restrictions on who can overwrite a state event, enforced by rules
7 and 8 of the [authorization rules](https://spec.matrix.org/latest/rooms/v11/#authorization-rules):

* Only users with a power level greater than or equal to the "required power level" for a state
event type may send state events of that type.
* State events with a `state_key` that equals a user ID may be overwritten only by the user whose
ID matches the state key.

With these restrictions, only a single piece of state for any state event type may have its write
access limited to a particular user (the state event whose `state_key` is set to the ID of the user
who has write access to it).

This is problematic if a user needs to publish multiple state
events of the same type in a room, but would like to set access control so
that only they can subsequently update the event. An example of this is if a
user wishes to publish multiple live location share beacons as per
[MSC3489](https://github.com/matrix-org/matrix-spec-proposals/pull/3489) and
[MSC3672](https://github.com/matrix-org/matrix-spec-proposals/pull/3672),
for instance one per device.
They will typically not want other users in the room to be able to overwrite the state event,
so there ought to be a mechanism to prevent other users from doing so.

## Proposal
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

In a future room version,
**if a state event's `state_key` *starts with* a user ID followed by an underscore, only the user
with that ID or users with a higher power level then them may overwrite that state event.**
This is an extension of the current behaviour where state events may be overwritten only by users
whose ID *exactly equals* the `state_key`.

As the spec currently enforces [a size limit of 255 bytes for both user IDs and state keys](
https://spec.matrix.org/unstable/client-server-api/#size-limits),
the size limit on state keys is increased to **511 bytes** to allow prefixing any currently-valid
state key with a maximum-length user ID (and a separator character).
The size of a state key suffix after a leading user ID and the separator character is limited to
**255 bytes** so that any such suffix may follow any user ID without the complete state key
ever surpassing the total state key size limit.
Similarly, the size of a state key without a leading user ID is limited to **255 bytes** so that any
state key without a leading user ID may be given one without ever surpassing the total size limit.

Users with a higher power level than a state event's original sender may overwrite the event
despite their user ID not matching the one in event's `state_key`. This fixes an abuse
vector where a user can immutably graffiti the state within a room
by sending state events whose `state_key` is their user ID.

Practically speaking, this means modifying the
[authorization rules](https://spec.matrix.org/v1.2/rooms/v9/#authorization-rules) such that rule 8:

> 8. If the event has a `state_key` that starts with an `@` and does not match the `sender`, reject.

becomes:

> 8. If the event has a `state_key`:
> 1. If the `state_key` starts with an `@`:
> 1. If the prefix of the `state_key` before the first `_` that follows the first `:` (or end
> of string) is not a valid user ID, reject.
> 1. If the size of the `state_key` without its leading user ID is greater than 256 bytes, reject.
> 1. If the leading user ID does not match the `sender`, and the `sender`'s power level is not
> greater than that of the user denoted by that ID, reject.
> 1. Otherwise, if size the `state_key` is greater than 255 bytes, reject.

Note that the size limit of 256 bytes after a leading user ID includes the separating `_`.

No additional restrictions are made about the content of the `state_key`, so any characters that
follow the `sender` + `_` part are only required to be valid for use in a `state_key`.

For example, to post a live location sharing beacon from
[MSC3672](https://github.com/matrix-org/matrix-spec-proposals/pull/3672)
for one of a user's devices:

```json=
{
"type": "m.beacon_info",
"state_key": "@stefan:matrix.org_{deviceid1}", // Ensures only the sender or higher PL users can update
"content": {
"m.beacon_info": {
"description": "Stefan's live location",
"timeout": 600000,
"live": true
},
"m.ts": 1436829458432,
"m.asset": {
"type": "m.self"
}
}
}
```

## Potential issues

### Incompatibility with domain names containing underscores

Although both [the spec](https://spec.matrix.org/unstable/appendices/#server-name)
and [RFC 1035 §2.3.1](https://www.rfc-editor.org/rfc/rfc1035#section-2.3.1)
forbid the presence of underscores in domain names,
there noneless exist resolvable domain names that contain underscores.
The proposed auth rule for parsing a leading user ID from an underscore-separated state key would
fail on such domain names.

Possible solutions include:
- using a different character to terminate a leading user ID in state keys. That character must be
one known to be absent from domain names in practice, and must also not be any character that
the spec allows to appear in a server name.
- refining the proposed auth rule for parsing a leading user ID such that it does not fail on domain
names that contain an underscore. One way to achieve this is to leverage the absence of
underscores from top-level domains.

## Alternatives
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved

[MSC3489](https://github.com/matrix-org/matrix-spec-proposals/pull/3489)
and [MSC3672](https://github.com/matrix-org/matrix-spec-proposals/pull/3672)
originally proposed that the event type could be made variable,
with an ID appended to each separately posted event so that each one could
separately be locked to the same user ID in the `state_key`. However, this is
problematic because you can't proactively refer to these event types in the
`events` field of the `m.room.power_levels` event to allow users to post
them - and they also are awkward for some client implementations to
manipulate.

An earlier draft of this MSC proposed putting a flag on the contents of the
event (outside of the E2EE payload) called `m.peer_unwritable: true` to indicate
if other users were prohibited from overwriting the event or not. However, this
unravelled when it became clear that there wasn't a good value for the `state_key`,
which needs to be unique and not subject to races from other malicious users.
By scoping who can set the `state_key` to be the user ID of the sender, this problem
goes away.

[MSC3760](https://github.com/matrix-org/matrix-spec-proposals/pull/3760)
proposes to include a dedicated `state_subkey` as the third component of what
makes a state event unique.
As an extension to this idea, a comment in [the discussion of this MSC](
https://github.com/matrix-org/matrix-spec-proposals/pull/3757#issuecomment-2099010555)
proposes allowing `state_key` to be an array of strings.
Either proposal allows for effectively including an owning user ID in a state key without having to
string-pack the user ID with another string.
However, either proposal would alter the nature of state events and state resolution.
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved

Another comment in [the discussion of this MSC](
https://github.com/matrix-org/matrix-spec-proposals/pull/3757#discussion_r1103877363)
proposes an optional top-level field for both state and non-state events that designates ownership
of the containing event to a particular user.
This would provide ownership semantics for not only state events, but also message events, which may
be used to restrict event replacements / redactions to only the designated owner of an event.
However, it remains to be decided how using this top-level field for state events should affect
state resolution; namely, whether it is possible to set multiple events with the same `state_key`
but different owners.

## Security considerations

This change requires a new room version, so will not affect old events.
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved

As this changes auth rules, the possibility of new attacks on state resolution must be considered.
For instance, if a user had higher power level at some point in the past, will they be able to
somehow abuse this to overwrite the state event, despite not being its owner?

When using a leading user ID in a `state_key` to restrict who can write the event, the character to
terminate the leading user ID was deliberately chosen to be an underscore, as it is not
allowed in [any form of server name](https://spec.matrix.org/v1.11/appendices/#server-name)
(either a DNS name or IPv4/6 address, with or without a numeric port specifier) and thus cannot be
confused as part of the server name of a leading user ID (with one caveat mentioned as a
[potential issue](#incompatibility-with-domain-names-containing-underscores)).
A pure prefix match will **not** be sufficient,
as `@matthew:matrix.org` will match a `state_key` of form `@matthew:matrix.org.evil.com:id1`.

This changes auth rules in a backwards incompatible way, which will break any
use cases which assume that higher power level users cannot overwrite state events whose
`state_key` is a different user ID. This is considered a feature rather than a bug,
fixing an abuse vector where users could send arbitrary state events
which could never be overwritten.

andybalaam marked this conversation as resolved.
Show resolved Hide resolved
## Unstable prefix

While this MSC is not considered stable, implementations should apply the behaviours of this MSC on
top of room version 10 or higher as `org.matrix.msc3757`.

## Dependencies

None