Skip to content

MSC3917: Cryptographically Constrained Room Membership#3917

Open
duxovni wants to merge 2 commits into
mainfrom
fayed/cryptographic-membership
Open

MSC3917: Cryptographically Constrained Room Membership#3917
duxovni wants to merge 2 commits into
mainfrom
fayed/cryptographic-membership

Conversation

@duxovni
Copy link
Copy Markdown

@duxovni duxovni commented Oct 25, 2022

@uhoreg uhoreg added e2e proposal A matrix spec change proposal client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. requires-room-version An idea which will require a bump in room version labels Oct 25, 2022
@turt2live turt2live added unassigned-room-version Remove this label when things get versioned. hacktoberfest-accepted labels Oct 25, 2022
Comment thread proposals/3917-cryptographic-membership.md Outdated
| Attack | Membership tree | State DAG | Signal |
|---|---|---|---|
| Homeserver admin inserts a new member | ✅ | ✅ | ✅ |
| Homeserver admin re-inserts a banned member | ❌ | ✅ | ❌ |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also applies to kicked members or the ones that left on their own. I think that in general is very far down, since that is a major downside, that any member that ever was in the room can be reinserted by the homeserver.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're correct, when I said "banned member", I wasn't implying that there's anything unique in this regard about the operation of banning as opposed to kicking or leaving.

This shortcoming is raised and discussed in the very first paragraph of the Potential Issues section, along with an argument for why this proposal is nevertheless valuable. The side-by-side comparison of the alternatives is further down in the document because I can't compare the alternative system before I've described what the alternative system is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wasn't quite sure where to put the comment, so this was just one place. But the problem isn't as much reinserting the member or authing new events, it is that the homeserver could just choose to never propagate the leave event or not provide it to a new initial sync, which would make it look like the user never left.

Specifically this:

It does not cryptographically verify
that the senders of those state events had the required power levels
at the time they issued the events, or that they didn't leave or get
banned between joining and issuing the events.

It talks about issuing events, which I needed to read several times to understand, that it means "issuing the original join event". I think if you are not already aware about the problem with rescinding a membership, then this looks like it is talking about sending new messages, that can't be verified that they should or should not be in the room. Maybe that can be fixed by changing it to something like this?:

It does not cryptographically verify
that the senders of those state events had the required power levels
at the time they issued the events. It also does not verify that the user
didn't leave, got kicked or banned in the meantime. The signature would still
be valid for their join event, they however should not be treated as a member
anymore.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

But the problem isn't as much reinserting the member or authing new events, it is that the homeserver could just choose to never propagate the leave event or not provide it to a new initial sync, which would make it look like the user never left.

This is in the first paragraph of the Alternatives section as the motivation for the alternative proposal which solves these problems.

Copy link
Copy Markdown
Author

@duxovni duxovni Oct 28, 2022

Choose a reason for hiding this comment

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

And to be clear, when I say "issuing events", I mean issuing invite events - it isn't just that banned members can remain in the room, it's that anyone who's ever been in the room can always invite others from the perspective of this cryptographic verification. The homeserver is still expected to enforce these sorts of permissions, but the homeserver can only tamper with a room's membership with the collaboration of someone who has been a legitimate member in the past.

Copy link
Copy Markdown
Author

@duxovni duxovni left a comment

Choose a reason for hiding this comment

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

The verification process relies on being able to access a user's invite event and their join event as separate entities, including after the user has left the room. Under the current spec, these would all be m.room.member events with the same state key, and would overwrite each other. The same also goes for older m.room.join_rules events.

I see two options here:

  • These signed state events can include the entire chain of stripped parent state events within their event content, just as in the case where a joining user is proving their membership in a parent space. For instance, a join event would also include a copy of the invite event that authorized it, and that copy of the invite event would include a copy of the inviter's cause-of-membership event, and so on back to the room's creation. (Disadvantage: the sizes of invite, join, and join-rules events will monotonically increase over time as the chains of inviters and invitees grow longer.)
  • Alternatively, we could explicitly mandate that old versions of these state events be kept around by the server, and shown to clients that explicitly request them by their event IDs. (Disadvantage: this may require homeserver implementers to change storage schemas based on the old assumption that stored state events are unique per room/type/state-key.)

