Conversation
|
Hi Should backup's public key be pusher's public key? Thanks |
|
MSC 3079: Low Bandwidth CS API may be an alternative to this as it makes it viable to rely on |
|
I think this and #3079 are likely to be complementary. If you are already using your platforms push provider running a permanent background process is still going to be more expensive than using the push provider, no matter how well optimised it is. #3979 with the extra work to add sync filters just for events which trigger push is a good solution to if you don't have (or don't want) and untrusted push provider. |
|
Indeed, they can be complementary but at the cost of an increased API surface. If you don't trust your push provider, that has knock on concerns though. Malicious push providers can do more subtle attacks than just sniff the message (e.g delay the delivery) so the point the provider is untrusted then really you shouldn't be using it at all. |
a flag to the http pusher kind
|
Removing the |
|
Have put this in the "initial review" column on the SCT Backlog board to get eyes on it, though it may already be under the watchful purview of @dbkr. Feel free to take it off the board if you want to re-claim it Dave. |
dbkr
left a comment
There was a problem hiding this comment.
This does add significant complexity (especially because it means Homeservers now have to do elliptic curve crypto too) but it does slot nicely into the gap for more secure (and/or more efficient) push in non-e2e rooms.
Would suggest the next step here is for someone with a crypto hat on to review the crypto side of it. It sounds like it would be quite heavyweight, equivalent to establishing a new megolm session per message?
| This is the same algorithm used currently in the unstable spec for megolm backup, as such it is | ||
| comptible with libolms PkEncryption / PkDecryption methods. |
There was a problem hiding this comment.
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.
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.
If you meant symmetric vs asymmetric cryptography, there is a paragraph on that under "Security considerations"
There was a problem hiding this comment.
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
There was a problem hiding this comment.
See my comment about HPKE here: #3013 (comment).
Which will be used in MSC4388.
| @@ -0,0 +1,209 @@ | |||
| # MSC3013: Encrypted Push | |||
There was a problem hiding this comment.
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.
|
Posting here, I commented on a resolved comment.. A (robust IMO) alternative would be to add a webpush pusher kind directly. The advantage of using web push directly:
Edit: To add a bit more context, the specifications already need to be updated:
PS: I'm OK to write an MSC for webpush pushkind. |
|
I think an MSC for a web push pusher would be useful down the road. Feel free to write it up while you're thinking about it @p1gp1g. |
|
Here it is: #4174 |
|
Team member @richvdh has proposed to close this. The next step is review by the rest of the tagged people: Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for information about what commands tagged team members can give me. |
|
MSC4174 doesn't replace this properly due to #4174 (comment). Web push only supports P-256, but if we want to actually send message contents over push, we should use better crypto than that (although I'm not sure if the crypto suggested in this MSC is good either) |
To clarify a bit: the content of E2EE encrypted messages are E2EE with the protocol supported by matrix (Olm/Megolm/MLS potentially), we are talking about metadata only here |
|
The goal of this MSC appears to be combining with #2782 to send full event content in pushes, not only metadata. |
|
In E2EE communications, the full content send via push notification is encrypted content and we only encrypt its metadata on top of that |
| # 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 |
There was a problem hiding this comment.
| be delivered, e.g. FCM (Google) or APNs (Apple) and an app-specific gateway (sygnal). In order to | |
| be delivered, e.g. FCM (Google) or APNs (Apple) and an app-specific gateway (such as [Sygnal](https://github.com/element-hq/sygnal)). In order to |
| } | ||
| ``` | ||
|
|
||
| Now, when the homeserver pushes out the message, it is to perform the `notification` dict as with the |
There was a problem hiding this comment.
Maybe it's just me but "perform" strikes me as slightly confusing here.
| Now, when the homeserver pushes out the message, it is to perform the `notification` dict as with the | |
| Now, when the homeserver pushes out the message, it is to construct the [`notification`](https://spec.matrix.org/v1.17/push-gateway-api/#post_matrixpushv1notify_request_notification) dict as with the |
| 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 |
There was a problem hiding this comment.
I think you also need vendor prefixes for the various new properties such as public_key?
| } | ||
| ``` | ||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
The maximum payload size on both APNS and FCM seems to be 4096 bytes which might be exceeded based on the size of the event contained in the notification. This is already a problem today when not using event_id_only but I assume it's somewhat aggravated here since encryption will increase the overall payload size?
| The field `public_key` is required. This key is an unpadded base64-encoded curve25519 | ||
| public key. This new field is not to be added to the actual push payload being sent to push gateways. | ||
|
|
||
| The field `counts_only_type` is an optional enum which denotes how push frames should handle counts-only |
There was a problem hiding this comment.
It's possibly just me but I'm finding the term "push frame" slightly irritating and I haven't met this terminology elsewhere yet. Can we not just say "push notification"?
It's unclear that MSC4174 does fully replace this MSC; however this MSC appears to be without an owner and has some substantial concerns. So I'm going to leave the FCP close proposal in place. |
|
I do not believe that MSC4174 replaces this, as we do not want to use the encryption scheme presented there with other push methods (which we are only accepting in MSC4174 because it comes as part of the Web Push bundle of standards and implementations). However, we also will want to use a different encryption scheme than what is currently presented here, and at this point, it's probably better to start with a new MSC when we're ready to define how to do that, so I agree with FCP closing at this point. |
Rendered
Signed-off-by: Sorunome mail@sorunome.de
Signed-off-by: Sorunome sorunome@famedly.com
Synapse PR: matrix-org/synapse#11512
FamedlySDK PR: https://gitlab.com/famedly/company/frontend/famedlysdk/-/merge_requests/868
Hedwig PR: https://gitlab.com/famedly/company/backend/services/hedwig/-/merge_requests/41
SCT Stuff:
FCP callFCP close tickyboxes