Skip to content

MSC4291: Room IDs as hashes of the create event #4291

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 22, 2025
Merged

MSC4291: Room IDs as hashes of the create event #4291

merged 8 commits into from
Jul 22, 2025
+175 −0

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented May 20, 2025 •
edited by turt2live
Loading

Rendered

(Written by Matthew as Matrix project lead (while on the clock as Element CTO) and Kegan as Element staff eng)

SCT Stuff:

FCP tickyboxes

MSC checklist

@turt2live turt2live added proposal A matrix spec change proposal needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels May 20, 2025
@turt2live turt2live marked this pull request as draft May 20, 2025 22:15
@turt2live

This comment was marked as resolved.

Sign in to view
@turt2live turt2live added proposal-placeholder This label is removed and replaced with `proposal` once the placeholder status is cleared. action-required Just a bright label to differentiate arbitrary proposals. and removed proposal A matrix spec change proposal labels Jul 3, 2025
@github-project-automation github-project-automation bot moved this to Tracking for review in Spec Core Team Workflow Jul 8, 2025
@turt2live turt2live moved this from Tracking for review to Ready for general review in Spec Core Team Workflow Jul 8, 2025
@kegsay @turt2live
@KitsuneRal @richvdh @Kladki @tulir
Add MSC4291: Room IDs as hashes of the create event
81cfe19 
Co-authored-by: Travis Ralston <travpc@gmail.com>
Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Matthias Ahouansou <matthias@ahouansou.cz>
@tulir tulir changed the title [WIP] Placeholder MSC4291: Room IDs as hashes of the create event Jul 10, 2025
@tulir tulir marked this pull request as ready for review July 10, 2025 16:02
@tulir tulir added requires-room-version An idea which will require a bump in room version proposal A matrix spec change proposal s2s Server-to-Server API (federation) room-spec Something to do with the room version specifications unassigned-room-version Remove this label when things get versioned. kind:maintenance MSC which clarifies/updates existing spec and removed needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal-placeholder This label is removed and replaced with `proposal` once the placeholder status is cleared. action-required Just a bright label to differentiate arbitrary proposals. labels Jul 10, 2025
@turt2live turt2live added the maybe v12? candidate for room version 12 label Jul 10, 2025
turt2live
turt2live reviewed Jul 10, 2025
View reviewed changes
proposals/4291-room-ids-as-hashes.md
Copy link
Member

@turt2live turt2live Jul 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Implementation requirements:

Some clients to test/verify (incomplete list):

  • matrix.to web
  • Element Web (/Desktop)
  • Legacy Element Android
  • Legacy Element iOS
  • matrix-rust-sdk
    • Element X Android
    • Element X iOS
    • Fractal
  • Cinny
  • Nheko
  • Quotient/Quaternion
  • NeoChat
  • Rory&::LibMatrix
  • matrix-bot-sdk (both Element fork and upstream; covers most moderation tooling/bots)
  • Draupnir
  • Hydrogen

Copy link
Contributor

@Gnuxie Gnuxie Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Draupnir will crash, and please do not assume that ticking off the matrix-bot-sdk will cover us in future. We have our own infrastructure for parsing matrix events and matrix identifiers.

  • https://github.com/the-draupnir-project/matrix-basic-types includes parsing and type brands for matrix identifiers (potentially illegally in the case of room identifiers, but we do so because it is important for us to be able to identify the origin of rooms).
  • https://github.com/Gnuxie/matrix-protection-suite/ parses events, and will crash if room_id is removed from the top level of events. Not sure whether that specifically is illegal or not. We have a tolerant strategy for handling other fields that don't match the schema (including all content), but this isn't the case for room_id and event_id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fractal doesn't crash with the changes in this MSC.

Copy link
Member

Choose a reason for hiding this comment

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

matrix.to appears to overly validate the room ID:
image

Copy link

Choose a reason for hiding this comment

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

Resolved in Rory&::LibMatrix as of https://cgit.rory.gay/matrix/LibMatrix.git/commit/?id=c39d18efd0e6a610d2dc29832407e99e0079bc13

@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jul 10, 2025
@turt2live
Copy link
Member

turt2live commented Jul 10, 2025 •
edited
Loading

MSCs proposed for Final Comment Period (FCP) should meet the requirements outlined in the checklist prior to being accepted into the spec. This checklist is a bit long, but aims to reduce the number of follow-on MSCs after a feature lands.

SCT members: please check off things you check for, and raise a concern against FCP if the checklist is incomplete. If an item doesn't apply, prefer to check it rather than remove it. Unchecking items is encouraged where applicable.

