MSC3202: Encrypted appservices

[turt2live]

Rendered

Implementation:

Synapse:

• Add experimental support for MSC3202: allowing application services to masquerade as specific devices. synapse#11538
• Add support for MSC3202: sending one-time key counts and fallback key usage states to Application Services. synapse#11617
• Send device list updates to application services (MSC3202) - part 1 synapse#11881
• MSC3202: Fix device_unused_fallback_keys -> device_unused_fallback_key_types synapse#12520
• Fix to-device messages not being sent to MSC3202-enabled appservices synapse#13235
• Fix appservice EDUs failing to send if the EDU doesn't have a room ID synapse#13236
• ... and more I might have missed. "MSC3202" as a search query should bring out the rest.

bot-sdk (appservice SDK):

• Structural: Add basic encryption support for simple bots turt2live/matrix-bot-sdk#142
• First implementation: Use rust-sdk for crypto turt2live/matrix-bot-sdk#187
• Second implementation: Replace rust-sdk bindings with ones that work turt2live/matrix-bot-sdk#236

Prototype proxy

• https://github.com/tulir/mautrix-syncproxy

[erikjohnston]

I think this looks good (modulo dave's comment: https://github.com/matrix-org/matrix-doc/pull/3202/files#r643211377), I would like to see a working implementation of an AS that does e2e before we land this though, as I suspect that there might be some subtle corners lurking

[tulir]

There's now a proxy implementation at https://github.com/tulir/mautrix-syncproxy, but it only does individual devices rather than all devices of an appservices (because one device is enough for everything and more would be very complicated to do with a /syncing proxy). It also implements the to-device part of MSC2409 (with the same individual device caveat).

Also added appservice-side SDK support in mautrix/go@72aa965 and mautrix/python@c41b515

[reivilibre]

what should be the HTTP and matrix-level error code if it's not valid?

[turt2live]

for lack of a better answer, let's reuse 400 M_EXCLUSIVE for now. We'll probably want to introduce an error code in the future.

[reivilibre]

How should the AS distinguish between users without keys and users with unchanged keys if you can omit them for both cases?

In my implementation thus far I've been sending empty dicts for the 'without keys' case. I'm now reading that this is unnecessary*, but would be interested in the answer to the above.

*I'm especially sceptical of 'no unused fallback keys' and 'no change in your unused fallback keys' having the same representation over the wire (and thus being indistinguishable). I personally think it's 'wrong' :)

[reivilibre]

MSC2732 has this to say about including the field in /sync: The device_unused_fallback_key_types parameter must be present if the server supports fallback keys. Clients can thus treat this field as an indication that the server supports fallback keys, and so only upload fallback keys to servers that support them.

My personal opinion here is that the per-user entries should be omissible if and only if there have been no changes for that user.
If we wanted to require an indication that the homeserver supports fallback keys, we'd only need to require that the outer dict be present (rather than all the users' entries) — don't know if it makes sense to require this though?

[reivilibre]

Agh, after reading my code, I notice that I also include empty lists for all the devices if there are no unused fallback keys. I think this is in keeping with what /sync does, but again, it would be nice to have this commented on specifically by the MSC so I know whether what I'm doing is right or wrong before it gets merged.

[turt2live]

In theory the appservice is doing its own state tracking for whether or not it has uploaded keys (of any variety). If for some reason it loses this state, it can do two things to recover:

1. Query for its own device keys, uploading new ones if none exist. If device keys do exist and the appservice doesn't know what they are, it can pick/create a new device for that user and upload fresh keys to that device.
2. Upload a new fallback key, even if unused.

So it should be perfectly safe to exclude fields in both cases, though the MSC is also assuming the server is working off a transaction model under the hood and over the wire: it's a lot easier to see that the appservice was already informed of a change when there's a transaction log to say that was sent.

Whether the server supports fallback keys or not is somewhat irrelevant for appservices: if an appservice requires it, it can just mark it in documentation for server compatibility. Plus, with the new Matrix versioning, the appservice can declare a minimum supported spec version that operators can compare against their homeserver installs. Clients are a bit different because it's a lot easier to accidentally/on purpose use a modern client against an old server, so the feature check becomes a bit more important (at least while fallback keys weren't in a spec release).

[reivilibre]

@turt2live Mostly reasonable, but I'm left with the question of: how is the appservice meant to distinguish between 'no difference in your fallback keys' and 'all your fallback keys have been consumed' if both of them are represented by omission?

My suggestion for how this should work would be: if a device entry is specified, then the list (UFBKs)/dict (OTK counts) is the whole new state for that device and replaces the AS's current state for that device. If a device entry is omitted, then there's no change.
In this system, 'all your fallback keys have been consumed' would be represented by the empty list [] of fallback keys. Thereafter it could theoretically be omitted if the state continues to be 'no keys'.

[turt2live]

The state change of being unused to used would be in a transaction, but further transactions don't need to include it. This is represented by [] already.

[reivilibre]

The state change of being unused to used would be in a transaction, but further transactions don't need to include it. This is represented by [] already.

Perfect; I think the wording of the '...users without keys can be omitted...' should be altered to clarity that

[turt2live]

General status update:

Implementations of the MSC have been added to the description.

This MSC is waiting for some larger scale tests to be carried out on sensibly sized bridges, not matrix.org, libera.chat, or t2bot.io, before this is in a comfortable state to move forward. Obviously it'd be great to solve the massive use cases as well, but the belief is that we can make that a future MSC's problem and cover the more general scenario with this MSC for now.

[richvdh]

The changes to hookshot to make use of this (matrix-org/matrix-hookshot#299) have now landed. @Half-Shot, @AndrewFerr: it would be great to get feedback on how well this works.

[turt2live]

fwiw, some early testing on t2bot.io shows that this is working well enough for the small to medium usecase, though obviously interested in whether there's something missing for reliable encryption that we need to consider.

[Half-Shot]

The changes to hookshot to make use of this (matrix-org/matrix-hookshot#299) have now landed. @Half-Shot, @AndrewFerr: it would be great to get feedback on how well this works.

Of relevance is matrix-org/matrix-hookshot#594, which is currently our tracking issue for crypto issues. We've encountered a few issues already, which may be down to the rust-sdk rather than this MSC. We'd really like to feel confident in hookshot's stability, and then we could probably give a green light from our side.

[Xiretza]

@Half-Shot:

We'd really like to feel confident in hookshot's stability, and then we could probably give a green light from our side.

matrix-org/matrix-hookshot#594 has been fixed, are there any stability issues remaining?