search

matrix-org / matrix-spec

The Matrix protocol specification
Apache License 2.0
188 stars 94 forks source link

`unsigned` should be stripped of disallowed keys when received over S2S #1524

Open dkasak opened 1 year ago

dkasak commented 1 year ago

We should spell out the fact that the unsigned field of PDUs received over S2S should be stripped of everything except an explicitly allowed set of keys, to prevent issues as described in https://github.com/matrix-org/synapse/issues/11080.

Synapse already has mitigations for this, as does Dendrite.

turt2live commented 1 year ago

imo we shouldn't need to strip anything at the protocol level, but should be clear that "untrusted event bodies" goes as far as unsigned too.

dkasak commented 1 year ago

@turt2live Did you read the replaces_state example in https://github.com/matrix-org/synapse/issues/11080? What do you propose Synapse should do instead if not strip that field when it receives it over federation?

turt2live commented 1 year ago

Synapse can strip it, but the protocol doesn't have to :)

dkasak commented 1 year ago

I guess so, but given all major servers now do it like that, I'm unsure of what benefit the added laxness buys us.

dkasak commented 3 months ago

The root problem here is that unsigned is essentially shared between the origin and destination homeserver, so clients have no hope in knowing which one set a particular item. And obviously this matters, because there is a greater degree of trust placed in the local homeserver than any arbitrary homeserver.