Multiple "Canonical JSON" definitions not distinct in spec

[neilalexander]

At the moment the spec has one definition for "Canonical JSON" in the appendices. In reality there are multiple places that we care about signing JSON, including but possibly not limited to:

• events for room versions 5 and earlier
• events for room versions 6 and later
• federation request body signing
• third-party invites
• device keys
• one-time keys
• cross-signing
• key backups

It isn't clear precisely which rules apply to which — did we at some point expect that the post-v6 change to strict canonical JSON enforcement should also apply to E2EE and federation request signing? Presumably we can't change federation request body signing if we still need to be able to send events for old rooms?

Need better clarity on precisely which behaviours to apply where (i.e. #1244, #1245).

(created from #1232)

[richvdh]

I think that the only place that the One True Definition in the appendices (https://spec.matrix.org/v1.3/appendices/#canonical-json) doesn't apply is in the signature for events for room versions 5 or earlier (and in fairness, the appendix does say that, though is vague about the exact differences).

did we at some point expect that the post-v6 change to strict canonical JSON enforcement should also apply to E2EE and federation request signing?

E2EE and Federation requests should always have followed the strict canonical JSON: any instance where they did not was a bug. The differences with events are:

• Once an event is created, it is set in stone. The same does not apply to most of the other objects mentioned here.
• For objects other than events, if implementations don't agree on whether an object is valid and correctly signed, the effects are constrained to that one particular object. For room events, it can cause the whole room to get out of sync.
• The problematic values (large numbers etc) have no legitimate place in most of the other objects, so the problem will only arise in the case of malicious activity - which will simply be rejected by an instance enforcing strict canonical json.

Presumably we can't change federation request body signing if we still need to be able to send events for old rooms?

Certainly we can. There is nothing that obliges us to be able to send events containing large numbers etc, even in old rooms. We can, and do, therefore enforce strict canonical JSON even on /send requests containing events in old rooms. (This does cause a problem if you attempt to do something like update a state event which contains a large number. IIRC Synapse rejects such attempts at the CS API.)

So... I think this is basically a duplicate of #1244?

[neilalexander]

E2EE and Federation requests should always have followed the strict canonical JSON: any instance where they did not was a bug.

Certainly we can. There is nothing that obliges us to be able to send events containing large numbers etc, even in old rooms. We can, and do, therefore enforce strict canonical JSON even on /send requests containing events in old rooms. (This does cause a problem if you attempt to do something like update a state event which contains a large number. IIRC Synapse rejects such attempts at the CS API.)

The CS API is one thing because there we can catch things before they ever reach the room. On the SS API it's a different story because we can't really safely drop things if they were accepted and became important to the fabric of a room because of a bug at the time, otherwise we end up breaking the room.

Let's say we have a room v3 state event with a large number in it. If the federation /_matrix/federation/v1/send request body is supposed to enforce the strict canonical JSON rules to generate the X-Matrix signature, then by extension we're saying it isn't possible for a /_matrix/federation/v1/send request body to contain that event which breaks those strict rules, even though that event was allowed by the room version at the time because that room version didn't require that strictness.

We have to canonicalise the request body because that's the only way to create a reproducible JSON blob to sign, but if we're always enforcing the strict set of rules in doing so, then we are potentially applying new strict rules to old non-strict JSON blobs.

[richvdh]

Let's say we have a room v3 state event with a large number in it. If the federation /send request body is supposed to enforce the strict canonical JSON rules to generate the X-Matrix signature, then by extension we're saying it isn't possible for a /send request body to contain that event which breaks those strict rules

That's true, you can't send such an event in /send (or rather, spec-compliant servers will reject such attempts). There's nothing stopping it being pulled in retrospectively though. Still, the best thing for a server to do is to not generate (new) events like that in the first place.

even though that event was allowed by the room version at the time because that room version didn't require that strictness.

Yes, but there's a difference between events which were created before the bug was discovered, and those that are created now. Any server which still allows events with malformed canonicaljson to be created can expect to have a bad time when doing so, but because they can still be backfilled, it's not going to break the fabric of the room.

(I've just realised that Synapse does not reject such requests, and I have filed matrix-org/synapse#13883 to track it.)