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

MSC2260: Update the auth rules for `m.room.aliases` events #2260

Open
wants to merge 7 commits into
base: master
from
@@ -0,0 +1,49 @@
# MSC2260: Update the auth rules for `m.room.aliases` events
This conversation was marked as resolved by richvdh

This comment has been minimized.

Copy link
@ara4n

ara4n Aug 30, 2019

Member

I've stared at this MSC a bunch now, and am finding it quite hard to review. I think part of it is because it doesn't state how m.room.aliases is meant to work (or at least how i understand it to be intended to be used), and precisely how it has failed, or why the proposal fixes the way it failed while solving the goals. Thus it's hard to tell if removing the auth specialcases on m.room.aliases is the right fix.

To try to gather my thoughts I've done a short alternative proposal at https://gist.github.com/ara4n/0ec423e7c321acbb53eed04ceb4dd7df, which i think fixes up the current problems without increasing complexity further.

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 30, 2019

Author Member

I've stared at this MSC a bunch now, and am finding it quite hard to review. I think part of it is because it doesn't state how m.room.aliases is meant to work (or at least how i understand it to be intended to be used), and precisely how it has failed, or why the proposal fixes the way it failed while solving the goals. Thus it's hard to tell if removing the auth specialcases on m.room.aliases is the right fix.

Yeah, part of this was due to me not really understanding the status quo when I wrote it, and partly due to there still being some open questions about the solution. I've tried to address the first part at least (with help from your bis proposal, thanks).


## Background

Currently, `m.room.aliases` is subject to specific [authorization
rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules). When these
rules were introduced, the intention was that `m.room.aliases` would be
maintained as an up-to-date list of the aliases for the room. However, this has
not been successful, and in practice the `m.room.aliases` event tends to drift
out of sync with the aliases (https://github.com/matrix-org/matrix-doc/issues/2262).

Meanwhile, `m.room.aliases` is open to abuse by remote servers who can add spam
or offensive aliases (https://github.com/matrix-org/matrix-doc/issues/625).

## Proposal

`m.room.aliases` exists to advertise the aliases available for a given
room. This is an ability which should be restricted to privileged users in the
room.

Therefore, the special-case for `m.room.aliases` is to be removed from the
[authorization
rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules). `m.room.aliases`
would instead be authorised following the normal rules for state events.
This conversation was marked as resolved by ara4n

This comment has been minimized.

Copy link
@ara4n

ara4n Aug 30, 2019

Member

So I wonder if it still makes sense to prevent users (or admins) from creating alias events which refer to aliases on remote servers. The argument is:

  • If it's an ordinary user, they have no right to claim that a remote server's directory points to this room.
  • If it's an admin, they might want to curate the list and advertise remote directory entries without actually going and signing up on an account on the remote server or asking someone on the server to do so.

In practice, I don't think i've ever heard in practice of a room admin feeling frustrated that they can't add a remote server's alias to their room, so my hunch is that we should keep the restriction that you can only advertise aliases from your account's server on a given room.

However, m.room.aliases should otherwise be subject to normal auth rules.

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 30, 2019

Author Member

In practice, I don't think i've ever heard in practice of a room admin feeling frustrated that they can't add a remote server's alias to their room,

I can see it being useful (especially if we lock down regular users' abilities to update the alias event), but I could go either way on it.

This comment has been minimized.

Copy link
@richvdh

richvdh Sep 2, 2019

Author Member

@ara4n: I'm not really following what you meant here. Is it still relevant?

This comment has been minimized.

Copy link
@ara4n

ara4n Sep 2, 2019

Member

i was just trying to campaign to keep 4a and 4b from the auth rules, to stop users from advertises aliases on remote servers. looks like we're aligned, so it's no longer relevant.


As a corollary, only users with the power level necessary to send the
`m.room.aliases` state event will be allowed to add entries to the room
directory. Server admins will continue to be able to remove entries from the
directory even if they do not have the right to send the `aliases` event (in
which case the `m.room.aliases` event will become outdated).

It also be logical to allow the contents of `m.room.aliases` events to be
This conversation was marked as resolved by richvdh

This comment has been minimized.

Copy link
@ara4n

ara4n Aug 29, 2019

Member
Suggested change
It also be logical to allow the contents of `m.room.aliases` events to be
It would also be logical to allow the contents of `m.room.aliases` events to be
redacted, as per [MSC2261](https://github.com/matrix-org/matrix-doc/issues/2261).

## Tradeoffs

Perhaps we could instead allow room admins the ability to redact malicious
`aliases` events? Or to issue new ones?

## Potential issues

1. This will bake in https://github.com/matrix-org/synapse/issues/1477 in a way
that cannot be fixed in the case that the server admin doesn't have ops in
the room.

2. This would allow room operators to add 'fake' aliases: for example, I could
create a room and declare one of its aliases to be
`#matrix:matrix.org`. It's not obvious that this will cause any problems in
practice, but it might lead to some confusion.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.