From 72e694ba0b85604f4f59c5fb84d79514a70f783a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Apr 2024 09:26:42 -0600 Subject: [PATCH 01/29] MSC4126: Deprecation of query string auth (#4126) * MSC: Deprecation of query string auth * Update proposals/4126-deprecate-query-string-auth.md Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/4126-deprecate-query-string-auth.md | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 proposals/4126-deprecate-query-string-auth.md diff --git a/proposals/4126-deprecate-query-string-auth.md b/proposals/4126-deprecate-query-string-auth.md new file mode 100644 index 00000000000..264555f7238 --- /dev/null +++ b/proposals/4126-deprecate-query-string-auth.md @@ -0,0 +1,74 @@ +# MSC4126: Deprecation of query string auth + +Presently, the Client-Server API allows clients to provide their access token via the `Authorization` +request header or via an `access_token` query string parameter, described [here](https://spec.matrix.org/v1.10/client-server-api/#using-access-tokens). +Clients are already encouraged to use the header approach, though the query string option exists for +largely backwards compatibility reasons. + +The query string approach is subject a number of security, usability, and practical concerns, discussed +on [matrix-spec#1780](https://github.com/matrix-org/matrix-spec/issues/1780): + +* The query string of an HTTP request is often logged by the client itself, middleware reverse proxy, + and application/homeserver as well. Though some of these layers may be aware of this issue, they + can trivially accidentally break to log sensitive credentials again. By contrast, headers are not + typically logged by default. + +* Users often copy and paste URLs from their clients to either get support or provide direct links + to content/media. While the media angle is largely expected to be resolved with [MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916), + users are currently able to right click images in their client and copy the URL - if this URL + includes authentication in the query string, the user will likely end up disclosing their access + token. The same scenario applies when copy/pasting request logs out of a client when getting + support. + +* Having two ways of doing things could lead to compatibility issues, where a client using the query + string approach is tried against a server which only supports the header. The client ends up not + working, leading to subpar user experience. + +* Most clients have already adopted the header approach, largely forgetting that the query string + even exists. Continuing to support the query string option leaves some maintenance burden for what + is effectively unused code. + +* Matrix has [decided](https://matrix.org/blog/2023/09/matrix-2-0/) to adopt OIDC for authentication, + which is based on OAuth 2.0, which [advises against](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.3.2) + the query string approach. + +With these conditions in mind, this proposal sets the query string approach on a path towards removal +from the Matrix specification. This affects the Client-Server API and [Identity Service API](https://spec.matrix.org/v1.10/identity-service-api/#authentication) +as both support the approaches described above. + +## Proposal + +For both the Client-Server API and Identity Service API, the `access_token` query string authentication +parameter becomes *deprecated*, and SHOULD NOT be used by clients (as already stated in the specification). +Deprecation is required for at least 1 spec version before removal under the [deprecation policy](https://spec.matrix.org/v1.10/#deprecation-policy). + +Removal from the specification requires a second MSC and at least 1 specification release to pass. This +is currently described as [MSC4127](https://github.com/matrix-org/matrix-spec-proposals/pull/4127). + +## Potential issues + +Clients which rely on the query string approach may stop working. This is considered acceptable for +the purposes of this MSC. + +## Alternatives + +Most alternatives are not practical as they would maintain the security risk described in the introduction +for this proposal. + +Alterations to the deprecation policy may be discussed in a future MSC to make this sort of removal +easier. + +## Security considerations + +Security considerations are described throughout this proposal. + +## Unstable prefix + +This proposal cannot feasibly have an unstable prefix. Clients are already discouraged from using +query string authentication and should switch to `Authorization` as soon as possible, regardless of +this MSC. + +## Dependencies + +This MSC has no direct dependencies itself. [MSC4127](https://github.com/matrix-org/matrix-spec-proposals/pull/4127) +requires this MSC to land first. From d6edcbd9461fe0e2755978cc20750381ae828d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= <76261501+zecakeh@users.noreply.github.com> Date: Tue, 30 Apr 2024 20:43:58 +0200 Subject: [PATCH 02/29] Upgrade typos CI action (#4135) --- .github/_typos.toml | 1 + .github/workflows/spell-check.yaml | 2 +- proposals/1711-x509-for-federation.md | 2 +- proposals/1884-replace-slashes-in-event_ids.md | 2 +- proposals/2140-terms-of-service-2.md | 2 +- proposals/2677-reactions.md | 2 +- proposals/2870-protect-acls-from-redaction.md | 2 +- proposals/3030-jump-to-date.md | 2 +- 8 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/_typos.toml b/.github/_typos.toml index d227af4cb88..2e7159b7681 100644 --- a/.github/_typos.toml +++ b/.github/_typos.toml @@ -8,3 +8,4 @@ OTKs = "OTKs" [default.extend-words] OTK = "OTK" OTKs = "OTKs" +Iy = "Iy" diff --git a/.github/workflows/spell-check.yaml b/.github/workflows/spell-check.yaml index 11f019388ee..ff444616bba 100644 --- a/.github/workflows/spell-check.yaml +++ b/.github/workflows/spell-check.yaml @@ -14,6 +14,6 @@ jobs: uses: actions/checkout@v4 - name: Check spelling of proposals - uses: crate-ci/typos@9be36f97fdbe645ee9a12449fb13aca856c2516a + uses: crate-ci/typos@f2c1f08a7b3c1b96050cb786baaa2a94797bdb7d # v1.20.10 with: config: ${{github.workspace}}/.github/_typos.toml diff --git a/proposals/1711-x509-for-federation.md b/proposals/1711-x509-for-federation.md index 7fd9056091f..ef351ade891 100644 --- a/proposals/1711-x509-for-federation.md +++ b/proposals/1711-x509-for-federation.md @@ -174,7 +174,7 @@ certificates comes with a number of downsides. Configuring a working, federating homeserver is a process fraught with pitfalls. This proposal adds the requirement to obtain a signed certificate to -that process. Even with modern intiatives such as Let's Encrypt, this is +that process. Even with modern initiatives such as Let's Encrypt, this is another procedure requiring manual intervention across several moving parts. On the other hand: obtaining an SSL certificate should be a familiar process to diff --git a/proposals/1884-replace-slashes-in-event_ids.md b/proposals/1884-replace-slashes-in-event_ids.md index bec8d7adccd..7ae91102ab0 100644 --- a/proposals/1884-replace-slashes-in-event_ids.md +++ b/proposals/1884-replace-slashes-in-event_ids.md @@ -88,7 +88,7 @@ right encoding. A potential extension would be to change *all* Base64 encodings to be URL-safe. This would address the inconsistency. However, it feels like a large job which would span the entire matrix ecosystem (far larger than - updating clients to URL-encode their URL prarameters), and again the + updating clients to URL-encode their URL parameters), and again the situation would be confusing while the transition was in progress. 2. Incompleteness. Event IDs are certainly not the only identifier which can diff --git a/proposals/2140-terms-of-service-2.md b/proposals/2140-terms-of-service-2.md index c7a4fbf8e8e..df505d9f92a 100644 --- a/proposals/2140-terms-of-service-2.md +++ b/proposals/2140-terms-of-service-2.md @@ -34,7 +34,7 @@ way that Homeservers do. ## Proposal -Throuhgout this proposal, $prefix will be used to refer to the prefix of the +Throughout this proposal, $prefix will be used to refer to the prefix of the API in question, ie. `/_matrix/identity/v2` for the IS API and `/_matrix/integrations/v1` for the IM API. diff --git a/proposals/2677-reactions.md b/proposals/2677-reactions.md index 41d85ebd054..18e789c024e 100644 --- a/proposals/2677-reactions.md +++ b/proposals/2677-reactions.md @@ -51,7 +51,7 @@ property should contains a `key` that indicates the annotation being applied. For example, when reacting with emojis, the `key` contains the emoji being used. -An event annotating another with the thumbs-up emoji would therefore have the following `m.relates_to` propperty: +An event annotating another with the thumbs-up emoji would therefore have the following `m.relates_to` property: ```json "m.relates_to": { diff --git a/proposals/2870-protect-acls-from-redaction.md b/proposals/2870-protect-acls-from-redaction.md index aa526bf9a09..48abc6f9d3d 100644 --- a/proposals/2870-protect-acls-from-redaction.md +++ b/proposals/2870-protect-acls-from-redaction.md @@ -40,7 +40,7 @@ the room administrator's understanding of their room's function. Another possible approach is to have servers prevent sending faulty ACL events by preventing their local clients from doing so, such as by rejecting redaction requests. This doesn't solve the issue -over federation, but can reduce the liklihood of such events making it to the federation layer. This +over federation, but can reduce the likelihood of such events making it to the federation layer. This is what Synapse currently does to help mitigate the issue. This is only effective if the server (or client, locally) implements it - it's common for both clients and servers to forget to add these checks during development, leading to occasional room breakage. This MSC instead tries to resolve the issue diff --git a/proposals/3030-jump-to-date.md b/proposals/3030-jump-to-date.md index 75f3af15022..189c9330059 100644 --- a/proposals/3030-jump-to-date.md +++ b/proposals/3030-jump-to-date.md @@ -152,7 +152,7 @@ the room was created. ## Alternatives We chose the current `/timestamp_to_event` route because it sounded like the -easist path forward to bring it to fruition and get some real-world experience. +easiest path forward to bring it to fruition and get some real-world experience. And was on our mind during the [initial discussion](https://docs.google.com/document/d/1KCEmpnGr4J-I8EeaVQ8QJZKBDu53ViI7V62y5BzfXr0/edit#bookmark=id.qu9k9wje9pxm) because there was some prior art with a [WIP implementation](https://github.com/matrix-org/synapse/pull/9445/commits/91b1b3606c9fb9eede0a6963bc42dfb70635449f) from @erikjohnston. The alternatives haven't been thrown out for a particular From 8eb75fe64ac006a14c286deafd87822a4791347b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 10 May 2024 22:31:03 +0100 Subject: [PATCH 03/29] MSC3939: Account locking (#3939) * MSCXXXX: Account locking * MSC number * Switch to 401 + soft_lougout * FCP updates/clarity * Mention appeals/information * v1.10 * Clarify why not 403 * Update proposals/3939-account-locking.md Co-authored-by: David Baker * Clarify UI expectations * Update proposals/3939-account-locking.md Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update proposals/3939-account-locking.md Co-authored-by: Hubert Chathi * Update proposals/3939-account-locking.md Co-authored-by: Hubert Chathi --------- Co-authored-by: Mathieu Velten Co-authored-by: Travis Ralston Co-authored-by: David Baker Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Hubert Chathi --- proposals/3939-account-locking.md | 91 +++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 proposals/3939-account-locking.md diff --git a/proposals/3939-account-locking.md b/proposals/3939-account-locking.md new file mode 100644 index 00000000000..c80f4b5e435 --- /dev/null +++ b/proposals/3939-account-locking.md @@ -0,0 +1,91 @@ +# MSC3939: Account locking + +Account locking is a common safety and security tool where server administrators +can prevent a user from usefully using their account. For example, too many failed +login attempts, escalation in moderation action, or out of band verification +needing to be completed. + +Currently, Matrix only supports deactivating an account. This is a destructive +action which often leads to the account leaving all joined rooms, among other +details. With account locking, the effect of making the user unable to access +their account is achieved without destroying that same account - the account +can always be unlocked. + +This proposal covers account locking, though leaves the specifics of use as an +implementation detail. APIs for administration of locking are also not provided. + +## Proposal + +When an account is locked, clients receive a `401 Unauthorized` error response +to most APIs with an `M_USER_LOCKED` error code and `soft_logout` set to `true`. +Excluded APIs are described below. We enable `soft_logout` to encourage clients +to make use of the [soft logout](https://spec.matrix.org/v1.9/client-server-api/#soft-logout) +semantics: keep encryption state, but otherwise render the account unusable. 401 +is used to support legacy clients by giving the user a semantically meaningful +experience: they may need to try logging in again, and when they do they may get +a more useful error message about their account status, though their session data +may be deleted by the client if it doesn't recognize the error code. Soft logout +aims to prevent total destruction of this data, however. + +Upon receiving the `M_USER_LOCKED` error, clients SHOULD retain session information +including encryption state and inform the user that their account has been locked. +Details about *why* the user's account is locked are not formally described by +this proposal, though future MSCs which cover informing users about actions taken +against their account should have such details. Clients may wish to make use of +[server contact discovery](https://spec.matrix.org/v1.10/client-server-api/#getwell-knownmatrixsupport) +in the meantime. + +Clients SHOULD hide the normal UI from the user when informing them that their +account is locked, preventing general use of the account. + +Clients SHOULD continue to `/sync` and make other API calls to more quickly detect +when the lock has been lifted. However, clients should rate-limit their requests. If unlocked, the APIs will either return a different +error code or a normal 200 OK/successful response. For example, `/sync` will return +to working as though nothing ever happened. If the error code changes to +`M_UNKNOWN_TOKEN`, the client should delete local session data as it normally +would when seeing the error code (and use soft logout as it normally would). This +is typically expected if the server admin logged the user out or the user logged +themselves out. + +Locked accounts are still permitted to access the following API endpoints: + +* [`POST /logout`](https://spec.matrix.org/v1.9/client-server-api/#post_matrixclientv3logout) +* [`POST /logout/all`](https://spec.matrix.org/v1.9/client-server-api/#post_matrixclientv3logoutall) + +When a user's account is locked, servers SHOULD NOT invalidate an account's access tokens in case the account becomes +unlocked: the user should be able to retain their sessions without having to log +back in. However, if a client requests a logout (using the above endpoints), the +associated access tokens should be invalidated as normal. + +## Potential issues + +This proposal does not communicate *why* a user's account is restricted. The human-readable `error` +field may contain some information, though anything comprehensive may not be surfaced to the user. +A future MSC is expected to build a system for both informing the user of the action taken against +their account and allow the user to appeal that action. + +## Alternatives + +[MSC3823](https://github.com/matrix-org/matrix-spec-proposals/pull/3823) covers +a similar concept, though is semantically different. See [matrix-org/glossary](https://github.com/matrix-org/glossary) +for details. + +Another similar concept would be "shadow banning", though this only applies to +moderation use cases. + +A 403 HTTP status code was considered instead of 401 with a `soft_logout`. A 403 +would indicate that the given action is denied, but otherwise keep the user logged +in. This could wrongly indicate [suspension](https://github.com/matrix-org/matrix-spec-proposals/pull/3823), +confusing the user. Instead, we provide a semantically similar experience where +the user gets soft logged out on legacy clients, preserving encryption and related +session data (assuming the client also supports soft logout). This can result in +some loss of other session data however, like device-specific settings. Users may +also be differently confused when they try to log back in and get cryptic error +messages (indicating wrong username/password), however as mentioned above in the +Potential Issues section, communicating actions taken against an account is a +concern for a future MSC. + +## Unstable prefix + +Until this proposal is considered stable, implementations must use +`ORG_MATRIX_MSC3939_USER_LOCKED` instead of `M_USER_LOCKED`. From 2534644decaac348c034f16f4ee17c1c365c3861 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Sun, 12 May 2024 20:24:47 +0100 Subject: [PATCH 04/29] MSC4132: Deprecate Linking to an Event Against a Room Alias (#4132) * Deprecate Linking to an Event Against a Room Alias * Pin the spec links Co-authored-by: Travis Ralston * Address feedback. * Address comments about via and the URIs that should be used. * Rewording. --------- Co-authored-by: Travis Ralston --- ...4132-deprecate-event-on-room-alias-uris.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 proposals/4132-deprecate-event-on-room-alias-uris.md diff --git a/proposals/4132-deprecate-event-on-room-alias-uris.md b/proposals/4132-deprecate-event-on-room-alias-uris.md new file mode 100644 index 00000000000..607c66110e7 --- /dev/null +++ b/proposals/4132-deprecate-event-on-room-alias-uris.md @@ -0,0 +1,58 @@ +# MSC4132: Deprecate Linking to an Event Against a Room Alias. + +## Introduction + +The spec supports 2 types of URI, the [Matrix scheme](https://spec.matrix.org/v1.10/appendices/#matrix-uri-scheme) +and [matrix.to](https://spec.matrix.org/v1.10/appendices/#matrixto-navigation) links which allow deep linking to +Matrix users, rooms and events. Event URIs can be constructed against either a room ID or a room alias the latter of +which is fragile issue as a room's alias is mutable and so event links may be broken by changes to the alias. Users +expect deep links to continue working so that older links continue to show the correct content. Therefore it is proposed +to deprecate event links against a room alias. + + +## Proposal + +As room IDs are immutable, event URIs built against these will continue to work for as long as the room is reachable by +the homeserver. In contrast, event URIs built against a room alias can break under the following circumstances: +- The room is upgraded, resulting in the alias pointing to the new room, which won't contain any events from the + ancestor. +- The alias is removed/changed resulting in a dead link that can't be resolved. + +The proposal would [deprecate](https://spec.matrix.org/v1.10/#deprecation-policy) the 2 following URIs: +- Link to `$event` in `#somewhere:example.org`: `matrix:r/somewhere:example.org/e/event` +- Link to `$event` in `#somewhere:example.org`: `https://matrix.to/#/%23somewhere:example.org/%24event%3Aexample.org` + +Specifically this means it would be marked in the spec as not to be used in future and clients should: +1. Stop making new event URIs with room aliases, generating the examples above as the following respectively: + - `matrix:roomid/opaqueid:example.org/e/event?via=example.com` + - `https://matrix.to/#/!opaqueid%3Aexample.org/%24event%3Aexample.org?via=example.com` +2. Continue to accept them as it will take time for other clients to update and legacy events will continue to exist. + +It is worth nothing that this change would require clients to always add the `via=` fields when building event URIs as +these are expected for URIs that reference a room ID. + + +## Potential issues + +Whilst most clients do take the sensible route and generate event URIs against a room ID (even when it has an alias), as +it is part of the spec these kinds of links likely exist somewhere in room history and on the World Wide Web. This would +mean that after deprecation clients may still want to handle these URIs when received. + +Additionally, whilst not a new problem some clients don't always update the `via` fields, which poses a problem for +rooms that have lots of single user homeservers: A single user leaving can result in the `via` to become non functional +which over time can result in unreachable room IDs (this is essentially the equivalent of the alias being removed). + + +## Alternatives + +An alternative would be to leave things as they are, although this wouldn't be the best choice for the user. + + +## Security considerations + +It is unlikely that accepting this change would introduce any security considerations. + + +## Dependencies + +N/A From b9cdd782b9f97cf5d01ae8e0f7bc39a4963d906e Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Sun, 19 May 2024 20:29:57 +0100 Subject: [PATCH 05/29] MSC3967: Do not require UIA when first uploading cross signing keys (#3967) * Placeholder * MSC3967: Do not require UIA when first uploading cross signing keys * Lint length * Apply suggestions from code review Co-authored-by: Denis Kasak * Suggestion from review * Issue with Synapse compliance is being tracked elsewhere * Line length * Update 3967-device-signing-upload-uia.md * Update proposals/3967-device-signing-upload-uia.md Co-authored-by: Travis Ralston * Update 3967-device-signing-upload-uia.md * Update 3967-device-signing-upload-uia.md * Update 3967-device-signing-upload-uia.md * Update proposals/3967-device-signing-upload-uia.md Co-authored-by: Travis Ralston * Update 3967-device-signing-upload-uia.md * Update 3967-device-signing-upload-uia.md * Update proposals/3967-device-signing-upload-uia.md Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Co-authored-by: Denis Kasak Co-authored-by: kegsay <7190048+kegsay@users.noreply.github.com> Co-authored-by: Travis Ralston Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/3967-device-signing-upload-uia.md | 122 ++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 proposals/3967-device-signing-upload-uia.md diff --git a/proposals/3967-device-signing-upload-uia.md b/proposals/3967-device-signing-upload-uia.md new file mode 100644 index 00000000000..bd322648df1 --- /dev/null +++ b/proposals/3967-device-signing-upload-uia.md @@ -0,0 +1,122 @@ +# MSC3967: Do not require UIA when first uploading cross signing keys + +When a user first sets up end-to-end encryption cross-signing, their client +uploads their cross-signing keys to the server. + +This [upload operation](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3keysdevice_signingupload) +requires a higher level of security by applying User-Interactive Auth (UIA) to +the endpoint. + +This creates a usability issue at the point of user registration where a client +will typically want to immediately set up cross-signing for a new user. + +The issue is that the client will immediately need the user to re-authenticate +even though the user just authenticated. + +This usability issue has given rise to workarounds such as a +[configurable grace period](https://matrix-org.github.io/synapse/v1.98/usage/configuration/config_documentation.html#ui_auth) +(`ui_auth`.`session_timeout`) in Synapse whereby UIA will not be required for +uploading cross-signing keys where authentication has taken place recently. + +This proposal aims to provide for a standard way to address this UIA usability +issue with respect to setting up cross-signing. + +## Proposal + +For the `POST /_matrix/client/v3/keys/device_signing/upload` endpoint, the +Homeserver MUST require User-Interactive Authentication (UIA) _unless_: + - there is no existing cross-signing master key uploaded to the Homeserver, OR + - there is an existing cross-signing master key and it exactly matches the + cross-signing master key provided in the request body. If there are any additional + keys provided in the request (self signing key, user signing key) they MUST also + match the existing keys stored on the server. In other words, the request contains + no new keys. If there are new keys, UIA MUST be performed. + +In these scenarios, this endpoint is not protected by UIA. This means the client does not +need to send an `auth` JSON object with their request. + +This change allows clients to freely upload 1 set of keys, but not modify/overwrite keys if +they already exist. By allowing clients to upload the same set of keys more than once, this +makes this endpoint idempotent in the case where the response is lost over the network, which +would otherwise cause a UIA challenge upon retry. + +## Potential issues + +See security considerations below. + + +## Alternatives + +There has been some discussion around how to improve the usability of +cross-signing more generally. It may be that an alternative solution is to +provide a way to set up cross-signing in a single request. + +## Security considerations + +This change could be viewed as a degradation of security at the point of setting +up cross-signing in that it requires less authentication to upload cross-signing +keys on first use. + +However, this degradation needs to be weighed against the typical real world +situation where a Homeserver will be applying a grace period and so allow a +malicious actor to bypass UIA for a period of time after each authentication. + +### Existing users without E2EE keys + +Existing user accounts who do not already have cross-signing keys set up will +now be able to upload keys without UIA. If such a user has their access token +compromised, an attacker will be able to upload a `master_key`, `self_signing_key` +and `user_signing_key`. This is a similar threat model to a malicious server admin +replacing these keys in the homeserver database. + +This does not mean: + - the attacker can "take over the account". It does not allow the attacker to + [login](https://spec.matrix.org/v1.10/client-server-api/#login) as they need to + know the password to the account. Likewise, an attacker cannot [logout all devices](https://spec.matrix.org/v1.10/client-server-api/#post_matrixclientv3logoutall) + nor can they [logout specific devices](https://spec.matrix.org/v1.10/client-server-api/#delete_matrixclientv3devicesdeviceid) + as these also go through UIA prompts. + - the device will appear as verified to other users. Other users need to verify the + public key [out-of-band](https://spec.matrix.org/v1.10/client-server-api/#short-authentication-string-sas-verification). + As the true owner of the account is not performing this verification, if an attacker + physically met up with other users it would become obvious that this is not the true owner, + and hence no verification would be performed. + +The main usability issue around this endpoint is requiring UIA, so it is critical +that we only require UIA when absolutely necessary for the security of the account. +In practice, this means requiring UIA when keys are _replaced_. There have been +suggestions to reintroduce a grace period (e.g after initial device login) or just +mandate it entirely for these old existing accounts. This would negatively impact +usability because: + - it introduces temporal variability which becomes difficult to debug. Not all users + would be subject to these timers/mandates. As a result, it needs to be possible + to detect in a bug report if the client is one of these special cases, and this is hard to do + reliably, particularly when bug reports from other servers are involved. The kinds of + bugs being debugged are typically around encryption, where there are complex interactions + between different devices and continually changing server-side state. These kinds of bugs + are incredibly time-sensitive as it is, and adding more temporal variability makes it even + harder to debug correctly. + - it introduces configuration variability which becomes difficult to debug. It's not + clear what the grace period should actually be. Anything less than 1 hour risks + catching initial x-signing requests from users who are on particularly awful networks. + However, even increasing this to 1 hour poses the risk that we incorrectly catch the + initial upload (e.g the client logs in on a bad GSM connection, then give up waiting + for it to login and close the app, only reopening it the next day). This becomes + difficult to debug in bug reports, as they just report HTTP 401s and it is unknown what + the HS configuration is for the time delay. This is seen already due to the use (or non-use) + of `ui_auth.session_timeout`. A spec-mandated grace period would mitigate some of these + concerns, which could be further improved by adding special error codes indicating that + the grace period had expired. However, this adds extra API surface that will likely be + poorly tested in clients, as it's unreasonable to wait 1+ hours in a test, hence some + configuration would be likely included for testing purposes anyway. + +For these reasons, this MSC does not specify a grace period or treat some user accounts +without existing cross-signing keys as special. + + +## Unstable prefix + +Not applicable as client behaviour need not change. + +## Dependencies + +None. From 712dd41e4f58a668c9078751819ae498abdf05ab Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 3 Jun 2024 17:10:15 +0100 Subject: [PATCH 06/29] MSC4115: membership information on events (#4115) * Proposal for membership information on events * unstable prefix * typo * Update proposals/4115-membership-on-events.md Co-authored-by: Matthew Hodgson * Update MSC4115 * Make the new property *optional*. * Describe linear-DAG situation. * Mention client-side calculation as an alternative. * spleling * Return state *after* the event This is much easier to calculate in Synapse. * Clarificatoins to scope of MSC * List of endpoints is incomplete * RFC2119 keywords * Remove non-issue potential issue * Expand on why doing it client side doesn't work * Update proposals/4115-membership-on-events.md Co-authored-by: Patrick Cloke * Update proposals/4115-membership-on-events.md * Update proposals/4115-membership-on-events.md * fix formatting --------- Co-authored-by: Matthew Hodgson Co-authored-by: Patrick Cloke --- proposals/4115-membership-on-events.md | 157 +++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 proposals/4115-membership-on-events.md diff --git a/proposals/4115-membership-on-events.md b/proposals/4115-membership-on-events.md new file mode 100644 index 00000000000..1ed8605a5c9 --- /dev/null +++ b/proposals/4115-membership-on-events.md @@ -0,0 +1,157 @@ +# MSC4115: membership metadata on events + +## Background + +Consider the following Event DAG: + +```mermaid +graph BT; + B[Bob joins]; + B-->A; + C-->A; + D-->B; + D-->C; +``` + +Bob has joined a room, but at the same time, another user has sent a message +`C`. + +Depending on the configuration of the room, Bob's server may serve the event +`C` to Bob's client. However, if the room is encrypted, Bob will not be on the +recipient list for `C` and the sender will not share the message key with Bob, +even though, in an absolute time reference, `C` may have been sent at a later +timestamp than Bob's join. + +Unfortunately, there is no way for Bob's client to reliably distinguish events +such as `A` and `C` that were sent "before" he joined (and he should therefore +not expect to decrypt) from those such as `D` that were sent later. + +(Aside: there are two parts to a complete resolution of this "forked-DAG" +problem. The first part is making sure that the *sender* of an encrypted event +has a clear idea of who was a member at the point of the event; the second part +is making sure that the *recipient* knows whether or not they were a member at +the point of the event and should therefore expect to receive keys for it. This +MSC deals only with the second part. The whole situation is discussed in more +detail at https://github.com/element-hq/element-meta/issues/2268.) + +A similar scenario can arise even in the absence of a forked DAG: clients +see events sent when the user was not in the room if the room has [History +Visibility](https://spec.matrix.org/v1.10/client-server-api/#room-history-visibility) +set to `shared`. (This is fairly common even in encrypted rooms, partly because +that is the default state for new rooms even using the `private_chat` preset +for the [`/createRoom`](https://spec.matrix.org/v1.10/client-server-api/#post_matrixclientv3createroom) +request, and also because history-sharing solutions such as +[MSC3061](https://github.com/matrix-org/matrix-spec-proposals/pull/3061) rely +on it.) + +As a partial solution to the forked-DAG problem, which will also solve the +problem of historical message visibility, we propose a mechanism for servers to +inform clients of their room membership at each event. + +## Proposal + +The `unsigned` structure contains data added to an event by a homeserver when +serving an event over the client-server API. (See +[specification](https://spec.matrix.org/v1.9/client-server-api/#definition-clientevent)). + +We propose adding a new optional property, `membership`. If returned by the +server, it MUST contain the membership of the user making the request, +according to the state of the room at the time of the event being returned. If +the user had no membership at that point (ie, they had yet to join or be +invited), `membership` is set to `leave`. Any changes caused by the event +itself (ie, if the event itself is a `m.room.member` event for the requesting +user) are *included*. + +In other words: servers MUST follow the following algorithm when populating +the `unsigned.membership` property on an event E and serving it to a user Alice: + +1. Consider the room state just *after* event E landed (accounting for E + itself, but not any other events in the DAG which are not ancestors of E). +2. Within the state, find the event M with type `m.room.member` and `state_key` + set to Alice's user ID. +3. * If no such event exists, set `membership` to `leave`. + * Otherwise, set `membership` to the value of the `membership` property of + the content of M. + +It is recommended that homeservers SHOULD populate the new property wherever +practical, but they MAY omit it if necessary (for example, if calculating the +value is expensive, servers might choose to only implement it in encrypted +rooms). Clients MUST in any case treat the new property as optional. + +For the avoidance of doubt, the new `membership` property is added to all +Client-Server API endpoints that return events, including, but not limited to, +[`/sync`](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3sync), +[`/messages`](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3roomsroomidmessages), +[`/state`](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3roomsroomidstate), +and deprecated endpoints such as +[`/events`](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3events) +and +[`/initialSync`](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3events). + + +Example event including the new property, as seen in the response to a request made by `@user:example.org`: + +```json5 +{ + "content": { + "membership": "join" + }, + "event_id": "$26RqwJMLw-yds1GAH_QxjHRC1Da9oasK0e5VLnck_45", + "origin_server_ts": 1632489532305, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@example:example.org", + "state_key": "@example:example.org", + "type": "m.room.member", + "unsigned": { + "age": 1567437, + // @user:example.org's membership at the time this event was sent + "membership": "leave", + "redacted_because": { + "content": { + "reason": "spam" + }, + "event_id": "$Nhl3rsgHMjk-DjMJANawr9HHAhLg4GcoTYrSiYYGqEE", + "origin_server_ts": 1632491098485, + "redacts": "$26RqwJMLw-yds1GAH_QxjHRC1Da9oasK0e5VLnck_45", + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@moderator:example.org", + "type": "m.room.redaction", + "unsigned": { + // @user:example.org's membership at the time the redaction was sent + "membership": "join", + "age": 1257 + } + } + } +} +``` + +## Potential issues + +None foreseen. + +## Alternatives + +1. https://github.com/element-hq/element-meta/issues/2268#issuecomment-1904069895 + proposes use of a Bloom filter — or possibly several Bloom filters — to + mitigate this problem in a more general way. It is the opinion of the author of + this MSC that there is room for both approaches. + +2. We could attempt to calculate the membership state on the client side. This + might help in a majority of cases, but it will be unreliable in the presence + of forked DAGs. It would require clients to implement the [state resolution + algorithm](https://spec.matrix.org/v1.10/rooms/v11/#state-resolution), which + would be prohibitively complicated for most clients. + +## Security considerations + +None foreseen. + +## Unstable prefix + +While this proposal is in development, the name `io.element.msc4115.membership` +MUST be used in place of `membership`. + +## Dependencies + +None. From 36c5ec47a2f0174455bb53a646ef12b4f689a2e8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 10 Jun 2024 11:11:05 +0100 Subject: [PATCH 07/29] MSC3916: Authentication for media (#3916) --- proposals/3916-authentication-for-media.md | 401 +++++++++++++++++++++ 1 file changed, 401 insertions(+) create mode 100644 proposals/3916-authentication-for-media.md diff --git a/proposals/3916-authentication-for-media.md b/proposals/3916-authentication-for-media.md new file mode 100644 index 00000000000..faf026bf2bc --- /dev/null +++ b/proposals/3916-authentication-for-media.md @@ -0,0 +1,401 @@ +# MSC3916: Authentication for media access, and new endpoint names + +Currently, access to media in Matrix has a number of problems including the following: + +* The only protection for media is the obscurity of the URL, and URLs are + easily leaked (eg accidental sharing, access + logs). [synapse#2150](https://github.com/matrix-org/synapse/issues/2150) +* Anybody (including non-matrix users) can cause a homeserver to copy media + into its local + store. [synapse#2133](https://github.com/matrix-org/synapse/issues/2133) +* When a media event is redacted, the media it used remains visible to all. + [synapse#1263](https://github.com/matrix-org/synapse/issues/1263) +* There is currently no way to delete + media. [matrix-spec#226](https://github.com/matrix-org/matrix-spec/issues/226) +* If a user requests GDPR erasure, their media remains visible to all. +* When all users leave a room, their media is not deleted from the server. + +These problems are all difficult to address currently, because access to media +is entirely unauthenticated. The first step for a solution is to require user +authentication. Once that is done, it will be possible to impose authorization +requirements to address the problems mentioned above. (See, for example, +[MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911) which +builds on top of this MSC.) + +This proposal supersedes [MSC1902](https://github.com/matrix-org/matrix-spec-proposals/pull/1902). + +## Proposal + +1. New endpoints + + The existing `/_matrix/media/v3/` endpoints become deprecated, and new + endpoints under the `/_matrix/client` and `/_matrix/federation` + hierarchies are introduced. Removal of the deprecated endpoints would be a + later MSC under [the deprecation policy](https://spec.matrix.org/v1.10/#deprecation-policy). + + The following table below shows a mapping between deprecated and new endpoint: + + | Deprecated | Client-Server | Federation | + | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------- | + | [`GET /_matrix/media/v3/preview_url`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3preview_url) | `GET /_matrix/client/v1/media/preview_url` | - | + | [`GET /_matrix/media/v3/config`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3config) | `GET /_matrix/client/v1/media/config` | - | + | [`GET /_matrix/media/v3/download/{serverName}/{mediaId}`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3downloadservernamemediaid) | `GET /_matrix/client/v1/media/download/{serverName}/{mediaId}` | `GET /_matrix/federation/v1/media/download/{mediaId}` | + | [`GET /_matrix/media/v3/download/{serverName}/{mediaId}/{fileName}`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3downloadservernamemediaidfilename) | `GET /_matrix/client/v1/media/download/{serverName}/{mediaId}/{fileName}` | - | + | [`GET /_matrix/media/v3/thumbnail/{serverName}/{mediaId}`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3thumbnailservernamemediaid) | `GET /_matrix/client/v1/media/thumbnail/{serverName}/{mediaId}` | `GET /_matrix/federation/v1/media/thumbnail/{mediaId}` | + + **Note**: [`POST /_matrix/media/v3/upload`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixmediav3upload) + and [`POST /_matrix/media/v1/create`](https://spec.matrix.org/v1.10/client-server-api/#post_matrixmediav1create) + are **not** modified or deprecated by this MSC: it is intended that they be brought into line with the other + endpoints by a future MSC, such as [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911). + +2. Removal of `allow_remote` parameter from `/download` and `/thumbnail` + + The current + [`/download`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3downloadservernamemediaid) + and + [`/thumbnail`](https://spec.matrix.org/v1.6/client-server-api/#get_matrixmediav3thumbnailservernamemediaid) + endpoints take an `allow_remote` query parameter, indicating whether the + server should request remote media from other servers. This is redundant + with the new endpoints, so will not be supported. + + Servers MUST NOT return remote media from `GET /_matrix/federation/v1/media/download` or + `GET /_matrix/federation/v1/media/thumbnail`. The `serverName` is omitted from + the endpoint's path to strongly enforce this - the `mediaId` in a request is + assumed to be scoped to the target server. + + `/_matrix/client/v1/media/download` and + `/_matrix/client/v1/media/thumbnail` return remote media as normal. + +3. Authentication on all endpoints + + Currently, the `/download` and `/thumbnail` endpoints have no authentication + requirements. Under this proposal, the new endpoints will be authenticated + the same way as other endpoints: they will require an `Authorization` header + which must be `Bearer {accessToken}` for `/_matrix/client`, or the signature + for `/_matrix/federation`. + + Clients SHOULD NOT use the [deprecated](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/4126-deprecate-query-string-auth.md) + `?access_token` query string authentication mechanism. The method is [pending removal](https://github.com/matrix-org/matrix-spec-proposals/pull/4127) + and is generally unsafe. See those MSCs for further details. + + **Note**: This fixes [matrix-spec#313](https://github.com/matrix-org/matrix-spec/issues/313). + +4. Updated response format + + * For the new `/_matrix/client` endpoints, the response format is the same as + the corresponding original endpoints. + + * To enable future expansion, for the new `/_matrix/federation` endpoints, + the response is + [`multipart/mixed`](https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html) + content with exactly two parts: the first MUST be a JSON object (and should have a + `Content-type: application/json` header), and the second MUST be the media item. + The media item may be served inline, as shown in the first example below, or + be a pointer to a URL containing the media item's bytes instead, represented + by the `Location` header described further below. + + No properties are yet specified for the JSON object to be returned. One + possible use is described by [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911). + + An example response: + + ``` + Content-Type: multipart/mixed; boundary=gc0p4Jq0M2Yt08jU534c0p + + --gc0p4Jq0M2Yt08jU534c0p + Content-Type: application/json + + {} + + --gc0p4Jq0M2Yt08jU534c0p + Content-Type: text/plain + + This media is plain text. Maybe somebody used it as a paste bin. + + --gc0p4Jq0M2Yt08jU534c0p + ``` + + The second part (media item bytes) MAY include a [`Location` header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location) + to point to the raw media object instead of having bytes itself. Servers + SHOULD NOT cache the `Location` header's value as the responding server may + have applied time limits on its validity. Servers which don't immediately + download the media from the provided URL should re-request the media and + metadata from the `/download` endpoint when ready for the media bytes. + + The `Location` header's URL does *not* require authentication, as it will + typically be served by a CDN or other non-matrix server (thus being unable + to verify any `X-Matrix` signatures, for example). + + Note that all other headers besides `Location` for the media item part are + ignored when `Location` is present. The `Content-Type`, `Content-Disposition`, + etc headers will be served from the `Location`'s URL instead. Similarly, + the body for the media item part is ignored and SHOULD be empty. + + An example response with a `Location` redirect would be: + + ``` + Content-Type: multipart/mixed; boundary=gc0p4Jq0M2Yt08jU534c0p + + --gc0p4Jq0M2Yt08jU534c0p + Content-Type: application/json + + {} + + --gc0p4Jq0M2Yt08jU534c0p + Location: https://cdn.example.org/ab/c1/2345.txt + + --gc0p4Jq0M2Yt08jU534c0p + ``` + + If the server were to `curl https://cdn.example.org/ab/c1/2345.txt`, it'd + get something similar to the following: + + ``` + Content-Type: text/plain + + This media is plain text. Maybe somebody used it as a paste bin. + ``` + + **Note**: For clarity, the above applies to the federation `/thumbnail` endpoint + as well as `/download`. + +5. Backwards compatibility mechanisms + + Servers SHOULD *stop* serving new media as unauthenticated within 1 spec release + of this proposal being released itself using a standard `404 M_NOT_FOUND` response. + Existing media should continue to be served from the unauthenticated endpoints + indefinitely for backwards compatibility. For example, if this proposal is + released in Matrix 1.11, then by Matrix 1.12 servers should freeze the old + unauthenticated endpoints by only serving media known to exist from before the + freeze. + + "New media" is any media which local users upload after the freeze is put in + place, and any remote media which becomes cached after the freeze as well. This + could be marked by a configuration option within the server software, or as part + of a scheduled/dedicated release which enacts the freeze for everyone who updates + to that version. + + This freeze schedule will have some undesirable side effects, particularly for + clients and servers which are slow to update or support the new endpoints. Newly + uploaded images, files, avatars, etc may appear "broken" or missing to users on + older software. Existing media should continue to work, however, reducing the + impact from 100% of media to a smaller percentage. + + Servers SHOULD consider whether their users' typical clients will break as + part of the freeze before enacting the freeze. Clients SHOULD update as soon + as reasonably possible to support authenticated media, particularly following + the spec release containing this MSC. Other considerations may include bridges, + deployment-specific use cases, and patch availability. + + It is worth noting that the matrix.org homeserver plans to freeze media relatively + quickly following this proposal's release in the specification. Details will be + published to the matrix.org blog closer to the spec release date. + + The following are specific backwards compatibility cases: + + a. New clients & servers with older servers: The [`M_UNRECOGNIZED`](https://spec.matrix.org/v1.10/client-server-api/#common-error-codes) + error behaviour should be followed to indicate that the server does not + support the new endpoints, particularly when partnered with a 404 HTTP status + code. Clients and servers should use the unauthenticated endpoints in this + case. The endpoints will not be frozen by the server, so should work for + 'new' media. + + b. Older clients & servers with newer servers: Mentioned above, servers are + strongly encouraged to freeze unauthenticated media access within a relatively + quick timeframe. Though media before the freeze should remain accessible, + clients and older federating servers may still see errors when accessing + new media, leading to client UI feeling "broken" or missing avatars. The + various considerations above are meant to reduce the impact of this case. + +6. Removal of `allow_redirect` parameter from `/download` and `/thumbnail` + + Clients MUST expect a 307 or 308 redirect when calling the new `/download` + and `/thumbnail` Client-Server API endpoints. + + Servers MUST expect the `Location` header in the media part of the new Server-Server + API `/download` and `/thumbnail` endpoints. Servers MUST NOT respond with a 307 or 308 redirect at + the top level for the endpoint - they can only redirect within the media part + itself. + + See [this comment on MSC3860](https://github.com/matrix-org/matrix-spec-proposals/pull/3860/files#r1005176480) + for further context on this change. + +### Effects on client applications + +Naturally, implementations will be required to provide `Authorization` headers +when accessing the new endpoints. This will be simple in some cases, but rather +more involved in others. This section considers some of those cases. + +#### IRC/XMPP bridges + +Possibly the largest impact will be on IRC and XMPP bridges. Since IRC and +XMPP have no media repository of their own, these bridges currently transform +`mxc:` URIs into `https:///_matrix/media/v3/download/` URIs and forward +those links to the remote platform. This will no longer be a viable option. + +One potential solution is for the bridges to provide a proxy. + +In this scenario, the bridge would have a secret HMAC key. When it +receives a matrix event referencing a piece of media, it should create a new URI +referencing the media, include an HMAC to prevent tampering. For example: + +``` +https:///media/{originServerName}/{mediaId}?mac={hmac} +``` + +When the bridge later receives a request to that URI, it checks the hmac, +and proxies the request to the homeserver, using its AS access +token in the `Authorization` header. + +The bridge might also choose to embed information such as the room that +referenced the media, and the time that the link was generated, in the URL. +Such mechanisms would allow the bridge to impose controls such as: + +* Limiting the time a media link is valid for. Doing so would help prevent + visibility to users who weren't participating in the chat. + +* Rate-limiting the amount of media being shared in a particular room (in other + words, avoiding the use of Matrix as a Warez distribution system). + +#### Icons for "social login" flows + +When a server supports multiple login providers, it provides the client with +icons for the login providers as `mxc:` media URIs. These must be accessible +without authentication (because the client has no access token at the time the +icons are displayed). + +Servers MAY flag the media as exceptions to the freeze described in part 5 of +this proposal ("Backwards compatibility mechanisms"). Clients SHOULD continue to +use the unauthenticated media download/thumbnail endpoints to access these icons +until a future MSC can improve the situation. + +The proposal author expects that the spec's transition to OIDC will ultimately +replace this mechanism, or that an MSC could be opened to allow HTTP(S) URLs in +place of `mxc:` URIs. + +(This was previously discussed in +[MSC2858](https://github.com/matrix-org/matrix-spec-proposals/pull/2858#discussion_r543513811).) + +## Potential issues + +* Setting the `Authorization` header is particularly annoying for web clients. + Service workers are seemingly the best option, though other options include + locally-cached `blob:` URIs. Clients should note that caching media can lead + to significant memory usage, particularly for large media. Service workers by + comparison allow for proxy-like behaviour. + + Cookies are a plausible mechanism for sharing session information between + requests without having to set headers, though would be a relatively bespoke + authentication method for Matrix. Additionally, many Matrix users have cookies + disabled due to the advertising and tracking use cases common across the web. + +* Users will be unable to copy links to media from web clients to share out of + band. This is considered a feature, not a bug. + +* Over federation, the use of the `Range` request header on the federation endpoints becomes + unclear as it could affect either or both parts of the response. There does not + appear to be formal guidance in [RFC 9110](https://www.rfc-editor.org/rfc/rfc9110#field.range) + either. There are arguments for affecting both and either part equally. Typically, + such a header would be used to resume failed downloads, though servers are + already likely to discard received data and fail the associated client requests + when the federation request fails. Therefore, servers are unlikely to use `Range` + at all. As such, this proposal does not make a determination on how `Range` + should be handled, and leaves it as an HTTP specification interpretation problem + instead. + +* The `Location` header support on the new federation endpoints could add a bit + of complexity to servers, though given the alternative of supporting CDNs and + similar is to place complexity into "edge workers" to mutate the response value. + Though the Matrix spec would be "simpler", the edge worker setup would be + fragmented where we have an opportunity for a common standard. + +## Alternatives + +* Allow clients to upload media which does not require authentication (for + example via a `public=true` query parameter). This might be particularly + useful for IRC/XMPP bridges, which could upload any media they encounter to + the homeserver's repository. + + The danger with this is that is that there's little stopping clients + continuing to upload media as "public", negating all of the benefits in this + MSC. It might be ok if media upload it was restricted to certain privileged + users, or applied after the fact by a server administrator. + +* We could simply require that `Authorization` headers be given when calling + the existing endpoints. However, doing so would make it harder to evaluate + the proportion of clients which have been updated, and it is a good + opportunity to bring these endpoints into line with the rest of the + client-server and federation APIs. + +* There's no real need to rename `GET /_matrix/media/v3/preview_url` and `GET + /_matrix/media/v3/config` at present, and we could just leave them in + place. However, changing them at the same time makes the API more consistent. + + Conversely, we should make sure to rename `POST /_matrix/media/v3/upload` + and `GET /_matrix/media/v3/create`. The reason to + delay doing so is because MSC3911 will make more substantial changes to these + endpoints, requiring another rename, and it is expected that both proposals + will be merged near to the same time as each other (so a double rename will + be confusing and unnecessary). However, if MSC3911 is delayed or rejected, we + should reconsider this. + +* Rather than messing with multipart content, have a separate endpoint for + servers to get the metadata for a media item. That would mean two requests, + but might make more sense than the federation endpoints providing the info directly. + + This is a plausible approach with no significant upsides or downsides when + compared to multipart responses. + + Similarly, custom headers could be used to carry the metadata on the response, + though again, there are no significant upsides or downsides to doing so. + + Readers may wish to refer to [this thread](https://github.com/matrix-org/matrix-spec-proposals/pull/3916/files#r1586878787) + on the MSC which covers the majority of the pros and cons for all 3 approaches. + +### Compared to MSC3796 (MSC701) + +[MSC701/3796](https://github.com/matrix-org/matrix-spec-proposals/issues/3796) +introduces a concept of "content tokens" which have authentication tie-in to +prevent anonymous users from accessing media. This is a similar problem space +to this proposal, though deals more in the event-to-media linking space instead. +Although the MSC is an early sketch, it's unclear if the problems noted on the +MSC itself are feasibly resolvable. + +### Compared to MSC2461 + +[MSC2461](https://github.com/matrix-org/matrix-spec-proposals/pull/2461) adds +authentication to the existing media endpoints, which as noted here in the +Alternatives is not likely to roll out quickly and leaves an inconsistency in +the spec. MSC2461 also introduces a client-visible flag for which kinds of media +may require authentication, making it similar to the alternative listed above +where on the federation side we could have two endpoints: one for information +and one for the media itself. MSC2461 simply makes the information client-visible +instead of server-visible. + +## Unstable prefix + +While this proposal is in development, the new endpoints should be named as follows: + +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/preview_url` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/config` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/{serverName}/{mediaId}` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/{serverName}/{mediaId}/{fileName}` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/{serverName}/{mediaId}` +* `GET /_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/{mediaId}` + * **Note**: This endpoint has a `.v2` in its unstable identifier due to the MSC changing after + initial implementation. The original unstable endpoint has a `serverName` and may still be + supported by some servers: `GET /_matrix/federation/unstable/org.matrix.msc3916/media/download/{serverName}/{mediaId}` + + The `serverName` was later dropped in favour of explicit scoping. See `allow_remote` details + in the MSC body for details. +* `GET /_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/{mediaId}` + * **Note**: This endpoint has a `.v2` in its unstable identifier due to the MSC changing after + initial implementation. The original unstable endpoint has a `serverName` and may still be + supported by some servers: `GET /_matrix/federation/unstable/org.matrix.msc3916/media/thumbnail/{serverName}/{mediaId}` + + The `serverName` was later dropped in favour of explicit scoping. See `allow_remote` details + in the MSC body for details. + +## Dependencies + +None. From 9acf41a92b9da3dd49187d78aff5b62596660b05 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 24 Jun 2024 11:07:41 -0600 Subject: [PATCH 08/29] MSC4151: Reporting rooms (Client-Server API) (#4151) * MSC: Reporting rooms (Client-Server API) * Add legal notes, and limit scope creep on those notes * Mention MSC3840 * Mention information disclosure. * Update proposals/4151-report-room.md Co-authored-by: Quentin Gliech --------- Co-authored-by: Quentin Gliech --- proposals/4151-report-room.md | 117 ++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 proposals/4151-report-room.md diff --git a/proposals/4151-report-room.md b/proposals/4151-report-room.md new file mode 100644 index 00000000000..0f5300eb4ce --- /dev/null +++ b/proposals/4151-report-room.md @@ -0,0 +1,117 @@ +# MSC4151: Reporting rooms (Client-Server API) + +The specification [already contains](https://spec.matrix.org/v1.10/client-server-api/#reporting-content) +a module for being able to report events, though this functionality does not extend to rooms. Being +able to report rooms is important for user safety: clients have room directories, invite lists, +links to rooms, etc which all don't have an event ID to reference. If the client has *any* event ID +for the room, it can use the existing 'report event' API to report the room instead. However, this +only works if the user has visibility on the event ID being reported too. + +These constraints are in addition to the legal obligations of clients to provide a safe user experience. +In some countries, such as the UK, it is required that users be able to report *any* kind of content +they see, and some app stores require similar reporting functionality for mobile apps. These obligations +impose further obligations not discussed in this proposal. For example, actually handling the reports +and informing the reporter how long it will take to process their request. These obligations are +expected to be discussed in a future, larger, MSC series which revamps reporting in Matrix. + +This proposal introduces an endpoint for reporting rooms, expanding the capabilities of the reporting +module. The scope of this proposal is intentionally narrow to ensure quick traversal of the MSC process. +Other, future, MSCs may further expand the suite of endpoints available to clients (like reporting +users, media, etc). + +## Proposal + +Taking inspiration from [`POST /rooms/:roomId/report/:eventId`](https://spec.matrix.org/v1.10/client-server-api/#post_matrixclientv3roomsroomidreporteventid), +a new endpoint is introduced: + +``` +POST /_matrix/client/v3/rooms/:roomId/report +{ + "reason": "" +} +``` + +`reason` is a human-readable string describing the reason for the report. The string may be blank, +but *must* be provided (to align with `/report/:eventId`). + +**Note**: `score` is not carried over from `/report/:eventId` because it has not proven useful. A +future MSC may introduce it. + +There are no restictions on who can report a room: knowing the room ID is sufficient. This is to +ensure that results from the room directory, invites, links, etc can all be reported. If the room +does not exist on the server, the endpoint returns `404 M_NOT_FOUND`. Otherwise, `200` with `{}` as +a response body. + +Like `/report/:eventId`, handling of the report is left as a deliberate implementation detail. + +## Safety considerations + +* Server admins may be exposed to harmful content through `reason`. This is an existing issue with + the reporting module, and difficult to fix. Applications which expose report reasons of any kind + are encouraged to place disclosures in the user experience path. For example, a page explaining + that the tool may contain harmful content before allowing the user temporary access, or the use of + spoiler tags on report reasons/content. + +* Clients should hide rooms the user reports by default to both discourage duplicate reports and to + remove the harmful content from the user's view, where possible. This may require filtering room + directory responses and room lists for the user, or an "ignore room API" like [MSC3840](https://github.com/matrix-org/matrix-doc/pull/3840). + + If the user is joined to a room, the client may wish to offer the user an option to leave the room. + +* Users may report whole rooms instead of events in that room, particularly during a harmful content + spam wave. Administrators and safety teams should be cautious to avoid shutting down or banning + whole rooms, as the room may be legitimate otherwise. Automated decision making is not suggested + for a similar reason. + +* 'Report flooding' is more easily possible with this new endpoint, where many users report a room + with the hope of getting it shut down/banned. Mentioned a few times in this proposal, automated + decision making is not recommended for this endpoint to prevent consequences like this from + happening. + +## Potential issues + +* Within the Trust & Safety environment, it is well known that `reason` alone is insufficient for an + informed report. Self-triage categories and mandatory `reason` for some of those categories help + improve a safety team's ability to handle a report. These features are not included in this proposal + as they require further thought and consideration - a future MSC may expand (or even deprecate) the + report endpoints to support this added information. + +* Reports are not federated. This is considered an issue for another MSC, like [MSC3843](https://github.com/matrix-org/matrix-spec-proposals/pull/3843). + +* Whether the local server is participating in a room is revealed through the new endpoint. The endpoint + is only available to local users however, and other ways of finding out the same information may + already be possible in Matrix (not verified). It is not immediately clear that disclosing this + information to local clients would cause harm to the server or its users. A future reporting over + federation proposal may wish to consider hiding the server's participation state, however. + +## Alternatives + +* Mentioned in the introduction, if a client has an event ID for something in the room, it can typically + use the existing event report endpoint to report the room. For example, using the creation event, + the user's own join event, or the most recent message in the room. This only works if the user is + able to see that event in the room, and further only if the client even has an event ID. Areas of + the client like the room directory do not expose an event ID the client could use. If they did, the + user may not have sufficient visibility on the event to be able to report it. + +* The event report API could be relaxed to support an empty string for the event ID, though this feels + effectively like a new endpoint anyways. This MSC introduces such an endpoint. + +* The event report API's history visibility check could also be removed, though, as per + [MSC2249](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2249-report-require-joined.md), + this is intentional behaviour. + +## Security considerations + +* Rate limiting is strongly recommended for this new endpoint. + +* Authentication is required for this new endpoint. + +## Unstable prefix + +While this proposal is not considered stable, implementations should use `/_matrix/client/unstable/org.matrix.msc4151/rooms/:roomId/report` +instead. Clients should note the [`M_UNRECOGNIZED` behaviour](https://spec.matrix.org/v1.10/client-server-api/#common-error-codes) +for servers which do not support the (un)stable endpoint. + +## Dependencies + +This MSC has no direct dependencies. From d04470eca4546303adfc9779371ce1523276ee4e Mon Sep 17 00:00:00 2001 From: Marcus Date: Mon, 24 Jun 2024 19:08:48 +0200 Subject: [PATCH 09/29] MSC2867: Marking rooms as unread (#2867) * marking rooms as unread Signed-off-by: Marcus Hoffmann * Add alternative suggestion by @turt2live Co-authored-by: Travis Ralston * mention that this msc lacks thread support * typo fix * clarify type of notification we are referring to * fix some small typos * Update proposals/2867-rooms_marked_unread.md grammar improvement Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Signed-off-by: Marcus Hoffmann Co-authored-by: Travis Ralston Co-authored-by: Andrew Morgan Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/2867-rooms_marked_unread.md | 81 +++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 proposals/2867-rooms_marked_unread.md diff --git a/proposals/2867-rooms_marked_unread.md b/proposals/2867-rooms_marked_unread.md new file mode 100644 index 00000000000..d58079c7c37 --- /dev/null +++ b/proposals/2867-rooms_marked_unread.md @@ -0,0 +1,81 @@ +# MSC2867: Marking rooms as unread + +There is currently no way to mark a room for later attention. A common use-case is receiving a +notification on the go and opening the corresponding room but then deciding to deal with it at a later time. + +This MSC introduces an option to manually mark an room as *Unread*. + +In the above use-case the user would just mark the room as unread and when later opening a matrix +client on their desktop PC that room would appear prominently in their room list waiting for attention. + +A related use-case solved by this proposal is wanting to mute a room's notifications while there's an +ongoing discussion but still flag it for catching up later. + +Both WhatsApp and Telegram offer this functionality. + +## Proposal + +We add a [room account data](https://matrix.org/docs/spec/client_server/r0.6.1#put-matrix-client-r0-user-userid-rooms-roomid-account-data-type) +field `m.marked_unread` which just stores the following: + +```json +{ + "unread": true | false +} +``` + +When this is true a client should show the room with an indeterminate unread marker. This marker should +be of similar visual importance to a non-highlight notification badge without the notification count. For example if +you have a red circle with small numbers inside for counting notifications next to a room, then a room +without notifications but marked as unread should have just the red circle. If a client gets new +notifications after marking a room as unread the notification count should be displayed instead as normal. + +The `m.fully_read` marker should not be touched when marking a room as unread to keep the users read position +in the room. + +Marking a room as read, if a client implements such a functionality, now involves sending a read receipt for the last +event, as well as setting `m.marked_unread` to false. + +The unread flag should be cleared when opening the room again. + +## Potential issues + +Client not aware of this feature will not clear the unread flag, when reading the room. In addition they'll obviously +not show the room with a special badge. This seems preferable to the alternative of clearing the unread flag of a room +without intending to because it didn't actually show up as unread. + +This proposal has no support for marking threads as unread. This should be handled in a future MSC. + +## Alternatives + +There are multiple possible alternatives here: + +* Marking individual messages as unread as discussed [here](https://github.com/matrix-org/matrix-doc/issues/2506): + This is a far more complex feature that has possible interactions with server-side implementations and notification + counts. This proposal aims to be a far more lightweight alternative. Looking at other messengers marking a room as + unread is a more common operation than marking messages as unread, so it could be argued that others already found + this to strike a good balance of complexity and use-cases covered. +* Modifying the `m.fully_read` marker instead of introducing a new `m.marked_unread` field: + Another idea was setting the `m.fully_read` marker to some earlier event which would then cause clients to show + unread messages again. This has two problems: + * It makes it harder to distinguish between rooms which happen to have unread messages that you don't necessarily + care about and rooms which were manually marked as such and thus should be shown much more prominently. + * When trying to work around this, by setting the marker at a special location like the room creation event, we completely + lose the user's actual read position in a room whenever they use this feature. +* Read receipts could be allowed to go "backwards" to restore notification + counts, though this is likely to be riddled with bugs and unexpected + behaviour for users. It's better for everyone to keep read receipts linearly + progressing forwards. + +## Security considerations + +None. + +## Unstable prefix + +While this feature is not yet fully specced, implementers can use the `com.famedly.marked_unread` room +account data type. + +Implementations using this unstable prefix in a released version should automatically migrate +user's unread rooms to `m.marked_unread` when this is released as stable. +This ensures user's unread rooms are not lost. From f2c6766e095581b63aa909568329cbd981915598 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Sun, 7 Jul 2024 20:40:12 +0300 Subject: [PATCH 10/29] Clarify "real name" in contributor requirements (#4160) This updates the sign-off requirements to match what most other matrix-org and element-hq repos already have. The change was first made in synapse: https://github.com/matrix-org/synapse/pull/3467 Signed-off-by: Tulir Asokan --- CONTRIBUTING.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d5fba857ab..e51f8ab382f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,10 +69,14 @@ include the line in your commit or pull request comment: Signed-off-by: Your Name -...using your real name; unfortunately pseudonyms and anonymous contributions -can't be accepted. Git makes this trivial - just use the -s flag when you do -``git commit``, having first set ``user.name`` and ``user.email`` git configs -(which you should have done anyway :) +We accept contributions under a legally identifiable name, such as +your name on government documentation or common-law names (names +claimed by legitimate usage or repute). Unfortunately, we cannot +accept anonymous contributions at this time. + +Git allows you to add this signoff automatically when using the `-s` +flag to `git commit`, which uses the name and email set in your +`user.name` and `user.email` git configs. ### Private sign off From bbe54bc1c22f17a7b0c9d9f99a4c0c89e0ded3a8 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 8 Jul 2024 16:49:04 +0200 Subject: [PATCH 11/29] MSC4159: Remove the deprecated name attribute on HTML anchor elements (#4159) Signed-off-by: Johannes Marbach --- proposals/4159-remove-anchor-name.md | 73 ++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 proposals/4159-remove-anchor-name.md diff --git a/proposals/4159-remove-anchor-name.md b/proposals/4159-remove-anchor-name.md new file mode 100644 index 00000000000..2442ead67e2 --- /dev/null +++ b/proposals/4159-remove-anchor-name.md @@ -0,0 +1,73 @@ +# MSC4159: Remove the deprecated name attribute on HTML anchor elements + +Some message types in `m.room.message`, such as `m.text`, permit including HTML in the event content. +The spec [recommends] that clients limit the HTML they render to prevent attacks and provides a list +of permitted HTML tags and attributes. In particular, it allows using the `name` attribute on `a` tags. +This attribute is obsolete according to the [WHATWG HTML Living Standard] which is why this proposal +attempts to remove it from the spec. + + +## Proposal + +The `name` attribute was originally introduced to define targets for linking to specific parts of a +webpage. As an example, including the named anchor `bar` on a site allows you to append +the fragment `#foo` to the URL to cause your browser to scroll the anchor into view after loading the page. + +In modern versions of HTML this feature has been superseded by the `id` attribute which extends targeted +linking to more than just `a` tags. As a result, the `name` attribute is marked deprecated in [MDN]. + +> Was required to define a possible target location in a page. In HTML 4.01, `id` and `name` could +> both be used on ``, as long as they had identical values. +> +> Note: Use the global attribute `id` instead. + +Furthermore, it is also tracked as [obsolete but conforming] in WHATWG. + +> Authors should not specify the `name` attribute on `a` elements. If the attribute is present, its value +> must not be the empty string and must neither be equal to the value of any of the IDs in the element's +> tree other than the element's own ID, if any, nor be equal to the value of any of the other `name` +> attributes on `a` elements in the element's tree. If this attribute is present and the element has an ID, +> then the attribute's value must be equal to the element's ID. In earlier versions of the language, this +> attribute was intended as a way to specify possible targets for fragments in URLs. The `id` attribute +> should be used instead. + +On top of the deprecation of the `name` attribute in HTML, it is unclear what this feature would ever have +been used for in the context of Matrix. It appears highly undesirable to let events define targeted links +into a client's UI, not least because the value of the `name` attribute would need to be unique on the +entire page. Additionally, linking to specific events is already possible via [matrix.to URIs]. + +Therefore, the `name` attributed is removed from the list of permitted attributes on `a` tags without a +replacement. + + +## Potential issues + +Use cases that currently depend on the `name` attribute will be broken once the attribute is removed from +the allowed list. No concrete use cases are known as of writing, however. + + +## Alternatives + +None. + + +## Security considerations + +None. + + +## Unstable prefix + +None. + + +## Dependencies + +None. + + +[MDN]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#name +[WHATWG HTML Living Standard]: https://html.spec.whatwg.org/ +[matrix.to URIs]: https://spec.matrix.org/v1.10/appendices/#matrixto-navigation +[obsolete but conforming]: https://html.spec.whatwg.org/#obsolete-but-conforming-features +[recommends]: https://spec.matrix.org/v1.10/client-server-api/#mroommessage-msgtypes From f07c4508a8461fa5d25189dfc45ed9f3b4f8e7df Mon Sep 17 00:00:00 2001 From: Josh Simmons Date: Thu, 25 Jul 2024 15:21:02 -0700 Subject: [PATCH 12/29] drop real/legal name requirement from DCO (#4172) Co-authored-by: Josh Simmons --- CONTRIBUTING.md | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e51f8ab382f..247cf0c36ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,23 +69,8 @@ include the line in your commit or pull request comment: Signed-off-by: Your Name -We accept contributions under a legally identifiable name, such as -your name on government documentation or common-law names (names -claimed by legitimate usage or repute). Unfortunately, we cannot -accept anonymous contributions at this time. - Git allows you to add this signoff automatically when using the `-s` flag to `git commit`, which uses the name and email set in your `user.name` and `user.email` git configs. -### Private sign off - -If you would like to provide your legal name privately to the Matrix.org -Foundation (instead of in a public commit or comment), you can do so by emailing -your legal name and a link to the pull request to dco@matrix.org. It helps to -include "sign off" or similar in the subject line. You will then be instructed -further. - -Once private sign off is complete, doing so for future contributions will not be required. - [1]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request From cf4cdf26634beeb279b0c5de9c555b13a18298e6 Mon Sep 17 00:00:00 2001 From: Matthias Ahouansou Date: Mon, 5 Aug 2024 08:34:53 +0000 Subject: [PATCH 13/29] MSC4163: Make ACLs apply to EDUs (#4163) --- proposals/4163-make-acls-apply-to-edus.md | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 proposals/4163-make-acls-apply-to-edus.md diff --git a/proposals/4163-make-acls-apply-to-edus.md b/proposals/4163-make-acls-apply-to-edus.md new file mode 100644 index 00000000000..48c72d5600c --- /dev/null +++ b/proposals/4163-make-acls-apply-to-edus.md @@ -0,0 +1,46 @@ +# MSC4163: Make ACLs apply to EDUs + +[Access Control Lists](https://spec.matrix.org/v1.11/client-server-api/#server-access-control-lists-acls-for-rooms) +(also known as ACLs) are used to prevent other servers from participating in a room at a federation level, +covering many federation API endpoints, including +[`/send`](https://spec.matrix.org/v1.11/server-server-api/#put_matrixfederationv1sendtxnid). However, while ACLs +are applied on a per-PDU basis on this endpoint, they are not applied to EDUs at all. Considering that some EDUs +are specific to certain rooms (e.g. read receipts & typing indicators), it makes sense to apply ACLs to them as well. + + +## Proposal + +All EDUs which are local to a specific room MUST have ACLs applied to them. This means that for the EDUs currently +in the spec, ACLs would only apply to receipts and typing notifications. Examples of how ACLs should be enforced +at the point of receiving a transaction for those two types of EDUs are as follows: + - For +[typing notifications (`m.typing`)](https://spec.matrix.org/v1.11/server-server-api/#typing-notifications), +the `room_id` field inside `content` should be checked, with the typing notification ignored if the `origin` +of the request is a server which is forbidden by the room's ACL. Ignoring the typing notification means that the EDU +MUST be dropped upon receipt. + - For [read receipts (`m.receipt`)](https://spec.matrix.org/v1.11/server-server-api/#receipts), all receipts +inside a `room_id` inside `content` should be ignored if the `origin` of the request is forbidden by the +room's ACL. + +## Potential issues + +None considered. + +## Alternatives + +Leave things as-is, which wouldn't be that big of a deal when you consider that this would only apply +to typing notifications and read receipts currently, which don't allow for very significant disruption inside +a room. However, as ACLs are meant to prevent certain servers from participating in a room at all, it makes +sense to apply ACLs to EDUs which are local to certain rooms, as they are a form of participation. + +## Security considerations + +None considered. + +## Unstable prefix + +None required, as no new fields or endpoints are added. + +## Dependencies + +None. From 4251928aed58f60c2211a1aa9322f6d680989282 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 19 Aug 2024 13:33:47 +0200 Subject: [PATCH 14/29] MSC4156: Migrate server_name to via (#4156) --- proposals/4156-server-name-to-via.md | 64 ++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 proposals/4156-server-name-to-via.md diff --git a/proposals/4156-server-name-to-via.md b/proposals/4156-server-name-to-via.md new file mode 100644 index 00000000000..d8719de33de --- /dev/null +++ b/proposals/4156-server-name-to-via.md @@ -0,0 +1,64 @@ +# MSC4156: Migrate `server_name` to `via` + +Room IDs in Matrix are generally not routable on their own. In the [room ID grammar] `!opaque_id:domain`, +the `domain` is the server that created the room. There is, however, no guarantee that this server is +still joined to the room at a later time. Therefore, room IDs don't provide a reliable resident server +to send requests to. Critically, the `domain` is not to be used as a routing server. It is purely a namespace. + +The spec partially addresses this issue by defining a [`via`] query parameter on room URIs that can be +used to list servers that have a high probability of being in the room in the distant future. Additionally, +some APIs such as [`/_matrix/client/v3/join/{roomIdOrAlias}`] can take a `server_name` query parameter +that has the same effect as `via`. + +The terminology difference between `server_name` and `via` can be slightly confusing which is why this +proposal attempts to standardize on `via`. + + +## Proposal + +The `server_name` query parameter on [`/_matrix/client/v3/join/{roomIdOrAlias}`] and +[`/_matrix/client/v3/knock/{roomIdOrAlias}`] is deprecated and a new parameter `via: [string]` is +introduced. + +Clients SHOULD use `via` when the homeserver they're talking to supports it. To do this, they MAY either +detect server support through the supported spec versions in [`/_matrix/client/versions`] or always include +both parameters (with identical values). + +Homeservers MUST ignore all `server_name` parameters if any `via` parameters are supplied. + + +## Potential issues + +As with any migration, some effort will be required to update client and server implementations. Additionally, +while the transitions isn't completed, the concurrent existence of both query parameters might lead to further +confusion. + + +## Alternatives + +None other than accepting status quo. + + +## Security considerations + +A client that supplies different `via` and `server_name` parameters could be served a different room depending +on which set of parameters the server uses to resolve the room ID. Tricking a client into doing this seems very +difficult though because [Matrix URIs], for instance, only have a single documented `via` parameter. + + +## Unstable prefix + +Until this proposal is accepted into the spec, implementations SHOULD refer to `via` as `org.matrix.msc4156.via`. + + +## Dependencies + +None. + + +[Matrix URIs]: https://spec.matrix.org/v1.11/appendices/#matrix-uri-scheme +[room ID grammar]: https://spec.matrix.org/v1.10/appendices/#room-ids +[`via`]: https://spec.matrix.org/v1.10/appendices/#routing +[`/_matrix/client/v3/join/{roomIdOrAlias}`]: https://spec.matrix.org/v1.10/client-server-api/#post_matrixclientv3joinroomidoralias +[`/_matrix/client/v3/knock/{roomIdOrAlias}`]: https://spec.matrix.org/v1.10/client-server-api/#post_matrixclientv3knockroomidoralias +[`/_matrix/client/versions`]: https://spec.matrix.org/v1.10/client-server-api/#get_matrixclientversions From 3b7108535b0475d98e25f07bf9ff7d1c5ecb0d42 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Wed, 28 Aug 2024 11:05:26 +0000 Subject: [PATCH 15/29] Add a stable flag to MSC3916 (#4180) In conversation with several client and homeserver developers it has come up that the current schedule for rolling out authenticated media is hard for them to follow if they have to support all of 1.11 at once. I think the value of authenticated media is high enough to warrant a stable flag to encourage a faster rollout of authenticated media. This allows servers to support authenticated media without supporting all of 1.11 already and the additional burden on client developers is minimal. This flag is already in use by serveral implementations: - https://github.com/neilalexander/harmony/commit/51b5c9803344fa1f33832cd6ccacd37db383ec84 - https://github.com/famedly/matrix-dart-sdk/commit/2dce08bab1006abe27d1931f4ba74a7fb693873c Signed-off-by: Nicolas Werner --- proposals/3916-authentication-for-media.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/proposals/3916-authentication-for-media.md b/proposals/3916-authentication-for-media.md index faf026bf2bc..de3a784a9da 100644 --- a/proposals/3916-authentication-for-media.md +++ b/proposals/3916-authentication-for-media.md @@ -396,6 +396,17 @@ While this proposal is in development, the new endpoints should be named as foll The `serverName` was later dropped in favour of explicit scoping. See `allow_remote` details in the MSC body for details. +## Stable flag + +After the proposal is accepted servers may advertise support for the stable +endpoints by setting `org.matrix.msc3916.stable` to `true` in the +`unstable_features` section of the +[versions endpoint](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientversions) +in addition to the usual version-based feature support. This option is provided +to encourage a faster rollout in the wider Matrix ecosystem until servers +support the full feature set of the respective version of the Matrix +specification. + ## Dependencies None. From abaaaee1c506f5be41e967378d577c96dbf97d83 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 2 Sep 2024 10:41:40 -0600 Subject: [PATCH 16/29] MSC4138: Update allowed HTTP methods in CORS responses (#4138) --- proposals/4138-update-cors-methods.md | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 proposals/4138-update-cors-methods.md diff --git a/proposals/4138-update-cors-methods.md b/proposals/4138-update-cors-methods.md new file mode 100644 index 00000000000..ecadc187abb --- /dev/null +++ b/proposals/4138-update-cors-methods.md @@ -0,0 +1,47 @@ +# MSC4138: Update allowed HTTP methods in CORS responses + +The [specification](https://spec.matrix.org/v1.10/client-server-api/#web-browser-clients) suggests +that servers allow a limited subset of the available [HTTP methods](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) +available in [CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) responses. However, it's +reasonable to expect the specification to use other methods in the future or as part of feature +detection. To permit these use cases early, this MSC proposes adding a few more allowable values to +the `Access-Control-Allow-Methods` header. + +## Proposal + +The [`Access-Control-Allow-Methods` header's recommended value](https://spec.matrix.org/v1.10/client-server-api/#web-browser-clients) +is updated to include the following: + +* `PATCH` - A plausibly useful HTTP method for future use. +* `HEAD` - Similar to `PATCH`, `HEAD` is plausibly useful for feature detection and cases like + [MSC4120](https://github.com/matrix-org/matrix-spec-proposals/pull/4120). + +The following methods are *not* included because they don't have foreseeable use in Matrix: + +* `CONNECT` +* `TRACE` + +## Potential issues + +None anticipated. + +## Alternatives + +No significant alternatives. + +## Security considerations + +CORS is meant to help ensure requests made by the client are properly scoped in the client. If the +client wishes to use an HTTP method not allowed by the server, the web browser will mask the +response with an error before the application can inspect it. Therefore, to increase future +compatibility, we append a few useful HTTP methods while still excluding ones which are (currently) +nonsensical. + +## Unstable prefix + +This proposal cannot have an unstable prefix due to the nature of CORS. Servers are already able to +go off-spec and serve different headers because the spec is merely a recommendation. + +## Dependencies + +This proposal has no dependencies. From 41495d235524fb8a7c5513a565b3fe479d848bd9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 3 Sep 2024 11:29:30 +0100 Subject: [PATCH 17/29] MSC4178: Additional Error Codes for requestToken endpoint (#4178) --- .../4178-threepid-medium-not-supported.md | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 proposals/4178-threepid-medium-not-supported.md diff --git a/proposals/4178-threepid-medium-not-supported.md b/proposals/4178-threepid-medium-not-supported.md new file mode 100644 index 00000000000..6496b5dadb7 --- /dev/null +++ b/proposals/4178-threepid-medium-not-supported.md @@ -0,0 +1,46 @@ +# MSC4178: Error codes for requestToken + +There are a number of ways that sending a token to validate a third party identifier can go wrong. +The requestToken API, however, has a very limited number of error codes that it can return. + +Firstly, homeservers may not always support adding email addresses or phone numbers to a user's account, +however, there is no error code to signal this situation. Synapse currently returns `M_UNKNOWN` +which leads to bad, untranslatable error messages. + +Secondly, the supplied third party identifier may be invalid. + +## Proposal + +Firstly, Add the `M_THREEPID_MEDIUM_NOT_SUPPORTED` code to be returned by both +[`POST /account/3pid/email/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidemailrequesttoken) +and +[`POST /account/3pid/msisdn/requestToken`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3account3pidmsisdnrequesttoken), +defined to mean that the homeserver does not support adding a third party identifier of that medium. + +Secondly, allow these endpoints to also return `M_INVALID_PARAM`, to indicate that the third party address +was not valid for that medium (eg. not a valid phone number). + +For both of these codes, HTTP status code 400 should be used. + +## Potential issues + +None foreseen. + +## Alternatives + +A better UX would be for servers to advertise what third party identifiers they support adding so that clients can +inform users before they try to do so. This should be in addition rather than as alternative though: the clearest +possible API will come from having both. + +## Security considerations + +None foreseen. + +## Unstable prefix + +This is sufficiently simple that proving it on a large scale is unnecessary. The code should not be used in the open +before the MSC has been accepted. + +## Dependencies + +None From f633d3006e7bb966a2bdbdb89bdf9c580adc8ce3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 4 Sep 2024 12:49:51 -0600 Subject: [PATCH 18/29] Update spec text for CORS MSC (#4187) --- proposals/4138-update-cors-methods.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proposals/4138-update-cors-methods.md b/proposals/4138-update-cors-methods.md index ecadc187abb..fedcb762449 100644 --- a/proposals/4138-update-cors-methods.md +++ b/proposals/4138-update-cors-methods.md @@ -10,7 +10,11 @@ the `Access-Control-Allow-Methods` header. ## Proposal The [`Access-Control-Allow-Methods` header's recommended value](https://spec.matrix.org/v1.10/client-server-api/#web-browser-clients) -is updated to include the following: +is updated to note that the HTTP methods described cover existing specified endpoints. Servers which +support additional endpoints or methods should add those methods as well. The specification will be +updated whenever a new method is supported by an endpoint. + +Examples of possible future-use methods include: * `PATCH` - A plausibly useful HTTP method for future use. * `HEAD` - Similar to `PATCH`, `HEAD` is plausibly useful for feature detection and cases like From 27bc9a50e94bc5781523246048630b6c308c6e6f Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Wed, 25 Sep 2024 12:13:19 +0200 Subject: [PATCH 19/29] Mandate a 'Security Considerations' section on MSCs (#4199) And link to lists of possible problems to think about. This is part of an effort to improve the overall security of Matrix during the design process. --- MSC_CHECKLIST.md | 2 +- proposals/0000-proposal-template.md | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/MSC_CHECKLIST.md b/MSC_CHECKLIST.md index 30e3643600f..88e32998dc3 100644 --- a/MSC_CHECKLIST.md +++ b/MSC_CHECKLIST.md @@ -42,9 +42,9 @@ clarification of any of these points. - [ ] Proposal text - [ ] Potential issues - [ ] Alternatives - - [ ] Security considerations - [ ] Dependencies - [ ] Stable identifiers are used throughout the proposal, except for the unstable prefix section - [ ] Unstable prefixes [consider](README.md#unstable-prefixes) the awkward accepted-but-not-merged state - [ ] Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”). - [ ] Changes have applicable [Sign Off](CONTRIBUTING.md#sign-off) from all authors/editors/contributors +- [ ] There is a dedicated "Security Considerations" section which detail any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.". See [RFC3552](https://datatracker.ietf.org/doc/html/rfc3552) for things to think about, but in particular pay attention to the [OWASP Top Ten](https://owasp.org/www-project-top-ten/). diff --git a/proposals/0000-proposal-template.md b/proposals/0000-proposal-template.md index 30d909e434d..41f76d2159b 100644 --- a/proposals/0000-proposal-template.md +++ b/proposals/0000-proposal-template.md @@ -85,14 +85,21 @@ idea. ## Security considerations +**All proposals must now have this section, even if it is to say there are no security issues.** + +*Think about how to attack your proposal, using lists from sources like +[OWASP Top Ten](https://owasp.org/www-project-top-ten/) for inspiration.* + *Some proposals may have some security aspect to them that was addressed in the proposed solution. This section is a great place to outline some of the security-sensitive components of your proposal, such as why a particular approach was (or wasn't) taken. The example here is a bit of a stretch and unlikely to actually be worthwhile of including in a proposal, but it is generally a good idea to list these kinds of concerns where possible.* -By having a template available, people would know what the desired detail for a proposal is. This is not -considered a risk because it is important that people understand the proposal process from start to end. +MSCs can drastically affect the protocol. The authors of MSCs may not have a security background. If they +do not consider vulnerabilities with their design, we rely on reviewers to consider vulnerabilities. This +is easy to forget, so having a mandatory 'Security Considerations' section serves to nudge reviewers +into thinking like an attacker. ## Unstable prefix From e5f33b9839c0f962b85ef2e73673d518619cb0c6 Mon Sep 17 00:00:00 2001 From: Catalan Lover <48515417+FSG-Cat@users.noreply.github.com> Date: Thu, 26 Sep 2024 12:27:11 +0200 Subject: [PATCH 20/29] Clean up MSC_CHECKLIST.md (#4200) Signed-off-by: Catalan Lover catalanlover@protonmail.com --- MSC_CHECKLIST.md | 54 +++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/MSC_CHECKLIST.md b/MSC_CHECKLIST.md index 88e32998dc3..f6bf3d5f8cd 100644 --- a/MSC_CHECKLIST.md +++ b/MSC_CHECKLIST.md @@ -11,40 +11,42 @@ MSC authors, feel free to ask in a thread on your PR or in the [#matrix-spec:matrix.org](https://matrix.to/#/#matrix-spec:matrix.org) room for clarification of any of these points. -- [ ] Are [appropriate - implementation(s)](https://spec.matrix.org/proposals/#implementing-a-proposal) - specified in the MSC’s PR description? +- [ ] Are [appropriate implementation(s)](https://spec.matrix.org/proposals/#implementing-a-proposal) + specified in the MSC’s PR description? - [ ] Are all MSCs that this MSC depends on already accepted? - [ ] For each new endpoint that is introduced: - - [ ] Have authentication requirements been specified? - - [ ] Have rate-limiting requirements been specified? - - [ ] Have guest access requirements been specified? - - [ ] Are error responses specified? - - [ ] Does each error case have a specified `errcode` (e.g. `M_FORBIDDEN`) and HTTP status code? - - [ ] If a new `errcode` is introduced, is it clear that it is new? + - [ ] Have authentication requirements been specified? + - [ ] Have rate-limiting requirements been specified? + - [ ] Have guest access requirements been specified? + - [ ] Are error responses specified? + - [ ] Does each error case have a specified `errcode` (e.g. `M_FORBIDDEN`) and HTTP status code? + - [ ] If a new `errcode` is introduced, is it clear that it is new? - [ ] Will the MSC require a new room version, and if so, has that been made clear? - - [ ] Is the reason for a new room version clearly stated? For example, + - [ ] Is the reason for a new room version clearly stated? For example, modifying the set of redacted fields changes how event IDs are calculated, thus requiring a new room version. - [ ] Are backwards-compatibility concerns appropriately addressed? - [ ] Are the [endpoint conventions](https://spec.matrix.org/latest/appendices/#conventions-for-matrix-apis) honoured? - - [ ] Do HTTP endpoints `use_underscores_like_this`? - - [ ] Will the endpoint return unbounded data? If so, has pagination been considered? - - [ ] If the endpoint utilises pagination, is it consistent with [the - appendices](https://spec.matrix.org/v1.8/appendices/#pagination)? + - [ ] Do HTTP endpoints `use_underscores_like_this`? + - [ ] Will the endpoint return unbounded data? If so, has pagination been considered? + - [ ] If the endpoint utilises pagination, is it consistent with + [the appendices](https://spec.matrix.org/v1.8/appendices/#pagination)? - [ ] An introduction exists and clearly outlines the problem being solved. - Ideally, the first paragraph should be understandable by a non-technical - audience + Ideally, the first paragraph should be understandable by a non-technical audience. - [ ] All outstanding threads are resolved - - [ ] All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative -- [ ] While the exact sections do not need to be present, the details implied by the proposal template are covered. Namely: - - [ ] Introduction - - [ ] Proposal text - - [ ] Potential issues - - [ ] Alternatives - - [ ] Dependencies + - [ ] All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative +- [ ] While the exact sections do not need to be present, + the details implied by the proposal template are covered. Namely: + - [ ] Introduction + - [ ] Proposal text + - [ ] Potential issues + - [ ] Alternatives + - [ ] Dependencies - [ ] Stable identifiers are used throughout the proposal, except for the unstable prefix section - - [ ] Unstable prefixes [consider](README.md#unstable-prefixes) the awkward accepted-but-not-merged state - - [ ] Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”). + - [ ] Unstable prefixes [consider](README.md#unstable-prefixes) the awkward accepted-but-not-merged state + - [ ] Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”). - [ ] Changes have applicable [Sign Off](CONTRIBUTING.md#sign-off) from all authors/editors/contributors -- [ ] There is a dedicated "Security Considerations" section which detail any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.". See [RFC3552](https://datatracker.ietf.org/doc/html/rfc3552) for things to think about, but in particular pay attention to the [OWASP Top Ten](https://owasp.org/www-project-top-ten/). +- [ ] There is a dedicated "Security Considerations" section which detail + any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.". + See [RFC3552](https://datatracker.ietf.org/doc/html/rfc3552) for things to think about, + but in particular pay attention to the [OWASP Top Ten](https://owasp.org/www-project-top-ten/). From 4c0ec1ea733c629d0efbd29baeeee990972fb764 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 30 Sep 2024 12:25:44 +0200 Subject: [PATCH 21/29] MSC4189: Allowing guests to access uploaded media (#4189) --- proposals/4189-guest-access-media-routes.md | 53 +++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 proposals/4189-guest-access-media-routes.md diff --git a/proposals/4189-guest-access-media-routes.md b/proposals/4189-guest-access-media-routes.md new file mode 100644 index 00000000000..df78a116763 --- /dev/null +++ b/proposals/4189-guest-access-media-routes.md @@ -0,0 +1,53 @@ +# MSC4189: Allowing guests to access uploaded media + +[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3916-authentication-for-media.md) +introduced new endpoints which require clients to provide a valid access token in order to access +media. The MSC failed to specify [guest access](https://spec.matrix.org/v1.11/client-server-api/#guest-access) +requirements for the new endpoints. + +This MSC specifies the missing guest access requirements on the new endpoints. + +## Proposal + +The following endpoints explicitly permit guest access, joining the +[list of other endpoints](https://spec.matrix.org/v1.11/client-server-api/#client-behaviour-13) +already in the specification: + +* [`GET /_matrix/client/v1/media/download/{serverName}/{mediaId}`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv1mediadownloadservernamemediaid) +* [`GET /_matrix/client/v1/media/download/{serverName}/{mediaId}/{fileName}`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv1mediadownloadservernamemediaidfilename) +* [`GET /_matrix/client/v1/media/thumbnail/{serverName}/{mediaId}`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv1mediathumbnailservernamemediaid) + +The rationale for the above endpoints is that being able to see events without the associated media +isn't very useful. + +For clarity, the following endpoints are *not* added to the guest access list, as their prior (now +deprecated) versions are not already included. A future MSC may change this with sufficient rationale. +Note that guests cannot currently *upload* files, but can send messages/events. + +* [`GET /_matrix/client/v1/media/config`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv1mediaconfig) +* [`GET /_matrix/client/v1/media/preview_url`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv1mediapreview_url) + +## Potential issues + +This MSC fixes an issue where guests cannot download images/files. + +## Alternatives + +None applicable. + +## Security considerations + +This MSC does not materially increase the threat profile for guests: guests could already download +media using the unauthenticated endpoints. + +## Unstable prefix + +Prefixed endpoints are excessive for this MSC. Implementations can enable guest access on the existing +endpoints safely, or continue to respond with "guest access forbidden" errors. No `/versions` flag +is specified for feature detection: clients with guest access tokens should expect failure until a +server advertises a specification version containing this MSC. Clients should continue trying to make +requests for the best user experience. + +## Dependencies + +This MSC has no dependencies. From 6b10266b14626696be005dfdfdca5626e91a7020 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 7 Oct 2024 11:42:53 +0200 Subject: [PATCH 22/29] MSC4170: 403 error responses for profile APIs (#4170) Relates to matrix-org/matrix-spec#1867 Signed-off-by: Johannes Marbach --- proposals/4170-profile-403.md | 133 ++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 proposals/4170-profile-403.md diff --git a/proposals/4170-profile-403.md b/proposals/4170-profile-403.md new file mode 100644 index 00000000000..5ebb5b11009 --- /dev/null +++ b/proposals/4170-profile-403.md @@ -0,0 +1,133 @@ +# MSC4170: 403 error responses for profile APIs + +Matrix currently defines the following [client-server APIs] for profile look-ups: + +- [`GET /_matrix/client/v3/profile/{userId}`] +- [`GET /_matrix/client/v3/profile/{userId}/avatar_url`] +- [`GET /_matrix/client/v3/profile/{userId}/displayname`] + +These endpoints also support look-up over federation via the accompanying +[server-server API]: + +- [`GET /_matrix/federation/v1/query/profile`] + +Each of these endpoints has a documented 404 response for the case that no profile +information is available. + +> 404 There is no profile information for this user or this user does not exist. +> +> 404 There is no avatar URL for this user or this user does not exist. +> +> 404 There is no display name for this user or this user does not exist. +> +> 404 The user does not exist or does not have a profile. + +However, `GET /_matrix/client/v3/profile/{userId}` additionally reserves a 403 +status code that is not available on the other endpoints and can be used to deny +profile look-ups. + +> 403 The server is unwilling to disclose whether the user exists and/or has profile information. + +Unfortunately, the concrete semantics of when to respond with 403 are not fully +spelled out in the spec and understanding prior proposals' intention requires some +archeology (see the history section below). + +The current proposal aims to restore consistency among the profile endpoints +by standardizing their 403 error response format and behaviour. + + +## Proposal + +For the endpoints in the client-server API + +- [`GET /_matrix/client/v3/profile/{userId}`] +- [`GET /_matrix/client/v3/profile/{userId}/avatar_url`] +- [`GET /_matrix/client/v3/profile/{userId}/displayname`] + +homeservers MUST at a minimum allow profile look-up for users that either share a room +with the requester or reside in a public room known to the homeserver (i.e, the same +requirements as [`POST /_matrix/client/v3/user_directory/search`])[^3]. In all other +cases, homeservers MAY deny profile look-up by responding with 403 `M_FORBIDDEN`. + +If a remote user is queried through the client-server endpoints and the query is not +denied per the preceding paragraph, homeservers SHOULD query the remote server for the +user's profile information. + +Homeservers MAY deny profile look-up over federation by responding with 403 `M_FORBIDDEN` +to [`GET /_matrix/federation/v1/query/profile`]. To be clear: there is no requirement to return +profiles of users in public or shared rooms over the federation API. + +Homeservers MAY choose whether to respond with 403 or 404 when the requested user does +not exist. If the server denies profile look-up in all but the required cases, 403 is +RECOMMENDED. + + +## Potential issues + +Synapse already complies with this proposal in its default configuration. However, +its `limit_profile_requests_to_users_who_share_rooms` config setting is only partially +compatible with this proposal because it disallows profile look-up for users in public +rooms that the requester does not share a room with. This inconsistency would need to +be fixed if this proposal is accepted into the spec. + + +## Alternatives + +None. + + +## Security considerations + +This proposal allows server administrators to lock down profile look-ups via the +client-server API for all situations except those in which the profile information +is already available to the requester via room membership. This complements the +existing ability to deny profile look-ups on the server-server API and, if configured +accordingly, increases privacy for users. + + +## History + +In [2017], the user directory API, [`POST /_matrix/client/v3/user_directory/search`], +which closely relates to the profile APIs, was introduced into the spec. Since its +inception, it contained the requirement for servers to consider (at least) users from +shared and public rooms in the search. + +Later, [MSC1301] proposed a 403 `M_USER_NOT_PUBLIC` response on all four profile +endpoints to optionally disallow profile look-up for users that the requester does +not share a room with. This MSC was never accepted, but in 2019 +was partially implemented by Synapse[^1]: it was only implemented for the client-server +endpoints, and an error code of `M_FORBIDDEN` was used rather than `M_USER_NOT_PUBLIC`. + +In 2021, Synapse implemented[^2] a switchable feature to disable profile look-up +over federation via a 403 `M_FORBIDDEN` response. [MSC3550] picked up on this +feature and introduced this response type in the spec though only on the +`GET /_matrix/client/v3/profile/{userId}` endpoint in the client-server API. + + +## Unstable prefix + +None because this proposal only affects HTTP status codes and Matrix error codes. + + +## Dependencies + +None. + + +[^1]: https://github.com/element-hq/synapse/commit/c0e0740bef0db661abce352afaf6c958e276f11d +[^2]: https://github.com/matrix-org/synapse/pull/9203/files#diff-2f70c35b9dd342bfdaaed445847e0ccabbad63aa9a208d80d38fb248cbf57602L311 +[^3]: As stated in https://github.com/matrix-org/matrix-spec/issues/633, the spec currently + doesn't clearly define what a public room is. This proposal does not aim to solve this + problem and instead only requires that the user directory and profile APIs use the same + definition. + +[`GET /_matrix/client/v3/profile/{userId}`]: https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3profileuserid +[`GET /_matrix/client/v3/profile/{userId}/avatar_url`]: https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3profileuseridavatar_url +[`GET /_matrix/client/v3/profile/{userId}/displayname`]: https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3profileuseriddisplayname +[`GET /_matrix/federation/v1/query/profile`]: https://spec.matrix.org/v1.11/server-server-api/#get_matrixfederationv1queryprofile +[`POST /_matrix/client/v3/user_directory/search`]: https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3user_directorysearch +[2017]: https://github.com/matrix-org/matrix-spec-proposals/pull/1096/files#diff-332ce28a7277b9375050644632f99c0e606acb751adc54c64c5faabf981ac7edR35 +[MSC1301]: https://github.com/matrix-org/matrix-spec-proposals/issues/1301 +[MSC3550]: https://github.com/matrix-org/matrix-spec-proposals/pull/3550 +[client-server APIs]: https://spec.matrix.org/v1.11/client-server-api/#profiles +[server-server API]: https://spec.matrix.org/v1.11/server-server-api/#get_matrixfederationv1queryprofile From c1fc612e867156afaf2beebcf7b5afd73d10fe98 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Mon, 28 Oct 2024 16:53:05 +0100 Subject: [PATCH 23/29] MSC2409: Proposal to send EDUs to appservices (#2409) * Initial proposal commit * Add body of proposal * rename file * finish up MSC * address issues * change key names and add unstable prefix * Clarifications; to-device handling * It's not exactly like sync * Move to-device messages * Copy edu_type behaviour * Add full transaction example * Add implementation notes for to-device cleanup * Use type instead of edu_type to match realities of implementations * Add note to say ephemeral can be omitted. * Improve wording on why we use a seperate array. Co-authored-by: Kevin Cox * push_ephemeral -> receive_ephemeral * Fix some typos and clarify EDU room association * Clarify EDU formatting * Explicitly list all event types * Delete to-device events to be moved to a new MSC * Update spec link and fix typo Co-authored-by: Patrick Cloke * Add private read receipt rules * Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Wrap lines * Apply suggestions from code review Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> * Explicitly mention to-device events are not here * Mention the possibility of more granular filtering --------- Co-authored-by: Will Hunt Co-authored-by: Travis Ralston Co-authored-by: Kevin Cox Co-authored-by: Tulir Asokan Co-authored-by: Patrick Cloke Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- proposals/2409-appservice-edus.md | 158 ++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 proposals/2409-appservice-edus.md diff --git a/proposals/2409-appservice-edus.md b/proposals/2409-appservice-edus.md new file mode 100644 index 00000000000..2f8effa27be --- /dev/null +++ b/proposals/2409-appservice-edus.md @@ -0,0 +1,158 @@ +# MSC2409: Proposal to send typing, presence and receipts to appservices + +*Note: This proposal is a continuation of [MSC1888](https://github.com/matrix-org/matrix-doc/pull/1888) +and deprecates that one.* + +The [appservice /transactions API](https://spec.matrix.org/v1.11/application-service-api/#put_matrixappv1transactionstxnid) +currently supports pushing PDU events (regular message and state events) +however it doesn't provision for EDU events (typing, presence and receipts). This means that bridges cannot +react to Matrix users who send any typing or presence information in a room the service is part of. + +There is an interest amongst the community to have equal bridging on both sides of a bridge, so that +read reciepts and typing notifications can be seen on the remote side. To that end, this proposal +specifies how these can be pushed to an appservice. + +## Proposal + +### Changes to the registration file + +In order that appservices don't get flooded with EDUs, appservices have to opt-in to receive them by +setting `receive_ephemeral` to true. A registration file could look like following: + +```yaml +id: "IRC Bridge" +url: "http://127.0.0.1:1234" +as_token: "30c05ae90a248a4188e620216fa72e349803310ec83e2a77b34fe90be6081f46" +hs_token: "312df522183efd404ec1cd22d2ffa4bbc76a8c1ccf541dd692eef281356bb74e" +sender_localpart: "_irc_bot" +# We want to receive EDUs +receive_ephemeral: true +namespaces: + users: + - exclusive: true + regex: "@_irc_bridge_.*" + aliases: + - exclusive: false + regex: "#_irc_bridge_.*" + rooms: [] +``` + +For now, receiving EDUs is all-or-nothing. A future MSC may add more granular +filtering capabilities for appservices. + +### Changes to the /transactions/ API + +The `PUT /_matrix/app/v1/transactions/{txnId}` API currently supports sending PDUs +via the `events` array. + +```json +{ + "events": [ + { + "content": { + "membership": "join", + "avatar_url": "mxc://domain.com/SEsfnsuifSDFSSEF#auto", + "displayname": "Alice Margatroid" + }, + "type": "m.room.member", + "event_id": "$143273582443PhrSn:domain.com", + "room_id": "!jEsUZKDJdhlrceRyVU:domain.com", + "sender": "@example:domain.com", + "origin_server_ts": 1432735824653, + "unsigned": { + "age": 1234 + }, + "state_key": "@alice:domain.com" + } + ] +} +``` + +This proposal would extend the `PUT /_matrix/app/v1/transactions/` endpoint to include a new key +`ephemeral` to behave similar to the various sections of the CS API `/sync` endpoint. The `ephemeral` key +MAY be omitted entirely if there are no ephemeral events to send. + +```json +{ + "ephemeral": [ + { + "type": "m.typing", + "room_id": "!jEsUZKDJdhlrceRyVU:domain.com", + "content": { + "user_ids": [ + "@alice:example.com" + ] + } + }, + { + "type": "m.receipt", + "room_id": "!jEsUZKDJdhlrceRyVU:domain.com", + "content": { + "$1435641916114394fHBLK:matrix.org": { + "m.read": { + "@rikj:jki.re": { + "ts": 1436451550453 + } + } + } + } + } + ], + "events": [ + // ... + ] +} +``` + +The reason for a new key rather than bundling the events into `events` is that +existing appservices may mistake them for PDUs and might behave erratically. +While `events` may now be a somewhat misleading name, this is an acceptable tradeoff. + +The array is effectively a combination of the `presence` and `ephemeral` sections of the +client-server `/sync` API. User-defined ephemeral events don't exist yet, which means there are +only three event types that can currently occur: +[`m.presence`](https://spec.matrix.org/v1.11/client-server-api/#mpresence), +[`m.typing`](https://spec.matrix.org/v1.11/client-server-api/#mtyping), +and [`m.receipt`](https://spec.matrix.org/v1.11/client-server-api/#mreceipt). + +This proposal does not cover any other types of events which are sent as EDUs in the federation API, +such as to-device events or other e2ee features. Those are left to a separate MSC. + +EDUs are formatted the same way as they are in C-S sync, with the addition of the `room_id` field +for room-scoped EDUs (`m.typing` and `m.receipt`). `room_id` is not present in the C-S API because +sync nests EDUs inside a room object, but appservices get a flat list of events in all rooms. + +### Expectations of when an EDU should be pushed to an appservice + +It is not clear at face value what should be pushed to an appservice. Appservices claim +namespaces of users which registers "interest" in the rooms where those users reside, as +well as claiming namespaces of rooms for explicit interest. However, not all EDUs are +associated with a single room (presence, etc). + +If the EDU is capable of being associated to a particular room (i.e. `m.typing` and `m.receipt`), +it should be sent to the appservice under the same rules as regular events (interest in the room +means sending it). For EDUs which are not associated with a particular room, the appservice +receives the EDU if it contextually *would* apply. For example, a presence update for a user an +appservice shares a room with (or is under the appservice's namespace) would be sent to the +appservice. + +For `m.receipt`, private read receipts (`m.read.private`) should only be sent for users within the +appservice's namespaces. Normal read receipts and threaded read receipts are always sent. + +## Potential issues + +Determining which EDUs to transmit to the appservice could lead to quite some overhead on the +homeserver side. Additionally, more network traffic is produced, potentially straining the local +network and the appservice more. As such, appservices have to opt-in to receive EDUs. + +## Security considerations + +The homeserver needs to accurately determine which EDUs to send to the appservice, as to not leak +any unnecessary metadata about users. Particularly `m.presence` could be tricky, as no `room_id` is present in +that EDU. + +## Unstable prefix + +In the transaction body, instead of `ephemeral`, `de.sorunome.msc2409.ephemeral` is used. + +In the registration file, instead of `receive_ephemeral`, `de.sorunome.msc2409.push_ephemeral` is used. From b76697ed1a1ef9f4f8b9867c1844c1dcdda68a82 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 28 Oct 2024 10:26:45 -0600 Subject: [PATCH 24/29] spelling --- proposals/2409-appservice-edus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2409-appservice-edus.md b/proposals/2409-appservice-edus.md index 2f8effa27be..20fa10c7fbc 100644 --- a/proposals/2409-appservice-edus.md +++ b/proposals/2409-appservice-edus.md @@ -9,7 +9,7 @@ however it doesn't provision for EDU events (typing, presence and receipts). Thi react to Matrix users who send any typing or presence information in a room the service is part of. There is an interest amongst the community to have equal bridging on both sides of a bridge, so that -read reciepts and typing notifications can be seen on the remote side. To that end, this proposal +read receipts and typing notifications can be seen on the remote side. To that end, this proposal specifies how these can be pushed to an appservice. ## Proposal From 22c774de581a4fc3ee64fd90f78a999e0d3dd952 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 29 Oct 2024 16:56:50 +0000 Subject: [PATCH 25/29] Revert "MSC3061: Sharing room keys for past messages (#3061)" This reverts commit 6a3ab1d64cdf9f429a86b0e9d4df927dd36f0280. Per https://matrix.org/blog/2024/10/security-disclosure-matrix-js-sdk-and-matrix-react-sdk/ and https://github.com/matrix-org/matrix-spec-proposals/pull/3061#issuecomment-2444845098, we are removing the merged status of this PR. --- proposals/3061-shareable-room-keys.md | 152 -------------------------- 1 file changed, 152 deletions(-) delete mode 100644 proposals/3061-shareable-room-keys.md diff --git a/proposals/3061-shareable-room-keys.md b/proposals/3061-shareable-room-keys.md deleted file mode 100644 index 39fc5b919bc..00000000000 --- a/proposals/3061-shareable-room-keys.md +++ /dev/null @@ -1,152 +0,0 @@ -# MSC3061: Sharing room keys for past messages - -In Matrix, rooms can be configured via the `m.room.history_visibility` state -event such that historical messages can be visible to all Matrix users -(`world_readable`), all room members (`shared`), room members from the time -that they are invited to a room (`invited`), or room members from the time that -they join a room (`joined`). However, currently in encrypted rooms, rooms with -the history visibility set to `world_readable` or `shared` are effectively -set to `invited` since other members generally do not send new members the keys -to decrypt messages sent before they were invited or joined a room. - -We define a "shared-history" flag that identifies keys for messages that were -sent when the room's visibility setting was set to `world_readable` or -`shared`. This allows clients to know which keys are "safe" to share with new -members so that they can decrypt historical messages. We also give examples of -ways in which this flag can be used. - - -## Proposal - -A room key (such as a megolm session) is flagged as having been used for shared -history when it was used to encrypt a message while the room's history -visibility setting was set to `world_readable` or `shared`. - -If the client does not have an `m.room.history_visibility` state event for the -room, or its value is not understood, the client should treat it as if its -value is `joined` for the purposes of determining whether the key is used for -shared history. This is in contrast with the normal processing of -`m.room.history_visibility` which defaults to `world_readable` when there is no -`m.room.history_visibility` state event or its value is not understood. This -is done so that, in the event of a bug that causes the client to fail to obtain -the state event, the client will fail in a secure manner. - -Internally, a client may use any mechanism it wants to keep track of this flag. -When a room key is marked as having been used for shared history: - -- `m.room_key` and `m.forwarded_room_key` messages used to share this key have - a `shared_history` property set to `true` e.g. - - ```json - { - "type": "m.room_key", - "content": { - "algorithm": "m.megolm.v1.aes-sha2", - "room_id": "!room_id", - "session_id": "session_id", - "session_key": "session_key", - "shared_history": true - } - } - ``` - -- the [`SessionData` type](https://spec.matrix.org/unstable/client-server-api/#definition-sessiondata) - in key backups (that is, the plaintext object that gets encrypted into the - `session_data` field) of this key has a `shared_history` property set to - `true` in the decrypted JSON structure e.g. - - ```json - { - "algorithm": "m.megolm.v1.aes-sha2", - "forwarding_curve25519_key_chain": [ - "hPQNcabIABgGnx3/ACv/jmMmiQHoeFfuLB17tzWp6Hw" - ], - "sender_claimed_keys": { - "ed25519": "aj40p+aw64yPIdsxoog8jhPu9i7l7NcFRecuOQblE3Y" - }, - "sender_key": "RF3s+E7RkTQTGF2d8Deol0FkQvgII2aJDf3/Jp5mxVU", - "session_key": "AgAAAADxKHa9uFxcXzwYoNueL5Xqi69IkD4sni8Llf...", - "shared_history": true - } - ``` - - and, -- the [`SessionData` type](https://spec.matrix.org/unstable/client-server-api/#key-export-format) - used in key exports has a `shared_history` property that is set to `true` for - this key e.g. - - ```json - { - "algorithm": "m.megolm.v1.aes-sha2", - "forwarding_curve25519_key_chain": [ - "hPQNcabIABgGnx3/ACv/jmMmiQHoeFfuLB17tzWp6Hw" - ], - "sender_claimed_keys": { - "ed25519": "aj40p+aw64yPIdsxoog8jhPu9i7l7NcFRecuOQblE3Y" - }, - "sender_key": "RF3s+E7RkTQTGF2d8Deol0FkQvgII2aJDf3/Jp5mxVU", - "session_key": "AgAAAADxKHa9uFxcXzwYoNueL5Xqi69IkD4sni8Llf...", - "shared_history": true - } - ``` - -When a client obtains a key that has the `shared_history` property set to -`true`, then it flags the key internally as having been used for shared -history. Otherwise, the key should not be flagged as such. - -When the room's history visibility setting changes to `world_readable` or -`shared` from `invited` or `joined`, or changes to `invited` or `joined` from -`world_readable` or `shared`, senders that support this flag must rotate their -megolm sessions. - -Clients may use this flag to modify their behaviour with respect to sharing -keys. For example, when the user invites someone to the room, they may -preemptively share keys that have this flag with the invited user. Other -behaviours may be possible, but must be careful not to guard against malicious -homeservers. See the "Security Considerations" section. - -## Potential issues - -Room keys from clients that do not support this proposal will not be eligible -for the modified client behaviour. - -The suggested behaviour in this MSC is to only share additional keys when -inviting another user. This does not allow users who join the room but were -not invited (for example, if membership is restricted to another space, or if -the room is publicly joinable) to receive the keys. Also, if the inviter does -not have all the keys available for whatever reason, the invitee has no way of -receiving the keys. This may be solved in the future when we have a mechanism -for verifying room membership. - -## Alternatives - -Rather than having the sender flagging keys, a client can paginate through the -room's history to determine the room's history visibility settings when the -room key was used. This would not require any changes, but has performance -problems. In addition, the server could lie about the room history while the -user is paginating through the history. By having the sender flag keys, this -ensures that the key is treated in a manner consistent with the sender's view -of the room. - -Rather than using a boolean flag, we could include the history visibility -setting as-is. For example, a `history_visibility` field could be added, which -is set to the history visibility setting (e.g. `world_readable`). This -produces an equivalent effect, but it pushes the processing of the history -visibility setting to the receiver rather than the sender. For consistency, it -is better for as much of the decision-making done by the sender, rather than -the receiver. - -## Security considerations - -Clients should still ensure that keys are only shared with authorized users and -devices, as a malicious homeserver could inject fake room membership events. -One way to ensure that keys are only shared with authorized users is to only -share keys with users when the client invites them, as the client is then -certain that the user is allowed to be in the room. Another way is to have a -mechanism of verifying membership, such as the method proposed in -[MSC3917](https://github.com/matrix-org/matrix-spec-proposals/pull/3917). - -## Unstable prefix - -Until this feature lands in the spec, the property name to be used is -`org.matrix.msc3061.shared_history` rather than `shared_history`. From 9deddd15583a6d1da61fb71c43f38c9d5e8c0481 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 5 Nov 2024 17:23:40 +0100 Subject: [PATCH 26/29] MSC2781: Remove the reply fallbacks from the specification (#2781) * MSC2781: Down with the fallbacks Signed-off-by: Nicolas Werner * Add a note about dropping the html requirement Signed-off-by: Nicolas Werner * Add an unstable prefix for removed fallbacks. Signed-off-by: Nicolas Werner * Add a section about fallbacks not being properly specified. Signed-off-by: Nicolas Werner * Add appendix about which clients do not support replies (and why, if possible) Signed-off-by: Nicolas Werner * Correct weechat status Signed-off-by: Nicolas Werner * Add another alternative Signed-off-by: Nicolas Werner * Document a few more issues with fallbacks Signed-off-by: Nicolas Werner * Update client status, remove proposal for edits and try to turn down the language a bit Signed-off-by: Nicolas Werner * Remove mistaken reference to the Qt renderer Signed-off-by: Nicolas Werner * Try to make motivation a bit clearer in the proposal Signed-off-by: Nicolas Werner * How do anchors work? Signed-off-by: Nicolas Werner * Drop reference to issues with edit fallbacks Signed-off-by: Nicolas Werner * Typos Signed-off-by: Nicolas Werner * Address review comments Signed-off-by: Nicolas Werner * More edits Move edit section to a single sentence in "interaction with other features". Spell out why the IRC example is there. Reword body stripping. Signed-off-by: Nicolas Werner * Implementation traps Signed-off-by: Nicolas Werner * Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Add dates to client status list Signed-off-by: Nicolas Werner * Mention pushrules proposal in the alternatives section Signed-off-by: Nicolas Werner * Update proposal to 2024 This also addresses several review comments from clokep and Travis. * Be explicit about removal * Apply suggestions from code review Thanks dbkr, richvdh and clokep! Co-authored-by: David Baker Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Patrick Cloke * Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update proposals/2781-down-with-the-fallbacks.md Co-authored-by: Patrick Cloke * Apply suggestions from code review Co-authored-by: Travis Ralston Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> * Simplify wording around invalid html and potential issues Signed-off-by: Nicolas Werner --------- Signed-off-by: Nicolas Werner Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: David Baker Co-authored-by: Patrick Cloke Co-authored-by: Travis Ralston Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- proposals/2781-down-with-the-fallbacks.md | 279 ++++++++++++++++++++++ 1 file changed, 279 insertions(+) create mode 100644 proposals/2781-down-with-the-fallbacks.md diff --git a/proposals/2781-down-with-the-fallbacks.md b/proposals/2781-down-with-the-fallbacks.md new file mode 100644 index 00000000000..65b796f81ab --- /dev/null +++ b/proposals/2781-down-with-the-fallbacks.md @@ -0,0 +1,279 @@ +# MSC2781: Remove reply fallbacks from the specification + +Currently the specification suggests clients should send and strip a +[fallback representation](https://spec.matrix.org/v1.10/client-server-api/#fallbacks-for-rich-replies) +of a replied to message. The fallback representation was meant to simplify +supporting replies in a new client, but in practice they add complexity, are +often implemented incorrectly and block new features. + +This MSC proposes to **remove** those fallbacks from the specification. + +Some of the known issues include: +* The content of reply fallback is [untrusted](https://spec.matrix.org/v1.10/client-server-api/#stripping-the-fallback). +* Reply fallbacks may leak history. ([#368](https://github.com/matrix-org/matrix-spec/issues/368)) +* Parsing reply fallbacks can be tricky. ([#350](https://github.com/matrix-org/matrix-spec/issues/350)) +* It is unclear how to handle a reply to a reply. ([#372](https://github.com/matrix-org/matrix-spec/issues/372)) +* Localization of replies is not possible when the content is embedded into the event. +* It is not possible to fully redact an event once it is replied to. This causes issues with Trust & Safety where + spam or other removed content remains visible, and may cause issues with the GDPR Right to be Forgotten. +* There are a variety of implementation bugs related to reply fallback handling. + +More details and considerations are provided in the appendices, but these are +provided for convenience and aren't necessary to understand this proposal. + +## Proposal + +Remove the [rich reply fallback from the +specification](https://spec.matrix.org/v1.10/client-server-api/#fallbacks-for-rich-replies). +Clients should stop sending them and should consider treating `` parts +as they treat other invalid html tags. + +Clients are not required to include a fallback in a reply since version 1.3 of +the +[specification](https://spec.matrix.org/v1.10/client-server-api/#rich-replies). +For this reason the reply fallback can be removed from the specification without +any additional deprecation period. + +A suggestion for the spec PR: An info box could be included to mention +the historical use of the reply fallback, suggesting that clients may encounter +such events sent by other clients and that clients may need to strip out such +fallbacks. + +Given clients have had enough time to implement replies completely, the +overall look & feel of replies should be unchanged or even improved by this +proposal. Implementing replies in a client should also be a bit easier with this +change. + +An extended motivation is provided at [the end of this document](#user-content-appendix-b-issues-with-the-current-fallbacks). + +## Potential issues + +Old events and events sent by clients implementing an older version of the +Matrix specification might still contain a reply fallback. So for at least some +period of time clients will still need to strip reply fallbacks from messages. + +Clients which don't implement rich replies may see messages without context, +confusing users. However, most replies are in close proximity to the original +message, making context likely to be nearby. Clients should also have enough +information in the event to render helpful indications to users while they work +on full support. + +Clients which aren't using +[intentional mentions](https://spec.matrix.org/v1.7/client-server-api/#mentioning-the-replied-to-user) +may cause some missed notifications on the receiving side. +[MSC3664](https://github.com/matrix-org/matrix-doc/pull/3664) and similar aim to +address this issue, and +[MSC4142](https://github.com/matrix-org/matrix-spec-proposals/pull/4142) tries +to improve the intentional mentions experience for replies generally. +Because intentional mentions are already part of the Matrix specification since +version 1.7, clients can be expected to implement those first, which should make +the impact on notifications minimal in practice. + +## Alternatives + +[MSC2589](https://github.com/matrix-org/matrix-doc/pull/2589): This adds the +reply text as an additional key. While this solves the parsing issues, it +doesn't address the other issues with fallbacks. + +One could also just stick with the current fallbacks and make all clients pay +the cost for a small number of clients actually benefitting from them. + +Lastly one could introduce an alternative relation type for replies without +fallback and deprecate the current relation type (since it does not fit the +[new format for relations](https://github.com/matrix-org/matrix-doc/pull/2674) +anyway). We could specify, that the server is supposed to send the replied_to +event in unsigned to the client, so that clients just need to stitch those two +events together, but don't need to fetch the replied_to event from the server. +It would make replies slightly harder to implement for clients, but it would be +simpler than what this MSC proposes. + +## Security considerations + +Overall this should **reduce** security issues as the handling of untrusted +HTML is simplified. For an example security issue that could be avoided, see +https://github.com/vector-im/element-web/releases/tag/v1.7.3 and the appendix. + +## Unstable prefix + +No unstable prefix should be necessary as clients aren't required to send reply +fallbacks for all messages since version 1.3 of the Matrix specification, which +changed the wording from "MUST" to "SHOULD". + +## Appendix A: Support for rich replies in different clients + +### Clients without rendering support for rich replies + +Of the 23 clients listed in the [Matrix client matrix](https://matrix.org/clients-matrix) +16 are listed as not supporting replies (updated January 2022): + +- Element Android: Relies on the reply fallback. +- Element iOS: [Does not support rich replies](https://github.com/vector-im/element-ios/issues/3517) +- weechat-matrix: Actually has an [implementation](https://github.com/poljar/weechat-matrix/issues/86) to send replies although it seems to be [broken](https://github.com/poljar/weechat-matrix/issues/233). Doesn't render rich replies. Hard to implement because of the single socket implementation, but may be easier in the Rust version. +- Quaternion: [Blocked because of fallbacks](https://github.com/quotient-im/libQuotient/issues/245). +- matrixcli: [Doesn't support formatted messages](https://github.com/ahmedsaadxyzz/matrixcli/issues/10). +- Ditto Chat: [Seems to rely on the fallback](https://gitlab.com/ditto-chat/ditto/-/blob/main/mobile/scenes/chat/components/Html.tsx#L38) +- Mirage: Supports rich replies, but [doesn't strip the fallback correctly](https://github.com/mirukana/mirage/issues/89) and uses the fallback to render them. +- Nio: [Unsupported](https://github.com/niochat/nio/issues/85). +- Pattle: Client is not being developed anymore. +- Seaglass: Doesn't support rich replies, but is [unhappy with how the fallback looks](https://github.com/neilalexander/seaglass/issues/51)? +- Miitrix: Somewhat unlikely to support it, I guess? +- matrix-commander: No idea, but doesn't look like it. +- gotktrix: [Seems to rely on the reply fallback](https://github.com/diamondburned/gotktrix/blob/5f2783d633560421746a82aab71d4f7421e4b99c/internal/app/messageview/message/mcontent/text/html.go#L437) +- Hydrogen: [Seems to use the reply fallback](https://github.com/vector-im/hydrogen-web/blob/c3177b06bf9f760aac2bfd5039342422b7ec8bb4/doc/impl-thoughts/PENDING_REPLIES.md) +- kazv: Doesn't seem to support replies at all +- Syphon: [Uses the reply fallback in body](https://github.com/syphon-org/syphon/blob/fa44c5abe37bdd256a9cb61cbc8552e0e539cdce/lib/views/widgets/messages/message.dart#L368) + +So in summary, 3/4 of the listed clients don't support replies. At least one +client doesn't support it because of the fallback (Quaternion). 3 of the command +line clients probably won't support replies, since they don't support formatted +messages and replies require html support for at least sending. + +Only one client implemented rich replies in the last 1.5 years after the +original list was done in October 2020. Other clients are either new in my list +or didn't change their reply rendering. I would appreciate to hear, why those +client developers decided not to support rich reply rendering and if dropping +the reply fallback would be an issue for them. + +Changes from 1.5 years ago as of January 2022: + +- Fractal: [Seems to support replies now!](https://gitlab.gnome.org/GNOME/fractal/-/merge_requests/941) +- Commune: Seems to support rich reply rendering and style them very nicely. +- NeoChat: [Supports rich replies](https://invent.kde.org/network/neochat/-/blob/master/src/utils.h#L21) +- Cinny: [Seems to support rich replies](https://github.com/ajbura/cinny/blob/6ff339b552e242f6233abd86768bb2373b150f77/src/app/molecules/message/Message.jsx#L111) +- gomuks: [Strips the reply fallback](https://github.com/tulir/gomuks/blob/3510d223b2d765572bf2e97222f2f55d099119f0/ui/messages/html/parser.go#L361) +- Lots of other new clients! + + +### Results of testing replies without fallback + +So far I haven't found a client that completely breaks without the fallback. +All clients that support rendering rich replies don't break, when there is no +fallback according to my tests (at least Nheko, Element/Web, FluffyChat and +NeoChat were tested and some events without fallback are in #nheko:nheko.im and +I haven't heard of any breakage). Those clients just show the reply as normal +and otherwise seem to work completely fine as well. Element Android and Element +iOS just don't show what message was replied to. Other clients haven't been +tested by the author, but since the `content` of an event is untrusted, a client +should not break if there is no reply fallback. Otherwise this would be a +trivial abuse vector. + + +## Appendix B: Issues with the current fallbacks + +This section was moved to the back of this MSC, because it is fairly long and +exhaustive. It lists all the issues the proposal author personally experienced +with fallbacks in their client and its interactions with the ecosystem. + +### Stripping the fallback + +To reply to a reply, a client needs to strip the existing fallback of the first +reply. Otherwise replies will just infinitely nest replies. +[While the spec doesn't necessarily require stripping the fallback in replies to replies (only for rendering)](https://spec.matrix.org/v1.1/client-server-api/#fallback-for-mtext-mnotice-and-unrecognised-message-types), +not doing so risks running into the event size limit, but more importantly, it +just leads to a bad experience for clients actually relying on the fallback. + +Stripping the fallback is not trivial. Multiple implementations had bugs in +their fallback stripping logic. The edge cases are not covered in the +specification in detail and some clients have interpreted them differently. +Common mistakes include: + +- Not stripping the fallback in body, which leads to a very long nested chain. +- Not dealing with mismatched `` tags, which can look like you were + impersonating someone. + +For the `body` extra attention needs to be paid to only strip lines starting +with `>` until the first empty line. Implementations either only stripped the +first line, stripped all lines starting with `>` until the first non empty line, +that does not start with `>` or stripped only the `formatted_body`. While those +are implementation bugs, they can't happen if you don't need to strip a +fallback. + +### Creating a new fallback + +To create a new fallback, a client needs to add untrusted html to its own +events. This is an easy attack vector to inject your own content into someone +elses reply. While this can be prevented with enough care, since Riot basically +had to fix this issue twice, it can be expected that other clients can also be +affected by this. + +### Requirement of html for replies + +The spec requires rich replies to have a fallback using html: + +> Rich replies MUST have a format of org.matrix.custom.html and therefore a formatted_body alongside the body and appropriate msgtype. + +This means you can't reply using only a `body` and you can't reply with an +image, since those don't have a `formatted_body` property currently. This means +a text only client, that doesn't want to display html, still needs to support +html anyway and that new features are blocked, because of fallbacks. + +### Format is unreliable + +While the spec says how a fallback "should" look, there are variations in use +which further complicates stripping the fallback or are common mistakes, when +emitting the fallback. Some variations include localizing the fallback, +missing suggested links or tags, using the body in replies to files or images +or using the display name instead of the matrix id. + +As a result the experience in clients relying on the fallback or stripping the +fallback varies depending on the sending client. + +### Replies leak history + +A reply includes the `body` of another event. This means a reply to an event can +leak data to users, that joined this room at a later point, but shouldn't be +able to see the event because of visibility rules or encryption. While this +isn't a big issue, there is still an issue about it: https://github.com/matrix-org/matrix-doc/issues/1654 + +This history leak can also cause abusive or redacted messages to remain visible +to other room members, depending on the client implementation of replies. + +Historically clients have also sometimes localized the fallbacks. In those cases +they leak the users language selection for their client, which may be personal +information. + +### Using the unmodified fallback in clients and bridges + +The above issues are minor, if reply fallbacks added sufficient value to +clients. Bridges usually try to bridge to native replies, so they need to +strip the reply fallback +(https://github.com/matrix-org/matrix-doc/issues/1541). Even the IRC bridge +seems to send a custom fallback, because the default fallback is not that +welcome to the IRC crowd, although the use cases for simple, text only bridges +is often touted as a good usecase for the fallback (sometimes even explicitly +mentioning bridging to IRC). As a result there are very few bridges, that +benefit from the fallback being present. + +Some clients do choose not to implement rich reply rendering, but the experience +tends to not be ideal, especially in cases where you reply to an image and now +the user needs to guess, what image was being replied to. + +As a result the fallbacks provide value to only a subset of the Matrix +ecosystem. + +### Fallbacks increase integration work with new features + +- [Edits explicitly mention](https://github.com/matrix-org/matrix-doc/pull/2676) + that a reply fallback should not be sent in the `m.new_content`. This causes + issues for clients relying on the fallback, because they won't show replies + once a message has been edited (see Element Android as a current example) + and similar edge cases. +- [Extensible events](https://github.com/matrix-org/matrix-doc/pull/1767) + require an update to the specification for fallbacks (because there is no + `body` or `formatted_body` anymore after the transition period). + [The current proposal](https://github.com/matrix-org/matrix-doc/pull/3644) + also intends to just drop the fallbacks in extensible events. + +### Localization + +Since the fallback is added as normal text into the message, it needs to be +localized for the receiving party to understand it. This however proves to be a +challenge, since users may switch languages freely in a room and it is not easy +to guess, which language was used in a short message. One could also use the +client's language, but that leaks the user's localization settings, which can be a +privacy concern and the other party may not speak that language. Alternatively a +client can just send english fallbacks, but that significantly worsens the +experience for casual users in non-english speaking countries. The specification +currently requires them to not be translated (although some clients don't follow +that), but not sending a fallback at all completely sidesteps the need for the +spec to specify that and clients relying on an english only fallback. From aa7a3fc035f1777ef7b4e3ce4527fc7eb8d328cd Mon Sep 17 00:00:00 2001 From: David Teller Date: Sun, 1 Dec 2024 18:34:40 +0100 Subject: [PATCH 27/29] MSC3823: Account Suspension (#3823) * MSC: A code for account suspension Signed-off-by: David Teller * WIP: Adding prefix * Update proposals/3823-code-for-account-suspension.md Co-authored-by: Travis Ralston * Apply suggestions from code review Co-authored-by: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> * Update for latest requirements * Add list of permitted/forbidden actions * Remove `href` from error code - this will be handled in a later MSC * Match modern template * Clarify MSC * Clarify differences in locking vs suspension, and future scope --------- Signed-off-by: David Teller Co-authored-by: Travis Ralston Co-authored-by: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Co-authored-by: Travis Ralston --- proposals/3823-code-for-account-suspension.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 proposals/3823-code-for-account-suspension.md diff --git a/proposals/3823-code-for-account-suspension.md b/proposals/3823-code-for-account-suspension.md new file mode 100644 index 00000000000..3458736c193 --- /dev/null +++ b/proposals/3823-code-for-account-suspension.md @@ -0,0 +1,80 @@ +# MSC3823: Account Suspension + +Unlike [account locking](https://spec.matrix.org/v1.12/client-server-api/#account-locking), suspension +allows the user to have a (largely) readonly view of their account. Homeserver administrators and +moderators may use this functionality to temporarily deactivate an account, or place conditions on +the account's experience. Critically, like locking, account suspension is reversible, unlike the +deactivation mechanism currently available in Matrix - a destructive, irreversible, action. + +This proposal introduces a concept of suspension to complement locking for server admins to use. Locking +is typically more used in narrow scenarios, where the server admin wants to prevent the account from +engaging any further. An example of this may be too many failed password attempts. Suspension is more +general purpose to create a barrier to further action - a "something weird is going on -> suspend" +kind of button. + +The error code introduced by this proposal is accompanied by guidelines on how servers can implement +suspension. APIs to invoke or clear suspension are not introduced, and left as an implementation detail. +These will typically be done through an administrator-only API. + +## Proposal + +When an account is suspended, any [Client-Server API](https://spec.matrix.org/v1.10/client-server-api/) +endpoint MAY return a 403 HTTP status code with `errcode` of `M_USER_SUSPENDED`. This indicates to +the user that the associated action is unavailable. + +Clients should note that for more general endpoints, like `/send/:eventType`, suspension MAY only be +applied to a subset of request parameters. For example, a user may be allowed to *redact* events but +not send messages. + +The specific list of permitted actions during suspension is left as a deliberate implementation +detail, however a server SHOULD permit the user to: + +* Log in/create additional sessions (which should also behave as suspended). +* See and receive messages, particularly via `/sync` and `/messages`. +* [Verify their other devices](https://spec.matrix.org/v1.10/client-server-api/#device-verification) + and write associated [cross-signing data](https://spec.matrix.org/v1.10/client-server-api/#cross-signing). +* [Populate their key backup](https://spec.matrix.org/v1.10/client-server-api/#server-side-key-backups). +* Leave rooms & reject invites. +* Redacting their own events. +* Log out/delete any device of theirs, including the current session. +* Deactivate their account, potentially with a deliberate time delay to discourage making a new + account right away. +* Change or add [admin contacts](https://spec.matrix.org/v1.10/client-server-api/#adding-account-administrative-contact-information), + but not remove. Servers are recommended to only permit this if they keep a changelog on contact information + to prevent misuse. + +The recommendation for users to continue receiving/reading messages is largely so the administrator +can maintain contact with the user, where applicable. Future MSCs may improve upon the admin<>user +communication, and account locking may also be used to prevent access to messages. + +The suggested set of explicitly forbidden actions is: + +* Joining or knocking on rooms, including accepting invites. +* Sending messages. +* Sending invites. +* Changing profile data (display name and avatar). +* Redacting other users' events (when a moderator/admin of a room, for example). + +## Potential issues + +This proposal does not communicate *why* a user's account is restricted. The human-readable `error` +field may contain some information, though anything comprehensive may not be surfaced to the user. +A future MSC is expected to build a system for both informing the user of the action taken against +their account and allow the user to appeal that action. + +## Alternatives + +No significant alternatives are plausible. `M_USER_DEACTIVATED` could be expanded with a `permanent` +flag, though ideally each error code should provide meaning on its own. + +The related concept of locking, as discussed in places like [MSC3939](https://github.com/matrix-org/matrix-spec-proposals/pull/3939) +and [matrix-org/glossary](https://github.com/matrix-org/glossary), is semantically different from +suspension. Suspension has the key difference that the user is *not* logged out of their client, +typically used by safety teams, while locking *does* log the user out and would primarily be used by +security teams. A future MSC may combine or split the two related error codes into a descriptive set +of capabilities the user can perform. + +## Unstable prefixes + +Until this proposal is considered stable, implementations must use +`ORG.MATRIX.MSC3823.USER_SUSPENDED` instead of `M_USER_SUSPENDED`. From 5370f489cd798b66f7879933e2cafa539ff199dc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 9 Dec 2024 22:24:59 +0000 Subject: [PATCH 28/29] MSC4225: Specification of an order in which one-time-keys should be issued (#4225) * Initial proposal * edits * typos * Add impl note about dropping old keys --- proposals/4225-one-time-key-ordering.md | 155 ++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 proposals/4225-one-time-key-ordering.md diff --git a/proposals/4225-one-time-key-ordering.md b/proposals/4225-one-time-key-ordering.md new file mode 100644 index 00000000000..1b7566962f4 --- /dev/null +++ b/proposals/4225-one-time-key-ordering.md @@ -0,0 +1,155 @@ +# MSC4225: Specification of an order in which one-time-keys should be issued + +The specification for the +[`/keys/claim`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysclaim) +endpoint does not specify any particular order in which one-time keys (OTKs) +should be returned, leading to the possibility that old keys can remain on the +server a long time. Clients may discard the private parts of such old keys, +leading to [unable-to-decrypt +errors](https://github.com/element-hq/element-meta/issues/2356). + +See the Appendix below for a more detailed discussion of the problem. + +## Proposal + +[`/_matrix/client/v3/keys/claim`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysclaim) +and [`/_matrix/federation/v1/user/keys/claim`](https://spec.matrix.org/v1.12/server-server-api/#post_matrixfederationv1userkeysclaim) +should return one-time keys in the order that they were uploaded via +[`/keys/upload`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysupload). All +keys uploaded in a given call to `/keys/upload` are considered equivalent in +this regard; no ordering is specified within them. + +This means that the server will retain only the most recently-uploaded one-time +keys, therefore significantly reducing the chance that clients will discard +private one-time keys that are later used. + +### Implementation note + +When servers first implement this proposal, they may still have old one-time +keys in their datastore, and this may take many years to resolve. + +It is suggested that one solution to this is, as a one-time job, to drop all +one-time keys older than, say, a week, from the database. Having done so, +clients will upload new one-time keys as soon as they come online; in the +meantime, fallback keys are available to allow conversation to continue. + +Servers might consider applying further heuristics: for example, keys produced +by libolm are far more likely to have been dropped by the client than those +produced by Vodozemac, because libolm only retained 100 keys, whereas Vodozemac +retains 5000. The two can be identified by the different lengths of key ID they +produce (6 characters vs 11 characters). + +## Potential issues + +This proposal is not a complete solution to the problem of premature discarding +of one-time keys: even if the server issues a recent one-time key, it is still +possible for a to-device message to be delayed so long that the recipient has +discarded the private part of the one-time key. It is, however, a significant +improvement. A possible future solution is for clients that expect to be used +in conditions of poor connectivity to keep old OTKs for longer. + +There are other ways in which the server and client can get out of sync with +respect to one-time keys, including by a [database +rollback](https://github.com/element-hq/element-meta/issues/2155), or +implementation defects. It is anticipated that other solutions will be found +for those situations. + +## Alternatives + +1. Mandate that clients keep the private parts of all uploaded but unused OTKs + indefinitely. + + Given that each one-time key is only 32 bytes (plus the ID, say 8 bytes), it + would certainly be possible for a client implementation to hold a very large + number of private OTKs without significant concerns about resource usage. In + particular, a OTK is much smaller than a Megolm key, and there is no limit + to the number of Megolm keys that a client has to retain. + + In short though, this just seems inefficient, compared to specifying that + OTKs should be issued in upload order. + +2. [MSC4162](https://github.com/matrix-org/matrix-spec-proposals/pull/4162) + proposes a mechanism by which a client can inform the server that it is + discarding certain OTKs, so that the server can also remove the public + keys. This seems a heavier-weight solution to the problem. + +3. A more invasive alternative would be to design an encryption stack which + does not rely on one-time keys. Setting aside whether the security + properties of such a protocol would be sufficient, this is considered well + out-of-scope for this proposal. + +## Security considerations + +Requiring keys to be allocated in upload order might leak information about the +user's traffic level, since key IDs are typically allocated sequentially: if I +issue two claims for Alice's OTKs a week apart, and I get sequential key IDs, I +know that nobody else has opened a conversation with her in that time. + +To mitigate this, we might consider having clients create key IDs +non-sequentially (whilst remaining unique). This is considered out-of-scope for +this MSC. + +## Unstable prefix + +None required. + +## Dependencies + +None. + +## Appendix: detailed explanation of the failure mode + +### Background + +End-to-end encryption in Matrix relies on individual devices sharing +[Megolm](https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/megolm.md) +message keys via [to-device +messages](https://spec.matrix.org/v1.12/client-server-api/#send-to-device-messaging) +which are themselves encrypted using the +[Olm](https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/olm.md) +ratchet. + +Suppose Alice wishes to send an Olm-encrypted message to Bob (with whom she +has not previously established an Olm session). When Bob logged in, his device +will have created a long-term identity key, as well as a number (typically 50) +of one-time keys. Each of these keys has a private part, which Bob's device +retains locally, and a public part, which Bob's device publishes to his +homeserver via the +[`/keys/upload`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysupload) endpoint. + +Now, to establish an Olm session, Alice needs to claim one of Bob's one-time +keys. She does this by calling the +[`/keys/claim`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysclaim) +endpoint. Together with Bob's identity key, this allows Alice to encrypt a +message that only Bob will be able to decrypt. + +Over time, Bob's supply of one-time keys will be depleted. The `/sync` endpoint +informs clients how many one-time keys remain unclaimed on the server, so that +it can generate new ones when required. + +See [One-time and fallback +keys](https://spec.matrix.org/v1.12/client-server-api/#one-time-and-fallback-keys) +which specifies much of this behaviour. + +### Problem + +Clearly, a device must retain the private part of each one-time key until that +key is used to establish an Olm session. However, for a number of reasons, +ranging from network errors to malicious activity, it is possible for a claimed +one-time key never to be used to establish an Olm session. + +This presents a problem: there is a limit to the number of private one-time +keys that a client can retain. Over time, as keys are repeatedly claimed, +replaced with newly-generated keys, but not actually used, the client must +start to discard older keys. + +Unfortunately, the Matrix specification does not currently specify any order in +which keys should be returned by `/keys/claim`. This means that homeservers can +legitimately issue one-time keys effectively at random. Over time, then, it is +easy to get into a situation where the server still holds some very old +one-time keys, for which the client has discarded the private parts. + +Suppose, when Alice claims one of Bob's one-time keys, she is issued one whose +private part Bob has discarded. Then, Bob will be unable to decrypt the Olm +message from Alice, and (assuming the Olm message contained Megolm keys), will +be unable to decrypt any room messages that Alice sends. From 51ebe01d754ad8e609b9753c1825657f8e210cff Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 10 Dec 2024 11:43:32 -0500 Subject: [PATCH 29/29] MSC4147: Including device keys with Olm-encrypted events (#4147) --- ...g-device-keys-with-olm-encrypted-events.md | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 proposals/4147-including-device-keys-with-olm-encrypted-events.md diff --git a/proposals/4147-including-device-keys-with-olm-encrypted-events.md b/proposals/4147-including-device-keys-with-olm-encrypted-events.md new file mode 100644 index 00000000000..9a2f46615b7 --- /dev/null +++ b/proposals/4147-including-device-keys-with-olm-encrypted-events.md @@ -0,0 +1,183 @@ +# MSC4147: Including device keys with Olm-encrypted to-device messages + +Summary: a proposal to ensure that messages sent from a short-lived (but +genuine) device can be securely distinguished from those sent from a spoofed +device. + +## Background + +When a Matrix client receives an encrypted message, it is necessary to +establish whether that message was sent from a device genuinely belonging to +the apparent sender, or from a spoofed device (for example, a device created by +an attacker with access to the sender's account such as a malicious server +admin, or a man-in-the-middle). + +In short, this is done by requiring a signature on the sending device's device +keys from the sending user's [self-signing cross-signing +key](https://spec.matrix.org/v1.12/client-server-api/#cross-signing). Such a +signature proves that the sending device was genuine. + +Current client implementations check for such a signature by +[querying](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysquery) +the sender's device keys when an encrypted message is received. + +However, this does not work if the sending device logged out in the time +between sending the message and it being received. This is particularly likely +if the recipient is offline for a long time. In such a case, the sending server +will have forgotten the sending device (and any cross-signing signatures) by +the time the recipient queries for it. This makes the received message +indistinguishable from one sent from a spoofed device. + +Current implementations work around this by displaying a warning such as "sent +by a deleted or unknown device" against the received message, but such +messaging is unsatisfactory: a message should be either trusted or not. + +We propose to solve this by including a copy of the device keys in the +Olm-encrypted message, along with the cross-signing signatures, so that the +recipient does not have to try to query the sender's keys. + +## Proposal + +The plaintext payload of to-device messages encrypted with the [`m.olm.v1.curve25519-aes-sha2` encryption +algorithm](https://spec.matrix.org/v1.12/client-server-api/#molmv1curve25519-aes-sha2) +is currently of the form: + +```json +{ + "type": "", + "content": "", + "sender": "", + "recipient": "", + "recipient_keys": { + "ed25519": "" + }, + "keys": { + "ed25519": "" + } +} +``` + +We propose to add a new property: `sender_device_keys`, which is a copy of what +the server would return in response to a +[`/keys/query`](https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3keysquery) +request, as the device keys for the sender's device. In other words, the +plaintext payload will now look something like: + +```json +{ + "type": "", + "content": "", + "sender": "", + "recipient": "", + "recipient_keys": { + "ed25519": "" + }, + "keys": { + "ed25519": "" + }, + "sender_device_keys": { + "algorithms": ["", ""], + "user_id": "", + "device_id": "", + "keys": { + "ed25519:": "", + "curve25519:": "" + }, + "signatures": { + "": { + "ed25519:": "", + "ed25519:": "", + } + } + } +} +``` + +If this property is present, the `keys`.`ed25519` property of the plaintext +payload must be the same as the `sender_device_keys`.`keys`.`ed25519:` +property. If they differ, the recipient should discard the event. + +As the `keys` property is now redundant, it may be removed in a future version +of the Matrix specification. + +## Potential issues + +Adding this property will increase the size of the to-device message. We found it +increased the length of a typical encrypted `m.room_key` message from about 1400 to 2400 +bytes (a 70% increase). This will require increased storage on the recipient +homeserver, and increase bandwidth for both senders and recipients. See +[Alternatives](#alternatives) for discussion of mitigation strategies. + +This proposal is not a complete solution. In particular, if the sender resets +their cross-signing keys, and also logs out the sending device, the recipient +still has no way to verify the sending device. The device signature in the Olm +message is meaningless. A full solution would require the recipient to be able +to obtain a history of cross-signing key changes, and to expose that +information to the user; that is left for the future. + +## Alternatives + +### Minor variations + +The `sender_device_keys` property could be added to the cleartext. That is, it could +be added as a property to the `m.room.encrypted` event. This information is +already public, as it is accessible from `/keys/query` (while the device is +logged in), and does not need to be authenticated as it is protected by the +self-signing signature, so it does not seem to need to be encrypted. However, +there seems to be little reason not to encrypt the information. In addition, by +including it in the encrypted payload, it leaves open the possibility of +it replacing the `keys` property, which must be part of the encrypted payload +to prevent an [unknown key-share attack](https://github.com/element-hq/element-web/issues/2215). + +The `sender_device_keys` property could be added to the cleartext by the sender's +homeserver, rather than by the sending client. Possibly within an `unsigned` +property, as that is where properties added by homeservers are customarily +added. It is not clear what advantage there would be to having this +information being added by the client. + +To mitigate the increased size of to-device events under this proposal, the +`sender_device_keys` could be sent only in pre-key messages (Olm messages +with `type: 0` in the `m.room.encrypted` event) — with the rationale that if +the Olm message is a normal (non-pre-key) message, this means that the +recipient has already decrypted a pre-key message that contains the +information, and so does not need to be re-sent the information), or if the +signatures change (for example, if the sender resets their cross-signing keys), +or if the sender has not yet sent their `device_keys`. However, this requires +additional bookkeeping, and it is not clear whether this extra complexity is +worth the reduction in bandwidth. + +### Alternative approach + +A more radical proposal to decrease the overhead in to-device messages is to +instead specify that `/keys/query` must include deleted devices as well as +active ones, so that they can be reliably queried. Since the origin server +might be unreachable at the time the recipient receives the message, such +device lists would need to be cached on the recipient homeserver. + +In other words, this approach would require all homeservers to keep a permanent +record of all devices observed anywhere in the federation, at least for as long +as there are undelivered to-device events from such devices. + +Transparently: we have not significantly explored this approach. We have a +working solution, and it is unclear that the advantages of this alternative +approach outweigh the opportunity cost and delay in rollout of an important +security feature. If, in future, the overhead of including the device keys +in the to-device messages is found to be significant, it would be worth +revisiting this. + +## Security considerations + +If a device is logged out, there is no indication why it was logged out. For +example, an attacker could steal a device and use it send a message. The user, +upon realizing that the device has been stolen, could log out the device, but +the message may still be sent, if the user does not notice the message and +redact it. + +## Unstable prefix + +Until this MSC is accepted, the new property should be named +`org.matrix.msc4147.device_keys`. + +## Dependencies + +None