From a80fdff1eafe19ee6f4e89cacc46ba8302f491e1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 29 Aug 2019 15:22:48 +0100 Subject: [PATCH 1/7] MSC2260: Update the auth rules for `m.room.aliases` events --- proposals/2260-change-aliases-auth-rules.md | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 proposals/2260-change-aliases-auth-rules.md diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md new file mode 100644 index 00000000000..b04ff872e32 --- /dev/null +++ b/proposals/2260-change-aliases-auth-rules.md @@ -0,0 +1,47 @@ +# MSC2260: Update the auth rules for `m.room.aliases` events + +## 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. FIXME: why is this, apart from +https://github.com/matrix-org/synapse/issues/1477, which could be fixed? + +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. + +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). + +## 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. From 3ab0b63a4f0edde2815a592b882b75bcdfc738fb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 29 Aug 2019 15:58:24 +0100 Subject: [PATCH 2/7] add some links --- proposals/2260-change-aliases-auth-rules.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index b04ff872e32..6c1b44abb8e 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -7,8 +7,7 @@ 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. FIXME: why is this, apart from -https://github.com/matrix-org/synapse/issues/1477, which could be fixed? +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). @@ -30,6 +29,9 @@ 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 +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 From 9ba4c7ff21d7bc598c6a98bd65e05f5653fba6f8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 29 Aug 2019 18:02:59 +0100 Subject: [PATCH 3/7] Update proposals/2260-change-aliases-auth-rules.md Co-Authored-By: Matthew Hodgson --- proposals/2260-change-aliases-auth-rules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index 6c1b44abb8e..4daeff8fc50 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -29,7 +29,7 @@ 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 +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 From 5ec80dd739a05227c1d6385315b748ab732ae0d6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 29 Aug 2019 18:33:23 +0100 Subject: [PATCH 4/7] add some more thoughts --- proposals/2260-change-aliases-auth-rules.md | 31 ++++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index 4daeff8fc50..dedb1f51b45 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -23,19 +23,36 @@ Therefore, the special-case for `m.room.aliases` is to be removed from the rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules). `m.room.aliases` would instead be authorised following the normal rules for state events. -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). +TBD: are you still allowed to add rooms to the directory when you don't have +the necessary power level? If so, presumably this happens without updating the +`m.room.aliases` event. So: + + * Is there any mechanism for syncing the alias list if you are later given + ops? + + * What if another user on your server has ops? What if Eve lacks ops and + secretly adds `#offensive_alias:example.com`, and then Bob (who has ops) + adds `#nice_alias:example.com`? How do we make sure that the offensive alias + isn't published by Bob? + +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 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? +Perhaps allowing room admins the ability to redact malicious `aliases` events +is sufficient? Given that a malicious user could immediately publish a new +`aliases` event (even if they have been banned from the room), it seems like +that would be ineffective. + +Or we could just allow room admins to issue new `m.room.aliases` events +(possibly restricting them to removing aliases, though it's TBD if state res +would work reliably in this case). However, that seems to suffer the same +problem as above. ## Potential issues From 4d45afd2c53ce951502df742c5b76897a193de20 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 30 Aug 2019 14:09:00 +0100 Subject: [PATCH 5/7] add some more clarifications --- proposals/2260-change-aliases-auth-rules.md | 57 ++++++++++++++------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index dedb1f51b45..1d058d50afd 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -2,27 +2,42 @@ ## 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). +The [`m.room.aliases`](https://matrix.org/docs/spec/client_server/r0.5.0#m-room-aliases) +state event exists to list the available aliases for a given room. This serves +two purposes: -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). + * It allows existing members of the room to discover alternative aliases, + which may be useful for them to pass this knowledge on to others trying to + join. -## Proposal + * Secondarily, it helps to educate users about how Matrix works by + illustrating multiple aliases per room and giving a perception of the size + of the network. + +However, it has problems: + + * Any user in the entire ecosystem can create aliases for rooms, which are + then unilaterally added to `m.room.aliases`, and room admins are unable to + remove them. This is an abuse + vector (https://github.com/matrix-org/matrix-doc/issues/625). + + * For various reasons, the `m.room.aliases` event tends to get out of sync + with the actual aliases (https://github.com/matrix-org/matrix-doc/issues/2262). -`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. +Note that `m.room.aliases` is subject to specific [authorization +rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules). -Therefore, the special-case for `m.room.aliases` is to be removed from the -[authorization +## Proposal + +We remove the special-case for `m.room.aliases` 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 would mean that only room moderators could add entries to the +`m.room.aliases` event, and would therefore solve the abuse issue. However, it +would increase the likelihood of `m.room.aliases` being out of sync with the +real aliases. + TBD: are you still allowed to add rooms to the directory when you don't have the necessary power level? If so, presumably this happens without updating the `m.room.aliases` event. So: @@ -56,11 +71,17 @@ problem as above. ## 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. +1. This will increase the number of ways that `m.room.aliases` differs from + reality, particularly if we allow people to add entries to directories when + they lack ops in the room, but also when server admins remove entries from + the directory (currently https://github.com/matrix-org/synapse/issues/1477 + could be fixed, but under this MSC it would be unfixable.) + +2. Often, all moderators of a room will be on one server, so much of the point of + `m.room.aliases` (that of advertising alternative aliases on other servers) + would be lost. -2. This would allow room operators to add 'fake' aliases: for example, I could +3. 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. From ca4aad14006d852b30641e9afca0f745808c22c4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 2 Sep 2019 17:35:25 +0100 Subject: [PATCH 6/7] updates --- proposals/2260-change-aliases-auth-rules.md | 88 +++++++++++++-------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index 1d058d50afd..a8fa6fdfed5 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -29,35 +29,41 @@ rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules). ## Proposal -We remove the special-case for `m.room.aliases` 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. +1. We modify the special-case for `m.room.aliases` in the [authorization + rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules), so that + `m.room.aliases` are subject to power-levels *as well as* the constraint + that the `state_key` must match the sender's domain. -This would mean that only room moderators could add entries to the -`m.room.aliases` event, and would therefore solve the abuse issue. However, it -would increase the likelihood of `m.room.aliases` being out of sync with the -real aliases. + In other words, we remove step 4c from the rules. -TBD: are you still allowed to add rooms to the directory when you don't have -the necessary power level? If so, presumably this happens without updating the -`m.room.aliases` event. So: +2. We also change the default `m.room.power_levels` event generated by + [`/createRoom`](https://matrix.org/docs/spec/client_server/r0.5.0#post-matrix-client-r0-createroom) + to give ordinary users the ability to send `m.room.aliases` events (ie, we + add an entry `"m.room.aliases": 0` to the `events` map). - * Is there any mechanism for syncing the alias list if you are later given - ops? +3. We prevent clients from sending `m.room.aliases` events via the `/state` (or + `/send`) APIs, to prevent users from removing listed aliases or generating + spam aliases without the relevant PL. - * What if another user on your server has ops? What if Eve lacks ops and - secretly adds `#offensive_alias:example.com`, and then Bob (who has ops) - adds `#nice_alias:example.com`? How do we make sure that the offensive alias - isn't published by Bob? + TBD: alternatively: switch to [`m.room.alias` + events](https://github.com/matrix-org/matrix-doc/issues/2259), and make + sending such events an alias for the directory APIs. -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). +4. We make it possible to redact the contents of `m.room.aliases` events, as + per [MSC2261](https://github.com/matrix-org/matrix-doc/issues/2261). -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). +This means: -## Tradeoffs +* Anyone can still create/delete aliases in their server's room directory + (subject to local permissions restrictions). + +* If the user has the PL required to send an `m.room.aliases` event (ie, + normally), the server will generate such an event on the user's behalf. + +* Room admins can handle alias spam by redacting abusive aliases and/or raising + the PL necessary to add new aliases. + +## Alternatives Perhaps allowing room admins the ability to redact malicious `aliases` events is sufficient? Given that a malicious user could immediately publish a new @@ -69,19 +75,33 @@ Or we could just allow room admins to issue new `m.room.aliases` events would work reliably in this case). However, that seems to suffer the same problem as above. +Or we could just restrict the ability to send `m.room.aliases` to moderators +(like any other state event). That seems like it would be overly restrictive in +the aliases published, though. + ## Potential issues -1. This will increase the number of ways that `m.room.aliases` differs from - reality, particularly if we allow people to add entries to directories when - they lack ops in the room, but also when server admins remove entries from - the directory (currently https://github.com/matrix-org/synapse/issues/1477 - could be fixed, but under this MSC it would be unfixable.) +1. This does little to address the drift of `m.room.aliases` from + reality. Indeed, it would exacerbate + https://github.com/matrix-org/synapse/issues/1477. + +2. Unless we replace `m.room.aliases` with `m.room.alias` + (https://github.com/matrix-org/matrix-doc/issues/2259), there are problems + around delayed updates to the aliases list: + + * Eve adds an offensive alias. She does not have the relevant PL to update + the `aliases` event, so this has no effect except to add the entry to the + directory, and it probably goes completely unnoticed. + + * Later, Bob (who is on the same server as Eve, but has the PL to update the + `aliases` event) adds a new alias to the room. -2. Often, all moderators of a room will be on one server, so much of the point of - `m.room.aliases` (that of advertising alternative aliases on other servers) - would be lost. + At this point, I am envisaging an implementation where the server will + automatically generate an `m.room.aliases` event, with Bob as the sender, + listing all of the current aliases for the room. In other words, other users + in the room will see "Bob added `#nice_alias` and `#evil_alias`". -3. 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. + An alternative impl is possible where the new `aliases` event is based on + the previous one and just adds the new alias; however that screams races to + me and (particularly once state resolution comes into play) I think it's + likely that the list is going to get out of sync. From 31fa78bf97020505a6a077ab50436724f87dced6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Jan 2020 17:07:51 +0000 Subject: [PATCH 7/7] Some clarifications based on feedback --- proposals/2260-change-aliases-auth-rules.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/proposals/2260-change-aliases-auth-rules.md b/proposals/2260-change-aliases-auth-rules.md index a8fa6fdfed5..862d88a2d3a 100644 --- a/proposals/2260-change-aliases-auth-rules.md +++ b/proposals/2260-change-aliases-auth-rules.md @@ -82,8 +82,10 @@ the aliases published, though. ## Potential issues 1. This does little to address the drift of `m.room.aliases` from - reality. Indeed, it would exacerbate - https://github.com/matrix-org/synapse/issues/1477. + reality. Indeed, it would exacerbate it: it increases the number of cases + in which we don't have permission to update the `aliases` event (for example: + `DELETE /directory/room` allows users to delete aliases without + (necessarily) having the abilility to update the `aliases` event). 2. Unless we replace `m.room.aliases` with `m.room.alias` (https://github.com/matrix-org/matrix-doc/issues/2259), there are problems @@ -105,3 +107,9 @@ the aliases published, though. the previous one and just adds the new alias; however that screams races to me and (particularly once state resolution comes into play) I think it's likely that the list is going to get out of sync. + + Yet another alternative impl is possible where we keep track (in the + directory table) of whether each alias was added by a user who had + rights to send the `aliases` event. Eve's `#evil_alias` is therefore marked + as evil, so when Bob adds his `#nice_alias` the server can exclude it from + the generated `m.room.aliases` event.