diff --git a/proposals/1954-remove-prev_event-from-essential-keys-list.md b/proposals/1954-remove-prev_event-from-essential-keys-list.md new file mode 100644 index 00000000..735fac01 --- /dev/null +++ b/proposals/1954-remove-prev_event-from-essential-keys-list.md @@ -0,0 +1,77 @@ +# Remove prev_content from the essential keys list + +Matrix supports the concept of event redaction. The ability to redact rather +than delete is necessary because some events e.g. membership events are +essential to the protocol and _cannot_ be deleted. Therefore we do not delete +events outright and instead redact them. This involves removing all keys from +an event that are not required by the protocol. The stripped down event is +thereafter returned anytime a client or remote server requests it. + + +## Proposal + +[The redaction algorithm](https://matrix.org/docs/spec/client_server/r0.4.0.html#redactions) +defines which keys must be retained through a redaction. Currently it lists +```prev_content``` as a key to retain, though in practice there is no need to +do so at the protocol level. + +The proposal is simply to remove ```prev_content``` from the essential keys +list. + +Note: the inclusion of ```prev_content``` in the essential keys list was +unintentional and should be considered a spec bug. Synapse (and other server +implementations) have not implemented the bug and already omit +```prev_content``` from redacted events. + + +## Tradeoffs + +When sending events over federation the events are [hashed and +signed](https://matrix.org/docs/spec/server_server/unstable.html#adding-hashes-and-signatures-to-outgoing-events), +this involves operating not only on the original event but also the redacted +form of the event. The redacted hash and redacted signed event are necessary if +the event is ever redacted in future. As a result, any change of the essential +keys list must be managed carefully. If disparate servers implement different +versions of the redaction algorithm (for a given event) attempts to send the +event over federation will fail. + +We _could_ manage this change via room versioning and create a new room +version that implements this MSC. However, because the federation already +omits the ```prev_content``` key by convention, implementing this MSC only in +the new room version would mean that the entire existing federation would not +be spec compliant. + +As a result it seems pragmatic to have the spec reflect reality, acknowledge +that the spec and federation have deviated and instead update the spec +retrospectively to describe the de-facto redaction algorithm. + +## Potential issues + +It is theoretically possible that a closed federation could exist whose servers +do follow the spec as is. This MSC would render those servers non-compliant with +the spec. On balance this seems unlikely and in the worst case those +implementors could add the change to a subsequent room version, eventually +reaching spec consistency as older room versions are deprecated. + +Another scenario is that a client may redact events according to the spec as is +and persist prev_content through the redaction, thereby diverting from that on +the server(s). Client authors will have to update their code to drop +```prev_content``` - however, given that prev_content should not be used in +important calculations and/or visualisations, this ought to be a relatively +non-invasive change. + + +## Security considerations + +A further reason to support removal of ```prev_content``` is the case where a +malicious user adds illegal or abusive content into a state event and then +overwrites that state event. The content would then be preserved through the +redaction. + +Additionally, there are plenty of reasons to have security concerns over a +precedent that the federation can deviate from the spec. + +## Conclusions +Removing ```prev_content``` is pragmatic response to the current situation. It +alligns the federation and the spec, and does so in a way that removes +unecessary overhead.