Clarify how implementations of federation /send are to apply Server Access Control Lists on a "per PDU basis"

[Gnuxie]

Link to problem area:
https://spec.matrix.org/v1.10/server-server-api/#server-access-control-lists-acls
https://spec.matrix.org/v1.10/client-server-api/#mroomserver_acl
https://spec.matrix.org/v1.10/server-server-api/#checks-performed-on-receipt-of-a-pdu

Issue
The description of the m.room.server_acl event says the following

The ACLs are applied to servers when they make requests

Note: Server ACLs do not restrict the events relative to the room DAG via authorisation rules, but instead act purely at the network layer to determine which servers are allowed to connect and interact with a given room.

The specification is incorrect that Server ACLs are only applied to the network layer, because in the server-server spec describing which endpoints need to be protected, the spec says:

When a remote server makes a request, it MUST be verified to be allowed by the server ACLs. If the server is denied access to a room, the receiving server MUST reply with a 403 HTTP status code and an errcode of M_FORBIDDEN.

The following endpoint prefixes MUST be protected:

/_matrix/federation/v1/send (on a per-PDU basis)

Which implies that federation/v1/send cannot reply with 403 and M_FORBIDDEN. If we make a reasonable assumption of the specification's intent, then we can assume that implementations must fail PDUs but it is unclear how or when with regards to the section describing checks performed on PDUs.

It is also unclear whether events are failed only when the federation send request originates from a server that is denied by the server ACL or whether the PDU itself originates from a server denied by the server ACL. It is understood to me that implementations can only reasonably perform the former, because backfilling is unprotected from server ACL. The reason being you would otherwise cause all historical events from a denied server to fail a per-PDU server ACL check.

Possible solution

A possible solution could be adding a step after 2. Passes signature checks, otherwise it is dropped. that checks the event's origin against the current state for m.room.server_acl when the request origin also matches against the current state for m.room.server_acl.

[Gnuxie]

Related: #1783

[richvdh]

The specification is incorrect that Server ACLs are only applied to the network layer,

I think it depends how you interpret "at the network layer". The point is that the ACLs are applied during the /send transaction, without particular regard to the events in question.

The only reason that /send is special here is because it can contain events for more than one room, some of which must be accepted, so we can't 403 the entire request. Instead, individual events are listed with errors in the /send response.

[richvdh]

... and the check is based on the server making the /send request, not the server mentioned in the sender of the event in question. (Indeed, we have no way of knowing if the sender is genuine or forged without doing a signature verification, which could involve requesting keys for the sender, which is all way too much activity if the /send request itself came from a server which is banned in the room in question.)

[Gnuxie]

Ok, with that in mind, would you be willing to accept a clarification to https://spec.matrix.org/v1.10/server-server-api/#transactions that specifies this behavior? Maybe it would be clearer as a subsection (4.1)? Where do you think is most appropriate?

[richvdh]

I would welcome a clarifying PR.

I'd suggest a paragraph or two within https://spec.matrix.org/v1.10/server-server-api/#server-access-control-lists-acls, and then give it a brief mention with a link "for more details" in https://spec.matrix.org/v1.10/server-server-api/#server-access-control-lists-acls.