Including `"signatures"` into event size calculations can become problematic

[ShadowJonathan]

Link to problem area: https://spec.matrix.org/v1.4/client-server-api/#size-limits

Issue

Currently, signatures is defined as the following schema:

  // ...

  "signatures": {
    "<domain>": {
      "ed25519:<key>:": "<signature>"
    }
  },

However, together with the following definition of event size limits, it can become problematic:

The complete event MUST NOT be larger than 65536 bytes, when formatted with the federation event format, including any signatures, and encoded as Canonical JSON.

I see three problems with this:

1. It is possible for a server to forget to proactively verify an event this way when a client uploads it, where, when sent over federation, together with an added signature, it suddenly becomes an event larger than 65kb, causing a receiving server to reject it.

2. The wording here, "any signature" is as ambiguous as "should", but this becomes problematic in the context of verifying events are within the limit, when "signatures" is the only field that's mutable after sending, making it possible that events suddenly become too large when additional signatures are added to it other than the originating server. Choosing "any" signature could also result in diverging implementations that pick-and-choose which signature should count towards the limit in that case.

3. As a continuation of the second problem, while there is wording in the spec that suggests only the origin server to attach a signature on the event (in the prelude of the server-server spec), as far as I know, there is no wording prohibiting servers to attach their signatures after the fact, and the above schema seems to indicate that more-than-one server's signature can be present in the signatures object for validation.

Solutions

Seeing as the above combination can create problems in a number of cases, but also seeing what created the intentions of the above, this can probably be solved in at least three ways;

1. "signatures" should be removed from the JSON object when it is validated for event size.

Seeing as how this is one of two mutable-after-creation fields (the other one being unsigned), it shouldn't suddenly "jam" an event like this, especially if due to these circumstances it could deny a whole room for federation (see this issue, where it originally came up), and especially when servers can arbitrarily add their own signatures, or an MSC/behaviour creates a runaway loop of servers adding their own signatures when passing events around (which could happen in P2P or similar developments, "chaining" an event's origins like that)

However, this would leave "signatures" unbounded in size over the network, potentially allowing a server to keep feeding the event to another servers with an enormous signatures object of nonsensical servers and keys, all of which the server would have to validate or fetch, or otherwise deal with. (DoS)

2. Redefine signatures to signature, with the following schema:

signature: {
    origin: "<domain>",
    keys: {
      "ed25519:<key>:": "<signature>"
    }
}

For all intents and purposes, this would make the signature an immutable part of the event, as the originating server can tentatively add the signature field at creation to check for size limits (making the defacto size limit a bit less than 65kb), and deny at the client-server endpoint if it doesnt.

But, once the signature is added, there is no way for other servers to add their own, keeping the event within the size limit as defined, even when it has been passed around.

This would require acknowledgement that this is a direction the SCT wants to go into, denying future extensibility of the "signatures" field by having other servers strategically add signatures to contribute to quicker or necessary additional verification.

3. Explicitly deny any other signature than the originating server in signatures.

This is an abridged version of no. 2, but it would not break backwards compatibility.

Related

https://github.com/cinnyapp/cinny/issues/866, where it originally came up

[ShadowJonathan]

From #matrix-spec, @deepbluev7 said:

Or you just say signatures can't be bigger than 4kb in the current spec version :3

I followed up with:

that would probably work well, together with isolating them from the event object like that [in solution 1]