Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

We should never be sending contents of messages to push servers. (SYN-687) #1540

Closed
matrixbot opened this issue May 5, 2016 · 11 comments
Closed
Labels
A-Push Issues related to push/notifications

Comments

@matrixbot
Copy link
Member

matrixbot commented May 5, 2016

Splitting this out from #1360.

We simply shouldn't be ever sending the plain text of push notifications to APNS or GCM - instead we should send them a prod to fire up the app and receive the message contents via Matrix (and so apply E2E crypto to it, if we want to display it in the UI).

(Imported from https://matrix.org/jira/browse/SYN-687)

(Reported by @ara4n)

@matrixbot
Copy link
Member Author

Jira watchers: @ara4n

@matrixbot
Copy link
Member Author

matrixbot commented May 5, 2016

Links exported from Jira:

relates to SYAND-139
relates to SYIOS-194
relates to #1360

@matrixbot matrixbot changed the title We should never be sending contents of messages to push servers. (SYN-687) We should never be sending contents of messages to push servers. (https://github.com/matrix-org/synapse/issues/1540) Nov 7, 2016
@matrixbot matrixbot changed the title We should never be sending contents of messages to push servers. (https://github.com/matrix-org/synapse/issues/1540) We should never be sending contents of messages to push servers. (SYN-687) Nov 7, 2016
@flackr
Copy link

flackr commented Jul 21, 2019

The web push-api subscription includes an encryption key so that the push notification payload is not visible to the gateway. I'd love to see synapse implement a push gateway which supports the web push-api.

I think the same technique could be used for sending the notifications to APNS or GCM and then on the client side it would decrypt the message before displaying the notification. If so, this could potentially save a lot of battery over waking up the radio again when the push message is handled to fetch the payload.

@richvdh
Copy link
Member

richvdh commented Feb 5, 2020

hans't this been fixed?

@richvdh richvdh added the A-Push Issues related to push/notifications label Feb 5, 2020
@ptman
Copy link
Contributor

ptman commented Oct 1, 2020

I believe it has

@T3chTobi
Copy link

I hope it is fixed. It would be a major security thread...

@kescherCode
Copy link
Contributor

It's a config option.
image

@reivilibre
Copy link
Contributor

Clients can also request event_id_only pushes when setting up a pusher with their homeserver.
RiotX (if I remember) used to have a configuration slider — I can't see this in today's Element Android but looking at my homeserver's pushers table, it uses the event_id_only format (I think this may now be the only supported option).

Element iOS has been event_id_only for as long as I can remember, for what it's worth.

@dkasak
Copy link
Member

dkasak commented Oct 20, 2021

So it seems that all a client needs to do to not leak message contents is to use the event_id_only format when setting up a pusher. The Element clients already do this.

In addition to this, Synapse implements the include_content setting referred to in the screenshot above. Setting this to false makes Synapse withhold the content even if not using event_id_only. We currently default to true here. Could we change the default to false for good measure? (It's been suggested this might require a spec change, but I don't think this is the case as the Push Gateway API says the content field may be withheld for any reason)

Other than potentially changing that default, I don't think there's anything else left to do here.

@reivilibre
Copy link
Contributor

In addition to this, Synapse implements the include_content setting referred to in the screenshot above. Setting this to false makes Synapse withhold the content even if not using event_id_only. We currently default to true here. Could we change the default to false for good measure?

The problem is that, if we set this option to false, clients can't then opt-in to having content sent to their push gateways, and some clients DO use trustworthy push gateways (c.f. UnifiedPush).

All good clients that care about this privacy issue should already opt-in to the event ID-only format.

(We agree that the default is strange in the API — ideally you'd have to request content whilst setting up a pusher if you cared about it — but that would need a spec change, sadly.)

@callahad
Copy link
Contributor

Also, in E2EE rooms the server doesn't have access to message content, and thus can't disclose any plaintext to APNS / FCM.

If we redesigned push today, we'd avoid configuration and make event_id_only the default, but that ship has sailed.

Calling this sufficiently resolved to close.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Push Issues related to push/notifications
Projects
None yet
Development

No branches or pull requests

10 participants