Skip to content
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

MSC3884: Sliding Sync Extension: E2EE #3884

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

MSC3884: Sliding Sync Extension: E2EE #3884

wants to merge 9 commits into from

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Sep 6, 2022

@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Sep 6, 2022
@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Sep 7, 2022
"left": ["@bob:example.com"]
},
"device_unused_fallback_key_types": [
"signed_curve25519"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field has to always be included, otherwise there is no way for the client to tell, if the fallback key got used.

HOWEVER:

You can actually probably solve that with the sliding sync API, by having the client explicitly tell the server about what fallback keys it wants to know about, when they are used. Then the server would have to include them everytime when they are used, but could not include them in the common case of them being unused. The same could be done for the one time keys count, where you specify a minimum number you want to have and if you have those, the server doesn't update you about it.

This is however a bit racy, as a client might be told about a used key, while it is still uploading a key. But I think that is already an issue we have today.

Basically the request parameters would then look something like this:

{
  "enabled": true,
  "minimum_device_one_time_keys_count": {
    "signed_curve25519": 50 // tell the server to notify the client, if the OTKs fall below this limit
  },
  "fallback_key_types": [ "signed_curve25519" ] // tell the server to notify the client, if no such key is present
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably requires the server to tell the client, if a key type is unsupported though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related spec issue: #3298

Unsupported keys are hard to distinguish from "no change".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea overall, especially with the "min" values. I'd be interested to hear if this makes things any easier on clients, or just saves a bit of bandwidth.

"signed_curve25519": 50
},
"device_lists": {
"changed": ["@alice:example.com"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the semantics for this? When do I get a "changed" event? Only for device lists that changed during this connection? Or does this work on the delta token? Maybe this should have its own since and next batch token? Key data is actually a LOT of data, so invalidating that on every reconnect would be pretty scary. And not being told your device list is possibly missing changed events, because the delta token expired is equally scary.

Copy link
Member Author

@kegsay kegsay Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics are the same as sync v2 regarding getting a "changed" or "left" event.

Currently, the proxy will aggregate changed/left responses from sync v2 then deliver this to the client, only purging the arrays server-side once the client has acknowledged the response. This does mean that if you're offline for a while, these arrays can be large. This means we will always tell you of a change in a user's device lists, as this aggregation process happens outside the connection scope (it's constantly accumulated in the proxy from the moment you first start syncing until the end of time).

We don't currently have delta token support in this extension, but it might be worthwhile to add it if we can get the semantics right.

Also worth noting that in practice the size of these arrays isn't a large part of the overall sync response, even if you have been offline for a while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is mostly, that it would be easy to get this wrong on the client side, if you don't treat all devices as changed after a reconnection, where the server doesn't send you change lists from before you reconnected. Currently clients mostly rely on getting all change events as long as they don't do a new initial sync, but with sliding sync the "initial sync" could possibly happen quite often and by accident and in that case you would need to treat ALL keys as stale. You can probably implement that correctly in a client, but some more explicit notification, that your delta token wasn't used and your whole lists should be treated as stale might be harder to get wrong.

@turt2live turt2live added the matrix-2.0 Required for Matrix 2.0 label Feb 27, 2023
@turt2live turt2live requested review from a team and turt2live February 28, 2023 02:38
@turt2live turt2live removed request for a team and turt2live June 6, 2023 19:48
MadLittleMods added a commit to element-hq/synapse that referenced this pull request May 7, 2024
Based on:

 - MSC3575: Sliding Sync (aka Sync v3): matrix-org/matrix-spec-proposals#3575
 - MSC3885: Sliding Sync Extension: To-Device messages: matrix-org/matrix-spec-proposals#3885
 - MSC3884: Sliding Sync Extension: E2EE: matrix-org/matrix-spec-proposals#3884
MadLittleMods added a commit to element-hq/synapse that referenced this pull request May 23, 2024
This is being introduced as part of Sliding Sync but doesn't have any sliding window component. It's just a way to get E2EE events without having to sit through a big initial sync  (`/sync` v2). And we can avoid encryption events being backed up by the main sync response or vice-versa.

Part of some Sliding Sync simplification/experimentation. See [this discussion](#17167 (comment)) for why it may not be as useful as we thought.

Based on:

 - matrix-org/matrix-spec-proposals#3575
 - matrix-org/matrix-spec-proposals#3885
 - matrix-org/matrix-spec-proposals#3884
H-Shay pushed a commit to H-Shay/hq_synapse that referenced this pull request May 31, 2024
…t-hq#17167)

This is being introduced as part of Sliding Sync but doesn't have any sliding window component. It's just a way to get E2EE events without having to sit through a big initial sync  (`/sync` v2). And we can avoid encryption events being backed up by the main sync response or vice-versa.

Part of some Sliding Sync simplification/experimentation. See [this discussion](element-hq#17167 (comment)) for why it may not be as useful as we thought.

Based on:

 - matrix-org/matrix-spec-proposals#3575
 - matrix-org/matrix-spec-proposals#3885
 - matrix-org/matrix-spec-proposals#3884
Mic92 pushed a commit to Mic92/synapse that referenced this pull request Jun 14, 2024
…t-hq#17167)

This is being introduced as part of Sliding Sync but doesn't have any sliding window component. It's just a way to get E2EE events without having to sit through a big initial sync  (`/sync` v2). And we can avoid encryption events being backed up by the main sync response or vice-versa.

Part of some Sliding Sync simplification/experimentation. See [this discussion](element-hq#17167 (comment)) for why it may not be as useful as we thought.

Based on:

 - matrix-org/matrix-spec-proposals#3575
 - matrix-org/matrix-spec-proposals#3885
 - matrix-org/matrix-spec-proposals#3884
MadLittleMods added a commit to element-hq/synapse that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:core MSC which is critical to the protocol's success matrix-2.0 Required for Matrix 2.0 proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants