From c0a6932b1bbaf235fef62833e958febd6a82e2c3 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 23 May 2022 15:34:11 +0100 Subject: [PATCH] Kick back joins with invalid authorising user IDs, use event from `"event"` key if returned in `RespSendJoin` --- federationapi/internal/perform.go | 8 +++++++- federationapi/routing/join.go | 30 +++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/federationapi/internal/perform.go b/federationapi/internal/perform.go index 7ccd68ef0..1fc58e12e 100644 --- a/federationapi/internal/perform.go +++ b/federationapi/internal/perform.go @@ -209,10 +209,16 @@ func (r *FederationInternalAPI) performJoinUsingServer( } r.statistics.ForServer(serverName).Success() - authEvents := respSendJoin.AuthEvents.UntrustedEvents(respMakeJoin.RoomVersion) + // If the remote server returned an event in the "event" key of + // the send_join request then we should use that instead. It may + // contain signatures that we don't know about. + if respSendJoin.Event != nil { + event = respSendJoin.Event + } // Sanity-check the join response to ensure that it has a create // event, that the room version is known, etc. + authEvents := respSendJoin.AuthEvents.UntrustedEvents(respMakeJoin.RoomVersion) if err = sanityCheckAuthChain(authEvents); err != nil { return fmt.Errorf("sanityCheckAuthChain: %w", err) } diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 23239d0e0..4b627f21f 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -15,6 +15,7 @@ package routing import ( + "encoding/json" "fmt" "net/http" "sort" @@ -174,6 +175,7 @@ func MakeJoin( // SendJoin implements the /send_join API // The make-join send-join dance makes much more sense as a single // flow so the cyclomatic complexity is high: +// nolint:gocyclo func SendJoin( httpReq *http.Request, request *gomatrixserverlib.FederationRequest, @@ -327,8 +329,34 @@ func SendJoin( } } + // If the membership content contains a user ID for a server that is not + // ours then we should kick it back. + var memberContent gomatrixserverlib.MemberContent + if err := json.Unmarshal(event.Content(), &memberContent); err != nil { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(err.Error()), + } + } + if memberContent.AuthorisedVia != "" { + _, domain, err := gomatrixserverlib.SplitID('@', memberContent.AuthorisedVia) + if err != nil { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(fmt.Sprintf("The authorising username %q is invalid.", memberContent.AuthorisedVia)), + } + } + if domain != cfg.Matrix.ServerName { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(fmt.Sprintf("The authorising username %q does not belong to this server.", memberContent.AuthorisedVia)), + } + } + } + // Sign the membership event. This is required for restricted joins to work - // in the case that the authorised via user is one of our own users. + // in the case that the authorised via user is one of our own users. It also + // doesn't hurt to do it even if it isn't a restricted join. signed := event.Sign( string(cfg.Matrix.ServerName), cfg.Matrix.KeyID,