Skip to content

MSC4361: Non-federating Membership Authentication Rule Amendments #4361

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

MSC4361: Non-federating Membership Authentication Rule Amendments #4361

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 29 additions & 0 deletions proposals/4361-non-federating-member-auth.md
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Implementation requirements:

  • Server

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# MSC4361: Non-federating Membership Authentication Rules

Specification version 1.16. Room Version 12<sup>*</sup> § 2.2 (4)<sup>[1]</sup> is the sole clause within the Authentication Rules conditioned on the `m.federate` property. The `m.federate` property is an immutable declaration within the `m.room.create` event which isolates a room to a specific origin<sup>†</sup>. The sole condition to achieve a non-federating room is to deny events with a `sender` from a remote server.

This method has several leakages. The "target" of an `m.room.member` event declared by its `state_key` is allowed to be a remote user. Compliant servers will both create, transmit<sup>‡</sup> and accept such an event, only to find later it cannot be further acted upon. This specification change aims to correct this leakage and subsequent ⊥ by amending Rule `5. If type is m.room.member` prepending a new sub-rule `1. If the content of the m.room.create event in the room state has the property m.federate set to false, and the state_key domain of the event does not match the sender domain of the create event, reject.` shifting-up existing rules.
Copy link
Member

Choose a reason for hiding this comment

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

Please wrap lines to ~120 characters


## Alternatives

Synapse has been observed in at least some cases to preclude the transmission and/or reception of events for non-federating rooms "manually." These restrictions are specified external to the authentication rules in some other places. This approach denudes the significance of authentication rules. One could ask: why specify _anything_ as authentication rules? This is the question a proponent of the status quo must bear.
Copy link
Member

Choose a reason for hiding this comment

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

My understanding of this proposal is that, when m.federate is false.

The "target" of an m.room.member event declared by its state_key is allowed to be a remote user.

This will pass auth checks locally on compliant HSes, since the auth rules only check the sender:

If the content of the m.room.create event in the room state has the property m.federate set to false, and the sender domain of the event does not match the sender domain of the create event, reject.

..but will be completely useless and inactionable. Either servers won't send it to the target server, or if they do, they can't convert it into a valid membership.

It seems sensible to just catch this at event creation time on compliant servers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The naive compliant server currently:

  1. Creates an invite which passes auth checks.
  2. Transmits the invite to the remote server where it's rightfully targeted.
  3. The remote server acknowledges the invite (as it either doesn't know not to and it still passes auth-checks anyway).
  4. Presents the invite to the remote user.
  5. The remote user accepts the invite.

Only then does their join event fail. This is the behavior of condu*.

Auth checks already fetch and scrutinize the create event for several reasons (m.federate being one). Are you suggesting it is better to fetch the create event to examine some other event outside the context of auth-rules instead, but while auth-rules already has that exact purpose?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, I'm not making any suggestions here. I'm wording the proposal in a way that makes sense to me, and I think it's a fine addition to make. I do wonder if this should be more restricted specifically for invites which has this problem, as opposed to member events in general?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I'm looking for is a reason why remote-user state keys shouldn't be summarily blocked. @anoadragon453 was chasing this exact line of thought too and I feel like we all need to meditate on this for a bit until that reason reveals itself. Conversely if none exists, and we only block invites now out of caution, we might regret when some foul play occurs in the future requiring yet another revisit to this.


## Security Considerations

While this proposal clearly increases security by reducing cross-origin leakage of events, the addition of any new restrictions is a theoretical Denial of Service vector which must be scrutinized during review. No such possibility is known to the author.

## Potential Issues

A new room version may be required. However, this author believes it is technically possible to backport this change to all prior room versions without any alteration to nominal correctness; leaked events would simply enter a failure state or remain ⊥.

---

[1]: https://spec.matrix.org/v1.16/rooms/v12/#authorization-rules

<sup><b>Special thanks to Dasha for assisting in the research for this proposal.</b></sup>

<sup>* In addition to all earlier stable room versions.</sup>

<sup>† Server or cluster sharing the same domain.</sup>

<sup>‡ Synapse has been observed to impose certain restrictions external to the auth-rules. See: Alternatives for discussion.</sup>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not super important on this short proposal but I think the built-in footnotes in GitHub's markdown would make for better readability. They're numbered and navigateable in both directions. See e.g. here for an example.