Comment on lines +63 to +72
This proposal has each user generate a new cryptographic signing key
called the Room Signing Key, or RSK. The RSK is used for signing
certain types of room state events that the user sends (specifically,
invitations, joins, and join rule changes), so that other room members
can verify that the events were genuinely sent by that user. The RSK
should be signed by the Master Signing Key, and stored and retrieved
alongside the user's other signing keys. This key will be identified
by the string `m.cross_signing.room_signing`, and will be published to
the `/keys/device_signing/upload` endpoint using the new optional
field `room_signing_key`, with usage `["room_signing"]`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

By signing the RSK using the MSK, this proposal creates a cryptographic publishable proof that a user participates in a room (if they perform an action which uses the RSK).

The signing key used within a Megolm session is unsigned by the user so cannot prove that a user sent the message in a way which could not be faked by another participant. Although a homeserver could produce various records they would not be signed by the user so again could be faked.

Although deniability is not an explicit security guarantee it should be acknowledged that this change removes it. A possible solution would be to add the users MSK directly to the membership event, and allow the RSK to either be signed or unsigned. Some mechanism in m.room.create and m.room.join_rules could be used to signal if an unsigned RSK is acceptable.

Verification of an RSK which is unsigned could be achieved using an inband message encrypted message to the room for example, which would inherit the authenticity guarantees of the Megolm session without creating a cryptographic proof.

@cvwright
Copy link
Copy Markdown

Is anyone actively working on an implementation of this?

@uhoreg
Copy link
Copy Markdown
Member

uhoreg commented Mar 31, 2023

We are planning on working on it, but are not currently doing so yet.

connecting the sender's MSK to their RSK, particularly in cases
where the sender may no longer be in the room or may have even
deactivated their account.
+ `parent_event_id`:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I wonder if we can borrow this idea of having a back-pointer to the previous membership state event to do something similar for leave and ban events. If so, we might be able to cheaply prevent a malicious server from re-adding a banned/left user by re-sending the content from an old join event.

Maybe all that would be required is to include the parent_event_id for ban and leave too, pointing back to the join event that is being "revoked" or undone. Then when a client receives a new join event for a banned or left member, it can compare the new signatures against those in the previous join event for that user (the one that was the parent for the leave or ban). Ed25519 signatures are randomized, so if the new join event is legitimate, then the new signature should not be identical to the old one. If the two signatures are bit-for-bit identical, then the client can be sure that they are looking at a replay attack.

Of course, this doesn't work for more complicated scenarios where a given user is repeatedly joined, then banned, then re-invited and re-joins, is banned again, and so on and so forth. Full protection would require fully verifying the DAG of state events on the client as described in the second half of this MSC. But still, checking the parent event is low-hanging fruit and should cover the most common case of a replay attack in practice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, as you say, it's not a complete fix. But it looks like it's easy enough to do and will catch some cases.

@Michael-Hollister
Copy link
Copy Markdown

Michael-Hollister commented May 16, 2023

We might want to consider having clients sign m.room.tombstone events to ensure users that created the event are verified room members . Since this MSC will require a new room version, a malicious homeserver could downgrade the room version to a previous version without cryptographic room membership verification by injecting a fake m.room.tombstone event in the room.