Checklist:

  • Are appropriate implementation(s) specified in the MSC’s PR description?
  • Are all MSCs that this MSC depends on already accepted?
  • For each new endpoint that is introduced:
    • Have authentication requirements been specified?
    • Have rate-limiting requirements been specified?
    • Have guest access requirements been specified?
    • Are error responses specified?
      • Does each error case have a specified errcode (e.g. M_FORBIDDEN) and HTTP status code?
        • If a new errcode is introduced, is it clear that it is new?
  • Will the MSC require a new room version, and if so, has that been made clear?
    • Is the reason for a new room version clearly stated? For example, modifying the set of redacted fields changes how event IDs are calculated, thus requiring a new room version.
  • Are backwards-compatibility concerns appropriately addressed?
  • Are the endpoint conventions honoured?
    • Do HTTP endpoints use_underscores_like_this?
    • Will the endpoint return unbounded data? If so, has pagination been considered?
    • If the endpoint utilises pagination, is it consistent with the appendices?
  • An introduction exists and clearly outlines the problem being solved. Ideally, the first paragraph should be understandable by a non-technical audience.
  • All outstanding threads are resolved
    • All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative
  • While the exact sections do not need to be present, the details implied by the proposal template are covered. Namely:
    • Introduction
    • Proposal text
    • Potential issues
    • Alternatives
    • Dependencies
  • Stable identifiers are used throughout the proposal, except for the unstable prefix section
    • Unstable prefixes consider the awkward accepted-but-not-merged state
    • Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”).
  • Changes have applicable Sign Off from all authors/editors/contributors
  • There is a dedicated "Security Considerations" section which detail any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.". See RFC3552 for things to think about, but in particular pay attention to the OWASP Top Ten.

turt2live
turt2live reviewed Jul 10, 2025
View reviewed changes
proposals/4291-room-ids-as-hashes.md
Comment on lines +81 to +88
It is also desirable to know the creator's server name without having to join and get the full create event
for Trust & Safety purposes. The create event in invites can be spoofed, as can the room ID, but spoofing the
room ID makes the invite unusable whereas spoofing the create event does not. We could rely on the `sender`
instead, but for this to be effective it would be important for servers to use the `sender` domain for routing
purposes, such that if the `sender` was invalid it would cause the invite to be unusable. On the contrary, users
unfamiliar with Matrix may see a room ID like `!OGEhHVWSdvArJzumhm:matrix.org` and think that `matrix.org` hosts/moderates/
is somehow responsible for the room in some way. By dropping the domain, we clearly express that the creator domain
may not be responsible for the contents of the room.

This comment was marked as resolved.

Sign in to view

TheArcaneBrony
TheArcaneBrony reviewed Jul 10, 2025
View reviewed changes
turt2live added a commit to matrix-org/matrix.to that referenced this pull request Jul 16, 2025
@turt2live
Support v12 room IDs
442ed2f 
MSC: matrix-org/matrix-spec-proposals#4291
Pre-disclosure: https://matrix.org/blog/2025/07/security-predisclosure/
@turt2live turt2live mentioned this pull request Jul 16, 2025
Merged
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Jul 16, 2025
@Gnuxie
Attempt to allow Draupnir to use room V12.
8457d50 
- Updated to matrix-basic-types 1.4.0 which changes
  the regex validating room ids.

- Changed the package override so that all dependencies
  use matrix-basic-types 1.4.0, including the matrix-protection-suite.

- Removed code that tries to store details about discovered rooms in
  the room takdedown protection. These were unreliable for so many
  reasons and also are now broken given the room origin cannot be
  extracted from the room id. Details for why this is can be found in
  the reviews of
  matrix-org/matrix-spec-proposals#4291.
@Gnuxie Gnuxie mentioned this pull request Jul 16, 2025
Merged
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Jul 16, 2025
@Gnuxie
Attempt to allow Draupnir to use room V12.
ca1fa2c 
- Updated to matrix-basic-types 1.4.0 which changes
  the regex validating room ids.

- Changed the package override so that all dependencies
  use matrix-basic-types 1.4.0, including the matrix-protection-suite.

- Removed code that tries to store details about discovered rooms in
  the room takdedown protection. These were unreliable for so many
  reasons and also are now broken given the room origin cannot be
  extracted from the room id. Details for why this is can be found in
  the reviews of
  matrix-org/matrix-spec-proposals#4291.
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Jul 16, 2025
@Gnuxie
Attempt to allow Draupnir to use room V12.
b3ebec5 
- Updated to matrix-basic-types 1.4.0 which changes
  the regex validating room ids.

