MSC4293: Redact on ban

[turt2live]

Rendered

Disclosure: I am Director of Standards Development at The Matrix.org Foundation C.I.C., Matrix Spec Core Team (SCT) member, employed by Element, and operate the t2bot.io service. This proposal is written and published as a Trust & Safety team member allocated in full to the Foundation.

---

MSC checklist

FCP tickyboxes

[Gnuxie]

yes please!

[turt2live]

Implementation requirements:

• Server - Add support for MSC4293 - Redact on Kick/Ban element-hq/synapse#18540
• Sending client - Set MSC4293 flag when autoredacting users mjolnir#612
• Receiving/applying client:
  • Redact on ban: Client implementation matrix-js-sdk#4867
  • web/timeline: implement MSC4293 gomuks/gomuks#626

Other non-qualifying (as of writing) implementations:

• feat(timeline): support MSC4293 redact-on-ban/kick matrix-rust-sdk#5273

[turt2live]

MSCs proposed for Final Comment Period (FCP) should meet the requirements outlined in the checklist prior to being accepted into the spec. This checklist is a bit long, but aims to reduce the number of follow-on MSCs after a feature lands.

SCT members: please check off things you check for, and raise a concern against FCP if the checklist is incomplete. If an item doesn't apply, prefer to check it rather than remove it. Unchecking items is encouraged where applicable.

Checklist:

• Are appropriate implementation(s)
  specified in the MSC’s PR description?
• Are all MSCs that this MSC depends on already accepted?
• For each new endpoint that is introduced:
  • Have authentication requirements been specified?
  • Have rate-limiting requirements been specified?
  • Have guest access requirements been specified?
  • Are error responses specified?
    • Does each error case have a specified errcode (e.g. M_FORBIDDEN) and HTTP status code?
      • If a new errcode is introduced, is it clear that it is new?
• Will the MSC require a new room version, and if so, has that been made clear?
  • Is the reason for a new room version clearly stated? For example,
    modifying the set of redacted fields changes how event IDs are calculated,
    thus requiring a new room version.
• Are backwards-compatibility concerns appropriately addressed?
• Are the endpoint conventions honoured?
  • Do HTTP endpoints use_underscores_like_this?
  • Will the endpoint return unbounded data? If so, has pagination been considered?
  • If the endpoint utilises pagination, is it consistent with
    the appendices?
• An introduction exists and clearly outlines the problem being solved.
  Ideally, the first paragraph should be understandable by a non-technical audience.
• All outstanding threads are resolved
  • All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative
• While the exact sections do not need to be present,
  the details implied by the proposal template are covered. Namely:
  • Introduction
  • Proposal text
  • Potential issues
  • Alternatives
  • Dependencies
• Stable identifiers are used throughout the proposal, except for the unstable prefix section
  • Unstable prefixes consider the awkward accepted-but-not-merged state
  • Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”).
• Changes have applicable Sign Off from all authors/editors/contributors
• There is a dedicated "Security Considerations" section which detail
  any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.".
  See RFC3552 for things to think about,
  but in particular pay attention to the OWASP Top Ten.

[turt2live]

With my SCT hat, I've verified the implementations (though welcome second/third opinions)

[turt2live]

This has gone through quite a few rounds of review in various T&S circles, and has healthy code review to show that teams are interested in supporting this feature.

With my SCT hat, I think this is ready for FCP, especially to help encourage more client implementations.

@mscbot fcp merge

[mscbot]

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

• @clokep
• @dbkr
• @uhoreg
• @turt2live
• @ara4n
• @anoadragon453
• @richvdh
• @tulir
• @erikjohnston
• @KitsuneRal

Concerns:

• General clarity/understanding of goals and alternatives

Once at least 75% of reviewers approve (and there are no outstanding concerns), 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 information about what commands tagged team members can give me.

[clokep]

Why redact the event itself instead of just stopping at it?

[turt2live]

err, sorry: the membership event is not redacted by the autoredaction stuff. It's just that since we're doing this without a room version, the flag is at risk of being redacted off the event (technically).

We otherwise keep redacting after the ban because of eventual consistency and delayed receipt of events.

[clokep]

I feel like this says the opposite?

until the membership event itself is redacted

[turt2live]

I can see how that might be interpreted that way, but it is not the intention. It's trying to clarify to server implementations that they won't be able to see the redact_events flag on the event if the membership event itself is redacted, and therefore the autoredact behaviour also ceases at that point in time.

[clokep]

I've read this a handful of times now and I can't interpret it any other way.

Suggested change

	If the sender is allowed to redact, the redaction behaviour continues until the membership event itself
	is redacted (thus removing the field), another membership event removes the field, or the flag is set
	If the sender is allowed to redact, the redaction behaviour continues until a membership event for
	that user is reached which does not have the flag set or has the flag set

[clokep]

Doesn't this potentially have weird behavior if a server redacts the events, redacts the membership event itself, then backfills events? It is unclear to me if this is covered the text below (or above in fallback behavior?)

