Pass through unsigned data in /keys/query
#17951
Conversation
richvdh
changed the title
unsigned data in /keys/query We'd like a mechanism by which a client can add "unsigned" data to their device keys, and have it be accessible by other clients involved in E2EE discussions. Most of this actually already works; the bit that doesn't is that *client-side* /keys/query strips out any "unsigned" data from the /keys/upload body. (Server-side /keys/query follows a different codepath and is fine).unsigned data in /keys/query
We'd like a mechanism by which a client can add "unsigned" data to their device keys, and have it be accessible by other clients involved in E2EE discussions. Most of this actually already works; the bit that doesn't is that *client-side* `/keys/query` strips out any "unsigned" data from the `/keys/upload` body. (Server-side `/keys/query` follows a different codepath and is fine). This commit adds an experimental option which modifies client-side `/keys/query` so that `unsigned` data is preserved.
richvdh
force-pushed
the
rav/unsigned_data_in_keys_query
branch
from
bf29370 to
c7f443a
Compare
| @@ -0,0 +1 @@ | |||
| Add experimental option to pass through unsigned data in `/keys/query` responses. | |||
There was a problem hiding this comment.
Seems relatively sane given we are already returning some data under unsigned .
It feels a bit weird that we're storing the unsigned data on /keys/upload already (not spec'ed) but it looks like we just store whatever JSON is given (doesn't seem like the best idea).
You explained the reason why we can't just bodge this in under any key other than unsigned is because of the following:
It's worth noting that we can't simply add this data to the body of the key uploaded with
/keys/upload, otherwise a change toapp_versionwill invalidate any cross-signing signatures, and hence require re-verification of the device. Instead, we must add it to theunsignedsection, but that isn't currently exposed via/keys/upload. In short, whatever we do, we need server-side and spec changes.
And the code around that seems to be at:
synapse/synapse/handlers/e2e_keys.py
Lines 1675 to 1681 in 4587dec
Are clients meant to send/store things on unsigned? From our other usages, it sorta seems like unsigned is reserved specifically for the server wanting to return extra data (not for clients to add things to).
There was a problem hiding this comment.
Thanks for the thoughtful feedback. A few answers that might help:
It feels a bit weird that we're storing the unsigned data on /keys/upload already (not spec'ed)
That's something I hope to clarify in the MSC that has yet to be written about this (MSC4229).
[we can't add this outside
unsigned]
And the code around that seems to be at:
Really, cross-signatures of devices are for the benefit of other clients. For example, if I sign one of my devices with my cross-signing key, then my other devices, and other users, know that device belongs to me. So the code that we care about for validating cross-signatures is really https://github.com/matrix-org/matrix-rust-sdk/blob/main/crates/matrix-sdk-crypto/src/types/cross_signing/self_signing.rs#L45-L53.
The code you link to is actually "just" validation: it's there to try to let clients know early on if they are uploading invalid signatures, rather than waiting for the signature to get all the way to a remote client, which doesn't have a good way to feed back to the original client. But sure, it still needs to be satisfied.
Anyway, the actual algorithm for checking a signature on a bit of JSON is specced here. Note in particular that unsigned is a special key that is not part of the signed data, which is why we can add stuff to it without invalidating the signature.
Are clients meant to send/store things on
unsigned?
Ok well, good question. I'm not actually aware of anywhere in the spec that clients put data into unsigned. That said, I don't think there's any particular rule for/against clients doing that. Again, it's something I'd look to straighten out with an MSC.
|
There's actually now a bit of uncertainty about whether our new data does actually need to go into |
We'd like a mechanism by which a client can add "unsigned" data to their device keys, and have it be accessible by other clients involved in E2EE discussions.
Most of this actually already works; the bit that doesn't is that client-side
/keys/querystrips out any "unsigned" data from the/keys/uploadbody. (Server-side/keys/queryfollows a different codepath and is fine).This PR adds an experimental option which modifies client-side
/keys/queryso thatunsigneddata is preserved.An MSC for this behaviour is pending, but will be matrix-org/matrix-spec-proposals#4229.
Fixes: #17943
Part of element-hq/element-meta#2467