-
Notifications
You must be signed in to change notification settings - Fork 382
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
Conversation
(if this is a WIP, please tag it in the title. If it's not, please add |
Co-Authored-By: Matthew Hodgson <[email protected]>
actually, it's clear that this is still WIP |
m.room.aliases
eventsm.room.aliases
events
m.room.aliases
eventsm.room.aliases
events
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still let users contribute entries to the event, because otherwise they will have to tell the moderator out-of-band to add their room. As you say, it also increases the risk that when the alias is removed from the directory in question, the event won't be updated if a normal user can't update it.
If a room admin deliberately wants to lock down who can advertise aliases to just be mods/ops, then they can do that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still let users contribute entries to the event,
I think this is different to what you said at https://gist.github.com/ara4n/0ec423e7c321acbb53eed04ceb4dd7df#gistcomment-3012860 though. Have you changed your mind or is one of us misunderstanding?
If we continue to allow regular users to update the aliases event without regard to power levels, then we still have an abuse problem that is hard to control.
Or do you mean that aliases
events should be subject to PLs, but for public rooms, we should emit a PL event that lowers the required level for aliases
to 0? In which case, I think this makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you changed your mind or is one of us misunderstanding?
I think i am misunderstanding. My assumption was that the special cases for 4a and 4b still take effect (meaning that you can't publish m.room.aliases on behalf of remote servers). However, on 4c, normal PL rules take effect - so it's possible to let anyone in the room advertise an alias via m.room.aliases
, or alternatively put a PL threshold and only let ops/admins do so. My hunch would be to allow anyone to set one by default (so default to PL threshold of 0 for that state event).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, so:
Or do you mean that aliases events should be subject to PLs, but for public rooms, we should emit a PL event that lowers the required level for aliases to 0? In which case, I think this makes sense.
this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, or just define the default PL for m.room.aliases
to be 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, or just define the default PL for m.room.aliases to be 0.
that seems an unnecessary complication.
I've updated the MSC to say that we will lower the PL for m.room.aliases
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is having default PL 0 for this really a good idea? I don't know if I am making a wild assumption here but I'm not sure how many everyday users will examine the power levels when creating a new room unless they have specific reason to.
This would therefore still leave lots of rooms owned by inexperienced users open to drive by-style attacks where someone just joins the room, adds a whole bunch of evil aliases and leaves again (possibly some hours before anyone with a suitable PL even notices).
"Defaults reveal the soul" and all that.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, in that scenario we still need the ability for the room admin to gag randoms from adding remote aliases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...or require the user to be in the room in order to set a m.room.alias
, so we can stop abuse by banning them, which you'd get if we fell through to the default auth rules.
another use case for this for trying to identify random rooms sitting on your server; the presence of an m.room.aliases event gives you more of a clue (programmatically) as to what they might be. |
thanks for updating it - we've converged on the same proposal, hopefully for the best. |
I think it's fair to say this is no longer WIP. There's a slight question mark over we do https://github.com/matrix-org/matrix-doc/issues/2259 first, which would make it work better, but increases the scope of the project. Otherwise I think Matthew and I are aligned on this proposal now. |
m.room.aliases
eventsm.room.aliases
events
This MSC is now implemented in synapse develop, in the One note: I haven't done anything to deal with the "delayed alias update" problem, as per https://github.com/matrix-org/matrix-doc/blob/rav/proposal/change_aliases_auth_rules/proposals/2260-change-aliases-auth-rules.md#potential-issues (#2). So:
I guess I could try the "keep track of which aliases should be in the room state" solution. |
well, time to propose FCP I guess: @mscbot fcp merge |
Team member @richvdh has proposed to merge this. The next step is review by the rest of the tagged people: Once at least 75% of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
It seems that we are reconsidering this approach and will try something else instead. @mscbot fcp cancel |
Having implemented this, we found the following: The core assumption was that everybody would still be able to add aliases to the list in Whilst this MSC opens the possibility of increasing the PL required to send an We've had a bit of a rethink, and have an alternative approach, which I shall write it up as a new MSC. |
Synapse 1.10.0 (2020-02-12) =========================== **WARNING to client developers**: As of this release Synapse validates `client_secret` parameters in the Client-Server API as per the spec. See [\#6766](#6766) for details. Updates to the Docker image --------------------------- - Update the docker images to Alpine Linux 3.11. ([\#6897](#6897)) Synapse 1.10.0rc5 (2020-02-11) ============================== Bugfixes -------- - Fix the filtering introduced in 1.10.0rc3 to also apply to the state blocks returned by `/sync`. ([\#6884](#6884)) Synapse 1.10.0rc4 (2020-02-11) ============================== This release candidate was built incorrectly and is superceded by 1.10.0rc5. Synapse 1.10.0rc3 (2020-02-10) ============================== Features -------- - Filter out `m.room.aliases` from the CS API to mitigate abuse while a better solution is specced. ([\#6878](#6878)) Internal Changes ---------------- - Fix continuous integration failures with old versions of `pip`, which were introduced by a release of the `zipp` library. ([\#6880](#6880)) Synapse 1.10.0rc2 (2020-02-06) ============================== Bugfixes -------- - Fix an issue with cross-signing where device signatures were not sent to remote servers. ([\#6844](#6844)) - Fix to the unknown remote device detection which was introduced in 1.10.rc1. ([\#6848](#6848)) Internal Changes ---------------- - Detect unexpected sender keys on remote encrypted events and resync device lists. ([\#6850](#6850)) Synapse 1.10.0rc1 (2020-01-31) ============================== Features -------- - Add experimental support for updated authorization rules for aliases events, from [MSC2260](matrix-org/matrix-spec-proposals#2260). ([\#6787](#6787), [\#6790](#6790), [\#6794](#6794)) Bugfixes -------- - Warn if postgres database has a non-C locale, as that can cause issues when upgrading locales (e.g. due to upgrading OS). ([\#6734](#6734)) - Minor fixes to `PUT /_synapse/admin/v2/users` admin api. ([\#6761](#6761)) - Validate `client_secret` parameter using the regex provided by the Client-Server API, temporarily allowing `:` characters for older clients. The `:` character will be removed in a future release. ([\#6767](#6767)) - Fix persisting redaction events that have been redacted (or otherwise don't have a redacts key). ([\#6771](#6771)) - Fix outbound federation request metrics. ([\#6795](#6795)) - Fix bug where querying a remote user's device keys that weren't cached resulted in only returning a single device. ([\#6796](#6796)) - Fix race in federation sender worker that delayed sending of device updates. ([\#6799](#6799), [\#6800](#6800)) - Fix bug where Synapse didn't invalidate cache of remote users' devices when Synapse left a room. ([\#6801](#6801)) - Fix waking up other workers when remote server is detected to have come back online. ([\#6811](#6811)) Improved Documentation ---------------------- - Clarify documentation related to `user_dir` and `federation_reader` workers. ([\#6775](#6775)) Internal Changes ---------------- - Record room versions in the `rooms` table. ([\#6729](#6729), [\#6788](#6788), [\#6810](#6810)) - Propagate cache invalidates from workers to other workers. ([\#6748](#6748)) - Remove some unnecessary admin handler abstraction methods. ([\#6751](#6751)) - Add some debugging for media storage providers. ([\#6757](#6757)) - Detect unknown remote devices and mark cache as stale. ([\#6776](#6776), [\#6819](#6819)) - Attempt to resync remote users' devices when detected as stale. ([\#6786](#6786)) - Delete current state from the database when server leaves a room. ([\#6792](#6792)) - When a client asks for a remote user's device keys check if the local cache for that user has been marked as potentially stale. ([\#6797](#6797)) - Add background update to clean out left rooms from current state. ([\#6802](#6802), [\#6816](#6816)) - Refactoring work in preparation for changing the event redaction algorithm. ([\#6803](#6803), [\#6805](#6805), [\#6806](#6806), [\#6807](#6807), [\#6820](#6820))
Synapse 1.12.0 (2020-03-23) =========================== No significant changes since 1.12.0rc1. Debian packages and Docker images are rebuilt using the latest versions of dependency libraries, including Twisted 20.3.0. **Please see security advisory below**. Security advisory ----------------- Synapse may be vulnerable to request-smuggling attacks when it is used with a reverse-proxy. The vulnerabilties are fixed in Twisted 20.3.0, and are described in [CVE-2020-10108](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-10108) and [CVE-2020-10109](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-10109). For a good introduction to this class of request-smuggling attacks, see https://portswigger.net/research/http-desync-attacks-request-smuggling-reborn. We are not aware of these vulnerabilities being exploited in the wild, and do not believe that they are exploitable with current versions of any reverse proxies. Nevertheless, we recommend that all Synapse administrators ensure that they have the latest versions of the Twisted library to ensure that their installation remains secure. * Administrators using the [`matrix.org` Docker image](https://hub.docker.com/r/matrixdotorg/synapse/) or the [Debian/Ubuntu packages from `matrix.org`](https://github.com/matrix-org/synapse/blob/master/INSTALL.md#matrixorg-packages) should ensure that they have version 1.12.0 installed: these images include Twisted 20.3.0. * Administrators who have [installed Synapse from source](https://github.com/matrix-org/synapse/blob/master/INSTALL.md#installing-from-source) should upgrade Twisted within their virtualenv by running: ```sh <path_to_virtualenv>/bin/pip install 'Twisted>=20.3.0' ``` * Administrators who have installed Synapse from distribution packages should consult the information from their distributions. The `matrix.org` Synapse instance was not vulnerable to these vulnerabilities. Advance notice of change to the default `git` branch for Synapse ---------------------------------------------------------------- Currently, the default `git` branch for Synapse is `master`, which tracks the latest release. After the release of Synapse 1.13.0, we intend to change this default to `develop`, which is the development tip. This is more consistent with common practice and modern `git` usage. Although we try to keep `develop` in a stable state, there may be occasions where regressions creep in. Developers and distributors who have scripts which run builds using the default branch of `Synapse` should therefore consider pinning their scripts to `master`. Synapse 1.12.0rc1 (2020-03-19) ============================== Features -------- - Changes related to room alias management ([MSC2432](matrix-org/matrix-spec-proposals#2432)): - Publishing/removing a room from the room directory now requires the user to have a power level capable of modifying the canonical alias, instead of the room aliases. ([\#6965](#6965)) - Validate the `alt_aliases` property of canonical alias events. ([\#6971](#6971)) - Users with a power level sufficient to modify the canonical alias of a room can now delete room aliases. ([\#6986](#6986)) - Implement updated authorization rules and redaction rules for aliases events, from [MSC2261](matrix-org/matrix-spec-proposals#2261) and [MSC2432](matrix-org/matrix-spec-proposals#2432). ([\#7037](#7037)) - Stop sending m.room.aliases events during room creation and upgrade. ([\#6941](#6941)) - Synapse no longer uses room alias events to calculate room names for push notifications. ([\#6966](#6966)) - The room list endpoint no longer returns a list of aliases. ([\#6970](#6970)) - Remove special handling of aliases events from [MSC2260](matrix-org/matrix-spec-proposals#2260) added in v1.10.0rc1. ([\#7034](#7034)) - Expose the `synctl`, `hash_password` and `generate_config` commands in the snapcraft package. Contributed by @devec0. ([\#6315](#6315)) - Check that server_name is correctly set before running database updates. ([\#6982](#6982)) - Break down monthly active users by `appservice_id` and emit via Prometheus. ([\#7030](#7030)) - Render a configurable and comprehensible error page if something goes wrong during the SAML2 authentication process. ([\#7058](#7058), [\#7067](#7067)) - Add an optional parameter to control whether other sessions are logged out when a user's password is modified. ([\#7085](#7085)) - Add prometheus metrics for the number of active pushers. ([\#7103](#7103), [\#7106](#7106)) - Improve performance when making HTTPS requests to sygnal, sydent, etc, by sharing the SSL context object between connections. ([\#7094](#7094)) Bugfixes -------- - When a user's profile is updated via the admin API, also generate a displayname/avatar update for that user in each room. ([\#6572](#6572)) - Fix a couple of bugs in email configuration handling. ([\#6962](#6962)) - Fix an issue affecting worker-based deployments where replication would stop working, necessitating a full restart, after joining a large room. ([\#6967](#6967)) - Fix `duplicate key` error which was logged when rejoining a room over federation. ([\#6968](#6968)) - Prevent user from setting 'deactivated' to anything other than a bool on the v2 PUT /users Admin API. ([\#6990](#6990)) - Fix py35-old CI by using native tox package. ([\#7018](#7018)) - Fix a bug causing `org.matrix.dummy_event` to be included in responses from `/sync`. ([\#7035](#7035)) - Fix a bug that renders UTF-8 text files incorrectly when loaded from media. Contributed by @TheStranjer. ([\#7044](#7044)) - Fix a bug that would cause Synapse to respond with an error about event visibility if a client tried to request the state of a room at a given token. ([\#7066](#7066)) - Repair a data-corruption issue which was introduced in Synapse 1.10, and fixed in Synapse 1.11, and which could cause `/sync` to return with 404 errors about missing events and unknown rooms. ([\#7070](#7070)) - Fix a bug causing account validity renewal emails to be sent even if the feature is turned off in some cases. ([\#7074](#7074)) Improved Documentation ---------------------- - Updated CentOS8 install instructions. Contributed by Richard Kellner. ([\#6925](#6925)) - Fix `POSTGRES_INITDB_ARGS` in the `contrib/docker/docker-compose.yml` example docker-compose configuration. ([\#6984](#6984)) - Change date in [INSTALL.md](./INSTALL.md#tls-certificates) for last date of getting TLS certificates to November 2019. ([\#7015](#7015)) - Document that the fallback auth endpoints must be routed to the same worker node as the register endpoints. ([\#7048](#7048)) Deprecations and Removals ------------------------- - Remove the unused query_auth federation endpoint per [MSC2451](matrix-org/matrix-spec-proposals#2451). ([\#7026](#7026)) Internal Changes ---------------- - Add type hints to `logging/context.py`. ([\#6309](#6309)) - Add some clarifications to `README.md` in the database schema directory. ([\#6615](#6615)) - Refactoring work in preparation for changing the event redaction algorithm. ([\#6874](#6874), [\#6875](#6875), [\#6983](#6983), [\#7003](#7003)) - Improve performance of v2 state resolution for large rooms. ([\#6952](#6952), [\#7095](#7095)) - Reduce time spent doing GC, by freezing objects on startup. ([\#6953](#6953)) - Minor perfermance fixes to `get_auth_chain_ids`. ([\#6954](#6954)) - Don't record remote cross-signing keys in the `devices` table. ([\#6956](#6956)) - Use flake8-comprehensions to enforce good hygiene of list/set/dict comprehensions. ([\#6957](#6957)) - Merge worker apps together. ([\#6964](#6964), [\#7002](#7002), [\#7055](#7055), [\#7104](#7104)) - Remove redundant `store_room` call from `FederationHandler._process_received_pdu`. ([\#6979](#6979)) - Update warning for incorrect database collation/ctype to include link to documentation. ([\#6985](#6985)) - Add some type annotations to the database storage classes. ([\#6987](#6987)) - Port `synapse.handlers.presence` to async/await. ([\#6991](#6991), [\#7019](#7019)) - Add some type annotations to the federation base & client classes. ([\#6995](#6995)) - Port `synapse.rest.keys` to async/await. ([\#7020](#7020)) - Add a type check to `is_verified` when processing room keys. ([\#7045](#7045)) - Add type annotations and comments to the auth handler. ([\#7063](#7063))
Rendered