From 61a886668344e78e850db8983650bfecaae817a4 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 16 Sep 2020 18:10:06 +0100 Subject: [PATCH 01/22] initial proposal --- proposals/XXXX-appservice-login.md | 79 ++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 proposals/XXXX-appservice-login.md diff --git a/proposals/XXXX-appservice-login.md b/proposals/XXXX-appservice-login.md new file mode 100644 index 00000000000..d8795e20653 --- /dev/null +++ b/proposals/XXXX-appservice-login.md @@ -0,0 +1,79 @@ +# MSC0000: Provide authentication method for appservice users + +Appservices within Matrix are increasingly attempting to support End-to-End Encryption. As such, they +need a way to generate devices for their users so that they can participate in E2E rooms. In order to +do so, this proposal suggests implementing an appservice extension to the +[`POST /login` endpoint](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-login). + +Appservice users do not usually need to login as they do not need their own access token, and do not +traditionally need a "device". However E2E encryption demands that all users in a room have a device +which means bridge users need to be able to generate a device on demand. + +## Proposal + +A new `type` is to be added to `POST /login`. + +`m.login.application_service` + +The `/login` endpoint may now take an `access_token` in the same way that other +authenticated endpoints do. No additional parameters should be specified in the request body. + +Example request + +```json +{ + "type": "m.login.application_service", + "identifier": { + "type": "m.id.user", + "user": "alice" + } +} +``` + +The response body should be unchanged from the existing `/login` specification. + +If +- The access token is not provided +- The access token does not correspond to a appservice +- The access token does not correspond to a appservice that manages this user +- Or the user has not previously been registered + +Then the servers should reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. + +Homeservers should ignore the `access_token` parameter if a type other than +`m.login.application_service` has been provided. + +The expected flow for appservices would be to `/register` their users, and +then `/login` to generate the appropraite device. + +## Potential issues + +This proposal means that there will be more calls to make when setting up a appservice user, when +using encryption. While this could be done during the registration step, this would prohibit creating +new devices should the appservice intentionally or inadvertently lost the client-side device data. + +## Alternatives + +One minor tweak to the current proposal could be to include the token as part of the auth data, rather than +being part of the header/params to the request. An argument could be made for either, but since the specification +expects the appservice to pass the token this way in all requests, including /register it seems wise to keep +it that way. + +Some community members have used implementation details such as a "shared secret" authentication method to +log into the accounts without having to use the /login process at all. Synapse provides such a function, +but also means the appservice can now authenticate as any user on the homeserver. + +A third option could be to create a new endpoint that simply creates a new device for an appservice user on demand. +Given the rest of the matrix eco-system does this with /login, and /login is already extensible with `type`, it would +create more work for all parties involved for little benefit. + +## Security considerations + +The /login endpoint will generate an access token which can be used to control the appservice user, which +is superflous as the appservice `as_token` should be used to authenticate all requests on behalf of ghosts. +This can safely be ignored or used, but is an extra token hanging around. + +## Unstable prefix + +Implementations should use `uk.half-shot.mscXXXX.login.application_service` for `type` given in the +`POST /login` until this lands in a released version of the specification. \ No newline at end of file From 3a649839c27a13ccddebeb1c3253cf69979cddeb Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 16 Sep 2020 18:12:07 +0100 Subject: [PATCH 02/22] 2778 --- .../{XXXX-appservice-login.md => 2778-appservice-login.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename proposals/{XXXX-appservice-login.md => 2778-appservice-login.md} (96%) diff --git a/proposals/XXXX-appservice-login.md b/proposals/2778-appservice-login.md similarity index 96% rename from proposals/XXXX-appservice-login.md rename to proposals/2778-appservice-login.md index d8795e20653..ba3ae1f1a74 100644 --- a/proposals/XXXX-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -1,4 +1,4 @@ -# MSC0000: Provide authentication method for appservice users +# MSC2778: Providing authentication method for appservice users Appservices within Matrix are increasingly attempting to support End-to-End Encryption. As such, they need a way to generate devices for their users so that they can participate in E2E rooms. In order to @@ -75,5 +75,5 @@ This can safely be ignored or used, but is an extra token hanging around. ## Unstable prefix -Implementations should use `uk.half-shot.mscXXXX.login.application_service` for `type` given in the +Implementations should use `uk.half-shot.msc2778.login.application_service` for `type` given in the `POST /login` until this lands in a released version of the specification. \ No newline at end of file From 4751785a8c7fea71552188cdfe1a0b6e97599d16 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 16 Sep 2020 18:26:32 +0100 Subject: [PATCH 03/22] Some spelling fixes --- proposals/2778-appservice-login.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index ba3ae1f1a74..22e06eb79dd 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -32,7 +32,8 @@ Example request The response body should be unchanged from the existing `/login` specification. -If +If: + - The access token is not provided - The access token does not correspond to a appservice - The access token does not correspond to a appservice that manages this user @@ -44,7 +45,7 @@ Homeservers should ignore the `access_token` parameter if a type other than `m.login.application_service` has been provided. The expected flow for appservices would be to `/register` their users, and -then `/login` to generate the appropraite device. +then `/login` to generate the appropriate device. ## Potential issues @@ -56,12 +57,13 @@ new devices should the appservice intentionally or inadvertently lost the client One minor tweak to the current proposal could be to include the token as part of the auth data, rather than being part of the header/params to the request. An argument could be made for either, but since the specification -expects the appservice to pass the token this way in all requests, including /register it seems wise to keep +expects the appservice to pass the token this way in all requests, including `/register`, it seems wise to keep it that way. Some community members have used implementation details such as a "shared secret" authentication method to log into the accounts without having to use the /login process at all. Synapse provides such a function, -but also means the appservice can now authenticate as any user on the homeserver. +but also means the appservice can now authenticate as any user on the homeserver. This seems undesirable from a +security standpoint. A third option could be to create a new endpoint that simply creates a new device for an appservice user on demand. Given the rest of the matrix eco-system does this with /login, and /login is already extensible with `type`, it would From 7d9304fdde9569a4cb0733ea058fc6ed1754f1f6 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 30 Nov 2020 17:13:54 +0000 Subject: [PATCH 04/22] Clarify _bridge_alice Co-authored-by: Tulir Asokan --- proposals/2778-appservice-login.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 22e06eb79dd..76871c888d7 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -25,7 +25,7 @@ Example request "type": "m.login.application_service", "identifier": { "type": "m.id.user", - "user": "alice" + "user": "_bridge_alice" } } ``` @@ -78,4 +78,4 @@ This can safely be ignored or used, but is an extra token hanging around. ## Unstable prefix Implementations should use `uk.half-shot.msc2778.login.application_service` for `type` given in the -`POST /login` until this lands in a released version of the specification. \ No newline at end of file +`POST /login` until this lands in a released version of the specification. From 5c0000431747a20467b3b9a4d50769e604182733 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 30 Nov 2020 17:16:41 +0000 Subject: [PATCH 05/22] Add notice about identifer --- proposals/2778-appservice-login.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 76871c888d7..b361b2aecff 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -30,6 +30,11 @@ Example request } ``` +Note: Implementations MUST use the `identifier.type`=`m.id.user` method of specifying the +localpart. The deprecated top-level `user` field **cannot** use this login flow type. This +is deliberate so as to coax developers into using the new identifier format when implementing +new flows. + The response body should be unchanged from the existing `/login` specification. If: From 7a3b7b38a5a92c5a67d948b34c69867751fb4b35 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 29 Apr 2021 15:58:31 +0100 Subject: [PATCH 06/22] Add implementations section --- proposals/2778-appservice-login.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index b361b2aecff..0de59e27f4a 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -84,3 +84,12 @@ This can safely be ignored or used, but is an extra token hanging around. Implementations should use `uk.half-shot.msc2778.login.application_service` for `type` given in the `POST /login` until this lands in a released version of the specification. + +## Implementations + +The proposal has been implemented by a homeserver, a bridge SDK and two bridges: + +- [synapse](https://github.com/matrix-org/synapse/pull/8320) +- [mautrix-python](https://github.com/tulir/mautrix-python/commit/12d7c48ca7c15fd3ff61608369af1cf69e289aeb) +- [mautrix-whatsapp](https://github.com/tulir/mautrix-whatsapp/commit/ead8a869c84d07fadc7cfcf3d522452c99faaa36) +- [matrix-appservice-bridge](https://github.com/matrix-org/matrix-appservice-bridge/pull/231/files#diff-5e93f1b51d50a44fcf0ca46ea1793c1cR851-R864) \ No newline at end of file From c9b7e9f22d44ff1da103e9d2f5698c8f728b67ab Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 29 Apr 2021 15:58:51 +0100 Subject: [PATCH 07/22] Reword need for token --- proposals/2778-appservice-login.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 0de59e27f4a..52c64475967 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -6,8 +6,10 @@ do so, this proposal suggests implementing an appservice extension to the [`POST /login` endpoint](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-login). Appservice users do not usually need to login as they do not need their own access token, and do not -traditionally need a "device". However E2E encryption demands that all users in a room have a device -which means bridge users need to be able to generate a device on demand. +traditionally need a "device". However, E2E encryption demands that at least one user in a room has a Matrix device +which means bridge users need to be able to generate a device on demand. In the past, bridge developers +have used the bridge bot's device for all bridge users in the room, but this causes problems should the bridge +wish to only join ghosts to a room (e.g. for DMs). ## Proposal From 2c817cb1c579aa6d47cab18edf46867669518597 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 29 Apr 2021 15:58:57 +0100 Subject: [PATCH 08/22] Add another advantage --- proposals/2778-appservice-login.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 52c64475967..1b7c45a3a71 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -11,6 +11,10 @@ which means bridge users need to be able to generate a device on demand. In the have used the bridge bot's device for all bridge users in the room, but this causes problems should the bridge wish to only join ghosts to a room (e.g. for DMs). +Another advantage this provides is that an appservice can now be used to generate access tokens for +any user in it's namespace without having to set a password for that user, which may be useful in the +case of software where maintaining password(s) in the configuration is undesirable. + ## Proposal A new `type` is to be added to `POST /login`. From cb7bbf3787b10ecbde67eb5afb9b773553c4f64e Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 29 Apr 2021 16:00:21 +0100 Subject: [PATCH 09/22] I don't think this is a concern --- proposals/2778-appservice-login.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 1b7c45a3a71..c91f81a8e5d 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -82,9 +82,7 @@ create more work for all parties involved for little benefit. ## Security considerations -The /login endpoint will generate an access token which can be used to control the appservice user, which -is superflous as the appservice `as_token` should be used to authenticate all requests on behalf of ghosts. -This can safely be ignored or used, but is an extra token hanging around. +None ## Unstable prefix From 03491c5cfd8b82295706e1a86648e19741dbff04 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 3 May 2021 17:42:38 +0100 Subject: [PATCH 10/22] Add security considerations --- proposals/2778-appservice-login.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index c91f81a8e5d..390e05478b8 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -82,7 +82,28 @@ create more work for all parties involved for little benefit. ## Security considerations -None +Appservices could use this new functionality to generate devices for any userId that are within it's namespace e.g. setting the +user namespace regex to `@.*:example.com` would allow appservice to control anyone on the homeserver. While this sounds scary, in practise +this is not a problem because: + +- Appservice namespaces are mainained by the homeserver admin. If the namespace were to change, then it's reasonable + to assume that the server admin is aware. There is no defense mechanism to stop a malicious server admin from creating new + devices for a given user's account as they could also do so by simply modifying the database. + +- While an appservice *could* try to masquerade as a user maliciously without the server admin expecting it, it would still + be bound by the restrictions of the namespace. Server admins are expected to be aware of the implications of adding new + appservices to their server so the burden of responsibility lies with the server admin. + +- Appservices already can /sync as any user using the `as_token` and send any messages as any user in the namespace, the only + difference is that without a dedicated access token they are unable to receive device messages. While in theory this + does make them unable to see encrypted messages, this is not designed to be a security mechanism. + +- An appservice trying to log in as a user will always create a new device, which means the user would be informed of the + new device on their existing sessions. It should be very obvious if a malicous appservice is creating new devices on your account. + +In conclusion this MSC only automates the creation of new devices for users inside an AS namespace, which is something +a server admin could already do. Appservices should always be treated with care and so with these facts in mind the MSC should +be considered secure. ## Unstable prefix From bf9cc0a5629236135344160af5ed87ab3cf5a247 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 3 May 2021 17:44:26 +0100 Subject: [PATCH 11/22] M_EXCLUSIVE --- proposals/2778-appservice-login.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 390e05478b8..3f79d8c307e 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -47,11 +47,12 @@ If: - The access token is not provided - The access token does not correspond to a appservice -- The access token does not correspond to a appservice that manages this user - Or the user has not previously been registered Then the servers should reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. +If the access token does not correspond to a appservice that manages this user, then the `errcode` should be `"M_EXCLUSIVE"`. + Homeservers should ignore the `access_token` parameter if a type other than `m.login.application_service` has been provided. From e781b75847c0651336da60e17a99f0bbfdb606e5 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 3 May 2021 18:09:27 +0100 Subject: [PATCH 12/22] Mention that /register provides a token but it's not helpful --- proposals/2778-appservice-login.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 3f79d8c307e..567f84119c0 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -81,6 +81,12 @@ A third option could be to create a new endpoint that simply creates a new devic Given the rest of the matrix eco-system does this with /login, and /login is already extensible with `type`, it would create more work for all parties involved for little benefit. +Finally, `POST /register` does already return a `device_id`, `access_token` for appservice users by default. However critically +this means that bridges will need to be designed to store the access_token and device_id from the point of creating the user, +so older bridges would be unable to get an access token for existing users as `POST /register` would fail. +It would difficult to log out these tokens if they got exposed additionally, as the AS would not be able to fetch a new access token. +Furthermore, the ability to generate access tokens for real users who registered elsewhere would not be possible with this mechanism. + ## Security considerations Appservices could use this new functionality to generate devices for any userId that are within it's namespace e.g. setting the From 4ca319db0942dea353d3cda73fdea483bcaa801d Mon Sep 17 00:00:00 2001 From: Christian Paul Date: Wed, 5 May 2021 17:04:25 +0200 Subject: [PATCH 13/22] Update proposals/2778-appservice-login.md --- proposals/2778-appservice-login.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 567f84119c0..da93f32e07e 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -12,7 +12,7 @@ have used the bridge bot's device for all bridge users in the room, but this cau wish to only join ghosts to a room (e.g. for DMs). Another advantage this provides is that an appservice can now be used to generate access tokens for -any user in it's namespace without having to set a password for that user, which may be useful in the +any user in its namespace without having to set a password for that user, which may be useful in the case of software where maintaining password(s) in the configuration is undesirable. ## Proposal @@ -124,4 +124,4 @@ The proposal has been implemented by a homeserver, a bridge SDK and two bridges: - [synapse](https://github.com/matrix-org/synapse/pull/8320) - [mautrix-python](https://github.com/tulir/mautrix-python/commit/12d7c48ca7c15fd3ff61608369af1cf69e289aeb) - [mautrix-whatsapp](https://github.com/tulir/mautrix-whatsapp/commit/ead8a869c84d07fadc7cfcf3d522452c99faaa36) -- [matrix-appservice-bridge](https://github.com/matrix-org/matrix-appservice-bridge/pull/231/files#diff-5e93f1b51d50a44fcf0ca46ea1793c1cR851-R864) \ No newline at end of file +- [matrix-appservice-bridge](https://github.com/matrix-org/matrix-appservice-bridge/pull/231/files#diff-5e93f1b51d50a44fcf0ca46ea1793c1cR851-R864) From 0c26298b938961219391322c47b05f6d68198422 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 5 May 2021 18:27:07 +0100 Subject: [PATCH 14/22] Update proposals/2778-appservice-login.md Co-authored-by: Christian Paul --- proposals/2778-appservice-login.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index da93f32e07e..8e38c4a997b 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -63,7 +63,7 @@ then `/login` to generate the appropriate device. This proposal means that there will be more calls to make when setting up a appservice user, when using encryption. While this could be done during the registration step, this would prohibit creating -new devices should the appservice intentionally or inadvertently lost the client-side device data. +new devices should the appservice intentionally or inadvertently have lost the client-side device data. ## Alternatives From 538ac0940eab651e8d4e81d8de7bcff0634e5e2a Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 5 May 2021 18:45:33 +0100 Subject: [PATCH 15/22] Update Alternatives section --- proposals/2778-appservice-login.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 8e38c4a997b..e4583c00d59 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -67,21 +67,41 @@ new devices should the appservice intentionally or inadvertently have lost the c ## Alternatives +### 1. Include the token in the `/login` request body + One minor tweak to the current proposal could be to include the token as part of the auth data, rather than being part of the header/params to the request. An argument could be made for either, but since the specification expects the appservice to pass the token this way in all requests, including `/register`, it seems wise to keep it that way. -Some community members have used implementation details such as a "shared secret" authentication method to +### 2. Use implementation specific "shared secret" authentication + +Some community members have used homeserver implementation details such as a "shared secret" authentication method to log into the accounts without having to use the /login process at all. Synapse provides such a function, -but also means the appservice can now authenticate as any user on the homeserver. This seems undesirable from a +but also means the appservice can now authenticate as any user on the homeserver. This is undesirable from a security standpoint. +### 3. Keep using `/register` solely + A third option could be to create a new endpoint that simply creates a new device for an appservice user on demand. Given the rest of the matrix eco-system does this with /login, and /login is already extensible with `type`, it would create more work for all parties involved for little benefit. -Finally, `POST /register` does already return a `device_id`, `access_token` for appservice users by default. However critically +Finally, `POST /register` does already return a `device_id` and `access_token` so appservices +could store this information rather than calling `POST /login` at all. This does however present a few problems: + +- Quite a few appservices which only support unencrypted messaging do not use/store the `device_id`/`access_token` from a register call. + In the event that an appservice eventually gains the ability to support encryption, they would be unable to fetch a new `device_id`/ + `access_token` for any existing users (as `/register` would fail for an existing user). +- If user tokens were lost or exposed, there is no way to programattically create new access tokens for these users. +- Finally, if a user was registered externally and the appservice would like to masquerade as it, it would be unable to fetch + an access token for that user. + +While `POST /register` does work, it is impactical as the sole method of fetching an access token. + + +Most appservices +which do not implement encryption do not store this information as neither the device_id or access_token are needed f However critically this means that bridges will need to be designed to store the access_token and device_id from the point of creating the user, so older bridges would be unable to get an access token for existing users as `POST /register` would fail. It would difficult to log out these tokens if they got exposed additionally, as the AS would not be able to fetch a new access token. From b8ab3d0c87e4ef07eed3f9d8de0ae9636b566d86 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 5 May 2021 23:03:15 +0100 Subject: [PATCH 16/22] Remove point about Element showing unexpected devices --- proposals/2778-appservice-login.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index e4583c00d59..ce0828029fc 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -125,9 +125,6 @@ this is not a problem because: difference is that without a dedicated access token they are unable to receive device messages. While in theory this does make them unable to see encrypted messages, this is not designed to be a security mechanism. -- An appservice trying to log in as a user will always create a new device, which means the user would be informed of the - new device on their existing sessions. It should be very obvious if a malicous appservice is creating new devices on your account. - In conclusion this MSC only automates the creation of new devices for users inside an AS namespace, which is something a server admin could already do. Appservices should always be treated with care and so with these facts in mind the MSC should be considered secure. From c8e0ed60624d3ec1cbbd39c73810dbcb54c56b11 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 5 May 2021 23:10:00 +0100 Subject: [PATCH 17/22] Hopefully improve words around M_EXCLUSIVE --- proposals/2778-appservice-login.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index ce0828029fc..1fac4e171ec 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -17,9 +17,7 @@ case of software where maintaining password(s) in the configuration is undesirab ## Proposal -A new `type` is to be added to `POST /login`. - -`m.login.application_service` +A new `type` is to be added to `POST /login`: `m.login.application_service` The `/login` endpoint may now take an `access_token` in the same way that other authenticated endpoints do. No additional parameters should be specified in the request body. @@ -43,15 +41,16 @@ new flows. The response body should be unchanged from the existing `/login` specification. -If: +If one of the following conditions are true: - The access token is not provided - The access token does not correspond to a appservice - Or the user has not previously been registered -Then the servers should reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. +Then the servers MUST reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. -If the access token does not correspond to a appservice that manages this user, then the `errcode` should be `"M_EXCLUSIVE"`. +If the access token DOES correspond to a appservice but the user is not inside it's namespace, +then the `errcode` should be `"M_EXCLUSIVE"`. Homeservers should ignore the `access_token` parameter if a type other than `m.login.application_service` has been provided. From 93dd264c307875dc4f377591ef7427e59374ade5 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 5 May 2021 23:14:48 +0100 Subject: [PATCH 18/22] words --- proposals/2778-appservice-login.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 1fac4e171ec..5ec9335071f 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -6,14 +6,14 @@ do so, this proposal suggests implementing an appservice extension to the [`POST /login` endpoint](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-login). Appservice users do not usually need to login as they do not need their own access token, and do not -traditionally need a "device". However, E2E encryption demands that at least one user in a room has a Matrix device -which means bridge users need to be able to generate a device on demand. In the past, bridge developers -have used the bridge bot's device for all bridge users in the room, but this causes problems should the bridge -wish to only join ghosts to a room (e.g. for DMs). +traditionally need a "device". However, E2E encryption demands that at least one user in a room has a +Matrix device which means bridge users need to be able to generate a device on demand. In the past, +bridge developers have used the bridge bot's device for all bridge users in the room, but this causes +problems should the bridge wish to only join ghosts to a room (e.g. for DMs). Another advantage this provides is that an appservice can now be used to generate access tokens for -any user in its namespace without having to set a password for that user, which may be useful in the -case of software where maintaining password(s) in the configuration is undesirable. +any user in its namespace without having to set a password for that user, which may be useful where +maintaining password(s) in the configuration is undesirable. ## Proposal From 16290a0fe5f528a8ca8ce138aefb66e4063aa9a8 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 6 May 2021 11:12:39 +0100 Subject: [PATCH 19/22] it's --- proposals/2778-appservice-login.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 5ec9335071f..5b9e0a48a10 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -49,7 +49,7 @@ If one of the following conditions are true: Then the servers MUST reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. -If the access token DOES correspond to a appservice but the user is not inside it's namespace, +If the access token DOES correspond to a appservice but the user is not inside its namespace, then the `errcode` should be `"M_EXCLUSIVE"`. Homeservers should ignore the `access_token` parameter if a type other than @@ -108,7 +108,7 @@ Furthermore, the ability to generate access tokens for real users who registered ## Security considerations -Appservices could use this new functionality to generate devices for any userId that are within it's namespace e.g. setting the +Appservices could use this new functionality to generate devices for any userId that are within its namespace e.g. setting the user namespace regex to `@.*:example.com` would allow appservice to control anyone on the homeserver. While this sounds scary, in practise this is not a problem because: From f9a5b34099e29f1f22c74b8a7266f5c9da162479 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 12 May 2021 18:41:56 +0100 Subject: [PATCH 20/22] Clarify /register / /login --- proposals/2778-appservice-login.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 5b9e0a48a10..43c024aab83 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -55,8 +55,8 @@ then the `errcode` should be `"M_EXCLUSIVE"`. Homeservers should ignore the `access_token` parameter if a type other than `m.login.application_service` has been provided. -The expected flow for appservices would be to `/register` their users, and -then `/login` to generate the appropriate device. +Appservices creating **new** users can still use the `/register` endpoint to generate an `access_token` / `device_id` +but for existing users, the `/login` endpoint can be used instead. ## Potential issues From fb091fed039fef50ac9b551fa89db8f6592a458c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 22 Jun 2021 20:25:46 -0600 Subject: [PATCH 21/22] Apply suggestions from code review Co-authored-by: Hubert Chathi Co-authored-by: penn5 Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- proposals/2778-appservice-login.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 43c024aab83..62bf5b04974 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -5,7 +5,7 @@ need a way to generate devices for their users so that they can participate in E do so, this proposal suggests implementing an appservice extension to the [`POST /login` endpoint](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-login). -Appservice users do not usually need to login as they do not need their own access token, and do not +Appservice users do not usually need to log in as they do not need their own access token, and do not traditionally need a "device". However, E2E encryption demands that at least one user in a room has a Matrix device which means bridge users need to be able to generate a device on demand. In the past, bridge developers have used the bridge bot's device for all bridge users in the room, but this causes @@ -44,13 +44,13 @@ The response body should be unchanged from the existing `/login` specification. If one of the following conditions are true: - The access token is not provided -- The access token does not correspond to a appservice +- The access token does not correspond to an appservice - Or the user has not previously been registered Then the servers MUST reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. -If the access token DOES correspond to a appservice but the user is not inside its namespace, -then the `errcode` should be `"M_EXCLUSIVE"`. +If the access token DOES correspond to an appservice but the user is not inside its namespace, +then the `errcode` must be `"M_EXCLUSIVE"`. Homeservers should ignore the `access_token` parameter if a type other than `m.login.application_service` has been provided. @@ -109,10 +109,10 @@ Furthermore, the ability to generate access tokens for real users who registered ## Security considerations Appservices could use this new functionality to generate devices for any userId that are within its namespace e.g. setting the -user namespace regex to `@.*:example.com` would allow appservice to control anyone on the homeserver. While this sounds scary, in practise +user namespace regex to `@.*:example.com` would allow appservice to control anyone on the homeserver. While this sounds scary, in practice this is not a problem because: -- Appservice namespaces are mainained by the homeserver admin. If the namespace were to change, then it's reasonable +- Appservice namespaces are maintained by the homeserver admin. If the namespace were to change, then it's reasonable to assume that the server admin is aware. There is no defense mechanism to stop a malicious server admin from creating new devices for a given user's account as they could also do so by simply modifying the database. From ec2c1f6f5399cbb8295454c57e588fa7fbc8b662 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 22 Jun 2021 20:29:26 -0600 Subject: [PATCH 22/22] Remove what appears to be leftover notes --- proposals/2778-appservice-login.md | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/proposals/2778-appservice-login.md b/proposals/2778-appservice-login.md index 62bf5b04974..fd509ab8941 100644 --- a/proposals/2778-appservice-login.md +++ b/proposals/2778-appservice-login.md @@ -2,7 +2,7 @@ Appservices within Matrix are increasingly attempting to support End-to-End Encryption. As such, they need a way to generate devices for their users so that they can participate in E2E rooms. In order to -do so, this proposal suggests implementing an appservice extension to the +do so, this proposal suggests implementing an appservice extension to the [`POST /login` endpoint](https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-login). Appservice users do not usually need to log in as they do not need their own access token, and do not @@ -47,7 +47,7 @@ If one of the following conditions are true: - The access token does not correspond to an appservice - Or the user has not previously been registered -Then the servers MUST reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. +Then the servers MUST reject with HTTP 403, with an `errcode` of `"M_FORBIDDEN"`. If the access token DOES correspond to an appservice but the user is not inside its namespace, then the `errcode` must be `"M_EXCLUSIVE"`. @@ -95,16 +95,8 @@ could store this information rather than calling `POST /login` at all. This does - If user tokens were lost or exposed, there is no way to programattically create new access tokens for these users. - Finally, if a user was registered externally and the appservice would like to masquerade as it, it would be unable to fetch an access token for that user. - -While `POST /register` does work, it is impactical as the sole method of fetching an access token. - -Most appservices -which do not implement encryption do not store this information as neither the device_id or access_token are needed f However critically -this means that bridges will need to be designed to store the access_token and device_id from the point of creating the user, -so older bridges would be unable to get an access token for existing users as `POST /register` would fail. -It would difficult to log out these tokens if they got exposed additionally, as the AS would not be able to fetch a new access token. -Furthermore, the ability to generate access tokens for real users who registered elsewhere would not be possible with this mechanism. +While `POST /register` does work, it is impactical as the sole method of fetching an access token. ## Security considerations @@ -116,7 +108,7 @@ this is not a problem because: to assume that the server admin is aware. There is no defense mechanism to stop a malicious server admin from creating new devices for a given user's account as they could also do so by simply modifying the database. -- While an appservice *could* try to masquerade as a user maliciously without the server admin expecting it, it would still +- While an appservice *could* try to masquerade as a user maliciously without the server admin expecting it, it would still be bound by the restrictions of the namespace. Server admins are expected to be aware of the implications of adding new appservices to their server so the burden of responsibility lies with the server admin. @@ -125,7 +117,7 @@ this is not a problem because: does make them unable to see encrypted messages, this is not designed to be a security mechanism. In conclusion this MSC only automates the creation of new devices for users inside an AS namespace, which is something -a server admin could already do. Appservices should always be treated with care and so with these facts in mind the MSC should +a server admin could already do. Appservices should always be treated with care and so with these facts in mind the MSC should be considered secure. ## Unstable prefix