-
Notifications
You must be signed in to change notification settings - Fork 385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSC4174: webpush push kind #4174
base: main
Are you sure you want to change the base?
Changes from all commits
24473e5
2eae5a3
0346332
201ddc9
2f71bf3
1f5b73c
e413f9e
faa3101
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
# MSC4174: Web Push pusher kind | ||
|
||
As stated in [MSC3013](https://github.com/matrix-org/matrix-spec-proposals/pull/3013), a first MSC about push notification encryption, that the present MSC is to replace: | ||
|
||
Push notifications have the problem that they typically go through third-party push providers in order to be delivered, | ||
e.g. FCM (Google) or APNs (Apple) and a push gateway (sygnal). In order to prevent these push providers and | ||
push gateways from being able to read any sensitive information the `event_id_only` format was introduced, which only | ||
pushes the `event_id` and `room_id` of an event down the push. After receiving the push message the client can hit the | ||
`GET /_matrix/client/r0/rooms/{roomId}/event/{eventId}` to fetch the full event, and then create the notification based | ||
on that. | ||
|
||
Even the `event_id_only` leaks some metadata that can be avoided. | ||
|
||
Today, web clients supporting push notifications (eg. hydrogen) needs to use a matrix to webpush gateway. This requires | ||
going over the specifications, because they use `endpoint`, and `auth` in the `PusherData` (hydrogen [[1]], sygnal [[2]]), | ||
while the specifications let understand that only `url` and `format` are allowed [[3]]. | ||
=> __PusherData already need to be updated__ to add `auth` and `endpoint`. | ||
|
||
Web Push is a standard for (E2EE) push notifications, defined with RFC8030+RFC8291+RFC8292 [[4]][[5]][[6]]: many libraries | ||
are already available and robuste: they are reviewed, and acknowledge by experts. | ||
|
||
Having a webpush push kind would provide push notifications without gateway to | ||
- Web app and desktop app | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Desktop apps which use Chromium, correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At least every Chromium based, Firefox based, and Safari There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So not really desktop apps, right? Not unless they're Electron or similar? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why, while the main providers of webpush servers are Firefox and Google for their web browsers nothing is stopping someone to host one themselves and use it in any context. Firefox one is open source. Actually you even can use Firefox official webpush server outside of any Firefox context. Sunup Android unified push provider is doing just that. |
||
- Android apps using UnifiedPush (MSC2970 was open for this and won't be required anymore) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Links showing UnifiedPush supports this would be helpful! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec will be updated next month, and will be defined as RFC8030+RFC8291+RFC8292 aka webpush Else, there is this comment: UnifiedPush/wishlist#15 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://unifiedpush.org/ Images shows Web Push now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please inline these into the proposal instead of adding them as comments. |
||
- Android apps using FCM (It is possible to push to FCM with webpush standard [[7]]) | ||
- Maybe other ? We have seen apple moving a lot into web push support [[8]] | ||
|
||
[1]: https://github.com/element-hq/hydrogen-web/blob/9b68f30aad329c003ead70ff43f289e293efb8e0/src/platform/web/dom/NotificationService.js#L32 | ||
[2]: https://github.com/matrix-org/sygnal/blob/main/sygnal/webpushpushkin.py#L152 | ||
[3]: https://spec.matrix.org/v1.9/client-server-api/#_matrixclientv3pushers_pusherdata | ||
[4]: https://www.rfc-editor.org/rfc/rfc8030 | ||
[5]: https://www.rfc-editor.org/rfc/rfc8291 | ||
[6]: https://www.rfc-editor.org/rfc/rfc8292 | ||
[7]: https://gist.github.com/mar-v-in/2a054e3a4c0a508656549fc7d0aaeb74#webpush | ||
[8]: https://developer.apple.com/documentation/usernotifications/sending-web-push-notifications-in-web-apps-and-browsers | ||
|
||
## Proposal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Examples of the request would be useful! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Client side, this is already ~ implemented by hydrogen just changing Server side, this has to be merged into the server: https://github.com/matrix-org/sygnal/blob/main/sygnal/webpushpushkin.py#L351 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the whole point to bypass sygnal though? Regardless, having examples in the MSC is important to show a full request/response. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely, I just wanted to say that most of the code in sygnal webpushpushkin can be used to write a synapse webpushpusher There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a requirement, but this is changing the architecture of Matrix, correct? By removing push gateways. Having an updated version of the diagram could be useful to show the simplification proposed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes them optional for some use cases, but it would still be needed for some others, for example Android and iOS apps both need a gateway because the push app key/certificate are per app, so some server side binding with the app needs to be done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It changes the architecture with this pushkind. It has the same architecture than Email pushkind.
This is actually possible to use webpush without a gateway for Android. Google does not document it and it requires to do the registration to the play services without firebase-messaging, but this is possible. Nevertheless, I think Android app will likely continue to use the gateway There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, that significantly reduces the usefulness of this. I was hoping we would be able to deprecate gateways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would it reduce usefulness?
Also, Apple may move to webpush too (or may need to with DSA, like Google). But the http pusher doesn't have to be removed (for iOS at least, and there might be some projects hacking with it) |
||
|
||
`PusherData` fields are now define as follow: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be clearer if the MSC only described the things it's adding, rather than redefining There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not very handy to not include the previous content. Every or |
||
- `format`: Required if `kind` is `http` or `webpush`, not used if `kind` is `email`. The format to send | ||
notifications in to Push Gateways. The details about what fields the homeserver should send to the push gateway | ||
are defined in the Push Gateway Specification. Currently the only format available is ’event_id_only'. | ||
- `url`: Required if `kind` is `http`, not used else. The URL to use to send notifications to. MUST be an | ||
HTTPS URL with a path of /_matrix/push/v1/notify | ||
- `endpoint`: Required if `kind` is `webpush`, not used else. The URL to send notification to, as defined as a | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`push resource` by RFC8030. MUST be an HTTPS URL. | ||
- `auth`: Required if `kind` is `webpush`, not used else. The authentication secret. This is 16 random bytes | ||
encoded in base64 url. | ||
|
||
The POST request to the endpoint dedicated to the creation, modification and deletin of pushers, | ||
`POST /_matrix/client/v3/pushers/set` now supports a new `kind`: `webpush`. | ||
- `kind`: Required: The `kind` of pusher to configure. `http` makes a pusher that sends HTTP pokes. `webpush` makes a | ||
pusher that sends Web Push encrypted messages. `email` makes a pusher that emails the user with unread notifications. | ||
`null` deletes the pusher. | ||
- `pushkey`: Required: This is a unique identifier for this pusher. The value you should use for this is the routing | ||
or destination address information for the notification, for example, the APNS token for APNS or the Registration ID | ||
for GCM. If your notification client has no such concept, use any unique identifier. Max length, 512 bytes. | ||
If the `kind` is "email", this is the email address to send notifications to. | ||
If the `kind` is `webpush`, this is the user agent public key encoded in base64 url. The public key comes from a ECDH | ||
keypair using the P-256 (prime256v1, cf. FIPS186) curve. | ||
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Links off to the sections of the RFCs that define how to do this would probably be good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see link to 25519 curve when ed25519 is mentioned, P-256 is a known curve. And this is already implemented in libraries |
||
|
||
A VAPID (Voluntary Application Server Identification, cf RFC8292) is often needed to be able to register with a push | ||
server. | ||
It is proposed to add a `m.webpush` capability to the `/capabilities` endpoint with this format: | ||
``` | ||
"m.webpush": { | ||
"enabled": true, | ||
"vapid": "BNbXV88MfMI0fSxB7cDngopoviZRTbxIS0qSS-O7BZCtG04khMOn-PP2ueb_X7Aeci42n02kJ0-JJJ0uQ4ELRTs" | ||
} | ||
``` | ||
It is also useful to decide if the client should register a pusher using `http` kind and and old style | ||
Sygnal WebPush semantic. A client that supports this kind of pusher should use it if the server supports it too, and | ||
not register another `http` pusher to avoid duplicate pushes. | ||
Comment on lines
+62
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following implementation experimentation this part has been added. |
||
|
||
## Potential issues | ||
|
||
While implemnting, one have to be carreful with RFC8291: many libraries use the 4th draft of this spec. Checking the | ||
Content-Encoding header is a good way to know if it the correct version. If the value is `aes128gcm`, then it uses | ||
the right specifications, else (`aesgcm`), then it uses the draft version. | ||
|
||
## Alternatives | ||
|
||
`pushkey` could be a random ID, and we can add `p256dh` in the `PusherData`. But it would require client to store it, | ||
while the public key already identify that pusher. And, client already use the PusherData that way. | ||
|
||
`vapid` parameter could be made optional considering it is officially not a requirement, however it seems | ||
existing big players push servers need it anyway to be able to subscribe, so it was decided to make it mandatory | ||
to avoid issues with those. | ||
|
||
## Security considerations | ||
|
||
Security considerations are listed by RFC8030 [[9]], there are mainly resolved with RFC8291 (Encryption) and | ||
RFC8292 (VAPID). | ||
|
||
Like any other federation request, there is a risk of SSRF. This risk is limited since the post data isn't | ||
arbitrary (the content is encrypted), and a potential malicious actor don't have access to the response. | ||
Nevertheless, it is recommended to not post to private addresses, with the possibility with a setting to | ||
whitelist a private IP. (Synapse already have ip_range_whitelist [[10]]) | ||
It is also recommended to not follow redirection, to avoid implementation issue where the destination is check | ||
before sending the request but not for redirections. | ||
|
||
Like any other federation request, there is a risk of DOS amplification. One malicious actor register many users | ||
to a valid endpoint, then change the DNS record and target another server, then notify all these users. This | ||
amplification is very limited since HTTPS is required and the TLS certificate of the target will be rejected. The | ||
request won't reach any functionnality of the targeted application. The home server can reject pusher if the response | ||
code is not one intended. | ||
|
||
[9]: https://www.rfc-editor.org/rfc/rfc8030#section-8 | ||
[10]: https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#ip_range_whitelist | ||
|
||
## Unstable prefix | ||
|
||
- Until this proposal is considered stable, implementations must use | ||
`org.matrix.msc4174.webpush` instead of `m.webpush`. | ||
|
||
## Dependencies | ||
|
||
- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation requirements:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@turt2live These requirements are now met
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client (Hydrogen) and Server (Synapse) implementations noted at #4174 (comment)