[turt2live]

it should be covered in the potential issues, but I'll aim to clarify in the text. The short version is: yes, it's weird, but not any more weird than usual. A server which requests events from a server that redacted them will receive redacted copies. This infectious behaviour is a feature, though if a server really wanted to it could ask other servers until it finds an unredacted copy.

[clokep]

I'm worried about the opposite -- receiving unredacted events and not knowing they should be redacted.

[turt2live]

That should already be addressed in the MSC, as a known limitation.

[clokep]

Can a user send this when leaving a room? Not in the case of kick but to self redact all.

[turt2live]

No, self-leaves are not permitted by this MSC. Rationale being it opens a can of worms I'd rather not get this MSC stuck in.

[clokep]

Ok, I think that's unclear that the behavior only applies if the sender and state key don't match then.

[dbkr]

I don't think I'm quite getting this proposal. So it's just a temporary measure to do mass redactions in old room versions? Why not just a separate 'mass redact' endpoint? Or, for that matter, have the HS look for mass redact events sent into old room versions and do them manually exactly as it would do here?

[Gnuxie]

MSC4194 is a prior exploration of a mass redact endpoint for this use case. A reason for collecting the responsibilities into a flag on the ban event are because of the following complications:

• The homeserver applying redactions would also need to send redactions for future soft failed events as well as already received historical events.
  • Toggling this behavior is complicated: should a user rejoin, the behavior would probably need to be tied to membership in the same way as in this proposal.
• The endpoint will always be used after a room member is banned. There isn't another use case with exception of self redaction which is intentionally being ignored by both proposals.
• Without mass redaction events, federating individual redaction events takes longer than a single ban event which can flag to clients to hide all content associated with a user before redaction events arrive. Which is critical in spam attacks¹.

Footnotes

1. And also consider that soft failure is common and it's not likely that there will be just "one" mass redact event needed to cleanup per spam incident. ↩

[turt2live]

As Gnuxie mentions, we can't/shouldn't be sending 1:1 redaction events anymore, especially post-ban (the original sender can't "self-redact", and the moderator who did the ban may have left). This MSC specifically tries to reduce the number of redaction events to zero if it can, or one in the worst case - mass redactions themselves would be the "one" case.

The issue with mass redact events is that they aren't backwards compatible to existing room versions, which will cause breakage. We could spend some time trying to fix this (fallback fields, different event types, etc), but those solutions don't look as great (in my opinion) and slow down the rollout of the feature due to broader use cases needing consideration.

We (T&S) expect that both redact-on-ban and mass redactions will be used in future room versions concurrently for different use cases, and require a more immediate solution to 1:1 redaction event sending.

I'll cover this in the alternatives section when I loop back around for an edit.

[dbkr]

I'm afraid I may be more confused if anything. I was assuming the HS would do something to make this backwards compatible but now I wonder if I've misunderstood that. What makes this backwards compatible given a mass redaction isn't? If this is not a short-term measure, in what circumstances would this be used vs a mass redaction? In any case, hopefully the edit should make things clearer. The info in the responses here probably needs to go into the MSC.

[nexy7574]

Later on in the proposal, it's said several times that sending redactions as fallback support is intended, so I'm getting mixed signals here. When this flag is passed, should the server send actual redactions or not? I assume it's "yes, send actual redactions, until greater adoption makes them redundant" from later paragraphs, however it is not clear.

[Gnuxie]

"yes, send actual redactions, until greater adoption makes them redundant"

If this is the implication then it shouldn't be. Redactions will always need to be sent in legacy room versions. And if the proposal has been written with otherwise then that's a pretty serious issue. @turt2live could you please clarify?

My understanding was that redaction events would have to be sent for all room versions prior to the inclusion of MSC4298.

[turt2live]

TLDR: "yes, send actual redactions if you want to, until greater adoption makes them redundant" (added emphasis mine) is accurate.

---

Fallback behaviour is a bit of an awkward case for MSCs. In this case, the intended change is meant to be additive to the spec with no actual requirement to send redaction events (even in existing room versions), but there are several implementations of Matrix clients and servers which already exist today that won't know how to handle this new behaviour, and thus senders are encouraged (not required) to send "fallback redactions".

The MSC does give some guidance on what fallback behaviour might look like, but leaves removal of that fallback behaviour up to the individual projects. One project may, for example, decide that pre-MSC4298 rooms will forever get fallback redactions while another decides that 3 months from spec release there is enough client implementations to safely drop the fallback support. Some implementations may also choose somewhere in the middle of that spectrum: instead of chasing events and retrying redactions, it could do a single query and send redactions once, leaving any errors to hopefully be picked up by the redact-on-ban flag.

Neither this MSC or MSC4298 make the fallback behaviour required, either for existing room versions or implementations of Matrix.

[turt2live]

@mscbot concern General clarity/understanding of goals and alternatives