-
Notifications
You must be signed in to change notification settings - Fork 388
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
MSC3013: Encrypted Push #3013
base: old_master
Are you sure you want to change the base?
MSC3013: Encrypted Push #3013
Changes from 1 commit
e835d5e
c57fd71
9677a5b
24686ca
c46a21c
7b45e05
d43e5ff
0fe441f
6d8e7ff
5e5d979
b66bad3
417591f
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,171 @@ | ||
# MSC3013: Encrypted Push | ||
|
||
Push notifications have the problem that they typically go through third-party gateways in order to | ||
be delivered, e.g. FCM (Google) or APNs (Apple) and an app-specific gateway (sygnal). In order to | ||
prevent these 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. | ||
|
||
This, however, introduces the issue of having to perform an additional HTTP request to be able to | ||
display the full event notification. On some systems (e.g. some weird vendor-specific android phones, | ||
or while driving through ~~rural germany~~ places with patchy cellular network availability) this isn't | ||
always. | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This proposal adds a method to encrypt the push message in a way only the recipient client can decrypt | ||
it, allowing the server to send the full event over push again, with this MSC and [MSC2782](https://github.com/matrix-org/matrix-doc/pull/2782). | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Proposal | ||
|
||
A new pusher kind `http.encrypted.curve25519-aes-sha2` is introduced. It behaves the same as the pusher | ||
kind `http`, except that the content it pushes is encrypted. That means it inherits all the pusher | ||
data requirements, all the formats etc. It adds a new pusher data field, `public_key`, which is a | ||
(optionally unpadded) base64-encoded curve25519 public key. This new field is not to be added to the | ||
actual push payload being sent to push gateways. As such, setting such a pusher could look as | ||
following: | ||
|
||
``` | ||
POST /_matrix/client/r0/pushers/set HTTP/1.1 | ||
Content-Type: application/json | ||
|
||
{ | ||
"lang": "en", | ||
"kind": "http.encrypted.curve25519-aes-sha2", | ||
"app_display_name": "Mat Rix", | ||
"device_display_name": "iPhone 9", | ||
"profile_tag": "xxyyzz", | ||
"app_id": "com.example.app.ios", | ||
"pushkey": "APA91bHPRgkF3JUikC4ENAHEeMrd41Zxv3hVZjC9KtT8OvPVGJ-hQMRKRrZuJAEcl7B338qju59zJMjw2DELjzEvxwYv7hH5Ynpc1ODQ0aT4U4OFEeco8ohsN5PjL1iC2dNtk2BAokeMCg2ZXKqpc8FXKmhX94kIxQ", | ||
"data": { | ||
"url": "https://push-gateway.location.here/_matrix/push/v1/notify", | ||
"format": "event_id_only", | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"public_key": "GkZgmbbxnYZfFtywxF4K7NUPqA50Kb7TEsyHeVWyHBI" | ||
}, | ||
"append": false | ||
} | ||
``` | ||
|
||
Now, when the homeserver pushes out the message, it is to perform the `notification` dict as with the | ||
http pusher, and then encrypt all of its contents, apart from the `devices` key, using the following | ||
algorithm: | ||
|
||
1. Generate an ephemeral curve25519 key, and perform an ECDH with the ephemeral key and the backup's | ||
public key to generate a shared secret. The public half of the ephemeral key, encoded using unpadded | ||
base64, becomes the `ephemeral` property of the new payload. | ||
2. Using the shared secret, generate 80 bytes by performing an HKDF using SHA-256 as the hash, with | ||
a salt of 32 bytes of 0, and with the empty string as the info. The first 32 bytes are used as the | ||
AES key, the next 32 bytes are used as the MAC key, and the last 16 bytes are used as the AES | ||
initialization vector. | ||
3. Stringify the JSON object, and encrypt it using AES-CBC-256 with PKCS#7 padding. This encrypted | ||
data, encoded using unpadded base64, becomes the `ciphertext` property of the new payload. | ||
4. Pass the raw encrypted data (prior to base64 encoding) through HMAC-SHA-256 using the MAC key | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
generated above. The first 8 bytes of the resulting MAC are base64-encoded, and become the `mac` | ||
property of the new payload. | ||
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's worth mentioning that the server should immediately drop the ephemeral secret key and shared secret after each use (or at least regularly regenerate them). If either secret is leaked then the push content will be decryptable.
Comment on lines
+85
to
+89
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. could we use AES-GCM or something modern rather than AES-CBC, for improved performance and to save having to make a separate MAC? (olm/megolm avoided GCM because GCM wasn't terribly widely supported at the time - I don't think that's true any 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. This code was intentionally designed to be similar to the current pk encryption in libolm, just without the bugs in it. But switching to a different cryptographic primitive solves that too, I guess. But it would need some of the crypto experts approval then. 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 only other (standard) use for libolm's pkencryption is in key backups, and we're moving away from that to symmetric key backup. So being consistent with other things is probably less of a concern. AES-GCM should be fine. |
||
|
||
This is the same algorithm used currently in the unstable spec for megolm backup, as such it is | ||
comptible with libolms PkEncryption / PkDecryption methods. | ||
Comment on lines
+91
to
+92
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. Note that we're now moving away from this though, towards symmetric encryption for key backup. It would also be good to have some words on why this encryption scheme was chosen & what cryptographic properties it gives (and doesn't give). 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. as far as soru is aware the PkEncryption / PkDecryption was also used for asymmetric SSSS, and it seems like a good idea to have some go-to symmetric / asymmetric algorithms to use for the matrix world, though that is obviously beyond the scope of this PR. 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. If you meant symmetric vs asymmetric cryptography, there is a paragraph on that under "Security considerations" 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. Another thing to note is that libolms PkEncryption / PkDecryption actually has a bug where it calculates the mac wrong (which soru discovered while working on encrypted push, uhoreg is aware about and said it is ok to talk about publicly as it only affects data integrity and not security itself), so until libolm gets fixed in that regard implementations need to do their own PkDecryption anyways |
||
|
||
### Example: | ||
Suppose a normal http pusher would push out the following content: | ||
```json | ||
{ | ||
"notification": { | ||
"event_id": "$3957tyerfgewrf384", | ||
"room_id": "!slw48wfj34rtnrf:example.com", | ||
"type": "m.room.message", | ||
"sender": "@exampleuser:matrix.org", | ||
"sender_display_name": "Major Tom", | ||
"room_name": "Mission Control", | ||
"room_alias": "#exampleroom:matrix.org", | ||
"prio": "high", | ||
"content": { | ||
"msgtype": "m.text", | ||
"body": "I'm floating in a most peculiar way." | ||
}, | ||
"counts": { | ||
"unread": 2, | ||
"missed_calls": 1 | ||
}, | ||
"devices": [ | ||
{ | ||
"app_id": "org.matrix.matrixConsole.ios", | ||
"pushkey": "V2h5IG9uIGVhcnRoIGRpZCB5b3UgZGVjb2RlIHRoaXM/", | ||
"pushkey_ts": 12345678, | ||
"data": {}, | ||
"tweaks": { | ||
"sound": "bing" | ||
} | ||
} | ||
] | ||
} | ||
} | ||
``` | ||
|
||
The following object would have to be json-encoded to encrypt: | ||
|
||
```json | ||
{ | ||
"event_id": "$3957tyerfgewrf384", | ||
"room_id": "!slw48wfj34rtnrf:example.com", | ||
"type": "m.room.message", | ||
"sender": "@exampleuser:matrix.org", | ||
"sender_display_name": "Major Tom", | ||
"room_name": "Mission Control", | ||
"room_alias": "#exampleroom:matrix.org", | ||
"prio": "high", | ||
"content": { | ||
"msgtype": "m.text", | ||
"body": "I'm floating in a most peculiar way." | ||
}, | ||
"counts": { | ||
"unread": 2, | ||
"missed_calls": 1 | ||
} | ||
} | ||
``` | ||
|
||
Resulting in the following final message being pushed out to the push gateway: | ||
|
||
```json | ||
{ | ||
"notification": { | ||
"ephemeral": "base64_of_ephemeral_public_key", | ||
"ciphertext": "base64_of_ciphertext", | ||
"mac": "base64_of_mac", | ||
"devices": [ | ||
{ | ||
"app_id": "org.matrix.matrixConsole.ios", | ||
"pushkey": "V2h5IG9uIGVhcnRoIGRpZCB5b3UgZGVjb2RlIHRoaXM/", | ||
"pushkey_ts": 12345678, | ||
"data": {}, | ||
"tweaks": { | ||
"sound": "bing" | ||
} | ||
} | ||
] | ||
} | ||
} | ||
``` | ||
|
||
## Potential issues | ||
|
||
It is currently implied that a homeserver could push the same notification out to multiple devices | ||
at once, by populating the `devices` array with more than one element. Due to the nature of cryptography, | ||
this won't be possible anymore. | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
It is still unclear how well this will work with iOS and its limitations, especially concerning badge-only | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
updates if a message was read on another device. | ||
|
||
If the gateway does additional processing, like marking call attempts differently, the relevant data | ||
musn't be encrypted. | ||
|
||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Security considerations | ||
|
||
In a first draft symmetric encryption was used. However, using asymmetric encryption seams like the | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
proper way to go here, as, in the case of the server being compromised, there is no need to re-negotiate | ||
a new key to encrypt the push message. | ||
|
||
## Unstable prefix | ||
|
||
Is this needed here? If so, how? Soru is kinda confused as the pusher kinds are just `http` and `email`, | ||
not having any `m.` prefix. |
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.
@dbkr
This isn't only good for non-e2ee rooms, but also good for them, as it means that further down the road, building ontop of this, we can easily tackle on more metadata into the push payload. Together with #3014 and a yet un-written MSC to add unread information per room, this would allow to display full push contents without having to perform any additional HTTP request, thus working significantly more reliable in sketchy network areas.
This additional usecase is actually also outlined in the introduction ^^"
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.
Can these details (seen in other comments) be written somewhere in the MSC?
For non-e2ee room it would allow to put content of the notification and then avoid additional http call.
But for e2e room it might not help as anyhow the client might need to sync (additional http call) the to_devices to get the megolm key.
On the benefit side it could allow to encrypt room_id/event_id metada, but one could arg that we could just remove them from the payload given that most client will sync on push anyhow.
Some potential problems: Not all push do a notification; clients have to process the push rules locally to know if it should notify.