-
Notifications
You must be signed in to change notification settings - Fork 372
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
MSC1796: improved e2e notifications #1796
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
8ef0bcc
MSC1796: improved e2e notifications
ara4n 4545f00
improve wording
ara4n 75db256
spell out that custom keywords need *everything* synced
ara4n a50b759
english
ara4n b771745
propose m.visible
ara4n d6f2b8f
Update proposals/1796-e2e-notifications.md
aaronraimist File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
# Proposal for improving notifications for E2E encrypted rooms | ||
|
||
## Problem | ||
|
||
Currently we have no way of receiving accurate notification counts for E2E | ||
encrypted rooms. This is because the server can't calculate the notification | ||
counts serverside because it can't read the contents of the messages to spot | ||
the notification keywords. | ||
|
||
We mitigate this problem to some extent for push notifications by having clients | ||
sync and decrypt all messages for their rooms when running in the background | ||
(which on iOS and Android is intended to be always running in the background), | ||
and then emitting local notifications for keywords. | ||
|
||
However, this is also not perfect: it consumes significant bandwidth & CPU | ||
whilst running in the background, as it has to download and decrypt every | ||
message whether or not that message is actually a notification. Also, if the | ||
app crashes, is terminated by the OS due to memory pressure, or if the client is | ||
offline, notifications will get lost unless the client backpaginates *all* | ||
traffic since it last synced (which we currently don't do, given the potential | ||
bandwidth, CPU, storage & time impact). | ||
|
||
This issue has been historically tracked at | ||
https://github.com/vector-im/riot-web/issues/2256 | ||
|
||
This should not be confused with the trivial Riot/Web bug which fails to make | ||
it highlight or play sounds for notifs in E2E rooms even when it can calculate | ||
them locally: https://github.com/vector-im/riot-web/issues/7489 | ||
|
||
## Solution | ||
|
||
We provide two classes of solution: the common case (which supports notifying | ||
for 'mentions'), and a rare case (which supports for notifying for custom | ||
configured keywords). | ||
|
||
### Reliable handling of mention notifications | ||
|
||
For notifying for mentions, we propose adding an `m.mentions` field | ||
which resides in the `contents` of `m.room.encrypted` events. This contains an | ||
array of the mxids who are mentioned in the body of a given event (if any). | ||
This leaks the metadata of who is mentioning who, but we consider this an | ||
acceptable risk given the read receipts, read markers, presence and sync traffic | ||
patterns already leak considerable visible metadata about who is talking to | ||
who. | ||
|
||
In practice, this would look like: | ||
|
||
```json | ||
{ | ||
"type": "m.room.encrypted", | ||
"content": { | ||
"algorithm": "m.megolm.v1.aes-sha2", | ||
"ciphertext": "AwgOEpABcEOmm0RXiNetf3+MwULKGsUxvpBeA+LpULBHIJe4O/N....", | ||
"device_id": "QEOYHMYOKQ", | ||
"sender_key": "1cBKte3VktfCJfKcK7L6REHR1Ng8jgA56Zhma9o0/js", | ||
"session_id": "kcZWPc2zqgJzaOYCNOLYMlfpe4APN6IGtPmJm/QJa9s", | ||
"m.mentions": [ | ||
"@matthew:matrix.org", | ||
"@Amandine:matrix.org" | ||
] | ||
} | ||
} | ||
``` | ||
|
||
This gives the server the necessary data to correctly calculate missed notifications | ||
due to mentions, even in E2E rooms. It also lets the server explicitly send badge | ||
update push notifications for missed mentions via APNS/FCM, even if the client | ||
is not running in the background. | ||
|
||
As a special case, `@room` is also allowed as a possible value of `m.mentions`, | ||
indicating when the message is attempting to notify the whole room. | ||
|
||
One possible extension here is to also define a `m.visible` field on the contents, | ||
which would be a boolean to let servers track whether a given message should | ||
contribute to unread message counts or not and so calculate unread room state | ||
and counts serverside rather than clientside. | ||
(See https://github.com/matrix-org/synapse/issues/2632) | ||
|
||
### Better handling of custom keyword notifications | ||
|
||
The only way to safely notify for per-user specified custom keywords is for | ||
the recipient to decrypt messages and scan them for keywords clientside. This | ||
means downloading **all** the history for rooms where we care about custom | ||
keywords to scan for missed ones. Specifically, the client would have to | ||
backpaginate any gappy syncs it saw (e.g. after returning from being offline) | ||
and potentially consume significant bandwidth/CPU/time as it caught up in the | ||
background on what it had missed. | ||
|
||
The good news is that custom keywords are fairly rare, and we can make this a | ||
user choice - i.e. warn them in room settings that their custom keyword notifs | ||
(if they have any) will not currently work unless they consciously burn | ||
bandwidth to maintain an ongoing full clientside copy of the room. | ||
|
||
The better news is that clients will also need to be doing this if they want to | ||
provide clientside search of E2E rooms, so this isn't that unreasonable thing to | ||
prompt the user to do, per room. For instance, then they first try to search in | ||
the room, it could prompt to turn on full sync. Similarly, if they have keyword | ||
notifs set, it can nag them to turn full sync. There are large parallels with | ||
configuring an IMAP client like Thunderbird to either download and index all | ||
mail clientside or not. | ||
|
||
## Tradeoffs | ||
|
||
The main two possible approaches here are either to decorate messages with | ||
plaintext metadata to aid counts/pushes, or to sync everything to the client | ||
and decrypt it here. | ||
|
||
Compromising by using both styles for different classes of traffic is hopefully | ||
a satisfactory tradeoff. | ||
|
||
It's possible there's some homomorphic encryption approach here where the server | ||
could calculate the sum of notifs without knowing what the actual number was and | ||
sending the encrypted result to the client to figure out, but this feels | ||
overengineered, difficult, likely bandwidth intensive, and doesn't solve the | ||
problem of the server needing to detect pushes. | ||
|
||
## Security considerations | ||
|
||
Malicious clients might lie about the notification metadata in order to spam | ||
users (or hide mentions). However, the risk of spam is not much worse than | ||
the user sending spam of any other kind (e.g. non-e2e spam which mentions the | ||
user in the body). Arguably the ability to hide mentions is actually a feature, | ||
and could save having to use l33tsp34k versions of displaynames when referencing | ||
someone and not wanting to bing them. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
leaving this here so I don't forget later:
My overall concern with this proposal is that it proposes that notifications still be handled server-side. Given the direction of e2e and extensible events, I continue to be convinced that server-side unread/notification counts are not the right approach. Instead, clients should be responsible for tracking this information as they are the best part of the stack to determine what the numbers should be.
Doing client-side notifications/unread counts does mean that clients are more complicated and duplicating efforts, however the server cannot possibly be expected to be as reliable anymore. I think it's better worth our time making it easier for clients to calculate counts and set expectations for how they work rather than continue to try and keep this server-side.
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.
That's not really the point @ara4n is making though. The point is that in the best case it just chews bandwidth and CPU (and hence battery). In the worst case, it is imperfect (read: misses notifications!!) because of the need to resync potentially large amounts of data which isn't feasible to do currently. This seems like a good use case for the server to do this work to me. Yes, there are privacy concerns, but it opens no new security fronts given the amount of metadata already being passed through the server's hands in the clear.