MSC4186: Simplified Sliding Sync

[erikjohnston]

Rendered

---

SCT Stuff:

FCP tickyboxes

MSC checklist

[turt2live]

Implementation requirements:

• Client (ideally multiple)
• Server

[anoadragon453]

Current implementations to my knowledge:

• Client implementation: matrix-rust-sdk (PR)
• Server implementation: has been implemented in Synapse across 10s of PRs. (Perhaps @MadLittleMods, the implementer, can give better link(s) here.)

[MadLittleMods]

Synapse server implementation

Maybe it's easiest to point to the main directories where the code lives:

• element-hq/synapse -> synapse/rest/client/sync.py#L782
• element-hq/synapse -> synapse/handlers/sliding_sync/
• element-hq/synapse -> tests/rest/client/sliding_sync/

The PRs between @MadLittleMods and @erikjohnston with the ~A-Sync label are also a pretty good encapsulation:

• https://github.com/element-hq/synapse/pulls?q=+is%3Apr+author%3AMadLittleMods+label%3AA-Sync
• https://github.com/element-hq/synapse/pulls?q=+is%3Apr+author%3Aerikjohnston+label%3AA-Sync

[x86pup]

conduwuit has implemented simplified sliding sync in x86pup/conduwuit#666

[richvdh]

@mscbot resolve unresolved threads

[anoadragon453]

All seems sane to me, other than some clarifying questions.

[anoadragon453]

In light of MSC4492, it would be worth specifying now that pos tokens are intended to be directionless.

[erikjohnston]

I think MSC4492 is a good illustration as to why I'm slightly nervous to spell out the exact properties. The intent here is that it works in the same as /v3/sync, and MSC4492 changes(/clarifies) the /v3/sync behaviour when/if it lands. Alternatively, we might decide MSC4492 isn't the right way forwards and tokens should encode direction, at which point we'd still want this MSC to align with /v3/sync behaviour.

Fundamentally, this MSC is not trying to change any behaviour around usages of tokens. We could add non-normative sections to remind people what the properties are at time of writing, but 🤷

[anoadragon453]

Should this be removed? Has Synapse been fixed?
This won't make it into the spec PR anyhow.

[erikjohnston]

I think as a non-normative flag about where the current Synapse implementation diverges from the MSC is helpful here. We're not fixing up the Synapse implementation until after this MSC lands.

[mscbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[jevolk]

Implementation	timeline array	invited-room state field	stripped-state element type
MSC4186 schema	timeline_events	stripped_state	StrippedState, never defined
Synapse	timeline	invite_state	stripped events via strip_event
Zendrite	timeline	both invite_state and stripped_state	ClientEvent
ruma (Tuwunel, Conduit, Continuwuity)	timeline	invite_state	AnyStrippedStateEvent
matrix-rust-sdk	consumes timeline	reads invite_state	AnyStrippedStateEvent via ruma
matrix-js-sdk	timeline	invite_state	IStrippedState

The schema names the timeline array timeline_events, but the example and every implementation use timeline; and stripped_state is typed StrippedState, a type the document never defines (only StateStub is). Aligning the schema with the example on the first, and defining StrippedState (or pointing to the /v3/sync stripped-state shape) on the second, would resolve both.

Separately, on the invite_state to stripped_state rename (changelog #2): Synapse and Element X still use invite_state, so Zendrite emits both (above) and notes in-source that the spec and the deployed clients disagree. Deciding which name servers should emit during the unstable period would let implementations converge instead of dual-emitting.

[erikjohnston]

The schema names the timeline array timeline_events, but the example and every implementation use timeline

Oh good catch. This has come from confused naming internally in Synapse. 3695caa

StrippedState, a type the document never defines (only StateStub is).

Ah yes, that is relying on the existing definition in the spec. We already point to /v3/sync in the comment for that field.

Separately, on the invite_state to stripped_state rename (changelog #2): Synapse and Element X still use invite_state, so Zendrite emits both (above) and notes in-source that the spec and the deployed clients disagree. Deciding which name servers should emit during the unstable period would let implementations converge instead of dual-emitting.

We've basically taken the view that we're not going to update the unstable implementations, as the majority of the changes are cosmetic and accept that the request/response shapes differ depending on if you're using the unstable vs stable prefix.

[jevolk]

Thanks for getting this across the line; we have been implementing the core of this on the server side and the proposal has been clear to follow.

One gap we noticed while doing it: the Connections section establishes that pos is server-issued and scoped to a (user, device, conn_id) connection, but neither that section nor Security considerations says what a server should do with a pos that was issued to a different user or device. The persisted per-connection state this API introduces is what makes that matter. With /v3/sync a next_batch is a stateless cursor, but here a pos selects a stored server-side connection, so a server that keys that lookup on the token alone, rather than on the authenticated user and device, could return another user's view.

Synapse already guards against this: it keys the stored-connection lookup on the requester's user, device, and conn_id, and a pos that does not match returns M_UNKNOWN_POS, on the stated basis that the pos is a user-supplied value that has to be confirmed as actually theirs. So this is a request to write the existing behavior into the spec rather than a report of a live bug. The proposal already defines M_UNKNOWN_POS for an unrecognized pos, so the same error covers the not-yours case. Would it be worth Security considerations stating something like:

Servers MUST reject a pos that was not issued to the requesting user and device, responding with M_UNKNOWN_POS.

That keeps a from-scratch implementation from treating the token as a bearer capability. Clarifying ask, not a blocker on the FCP.

[erikjohnston]

Yes, that's a good point. Technically the MSC requires that the server only send down information the user is allowed to see, but there is definitely a potential footgun here.

I've made checking ownership of pos mandatory, and update the security considerations in 06bbf14

[jevolk]

Thanks for adding the pos-ownership requirement. One more for Security considerations, on the membership-cache flush in the lazy-loading section.

The text says that because membership updates are not necessarily sent when limited is true, clients must flush any membership cache on a limited response, and it gives the reason that cache exists: a client needs the membership list "to send an encrypted message to the room". Putting those together, the flush has an E2EE consequence the Security section does not call out. After a gappy sync a client has dropped its membership list, and if it sends an encrypted message before refetching it (or if the refetch races the send), it computes the recipient set over a stale member set and can miss a member who joined during the gap, so that member's devices cannot decrypt.

This is the room-membership axis, which this MSC mandates, and it is distinct from device-list change delivery (handled by the e2ee extension¹): a client with perfectly tracked device lists can still miss a recipient if its membership list is stale, and neither this section nor the e2ee extension analyzes that interaction. It seems worth Security considerations noting that after a limited response a client must refetch the membership list (not merely invalidate it) before using it as an encryption recipient set.

Footnotes

1. MSC3884 (Sliding Sync Extension: E2EE); its Security considerations section adds nothing beyond /sync: https://github.com/matrix-org/matrix-spec-proposals/blob/5ff5682b114354577dad775dd2d9b653cf531f0f/proposals/3884-sliding-sync-e2ee.md?plain=1#L81-L83 ↩

[mscbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.