Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This event is defined in MSC4268. MSC4268: matrix-org/matrix-spec-proposals#4268
This event is defined in MSC4268. MSC4268: matrix-org/matrix-spec-proposals#4268
This event is defined in MSC4268. MSC4268: matrix-org/matrix-spec-proposals#4268
This event is defined in MSC4268. MSC4268: matrix-org/matrix-spec-proposals#4268
…cryption-info feat: Add `forwarder: ForwarderInfo` to `EncryptionInfo`. Introduces `ForwarderInfo` which which exposes information about the forwarder of the keys with which an event was encrypted if they were shared as part of an [MSC4268](matrix-org/matrix-spec-proposals#4268) room key bundle.
Expose information about the forwarder for events that were decrypted using a key from an [MSC4268](matrix-org/matrix-spec-proposals#4268) key bundle. Signed-off-by: Skye Elliot <actuallyori@gmail.com>
2abbf2b to
b7056c2
Compare
| however, that this process must be resilient to Bob's client being restarted | ||
| before the download/import completes. | ||
|
|
||
| The definition of "recently" is left up to clients. (They should consider |
There was a problem hiding this comment.
I think this is a mistake. This value determines how long the key bundle must be stored on the server (e.g. in the case of a partial download before the client was closed). By leaving "recently" undefined, we lose any ability to set reasonable TTLs on this data.
As a general design goal, I do not think we should be writing APIs which allow unbounded data to accumulate. This is what this is doing.
There was a problem hiding this comment.
Imposing a limit in the spec feels like we'd also be limiting use cases. Instead, because clients with slightly obscure requirements (such as availability) are typically attached to servers which are aware of those requirements, we can likely move this timeout in part to the server.
The client should still make its own decisions about what to request, but the server should also decide how long it keeps bundles around for. These two values may not be the same, and can lead to download failures: this is acceptable, in my opinion. A future MSC can introduce a re-request mechanism if it becomes a high enough priority issue.
(this is non-blocking for FCP in my opinion: I'm fine with the text as-written, but would appreciate a note that the server might delete things before the client can request it)
There was a problem hiding this comment.
@turt2live I basically agree with you, but it's worth noting that a server has no means of distinguishing a key bundle from any other piece of encrypted media (other than via side-channels), so it might be hard for a server to apply a sensible timeout.
We could find some way to decorate it, if we considered it important.
There was a problem hiding this comment.
Yea, I'd expect it to be decorated by some timeout or ephemeral flag that could theoretically be used by other media (but won't be)
There was a problem hiding this comment.
Ok, so I guess that would mean:
- Stick a query parameter of some kind (
media_type, maybe?) on/_matrix/media/v3/upload, and put something in the metadata part of the response in/_matrix/federation/v1/media/download/{mediaId}. - Leave the actual TTLs up to HS impls, for now at least.
- What would happen if a client forgot to set the flag? Should we add another query param on
/_matrix/client/v1/media/download/{serverName}/{mediaId}saying "only give me the media if it has the key-bundle flag set", so that we can enforce it being set?
I guess this might also help solve the incompatibility with MSC3911?
We (Element crypto team) are keen to release this feature soon, rather than get it too bogged down in spec hell, so I'd appreciate a temperature reading on this from SCT members (and @kegsay, as the OP on this thread): maybe you could use the apache voting scale to express your feelings on shipping without a way for clients to indicate that a specific media upload is actually a key bundle.
There was a problem hiding this comment.
I personally think it would be beneficial to flag to the server that this is a key bundle. However, we know that MSC3911 is something that we'll (eventually) do and so we'll need to annotate these bundles in some way anyway, so I don't really care if we punt that question until then.
I think we should note in the MSC that servers may delete media based on last usage, and so clients cannot assume that the bundle will be around forever (and downloading may fail).
MSC ChecklistThis document contains a list of final checks to perform on an MSC before it Spec Core Team (SCT) members, please ensure that all of the following checks MSC authors, feel free to ask in a thread on your PR or in the
|
|
I think this is ready for FCP. @mscbot fcp merge |
|
Team member @richvdh has proposed to merge this. The next step is review by the rest of the tagged people: Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for information about what commands tagged team members can give me. |
turt2live
left a comment
There was a problem hiding this comment.
no blocking concerns - thank you!
| however, that this process must be resilient to Bob's client being restarted | ||
| before the download/import completes. | ||
|
|
||
| The definition of "recently" is left up to clients. (They should consider |
There was a problem hiding this comment.
Imposing a limit in the spec feels like we'd also be limiting use cases. Instead, because clients with slightly obscure requirements (such as availability) are typically attached to servers which are aware of those requirements, we can likely move this timeout in part to the server.
The client should still make its own decisions about what to request, but the server should also decide how long it keeps bundles around for. These two values may not be the same, and can lead to download failures: this is acceptable, in my opinion. A future MSC can introduce a re-request mechanism if it becomes a high enough priority issue.
(this is non-blocking for FCP in my opinion: I'm fine with the text as-written, but would appreciate a note that the server might delete things before the client can request it)
Co-authored-by: Hubert Chathi <hubert@uhoreg.ca>
| until we have authenticated backups (MSC4048). This is tracked at | ||
| https://github.com/matrix-org/matrix-rust-sdk/issues/5110. | ||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
How big can this bundle get? Will (de)serializing it cause potential memory issues with clients? Do we need to allow for multiple?
There was a problem hiding this comment.
It's limited only by the size limits on the media store (e.g. 50MB by default for Synapse, 10MB for matrix.org's free tier). Some discussion about this here.
We're basically assuming that clients can safely decrypt and hold in memory anything that the media store can send them, so I'm not too worried about us blowing out memory limits.
At the other end, it has to be said that matrix.org's paid tiers have been introduced since we started this project, and it's not inconceivable that people will hit the 10MB limit if they invite someone to a massive encrypted room. That said (a) that sounds like the level of usage where they should be either paying or going elsewhere, and (b) even if we want to support multiple bundles in future, I'd much rather leave that until we're sure we need it.
There was a problem hiding this comment.
Thanks, I'm not sure if any of this is worth adding to the MSC? Maybe just a "clients may generate a file too large to upload, in which case they should do X"? Not sure it is worth it as a callout.
| * Ideally, the bundle would be deleted once the recipient has successfully | ||
| downloaded it. An implementation is left for a future MSC, such as | ||
| [MSC4425](https://github.com/matrix-org/matrix-spec-proposals/pull/4425). This | ||
| is tracked at https://github.com/matrix-org/matrix-rust-sdk/issues/5113. |
There was a problem hiding this comment.
Deleting the bundles being out of scope feels very not nice, but I guess it's not the end of the world since it's encrypted with AES-256 which isn't expected to be cracked by quantum computers
| @@ -0,0 +1,468 @@ | |||
| # MSC4268: Sharing room keys for past messages | |||
There was a problem hiding this comment.
My two main concerns on this MSC are:
-
Without the ability to delete key bundles once they've been consumed by the invited user, we accumulate encrypted key material in a way which is tantamount to harvest-now-decrypt-later attacks. However, given MSC4425 (or other ephemeral MSCs) are on the radar, I'm (just about) happy to for this to progress on the understanding that we follow up afterwards rapidly with MSC4425 or similar.
-
In terms of use experience, it's really not obvious to me that the common case should be to share all possible keys with an invited user. Sometimes this will be important (e.g. knowledge transfer to a new user joining a team/org) - but sometimes it will be very undesirable even if the room has shared history viz, particularly if the history viz been changed back and forth over time. I suspect that an approach similar to the WhatsApp idiom of "how much history do you want to share with the new user? nothing? 1 msg? 100 msgs? everything?" might help with this. However: this is a client UX question and is already supported by the MSC.
So, neither of these points end up being blocking, although both make me quite uncomfortable with the proposal. I'll checkbox it to proceed, but please can we track these concerns on the Element crypto-team agenda so they're not lost.
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
A proposal for sharing the keys to existing room messages when inviting someone to a room.
This reintroduces the functionality previously proposed in MSC3061.
Conflict of Interest declaration: I am employed by Element. This MSC was written as part of my work on the Element Cryptography team.
The functionality discussed here is implemented in current versions of Element Web and Element X.
Rendered
MSC checklist
FCP tickyboxes