Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] MSC3956: Extensible Events - Encrypted Events #3956

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

turt2live
Copy link
Member

@turt2live turt2live added proposal A matrix spec change proposal kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Jan 13, 2023
## Proposal

**Author's note**: There is fairly strong, and reasonable, opposition to having encryption be a content
block. This could theoretically allow an event to be partially encrypted, which is undesirable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not agreeing nor disagreeing with the sentiment here, but it is already possible to have partially encrypted events since m.relates_to is always in cleartext; it does create complexity though of how that interacts with a decrypted plaintext version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, though the concern is more that someone will stick m.markup as a fallback on an encrypted event, which defeats the purpose of encryption. This proposal as-is also theoretically allows for weird parts of the event to be encrypted, though that's not intended to be valid.


This allows the `m.encrypted` content block to be reused by other event types, if required.

For clarity, this is *not* intended to allow unencrypted fallback on an encrypted event - doing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to have the spec say that a server impl should refuse a cleartext fallback if m.encrypt is present.

While this doesn't protect against people modifying their servers this would protect against faulty client implementations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On further thinking and talking to people: wouldn't it make sense to even enforce this in the auth rules? Is there any case where checking wouldn't be trivial or where you need plaintext next to encrypted?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasoning behind doing this via auth rules would be that this is a security concern. So this would prevent a security relevant issue which would justify having strict validation of the event imho as security outweighs most use cases I can think of.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this stage, auth rules feel overly strict for the general protocol: while I can agree with the principle that information should be enforced as encrypted, the general purpose nature of the protocol would call for a less constraining approach.

Also at this stage, I currently believe that adding auth rules to specific event types would be a design failure. Ideally, there aren't exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants