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

[WIP] MSC4051: Using the create event as the room ID #4051

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

turt2live
Copy link
Member

@turt2live turt2live 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. needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. kind:maintenance MSC which clarifies/updates existing spec labels Sep 3, 2023
jplatte added a commit to ruma/ruma that referenced this pull request Sep 28, 2023
Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ matrix-org/matrix-spec-proposals#4051
jplatte added a commit to ruma/ruma that referenced this pull request Sep 28, 2023
Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ matrix-org/matrix-spec-proposals#4051
jplatte added a commit to ruma/ruma that referenced this pull request Sep 28, 2023
Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ matrix-org/matrix-spec-proposals#4051
jplatte added a commit to ruma/ruma that referenced this pull request Sep 28, 2023
Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ matrix-org/matrix-spec-proposals#4051
jplatte added a commit to ruma/ruma that referenced this pull request Sep 28, 2023
Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ matrix-org/matrix-spec-proposals#4051
@andrewzhurov
Copy link

Seems like a good idea. Event id, being a hash, would uniquely identify a room.
Room is a communication context. Oftentimes communication within a Room leads to new conversations that is losely related to the original context, we model it as Threads.
Perhaps we could go a step further and make Threads their own Room (a separate Matrix Event Graph), that forked from a particular event of a parent Thread. Would improve efficiency on scroll, as one scrolling the parent Thread would not need to load child's Thread events (that are not displayed anyways).

A new room version is established to accommodate the event format, redaction, and identifier changes
from this proposal.

The `room_id` field is removed from events, and no longer protected from redaction. The create event's

Choose a reason for hiding this comment

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

room_id is still to be kept in the encrypted payload of E2EE messages, right ? I think it should probably be mentioned in the MSC, perhaps as a security consideration.

Choose a reason for hiding this comment

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

What do you mean? Clients don't encrypt the room id when sending events

Copy link
Contributor

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Ok, then thats probably hidden by the various view source implementations in Element X Android / Android and Web

Copy link
Member Author

Choose a reason for hiding this comment

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

Clarification on what the concern is here would be appreciated. I'm having trouble parsing both the original comment and replies.

Links to existing spec are helpful.

Choose a reason for hiding this comment

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

If I understand the proposal right, nowhere in it does it state to keep room_id for events outgoing from the client inside m.room.encrypted, to address:

We include the room ID in the payload, because otherwise the homeserver would be able to change the room a message was sent in.
Link https://spec.matrix.org/v1.11/client-server-api/#mmegolmv1aes-sha2

Copy link
Member Author

Choose a reason for hiding this comment

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

This MSC doesn't really operate at that layer, so would not affect encryption. It only affects the federation and room behaviour characteristics.

I can try to make this clearer, but clients should see no material difference before and after this MSC.

Choose a reason for hiding this comment

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

Yea, I would suggest to make it clearer in the MSC exactly which usages of room_id it affects

Copy link
Member

Choose a reason for hiding this comment

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

With my crypto hat on, since has security implications, I think it would be worth clarifying somewhere that this MSC doesn't affect the encrypted payload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. 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) unassigned-room-version Remove this label when things get versioned.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants