Clarify that join events only need to be signed by resident servers if using join_authorised_via_users_server

[erikjohnston]

Link to problem area: https://spec.matrix.org/v1.9/server-server-api/#put_matrixfederationv2send_joinroomideventid

The join event only needs a signature from a resident server if joining via the restricted join rules flow, i.e. join_authorised_via_users_server is specified.

c.f. MSC3083 and Room Version 8 auth rules

[progval]

For context: As mentioned in matrix-org/synapse#16717 / element-hq/synapse#16717, when joining a room, Conduit relies on the behavior currently written in the spec, which Synapse does not implement; and the change @erikjohnston suggests would make the spec consistent with Synapse.

[timokoesters]

If the signature is not needed, the server does not need to send the event at all.

[richvdh]

If the signature is not needed, the server does not need to send the event at all.

Could you be clearer about what you mean here? Naively: if servers don't send out join events, how do other servers in the room know that a user has joined?

[timokoesters]

The response to /send_join has a field called event which is used for restricted joins. As far as I can tell, @erikjohnston wants the signature on this event field to be optional. I argue, that instead of making that single field optional, we can make the entire event field optional and don't include it if the signature is not needed.

[Kladki]

Seems like the current behavior in the spec was intended, so Synapse is likely in the wrong here.

[clokep]

Seems like the current behavior in the spec was intended, so Synapse is likely in the wrong here.

I'm unsure if you're agreeing or disagreeing with the behavior in the issue. But the current spec and the MSC agree that the resident server only needs to sign the event in the case of a restricted join.

This is explained in the description of join_authorised_via_users_server, but could probably be clearer.

[Kladki]

I'm unsure if you're agreeing or disagreeing with the behavior in the issue.

I'm disagreeing, because what the spec says is very explicit.

But the current spec and the MSC agree that the resident server only needs to sign the event in the case of a restricted join

The spec doesn't seem to, and requires it if the room version supports restricted joins:

event SignedMembershipEvent Required if the room version supports restricted join rules. The signed copy of the membership event sent to other servers by the resident server, including the resident server’s signature.

https://spec.matrix.org/v1.10/server-server-api/#put_matrixfederationv2send_joinroomideventid

[clokep]

But the current spec and the MSC agree that the resident server only needs to sign the event in the case of a restricted join

The spec doesn't seem to, and requires it if the room version supports restricted joins:

A separate part of the spec is what I was referring to:

join_authorised_via_users_server: Required if the room is restricted and is joining through one of the conditions available. If the user is responding to an invite, this is not required.

[...]

The resident server which owns the provided user ID must have a valid signature on the event.

https://spec.matrix.org/v1.10/server-server-api/#_matrixfederationv2send_joinroomideventid_membership-event-content

---

The MSC is explicit that not all events have to be signed by the resident server, see footnote 3:

[A join event issued via /send_join is signed by not just the requesting server, but also the resident server] seems like an improvement regardless since the resident server is accepting the event on behalf of the joining server and ideally this should be verifiable after the fact, even for current room versions. Requiring all events to be signed and verified in this way is left to a future MSC.

The text in the spec may not be fully clear, but the behavior in conduit is incorrect. The resident server signature is only required when a user is joining via a restricted join. I would welcome an MSC that has the resident server always sign this event.

[Kladki]

Ah, seems you're correct, my bad. I will write a spec PR shortly, since this seems like an easy fix.

[richvdh]

Must admit that I am slightly struggling to wrap my head around this, so let me try to set it out step by step.

As I understand it:

1. The event property in the send_join response was added by MSC3083, and specced in Room versions 8 and 9: Restricted rooms matrix-spec-proposals#3387.

2. The current situation is that Synapse always returns an event, but only signs it when the room has a restricted join rule, and the user is joining via join_authorised_via_users_server.

3. The MSC says: "In order for the joining server to receive the proper signatures the join event will be returned via /send_join in the event field." By implication, in this situation, the resident server's signature must be included in the event property.

   IMHO, the MSC is unclear as to exactly when this applies. The footnote implies it was not intended to apply to all /send_join requests, but there are various potential interpretations as to when it should apply; for example must it be present for:

   • all joins to rooms of a version where restricted join rules are possible (ie, v8 and later).
   • all joins to rooms where the join_rules are set to restricted, irrespective of whether the user is responding to an invite.
   • only for joins to rooms where join_authorised_via_users_server, irrespective of whether the user is actually joining a room where that field is relevant.
   • only for joins to rooms where the join_rules are set to restricted, and the user is joining by virtue of an allow rule in the join restrictions.

   Synapse seems to have used the fourth interpretation.

4. As of spec v1.10, the spec says that event is "required if the room version supports restricted join rules". In other words, it has used the first interpretation above.

5. PR #1834 has modified the wording of the spec, but appears to use the third interpretation above.

6. Neither the spec, nor the MSC, make any mention of the expected behaviour of event when the "restricted join" condition does not apply. The implication is that it will not normally be present, and indeed it is hard to see how Synapse's current behaviour is useful.

   I don't think we have any precedent for this. There are two ways of looking at it:

   • It's an unspecced property. To allow protocol evolution, in general, Matrix would say that implementations are allowed to return any additional unspecced property they like and that the client should just ignore any unrecognised properties. Under this interpretation, Synapse's behaviour is legitimate, if unconventional.
   • event is a specced property, and if Synapse is going to return it, even when it is not required, it had better match the spec. Since Synapse does not include the resident server's signature, it is non-spec-compliant under this interpretation.

In other words: there are two factors at play:

• When, exactly, it is necessary to return a signed event. It seems clear that Synapse and the spec do not currently agree on this behaviour, though both are, arguably, consistent with the MSC. PR #1834 is an attempt to fix this, but unfortunately uses yet a different interpretation of the MSC.

• In situations where event is not necessary per the spec, should the resident server's signature be populated? Synapse does not populate it, which, if you squint hard enough, could be legitimate; I favour the view that Synapse's behaviour is, at least, very unhelpful.

[richvdh]

I've probably missed something here, but my proposal for dealing with it would be:

• Let's have another go at Clarify that the event field of the send_join is only required when performing a restricted join #1834, using the same interpretation of "when we need to return a signed event" as Synapse
• Let's stop Synapse returning unsigned events.

[clokep]

In situations where event is not necessary per the spec, should the resident server's signature be populated?

I think this is a backwards way of looking at it.

I would reword your two factors:

1. When does the resident server sign the event?
2. When does the resident server return the event?

For the first item, I'm of the opinion it is clear in the MSC: the signature should only be populated in situations where the join is authorized via one of the allow rules. In other situations the resident server should not sign the event.

For the second item, which seems to be the crux of this issue, I disagree that Synapse uses the fourth interpretation. It uses the first interpretation, but is lazy about it and just always returns the event (as an extra field for earlier room versions, as a required field for room versions 8+).

If you take my interpretation about when to sign as correct then event must be returned if it is signed. But correct server behavior is to always return it for newer room versions. The intention behind this was prep for having the resident always sign the event, but we never went down that route/couldn't convince ourselves it made sense.

(Note much of this is from my memory between conversations with @erikjohnston while designing this feature. I agree the MSC is vague, but the above was the intention in the wording.)

[clokep]

I've probably missed something here, but my proposal for dealing with it would be:

• Let's have another go at Clarify that the event field of the send_join is only required when performing a restricted join #1834, using the same interpretation of "when we need to return a signed event" as Synapse

• Let's stop Synapse returning unsigned events.

I think this makes sense if by the first one you mean event and not signed event, which always has to be returned.

[richvdh]

In situations where event is not necessary per the spec, should the resident server's signature be populated?

I think this is a backwards way of looking at it.

I would reword your two factors:

1. When does the resident server sign the event?
2. When does the resident server return the event?

Mmm ok. I think the point is that the MSC, and spec, only attempt to define behaviour in the first situation. As far as I can see, there is no mention in the MSC about situations in which it makes sense to return the event without having also signed it.

For the first item, I'm of the opinion it is clear in the MSC: the signature should only be populated in situations where the join is authorized via one of the allow rules. In other situations the resident server should not sign the event.

Ok. Is that the same as my "fourth interpretation" ("only for joins to rooms where the join_rules are set to restricted, and the user is joining by virtue of an allow rule in the join restrictions")? And the same as what Synapse is doing? I think it is.

But correct server behavior is to always return it [event] for newer room versions.

According to what definition? I don't see this in any MSCs, other than being mentioned as a future aspiration in MSC3083.

I think this makes sense if by the first one you mean event and not signed event, which always has to be returned.

Again, I dispute that.

[richvdh]

I've probably missed something here, but my proposal for dealing with it would be:

• Let's have another go at Clarify that the event field of the send_join is only required when performing a restricted join #1834, using the same interpretation of "when we need to return a signed event" as Synapse
• Let's stop Synapse returning unsigned events.

I think this makes sense if by the first one you mean event and not signed event, which always has to be returned.

Having discussed this further with @clokep (https://matrix.to/#/!NasysSDfxKxZBzJJoE:matrix.org/$0aRyYMjrzKlDeRHFSFQiNGQv_GgqlWaxGR6HFPhOmHs?via=matrix.org&via=envs.net&via=element.io etc), I think we're actually aligned on the solution here. Using @clokep's wording:

Servers should return event only when the resident server signs it, and they should sign it only during a restricted join.

This requires a change to the spec to be clearer about when event should be returned, and a change to Synapse so that it does not return event except when it is a restricted join.

[clokep]

I tried to clarify this again in #1840.