-
Notifications
You must be signed in to change notification settings - Fork 412
MSC4311: Ensuring the create event is available on invites #4311
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
b7ec690
MSC: Ensuring the create event is available on invites and knocks
turt2live a982f53
Clarify that we're not replacing auth checks on create events
turt2live 78a1387
Acknowledge disparity in unaffected room formats
turt2live 4971929
This isn't a word despite what spellcheck thinks
turt2live 7d2c5f8
add migration steps
turt2live eeb4640
Just use the normal stripped_state format
turt2live 05b8b49
Rewrite problem statement and solution components to match new scope
turt2live 5ac4656
review: improve intro
turt2live e651b61
review: clarify format
turt2live afff25e
review: fix knocks
turt2live 72f6a3a
review: fix room_id calculation
turt2live bef6540
review: rework migration wording
turt2live 2104c97
Fully format all events in stripped state
turt2live 05b3d96
Spelling
turt2live 249dec3
Make migration normative
turt2live d34f796
Apply suggestions from code review
turt2live b8ec2f6
Adjust requirements on servers
turt2live File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
# MSC4311: Ensuring the create event is available on invites and knocks | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
Historically, when processing an invite or knock, safety tooling would parse the room ID despite | ||
[being opaque](https://spec.matrix.org/v1.15/appendices/#room-ids) to determine the server which | ||
originally created the room. If that server was considered abusive, the incoming invite or outbound | ||
knock may be rejected or blocked early by the tooling. This approach is preferred because the user | ||
richvdh marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
sending the invite may not be on the same server as the user who created the room, but is often still | ||
checked by safety tooling. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
With [MSC4291](https://github.com/matrix-org/matrix-spec-proposals/pull/4291), room IDs lose their | ||
domain component. This, combined with [Stripped State](https://spec.matrix.org/v1.15/client-server-api/#stripped-state) | ||
recommending rather than requiring the `m.room.create` event, makes the above check harder if not | ||
impossible in some cases. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
This MSC shifts the `m.room.create` event to a *required* stripped state event, and imposes validation | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
to ensure the event matches the room. To support the new validation, the `m.room.create` event must | ||
be formatted as a full PDU in the stripped state of [invites](https://spec.matrix.org/v1.15/server-server-api/#put_matrixfederationv1inviteroomideventid) | ||
and [knocks](https://spec.matrix.org/v1.15/server-server-api/#get_matrixfederationv1make_knockroomiduserid) | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
over federation. Together, these changes allow safety tooling (and servers) to better validate invites | ||
in particular. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
|
||
## Proposal | ||
|
||
On the Client-Server API, `m.room.create` MUST be provided in [Stripped State](https://spec.matrix.org/v1.15/client-server-api/#stripped-state), | ||
where available. No other changes are proposed to the Client-Server API. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
Over federation, for room versions affected by [MSC4291](https://github.com/matrix-org/matrix-spec-proposals/pull/4291), | ||
the `m.room.create` event MUST be included in [`invite_room_state`](https://spec.matrix.org/v1.15/server-server-api/#put_matrixfederationv1inviteroomideventid) | ||
and [`knock_room_state`](https://spec.matrix.org/v1.15/server-server-api/#get_matrixfederationv1make_knockroomiduserid) | ||
and MUST be a properly-formatted PDU according to that room version's event format specification. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
If the `m.room.create` event is not present, not a PDU, or not for the room ID specified, the server | ||
MUST fail to continue processing the invite or knock. For invites, this is a `400 M_MISSING_PARAM` | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
standard Matrix error (new to the endpoint). For knocks, this means the server drops the `make_knock` | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
response and never completes a `send_knock`. For both operations, the associated Client-Server API | ||
request is failed with `500 M_BAD_STATE`. A 4xx error isn't used for the Client-Server API because | ||
there's nothing the client can materially do differently to fix that request. | ||
|
||
For room versions *not* affected by MSC4291, servers SHOULD include the properly-formatted `m.room.create` | ||
PDU. This is not made mandatory to avoid a situation where servers trust data that shouldn't be trusted | ||
for the reasons described by MSC4291. | ||
|
||
To determine whether a room is "affected" by MSC4291, servers MUST inspect the `room_id` rather than | ||
the create event's `room_version`. Specifically, a room ID with a domain component is *not* affected | ||
while one without a domain component (and happens to be `!<43 unpadded urlsafe base64 chars>`) *is* | ||
affected. This is done to ensure the server is not potentially confused by a malicious server providing | ||
a create event for a different, unaffected, room. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
When a room is affected, the server MUST validate the `m.room.create` event as follows for the purposes | ||
of the above: | ||
|
||
1. If the event has a `room_id`, reject. | ||
2. If the event does not otherwise comply with the event format for its self-described room version, | ||
reject. | ||
3. If the event fails [signature checks](https://spec.matrix.org/v1.15/server-server-api/#validating-hashes-and-signatures-on-received-events), | ||
reject. The content hash check MAY be skipped as the event can safely be redacted prior to all of | ||
these checks. | ||
4. If the event's [reference hash](https://spec.matrix.org/v1.15/server-server-api/#calculating-the-reference-hash-for-an-event) | ||
does not match the event ID contained in the room ID, reject. | ||
5. Otherwise, allow. | ||
|
||
|
||
## Potential issues | ||
|
||
* This technique is not applied to other state events present in stripped state. A future MSC or | ||
series of MSCs is expected to address this particular concern. | ||
|
||
* Some server implementations allow safety tooling and other applications to hook into them between | ||
the Federation API and Client-Server API. Such implementations are encouraged to make the create | ||
event reasonably available in its full form to those applications. Typically, this will be an internal | ||
representation of the event which still has the capability to serialize down to a PDU. | ||
|
||
|
||
## Alternatives | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This proposal fills a potential gap in information created by MSC4291, making the alternatives roughly | ||
equivalent to "don't do this". | ||
|
||
|
||
## Security considerations | ||
|
||
Security considerations are made throughout, especially in areas where an implementation may accidentally | ||
trust data it shouldn't. | ||
|
||
|
||
## Unstable prefix | ||
|
||
This proposal does not require an unstable prefix as the behaviour can be accomplished without overly | ||
affecting client or server implementations. | ||
|
||
|
||
## Migration | ||
|
||
*This section is non-normative for spec writing purposes. It only affects implementations which have | ||
implemented room version 12 upon its release.* | ||
|
||
Room version 12 contains MSC4291 and is expected to be used in production prior to this proposal | ||
becoming stable itself. To account for this, servers SHOULD treat "MUST" as "MAY" throughout this | ||
proposal, with the exception of the Client-Server API changes, until 1 full spec release cycle has | ||
passed since this MSC's own release in the specification. | ||
|
||
This translates to a timeline anywhere between 2 and 6 months, depending on ecosystem rollout. An | ||
example *possible* release schedule is: | ||
|
||
1. August 2025 - Matrix 1.16 is released with Room Version 12. | ||
2. October 2025 - Matrix 1.17 is released with this proposal; servers use "MAY" keywords. | ||
3. January 2026 - Matrix 1.18 is released; servers switch to "MUST" keywords. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
Servers MAY switch to "MUST" keywords early if their local ecosystems are prepared for the change. | ||
turt2live marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
|
||
## Dependencies | ||
|
||
This proposal requires [MSC4291](https://github.com/matrix-org/matrix-spec-proposals/pull/4291) in | ||
order to make any amount of sense. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation requirements:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ruma (server; sending):
knock
intomembership
, allow non-stripped events to be represented in membership handshakes, unstable support for MSC4311 ruma/ruma#2171sync_events
, unstable support for MSC4311 ruma/ruma#2173There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meowlnir (client/bot replacing room ID parsing with creator parsing in invite antispam): maunium/meowlnir@eeccd4a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though servers haven't actually added the validation on the stripped state yet, per the migration section of the MSC, no one has indicated that it's a nightmare to do so. I'm inclined to consider that "implementation" for the purposes of starting FCP, but welcome (particularly SCT) opinions on whether that's valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server returning and receiving: https://gitlab.com/famedly/conduit/-/commit/532b17ade86b4373db9f3bcca6af6815dd9b5b10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This MSC now requires all stripped state events to be PDUs over federation. I believe element-hq/synapse#18822 and matrix-org/complement#796 demonstrate it's not impossible, even if the code itself is bad.