the RRK should *be* the room ID. Similar to
[MSC1228](https://github.com/matrix-org/matrix-spec-proposals/pull/1228),
the new format of a room ID will be `![unpadded urlsafe-base64ed
ed25519 public key]`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Limiting the key type to ed25519 is bad practice considering potential future threats of quantum computing. So a mechanism should be established to allow cryptoagility. See also https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Crypto/Migration_to_Post_Quantum_Cryptography.pdf?__blob=publicationFile&v=2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO, we shouldn't overcomplicate it with clunky cryptoagility mechanisms. The primary agility mechanism is the ability to issue a new room version, which would specify the use of a new signing algorithm.

"content": {
"room_version": "9",
"creator": "@alice:example.com",
"room_root_key" : "/ZK6paR+wBkKcazPx2xijn/0g+m2KCRqdCUZ6agzaaE",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

presumably this is supposed to match the room_id

"sender": "@alice:example.com",
"content": {
"room_version": "9",
"creator": "@alice:example.com",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

creator was removed by msc2175. presumably the intention is not to reintroduce it.

communications channel the URIs were sent through, which is again a
fundamental limit.

## Potential issues
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's worth emphasising that, in order to validate a given event, the client must have access to all of the events in its chain of trust, and this is not currently available via the /sync api.

For example, if I join an invite-only room where Alice is only a member, /sync will give me Alice's join event, but not the invite from Bob event that allowed the join to take place. Similarly Bob may have left the room since inviting Alice, in which case my client will get Bob's leave event rather than his join event, so can't validate the invite he sent to Alice. It gets even worse when Matrix state resolution is thrown into the mix (since state res can legitimately cause state events to be replaced by earlier instances of the state.)

All these events should be in the auth chain, so the server should have them, and an easy solution is for the client to request said events via a /context request. That won't be terribly efficient, but a more efficient mechanism could be designed later.

More of a concern is that, in a room with shared or joined history visiblity, many of the events in the chain won't be accessible to a newly-joined user.

We could probably relax the history visibility rules in some way to make old m.room.member and m.room.join_rules events visible even to newly-joined users, but we would need to be mindful of the potential resultant metadata leak (eg, getting a complete list of everyone who has ever participated in a room).

Comment on lines +7 to +8
members will not receive keys for past messages, since those are only
shared by existing members when they invite new members, but the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is referring to MSC3061, which is pending disclosure of security concerns.

Comment on lines +46 to +52
verify others' membership in the room, requires knowing the RRK. Users
who disagree about a room's RRK are, for all intents and purposes, not
actually members of the same room. For this reason, we propose that
the RRK should *be* the room ID. Similar to
[MSC1228](https://github.com/matrix-org/matrix-spec-proposals/pull/1228),
the new format of a room ID will be `![unpadded urlsafe-base64ed
ed25519 public key]`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This conflicts with room version 12 / MSC4291. In particular, it re-introduces the problem that MSC4291 set out to solve: that you can have multiple create events for the same room ID.

I'm not sure how we can best prevent both (1) having multiple create events for the same room ID and (2) having multiple rooms with the same RSK.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would we want to prevent multiple rooms having the same RSK?

I think the way to solve this pain point is to simply make the RSK public key become part of the create event. This binds the RSK to the room while retaining the uniqueness of the create event in room version 12.

join_rule_type==>|Public|check_sender
join_rule_type ==>|Restricted|any_old_rooms{"Does the restricted<br/>join rule include any rooms<br/>whose IDs are not RRKs?"}
any_old_rooms==>|Yes|check_sender
any_old_rooms==>|No|join_state{"Based on<br/>the list of state events<br/>provided in the <code>join</code> event,<br/>does the user's cause-of-membership event<br/>at the start of the list<br/>pass verification?"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what "list of state events"?

any_old_rooms==>|Yes|check_sender
any_old_rooms==>|No|join_state{"Based on<br/>the list of state events<br/>provided in the <code>join</code> event,<br/>does the user's cause-of-membership event<br/>at the start of the list<br/>pass verification?"}
join_state -.-> start
join_state ==>|Yes|joinrule_match{"Does the join rule event<br/>include a room ID that<br/>matches the room ID from<br/>the provided state?"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what provided state?

join_rsk_msk ==>|Yes|why_join{{"Look up<br/>the <code>join</code> event's<br/>parent event ID"}}

why_join ==>|<code>invite</code> event|invite_kind{{"How was<br/>the <code>invite</code> event<br/>created?"}}
invite_kind ==>|Sent directly<br/>by a user|invite_rrk{"Does the event<br/>contain the correct RRK<br/>and the user's MSK?"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
invite_kind ==>|Sent directly<br/>by a user|invite_rrk{"Does the event<br/>contain the correct RRK<br/>and the user's MSK?"}
invite_kind ==>|Sent directly<br/>by a user|invite_rrk{"Does the invite event<br/>contain the correct RRK<br/>and the inviter's MSK?"}

why_join ==>|<code>invite</code> event|invite_kind{{"How was<br/>the <code>invite</code> event<br/>created?"}}
invite_kind ==>|Sent directly<br/>by a user|invite_rrk{"Does the event<br/>contain the correct RRK<br/>and the user's MSK?"}
invite_rrk ---->|No|reject
invite_rrk =====>|Yes|check_sender{"Does the event<br/>have a valid signature<br/>by the sender's RSK?"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
invite_rrk =====>|Yes|check_sender{"Does the event<br/>have a valid signature<br/>by the sender's RSK?"}
invite_rrk =====>|Yes|check_sender{"Does the join event's parent<br/>have a valid signature<br/>by the sender's RSK?"}

Comment on lines +293 to +296
As verification of the entire membership event chain *requires*
knowing the correct RRK for a room, it is critical that when joining a
new room, clients receive its RRK in a way that cannot easily be
falsified by the homeserver. In the case where a user is being
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm a bit confused. Earlier we say that RRK == room id. How is there scope for falsification?

Comment on lines +27 to +29
or implicit) from another verified possible room member. As long as
state events are transmitted successfully, all such users will be
verified as possible room members.** In this context, a user is
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As long as state events are transmitted successfully, all such users will be
verified as possible room members.

I'm not quite clear on what this sentence wants to convey.

the RRK should *be* the room ID. Similar to
[MSC1228](https://github.com/matrix-org/matrix-spec-proposals/pull/1228),
the new format of a room ID will be `![unpadded urlsafe-base64ed
ed25519 public key]`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO, we shouldn't overcomplicate it with clunky cryptoagility mechanisms. The primary agility mechanism is the ability to issue a new room version, which would specify the use of a new signing algorithm.

actually members of the same room. For this reason, we propose that
the RRK should *be* the room ID. Similar to
[MSC1228](https://github.com/matrix-org/matrix-spec-proposals/pull/1228),
the new format of a room ID will be `![unpadded urlsafe-base64ed
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We've considered introducing urlsafe Base64 for other things, such as the Curve25519 key used for dehydrated devices in MSC3814, but ultimately decided against it since we use the non-urlsafe version in other places. I think it's better to avoid having some identifiers be urlsafe and some not, and just continue the current practice of using non-urlsafe Base64 everywhere.

Clients should ensure to URL-encode and decode any strings they are interpolating into URLs anyway.

Comment on lines +79 to +82
+ `room_root_key`: An Ed25519 public key generated for this specific
room by the room creator, henceforth called the Room Root Key
(RRK), that will serve as the root of the room membership
signature tree.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why? We're already using it as the room ID. Inserting it in yet another place just creates an opportunity for the two to desync and the associated security concerns.

Comment on lines +46 to +52
verify others' membership in the room, requires knowing the RRK. Users
who disagree about a room's RRK are, for all intents and purposes, not
actually members of the same room. For this reason, we propose that
the RRK should *be* the room ID. Similar to
[MSC1228](https://github.com/matrix-org/matrix-spec-proposals/pull/1228),
the new format of a room ID will be `![unpadded urlsafe-base64ed
ed25519 public key]`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would we want to prevent multiple rooms having the same RSK?

I think the way to solve this pain point is to simply make the RSK public key become part of the create event. This binds the RSK to the room while retaining the uniqueness of the create event in room version 12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client-server Client-Server API e2e hacktoberfest-accepted kind:feature MSC for not-core and not-maintenance stuff 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 unassigned-room-version Remove this label when things get versioned.

Projects

None yet

Development

Successfully merging this pull request may close these issues.