Events should have attributes indicating the encryption state.

[poljar]

Every room event might have been an encrypted event, e.g. a client will turn a m.room.encrypted event into a m.room.message event.

Clients need to present some info about this to users. For example an event wasn't encrypted but is posted in an encrypted room, clients might want to decorate such an event with a warning.

Clients will want to show:

• If an event was encrypted or not
• If an event is successfully verified or not
• Let clients record the sender key coming from the m.room.encrypted event, so if the trust state of the device holding the key changes, clients can change the presented verification state of the event.

I suggest we add an optional EncryptionInfo attribute to the events containing something like:

struct EncryptionInfo {
    verified: bool,
    sender_key: String,
    device_id: String,
    session_id: String
}

Most of those attributes will come from the encrypted version of the event, the m.room.encrypted event. The lack of the EncryptionInfo would indicate that the event was unencrypted.

[jplatte]

I still don't yet know that much about the matrix protocol and can't really comment on this proposal at the moment, so don't expect this issue to move forward soon unless Jimmy happens to have more time soon. I'm however interested and will try to understand the issue and your proposed solution when time permits.

[poljar]

Oh I don't expect people to magically implement stuff that I open here. But I would like to discuss this before I or someone else spends time implementing this.

The gist of the issue is that m.room.encrypted events turn into most m.room.* events. Client libraries will want to perform the conversion and present the unencrypted events to the clients, yet clients will want to know about some of the info from the m.room.encrypted event.

Client libraries could provide this in a separate struct but I think it's much more convenient if this is presented in the same struct.

[valkum]

The encryption library could wrap the ruma_event in a wrapper. So you basically have conversion of m.room.* => Wrap(m.room.*, encrypted=false) and for m.room.encrpyted => Wrap(m.room.*, encrypted=true)

[poljar]

Yes you can, and I already mentioned this in my last sentence of the previous comment.

The reason why I think having this in the Event itself is a better option is because a crypto lib could just swap out the encrypted events with non-encrypted ones in the sync response.

More concretely, iterate over the events in a room timeline, if the event is encrypted decrypt it and swap it out for the decrypted one, if not leave them be.

This isn't possible with a wrapper struct.

[valkum]

I think it only makes sense if decryption is a first party component of ruma-*.
What about states where you don't have the decryption key for a message. That would be additional information for a client in the place of an event.

I think it would be cleaner if an encryption component would wrap all of ruma and expose a similar api.

[poljar]

What would that mean, being a first party component?

Doesn't the ruma-client API have all the definitions for encryption support? Seems like some layers already have encryption support? What would be needed for it to become first party? I'm quite confused what this would mean.

I don't understand what you mean about the states where you don't have decryption keys? If you don't have the keys an m.room.encrypted event won't be decrypted, it stays the way it is. Now keys may arrive at a later point and the client lib could cache such events and decrypt them later or it could present an API that converts m.room.encrypted events into m.room.* events and let the library user decide if they want to cache such events.

If you mean the states where an event wasn't decrypted in the first place such a field would be obviously left as None. So the field would be an Option<EncryptionInfo>. That was stated in my original comment already.

I'm not sure I understand, or at least I strongly disagree what cleaner means in this case. Wrapping all of Ruma seems like a pointless endeavor considering what I ask for. And every library supporting encryption would need to do that as well (unless you suggest to create a wrapper ruma-events crate that includes one struct field inside the events).

The field could be behind a feature flag, and people who don't care about encryption at all can disable it. It probably makes sense to disable it by default. Nobody is required to do huge amounts of work to add sensible encryption info to the events, nobody pays the cost of an additional encryption field in the struct. Everybody is happy.

Your proposal doesn't seem to be well thought out and seems to dismiss encryption as non-interesting or a non-crucial feature of Matrix for some vague idea of cleanliness. Please clarify what would be cleaner in that case and why it's such a big deal to include an optional field there.

[valkum]

With 'first party' I meant that ruma-client should handle all the encryption/decryption including storing the encryption session keys as well as handling device verification.

If this is not the case and third party encryption/decryption crates are needed for e2ee support I see no point in exposing the encryption state in the downstream crate that mostly handles raw matrix events.

[jplatte]

ruma-events and ruma-client are not tightly coupled in that way and I don't see why they should be.

ruma-events can ship this kind of convenience functionality without ruma-client having builtin encryption / decryption support and ruma-client can add automatic encryption / decryption without ruma-events having this kind of functionality.

[jplatte]

@poljar have you found a good way to solve this inside rust-matrix-sdk? Do you still wish Ruma would be more helpful regarding this?

[poljar]

As for giving clients the encryption info we can modify the event emitter methods to contain an Option<EncryptionInfo> thing.

This would still be nice if people wish to consume the whole sync response as is instead of using the event emitter.

Another use-case, if we store the events in the state store the encryption info should be preserved.

For the state store I could wrap the events without a lot of fuss, but it would be a pain for the sync response use-case.

So I can live without it but it would be nice to have.

[jplatte]

This would still be nice if people wish to consume the whole sync response as is instead of using the event emitter.

Where / when does this use case come up?