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

MSC2228: Self destructing events #2228

Open
wants to merge 14 commits into
base: master
from

Conversation

@ara4n
Copy link
Member

commented Aug 11, 2019

A new proposal for self-destructing events via redactions, based on lessons learned from #1763.

Rendered

MSC for self destructing events
based on lessons learned from #1763

@ara4n ara4n changed the title MSC for self destructing events MSC2228 for self destructing events Aug 11, 2019

ara4n added a commit that referenced this pull request Aug 11, 2019

@ara4n ara4n changed the title MSC2228 for self destructing events MSC2228: Self destructing events Aug 11, 2019

ara4n added 3 commits Aug 11, 2019
@ara4n

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2019

I think this is good for review; if people think it's on the right track, I'll propose FCP (although N.B. we have no urgent plans to implement this in synapse - just wanted to rescue the content from #1763 and make it saner, particularly if anyone else wanted to implement it in their server or contribute it)

@ara4n ara4n requested a review from matrix-org/spec-core-team Aug 11, 2019

@turt2live turt2live self-requested a review Aug 11, 2019

@turt2live
Copy link
Member

left a comment

largely the concept seems straightforward to me. This also seems like a good candidate for proposing FCP early and letting checkmarks accumulate over time.

proposals/2228-self-destructing-events.md Outdated Show resolved Hide resolved
ara4n added 2 commits Aug 14, 2019
ara4n and others added 2 commits Aug 17, 2019
Update proposals/2228-self-destructing-events.md
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Update proposals/2228-self-destructing-events.md
Co-Authored-By: Travis Ralston <travpc@gmail.com>
@ara4n

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2019

@mscbot fcp merge

@mscbot

This comment has been minimized.

Copy link
Collaborator

commented Aug 17, 2019

Team member @ara4n has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

Once a majority of reviewers approve (and none object), 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 info about what commands tagged team members can give me.

@ara4n

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2019

(it feels premature to suggest fcp on this without having checked whether the impl works though - if we have to change it based on the impl, are we going to have to fcp it again?)

@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Theory is we don't design a system that sucks to implement. Minor changes don't need approval (they're at the discretion of the person reviewing the spec PR), however major changes need a MSC against the MSC to be incorporated in the spec.

@joepie91

This comment has been minimized.

Copy link

commented Aug 18, 2019

Something that I'm missing in the MSC: what's the concrete threat model that this MSC is meant to address? The referenced MSC 1763 has a lengthy list of goals, but those also encompass general message retention goals, and it's not clear which of those points are meant to be addressed by this derived MSC, or whether there might even be any new ones.

This is especially important because "self-destructing messages" is a topic that's fraught with potential design mistakes in general.

For example, if the intention is to only expose a given piece of information to recipients for a limited period of time because the recipients are not trusted to behave in good faith, such a feature could actually make things worse by suggesting a reliable auto-deletion mechanism where none exists (as a malicious recipient could simply run their own HS and refuse to implement the server-side component of this MSC).

On the other hand, if the purpose is to opportunistically protect information from being disclosed in a future server compromise, self-destructing events could be a reasonable approach; though at that point, the question should be asked whether this is not better solved through opportunistic E2EE.

In summary: as it stands, it's not clear what threat model this MSC is meant to address, and as such its effectiveness from a security perspective can't really be meaningfully reviewed by third parties.


We could let the user specify an expiry time for messages relative to when
they were sent rather than when they were read. However, I can't think of a
good enough use case to justify complicating the proposal with that feature.

This comment has been minimized.

Copy link
@uhoreg

uhoreg Aug 18, 2019

Member

TBH, having the expiry based on the send time rather than the receive time seems more natural to me, as it seems more predictable.

This comment has been minimized.

Copy link
@ara4n

ara4n Sep 4, 2019

Author Member

it may be more predictable for the sender, but we should surely consider the UX from the receiver's perspective, where it will just be frustrating to rush to read messages relative to when the sender sent them rather than when the receiver read them. Imagine how annoying it would be to receive a msg with a 5s timeout and discover that 4s of synapse latency meant you only had 1s to open the push, launch the app, sync and read it before your client gleefully deletes it...

That said, there may be a fairly obscure use case here around time-limited promotions - where a user sends a message to a room saying "You have 30 mins from now to download my exclusive artwork!!!" and then posts a link which autodestructs 30 mins after sending.

So perhaps we do want to include this after all.

@Half-Shot

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@mscbot concern Edits to the timeout field are currently undefined behaviour. See https://github.com/matrix-org/matrix-doc/pull/2228/files#r315175179

@Half-Shot

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

I can't raise concerns because I am a mere observer, but it feels premature to call FCP on a MSC with open questions inside the proposal document.

@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

raising on half-shot's behalf:

@mscbot concern Edits to the timeout field are currently undefined behaviour. See https://github.com/matrix-org/matrix-doc/pull/2228/files#r315175179

@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

@mscbot concern Should have a mention of self-destructing redactions

@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

@mscbot concern Unclear if m.synthetic is actually part of this proposal

@ara4n

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@joepie91 have tried to incorporate your threat model concerns in b8f172e. (please use threads for comments rather than the main linear comment list, for ease of threaded responses :)

ara4n added 5 commits Sep 4, 2019
@Half-Shot

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

I am wondering if it would be helpful for the server to stick in a lifetime value in unsigned as we do already for age.

Given the server should be the authority on how long an event has left to live rather than the client (which is almost certainly out of sync with the server's time)

@gerg5c42g542g2c54g52c

This comment has been minimized.

Copy link

commented Sep 5, 2019

Given the server should be the authority on how long an event has left to live rather than the client (which is almost certainly out of sync with the server's time)

Is this the case in Signal too?

@turt2live turt2live added this to Review stages in Client-server r0.6 proposals Sep 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.