MSC3757: Restricting who can overwrite a state event

[andybalaam]

Rendered

Implementations:

• Support MSC3757: Restricting who can overwrite a state event element-hq/synapse#17513
• rtc: Handle non-MXID call member event state keys matrix-rust-sdk#3836
• Loosen type of call member event state keys ruma/ruma#1885
• Add tests for MSC3757 complement#733

Written by @ara4n , with contributions from @Johennes and @andybalaam .

Shepherd: @AndrewFerr

---

SCT Stuff:

FCP closure tickyboxes

MSC checklist

[ShadowJonathan]

One nit, else this looks sound.

[turt2live]

My general concern is that we already have JSON available to us, so we should use that. String packing works in areas where we don't have as fine control (voip call candidates, for example), but for something like this we can and should afford fields.

[andybalaam]

Since the SCT came down on the side of this proposal vs MSC3760 I consider this resolved. Please unresolve if you disagree.

[clokep]

I'm not sure where the conversation about the SCT preferring this over MSC3760 happened, but I'm strongly against the current proposal. String packing is going to cause bugs, having separate fields seems better in every sense.

Hence my concern I've left about insufficient alternatives being explored.

[fkwp]

I think there’s general agreement that string packing is not a great long-term idea. This is not only a topic in the context of this MSC but also in the context of MSC4143.

The reason it’s being considered is that it’s an pragmatic change — lightweight for SDK maintainers to adopt and gives already (amongst others) the nice feature of pragmatic name-spacing which is a powerful property.

In contrast a big-bang change is disruptive and complex. Probably leading to a situation where nobody currently has time to implement it.

Hence, I am proposing deliver value without disruption (=using string packing) over disruptive and complex

[Xiretza]

Despite frequent appearances to the contrary, matrix is intended to be a production-quality protocol, not some finished-over-a-weekend demo. You don't do "pragmatic" hacks in such a project, especially not for a new feature. If the feature requires "complex and disruptive" changes, so be it - this PR has been sitting around for three and a half years now, it can wait a couple more until those changes are worked out.

[richvdh]

Right now, I am minded that the problems with this proposal (fundamentally: it's a hack that we don't want to have to support in the long term) outweigh the advantages of landing it swiftly.

[turt2live]

Concerns-I-have-raised update:

@mscbot resolve Alternatives section is missing some alternatives.
@mscbot resolve Specific alternative of top-level access control is not sufficiently discounted.

Introduction does not sufficiently list benefiting MSCs/features.

The introduction currently relies on location sharing to drive it, which is a deprioritized feature at the spec level. I highly suggest adding the VoIP impact to the introduction to naturally drive this MSC up the list.

Insufficient implementation - Complement tests are required for this MSC.

This still appears to be the case.

[AndrewFerr]

Introduction does not sufficiently list benefiting MSCs/features.

The introduction currently relies on location sharing to drive it, which is a deprioritized feature at the spec level. I highly suggest adding the VoIP impact to the introduction to naturally drive this MSC up the list.

2a77266

Insufficient implementation - Complement tests are required for this MSC.

This still appears to be the case.

Is matrix-org/complement#733 not sufficient?

[Xiretza]

This part can most charitably be described as an awful hack. There should never be a reason to encode multiple independent values into a single string field inside a structured data format like JSON, let alone then enforcing strict length limits on different parts. No, this is terrible, the section "Multi-component state keys" describes what a real solution would have to look like.

[dbkr]

But above you've said that resolvable domain names with underscores exist: wouldn't this allow the attack you talk about below?

[turt2live]

@mscbot resolve Introduction does not sufficiently list benefiting MSCs/features.
@mscbot resolve Insufficient implementation - Complement tests are required for this MSC.

non-blocking

[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.

[Xiretza]

Is "attempts to pack structured data into a JSON string" not a valid concern in the opinion of the SCT?

[turt2live]

Please use threads for conversation, as otherwise there's a 95% chance we'll never see it.

The string packing concern is tracked as alternatives not being sufficiently explored, per https://github.com/matrix-org/matrix-spec-proposals/pull/3757/files#r2192809774

[dkasak]

To be honest, it's not clear to me that MatrixRTC call events should be state events. Call state is very ephemeral information, while room state is generally meant for longer-lived state, like room configuration.

Areas where room state did end up being used for quicker evolving information are precisely those that showcase many problems with such an approach: the need to retain stricter ownership over a subset of state events; the need to allow less privileged users to send state events, which brings about state spam concerns; the large volume of events calling for a mechanism to delete state, which turns out to be an almost impossible feat. Not to mention that state events in today's Matrix are unencrypted. It all screams bad fit to me.

I realise this is coming a bit late to the party given when MatrixRTC implementations are already running in the wild. But given that this is used as the sole example of a requirement for this MSC and there is widespread dissatisfaction with string packing, I think it needs to be raised.

[kegsay]

I second this. State events is the wrong place for this data, and by putting it in this wrong place we are creating a rod for our own back. String packing is, in the words of other people, a horrible hack. Does it work? Undeniably yes. Do we want to bake this into the protocol permanently? No. This is precisely the job of the SCT to block this kind of thing imo.

Now this does beg the question: what is the right place then? I've banged on internally that there isn't really a right place for this kind of data currently, and have been spending my downtime slowly crafting a proposal for an alternative. Effectively, all we want is:

• Custom EDUs which can be sent by clients
• persistent EDUs which guarantee delivery of the latest value sent on a per-room, per-user basis.

Thus sending a state event only you can modify becomes a persistent EDU (quite an oxymoron given the E stands for Ephemeral!) with whatever custom type/content you want.

The main driver for this MSC afaict is MatrixRTC so we can even drop the "custom" part for now and only have fixed known EDUs for now to reduce the scope and risk of the changes.

I'd be much happier to thumbs up a persistent EDU proposal than shoehorn in owned state events.

[richvdh]

State events is the wrong place for this data

Agreed.

Apart from the ephemerality of the data, another thing that makes it different to room state is that there is never any real reason to do state resolution. We only ever care about the "current" state, and (unlike regular state) there is a single server that can and should be trusted to be authoritative.

[kegsay]

I'd be much happier to thumbs up a persistent EDU proposal than shoehorn in owned state events.

#4354 is this.

[turt2live]

This MSC is blocked on conversations around possibly using non-state-events for VoIP becoming resolved.

[turt2live]

The SCT has determined that this MSC will not be accepted, and is (currently) favouring approaches like MSC4354 instead.

@mscbot fcp cancel
@mscbot fcp close

[mscbot]

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

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

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.

[MadLittleMods]

Suggested change

	there noneless exist resolvable domain names that contain underscores.
	there nonetheless exist resolvable domain names that contain underscores.