mlswg / mls-protocol

MLS protocol
https://messaginglayersecurity.rocks
Other
234 stars 60 forks source link

Don't blank leaves that aren't known to a removed member #838

Closed bifurcation closed 1 year ago

bifurcation commented 1 year ago

Fixes #834

mulmarta commented 1 year ago

While it sounds like a nice idea, I'm rather reluctant to make such changes at this stage. There has been a number of analyses of MLS and most of them are unlikely to be updated. I haven't been able to find an attack in a couple of days, but I don't feel certain enough that this is secure.

Would it be possible to make this an extension instead? Or maybe (also) a part of MLS 2.0?

TWal commented 1 year ago

Yes, this could be defined cleanly in an extension. It could be a separate proposal, i.e. we could have Remove (as we have now) and FastRemove (the new version I proposed) that live together.

About attacks on such a change, I really think it only boils down to whether it breaks the parent-hash invariant (the one checked when joining a group) or not. Here is an intuition: if the current MLS is secure, and FastRemove don't break the parent-hash invariants but introduce a security flaw (e.g. break the TreeKEM secrecy invariant), then there would be an attack on the current MLS: the attacker could apply FastRemove on a tree (no need for this to be an actual proposal) and invite someone in a group with this tree. The new tree would be accepted by the joining while not satisfying the TreeKEM secrecy invariant, hence an attack on the current MLS.

raphaelrobert commented 1 year ago

I would also like to put the optimisation into perspective again: This only makes removes faster in the narrow time window after a new member joined and before said member did its initial update. Furthermore, this only applies to the removal of new members. With that in mind, the optimisation should be more accurately renamed as "More efficient removal of new joiners". New members are strongly encouraged to do an update as quickly as possible (even when the general update policy is more relaxed), therefore I expect this time window to be relatively short in an overwhelming majority of MLS deployments.

The risk @mulmarta describes is real, and I don't think it is justified by the marginal performance gain.

seanturner commented 1 year ago

Discussed at 12/16/2022 interim. Decided to close, but it can be revived later as an extension.