- Changed the package override so that all dependencies
  use matrix-basic-types 1.4.0, including the matrix-protection-suite.

- Removed code that tries to store details about discovered rooms in
  the room takdedown protection. These were unreliable for so many
  reasons and also are now broken given the room origin cannot be
  extracted from the room id. Details for why this is can be found in
  the reviews of
  matrix-org/matrix-spec-proposals#4291.
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Jul 16, 2025
@Gnuxie
Attempt to allow Draupnir to use room V12. (#918)
1f8da29 
- Updated to matrix-basic-types 1.4.0 which changes
  the regex validating room ids.

- Changed the package override so that all dependencies
  use matrix-basic-types 1.4.0, including the matrix-protection-suite.

- Removed code that tries to store details about discovered rooms in
  the room takdedown protection. These were unreliable for so many
  reasons and also are now broken given the room origin cannot be
  extracted from the room id. Details for why this is can be found in
  the reviews of
  matrix-org/matrix-spec-proposals#4291.
@joshqou
Copy link

joshqou commented Jul 17, 2025

This proposal on it's own is fine if changed slightly, however packaging it alongside a mandatory security-related version bump is very neglectful behaviour towards both paid and unpaid maintainers of matrix software. There are many legitimate reasons as to why origin homeservers should be identifiable just from their room id, some of which are specifically outlined in this proposal, however are ignored in favour of breaking many moderation tools for... a very specific edge-case where a server gains knowledge of a room which could only be created by a malicious server?

I don't see any reason as to why this new format wouldn't be able to retain all of the existing information while also providing protection against impersonation or id "downgrading". i.e: !:example.com@$createid

Given those at the foundation and at element have given their approval I doubt anything can be said or done that would change this MSC in any way, however I am leaving this comment in spite of that to voice my own disapproval.

@HarHarLinks HarHarLinks mentioned this pull request Jul 17, 2025
Open
HarHarLinks
HarHarLinks reviewed Jul 17, 2025
View reviewed changes
FSG-Cat
FSG-Cat reviewed Jul 17, 2025
View reviewed changes
@zecakeh zecakeh mentioned this pull request Jul 21, 2025
Merged
@mscbot
Copy link
Collaborator

mscbot commented Jul 21, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jul 21, 2025
@turt2live
Copy link
Member

(FCP is actually not finished due to outstanding comments)

@turt2live turt2live added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed finished-final-comment-period labels Jul 21, 2025
Half-Shot
Half-Shot reviewed Jul 22, 2025
View reviewed changes
Half-Shot
Half-Shot reviewed Jul 22, 2025
View reviewed changes
@turt2live
Copy link
Member

Review feedback from the last several days, including from various rooms, has been addressed and as such the FCP is now finished.

@turt2live turt2live added finished-final-comment-period and removed final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jul 22, 2025
@turt2live turt2live merged commit b1f4ea8 into main Jul 22, 2025
1 check passed
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jul 22, 2025
@turt2live turt2live moved this from In FCP to Requires spec writing in Spec Core Team Workflow Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KitsuneRal KitsuneRal KitsuneRal left review comments

@kegsay kegsay kegsay left review comments

@zecakeh zecakeh zecakeh left review comments

@Half-Shot Half-Shot Half-Shot left review comments

@HarHarLinks HarHarLinks HarHarLinks left review comments

@tulir tulir tulir left review comments

@turt2live turt2live turt2live left review comments

@CobaltCause CobaltCause CobaltCause left review comments

@Gnuxie Gnuxie Gnuxie left review comments

@spaetz spaetz spaetz left review comments

@TheArcaneBrony TheArcaneBrony TheArcaneBrony left review comments

@JadedBlueEyes JadedBlueEyes JadedBlueEyes left review comments

@FSG-Cat FSG-Cat FSG-Cat left review comments

@jevolk jevolk jevolk approved these changes

@anoadragon453 anoadragon453 anoadragon453 approved these changes

Assignees
No one assigned
Labels
kind:core MSC which is critical to the protocol's success maybe v12? candidate for room version 12 proposal A matrix spec change proposal requires-room-version An idea which will require a bump in room version room-spec Something to do with the room version specifications s2s Server-to-Server API (federation) spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec unassigned-room-version Remove this label when things get versioned.
Projects
Spec Core Team Workflow
Status: Requires spec writing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.