From 8a6152ca70aa86df651db54efb3b1ec68ec9ec78 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 4 Jun 2020 10:53:39 +0100 Subject: [PATCH 1/4] Enable room version 6 (#1087) * Return bad request on CS API /send if bad JSON * Return some more M_BAD_JSON in the right places * nolint because damnit gocyclo all I added was a type check for an error * Update gomatrixserverlib * Update gomatrixserverlib * Update sytest-whitelist * Update gomatrixserverlib * Update sytest-whitelist * NotJSON -> BadJSON --- clientapi/routing/membership.go | 8 +++++++- clientapi/routing/profile.go | 20 ++++++++++++++++++-- clientapi/routing/sendevent.go | 5 +++++ federationapi/routing/join.go | 7 ++++++- federationapi/routing/leave.go | 5 +++++ go.mod | 2 +- go.sum | 4 ++-- sytest-whitelist | 12 ++++++++++++ 8 files changed, 56 insertions(+), 7 deletions(-) diff --git a/clientapi/routing/membership.go b/clientapi/routing/membership.go index cd0ce7328..74d92a059 100644 --- a/clientapi/routing/membership.go +++ b/clientapi/routing/membership.go @@ -276,7 +276,13 @@ func checkAndProcessThreepid( Code: http.StatusNotFound, JSON: jsonerror.NotFound(err.Error()), } - } else if err != nil { + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return inviteStored, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + } + if err != nil { util.GetLogger(req.Context()).WithError(err).Error("threepid.CheckAndProcessInvite failed") er := jsonerror.InternalServerError() return inviteStored, &er diff --git a/clientapi/routing/profile.go b/clientapi/routing/profile.go index 1c1ee8036..dbf6ef1d9 100644 --- a/clientapi/routing/profile.go +++ b/clientapi/routing/profile.go @@ -91,6 +91,7 @@ func GetAvatarURL( } // SetAvatarURL implements PUT /profile/{userID}/avatar_url +// nolint:gocyclo func SetAvatarURL( req *http.Request, accountDB accounts.Database, device *authtypes.Device, userID string, producer *producers.UserUpdateProducer, cfg *config.Dendrite, @@ -156,7 +157,14 @@ func SetAvatarURL( events, err := buildMembershipEvents( req.Context(), memberships, newProfile, userID, cfg, evTime, rsAPI, ) - if err != nil { + switch e := err.(type) { + case nil: + case gomatrixserverlib.BadJSONError: + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + default: util.GetLogger(req.Context()).WithError(err).Error("buildMembershipEvents failed") return jsonerror.InternalServerError() } @@ -205,6 +213,7 @@ func GetDisplayName( } // SetDisplayName implements PUT /profile/{userID}/displayname +// nolint:gocyclo func SetDisplayName( req *http.Request, accountDB accounts.Database, device *authtypes.Device, userID string, producer *producers.UserUpdateProducer, cfg *config.Dendrite, @@ -270,7 +279,14 @@ func SetDisplayName( events, err := buildMembershipEvents( req.Context(), memberships, newProfile, userID, cfg, evTime, rsAPI, ) - if err != nil { + switch e := err.(type) { + case nil: + case gomatrixserverlib.BadJSONError: + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + default: util.GetLogger(req.Context()).WithError(err).Error("buildMembershipEvents failed") return jsonerror.InternalServerError() } diff --git a/clientapi/routing/sendevent.go b/clientapi/routing/sendevent.go index aa7317094..a8f8893ba 100644 --- a/clientapi/routing/sendevent.go +++ b/clientapi/routing/sendevent.go @@ -154,6 +154,11 @@ func generateSendEvent( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return nil, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(req.Context()).WithError(err).Error("internal.BuildEvent failed") resErr := jsonerror.InternalServerError() diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 617a6cab0..5b4e8db3e 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -102,6 +102,11 @@ func MakeJoin( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("internal.BuildEvent failed") return jsonerror.InternalServerError() @@ -157,7 +162,7 @@ func SendJoin( if err != nil { return util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.NotJSON("The request body could not be decoded into valid JSON. " + err.Error()), + JSON: jsonerror.BadJSON("The request body could not be decoded into valid JSON: " + err.Error()), } } diff --git a/federationapi/routing/leave.go b/federationapi/routing/leave.go index bd226d7ee..62ca11457 100644 --- a/federationapi/routing/leave.go +++ b/federationapi/routing/leave.go @@ -76,6 +76,11 @@ func MakeLeave( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("internal.BuildEvent failed") return jsonerror.InternalServerError() diff --git a/go.mod b/go.mod index 021cc02a4..5510e8e0e 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 - github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca + github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 github.com/mattn/go-sqlite3 v2.0.2+incompatible diff --git a/go.sum b/go.sum index 3f235d51e..5df03cbaa 100644 --- a/go.sum +++ b/go.sum @@ -356,8 +356,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 h1:Yb+Wlf github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3/go.mod h1:e+cg2q7C7yE5QnAXgzo512tgFh1RbQLC0+jozuegKgo= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 h1:Hr3zjRsq2bhrnp3Ky1qgx/fzCtCALOoGYylh2tpS9K4= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca h1:s/dJePRDKjD1fGeoTnEYFqPmp1v7fC6GTd6iFwCKxw8= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f h1:G0B8Yu/5RVg4PiNhr9Adw3g7RLmQ95wnOL7bz6tUKrU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f h1:pRz4VTiRCO4zPlEMc3ESdUOcW4PXHH4Kj+YDz1XyE+Y= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f/go.mod h1:y0oDTjZDv5SM9a2rp3bl+CU+bvTRINQsdb7YlDql5Go= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 h1:ntrLa/8xVzeSs8vHFHK25k0C+NV74sYMJnNSg5NoSRo= diff --git a/sytest-whitelist b/sytest-whitelist index 6236b28ed..c1c0c13ac 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -301,3 +301,15 @@ Can send messages with a wildcard device id to two devices Wildcard device messages wake up /sync # TODO: separate PR for: Wildcard device messages over federation wake up /sync Can send a to-device message to two users which both receive it using /sync +User can create and send/receive messages in a room with version 6 +local user can join room with version 6 +User can invite local user to room with version 6 +remote user can join room with version 6 +User can invite remote user to room with version 6 +Remote user can backfill in a room with version 6 +Inbound: send_join rejects invalid JSON for room version 6 +Outbound federation rejects backfill containing invalid JSON for events in room version 6 +Invalid JSON integers +Invalid JSON special values +Invalid JSON floats +Outbound federation will ignore a missing event with bad JSON for room version 6 From 225b72bd42fb104bccd4a6b98e770b7473eb00f7 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 4 Jun 2020 10:54:10 +0100 Subject: [PATCH 2/4] Don't reset counters before successful outgoing federation request (#1089) * Don't reset counters before successful outgoing federation request on incoming federation request * Comments --- federationsender/queue/destinationqueue.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/federationsender/queue/destinationqueue.go b/federationsender/queue/destinationqueue.go index 4ab610de7..bf9042f45 100644 --- a/federationsender/queue/destinationqueue.go +++ b/federationsender/queue/destinationqueue.go @@ -59,8 +59,11 @@ func (oq *destinationQueue) retry() { // and then skip ahead a lot which feels non-ideal but equally we can't persist thousands of events // in-memory to maybe-send it one day. Ideally we would just shove these pending events in a database // so we can send a lot of events. - oq.statistics.Success() - // if we were backing off, swap to not backing off and interrupt the select. + // + // Interrupt the backoff. If the federation request that happens as a result of this is successful + // then the counters will be reset there and the backoff will cancel. If the federation request + // fails then we will retry at the current backoff interval, so as to prevent us from spamming + // homeservers which are behaving badly. // We need to use an atomic bool here to prevent multiple calls to retry() blocking on the channel // as it is unbuffered. if oq.backingOff.CAS(true, false) { From 8c3f51d624aea751e61575e3d2c401f976d1f8ef Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 4 Jun 2020 11:13:40 +0100 Subject: [PATCH 3/4] Update are-we-synapse-yet (#1092) --- are-we-synapse-yet.list | 26 +++++++++++++++++++++++--- are-we-synapse-yet.py | 2 ++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/are-we-synapse-yet.list b/are-we-synapse-yet.list index cb90d6282..c088c8b5e 100644 --- a/are-we-synapse-yet.list +++ b/are-we-synapse-yet.list @@ -455,6 +455,19 @@ rmv User can invite remote user to room with version 5 rmv Remote user can backfill in a room with version 5 rmv Can reject invites over federation for rooms with version 5 rmv Can receive redactions from regular users over federation in room version 5 +rmv User can create and send/receive messages in a room with version 6 +rmv User can create and send/receive messages in a room with version 6 (2 subtests) +rmv local user can join room with version 6 +rmv User can invite local user to room with version 6 +rmv remote user can join room with version 6 +rmv User can invite remote user to room with version 6 +rmv Remote user can backfill in a room with version 6 +rmv Can reject invites over federation for rooms with version 6 +rmv Can receive redactions from regular users over federation in room version 6 +rmv Inbound federation rejects invites which include invalid JSON for room version 6 +rmv Outbound federation rejects invite response which include invalid JSON for room version 6 +rmv Inbound federation rejects invite rejections which include invalid JSON for room version 6 +rmv Server rejects invalid JSON in a version 6 room pre Presence changes are reported to local room members f,pre Presence changes are also reported to remote room members pre Presence changes to UNAVAILABLE are reported to local room members @@ -536,11 +549,11 @@ std Can recv device messages until they are acknowledged std Device messages with the same txn_id are deduplicated std Device messages wake up /sync std Can recv device messages over federation -std Device messages over federation wake up /sync +fsd Device messages over federation wake up /sync std Can send messages with a wildcard device id std Can send messages with a wildcard device id to two devices std Wildcard device messages wake up /sync -std Wildcard device messages over federation wake up /sync +fsd Wildcard device messages over federation wake up /sync adm /whois nsp /purge_history nsp /purge_history by ts @@ -578,6 +591,7 @@ frv A pair of servers can establish a join in a v2 room fsj Outbound federation rejects send_join responses with no m.room.create event frv Outbound federation rejects m.room.create events with an unknown room version fsj Event with an invalid signature in the send_join response should not cause room join to fail +fsj Inbound: send_join rejects invalid JSON for room version 6 fed Outbound federation can send events fed Inbound federation can receive events fed Inbound federation can receive redacted events @@ -636,6 +650,7 @@ fst Name/topic keys are correct fau Remote servers cannot set power levels in rooms without existing powerlevels fau Remote servers should reject attempts by non-creators to set the power levels fau Inbound federation rejects typing notifications from wrong remote +fau Users cannot set notifications powerlevel higher than their own fed Forward extremities remain so even after the next events are populated as outliers fau Banned servers cannot send events fau Banned servers cannot /make_join @@ -833,4 +848,9 @@ gst Guest user can call /events on another world_readable room (SYN-606) gst Real user can call /events on another world_readable room (SYN-606) gst Events come down the correct room pub Asking for a remote rooms list, but supplying the local server's name, returns the local rooms list -std Can send a to-device message to two users which both receive it using /sync \ No newline at end of file +std Can send a to-device message to two users which both receive it using /sync +fme Outbound federation will ignore a missing event with bad JSON for room version 6 +fbk Outbound federation rejects backfill containing invalid JSON for events in room version 6 +jso Invalid JSON integers +jso Invalid JSON floats +jso Invalid JSON special values \ No newline at end of file diff --git a/are-we-synapse-yet.py b/are-we-synapse-yet.py index ffed8d384..5d5128479 100755 --- a/are-we-synapse-yet.py +++ b/are-we-synapse-yet.py @@ -50,6 +50,7 @@ test_mappings = { "fpb": "Public Room API", "fdk": "Device Key APIs", "fed": "Federation API", + "fsd": "Send-to-Device APIs", }, "client_apis": { @@ -99,6 +100,7 @@ test_mappings = { "ign": "Ignore Users", "udr": "User Directory APIs", "app": "Application Services API", + "jso": "Enforced canonical JSON", }, } From feb32ba365a460e4dcd3e77d0d4aed61d7579610 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Thu, 4 Jun 2020 11:14:08 +0100 Subject: [PATCH 4/4] Encode v3 event IDs correctly (#1090) --- federationapi/routing/routing.go | 94 +++++++------------------------- internal/basecomponent/base.go | 13 ++++- internal/httpapi.go | 9 ++- 3 files changed, 39 insertions(+), 77 deletions(-) diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index b75a7941d..e6c6df658 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -37,6 +37,9 @@ const ( ) // Setup registers HTTP handlers with the given ServeMux. +// The provided publicAPIMux MUST have `UseEncodedPath()` enabled or else routes will incorrectly +// path unescape twice (once from the router, once from MakeFedAPI). We need to have this enabled +// so we can decode paths like foo/bar%2Fbaz as [foo, bar/baz] - by default it will decode to [foo, bar, baz] // // Due to Setup being used to call many other functions, a gocyclo nolint is // applied: @@ -76,11 +79,7 @@ func Setup( v1fedmux.Handle("/send/{txnID}", internal.MakeFedAPI( "federation_send", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return Send( httpReq, request, gomatrixserverlib.TransactionID(vars["txnID"]), cfg, rsAPI, producer, eduProducer, keys, federation, @@ -90,11 +89,7 @@ func Setup( v2fedmux.Handle("/invite/{roomID}/{eventID}", internal.MakeFedAPI( "federation_invite", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return Invite( httpReq, request, vars["roomID"], vars["eventID"], cfg, producer, keys, @@ -110,11 +105,7 @@ func Setup( v1fedmux.Handle("/exchange_third_party_invite/{roomID}", internal.MakeFedAPI( "exchange_third_party_invite", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return ExchangeThirdPartyInvite( httpReq, request, vars["roomID"], rsAPI, cfg, federation, producer, ) @@ -123,11 +114,7 @@ func Setup( v1fedmux.Handle("/event/{eventID}", internal.MakeFedAPI( "federation_get_event", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetEvent( httpReq.Context(), request, rsAPI, vars["eventID"], cfg.Matrix.ServerName, ) @@ -136,11 +123,7 @@ func Setup( v1fedmux.Handle("/state/{roomID}", internal.MakeFedAPI( "federation_get_state", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetState( httpReq.Context(), request, rsAPI, vars["roomID"], ) @@ -149,11 +132,7 @@ func Setup( v1fedmux.Handle("/state_ids/{roomID}", internal.MakeFedAPI( "federation_get_state_ids", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetStateIDs( httpReq.Context(), request, rsAPI, vars["roomID"], ) @@ -162,8 +141,7 @@ func Setup( v1fedmux.Handle("/event_auth/{roomID}/{eventID}", internal.MakeFedAPI( "federation_get_event_auth", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars := mux.Vars(httpReq) + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetEventAuth( httpReq.Context(), request, rsAPI, vars["roomID"], vars["eventID"], ) @@ -172,7 +150,7 @@ func Setup( v1fedmux.Handle("/query/directory", internal.MakeFedAPI( "federation_query_room_alias", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return RoomAliasToID( httpReq, federation, cfg, rsAPI, fsAPI, ) @@ -181,7 +159,7 @@ func Setup( v1fedmux.Handle("/query/profile", internal.MakeFedAPI( "federation_query_profile", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetProfile( httpReq, accountDB, cfg, asAPI, ) @@ -190,11 +168,7 @@ func Setup( v1fedmux.Handle("/user/devices/{userID}", internal.MakeFedAPI( "federation_user_devices", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetUserDevices( httpReq, deviceDB, vars["userID"], ) @@ -203,11 +177,7 @@ func Setup( v1fedmux.Handle("/make_join/{roomID}/{eventID}", internal.MakeFedAPI( "federation_make_join", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { roomID := vars["roomID"] eventID := vars["eventID"] queryVars := httpReq.URL.Query() @@ -232,11 +202,7 @@ func Setup( v1fedmux.Handle("/send_join/{roomID}/{eventID}", internal.MakeFedAPI( "federation_send_join", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { roomID := vars["roomID"] eventID := vars["eventID"] res := SendJoin( @@ -254,11 +220,7 @@ func Setup( v2fedmux.Handle("/send_join/{roomID}/{eventID}", internal.MakeFedAPI( "federation_send_join", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { roomID := vars["roomID"] eventID := vars["eventID"] return SendJoin( @@ -269,11 +231,7 @@ func Setup( v1fedmux.Handle("/make_leave/{roomID}/{eventID}", internal.MakeFedAPI( "federation_make_leave", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { roomID := vars["roomID"] eventID := vars["eventID"] return MakeLeave( @@ -284,11 +242,7 @@ func Setup( v2fedmux.Handle("/send_leave/{roomID}/{eventID}", internal.MakeFedAPI( "federation_send_leave", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { roomID := vars["roomID"] eventID := vars["eventID"] return SendLeave( @@ -306,22 +260,14 @@ func Setup( v1fedmux.Handle("/get_missing_events/{roomID}", internal.MakeFedAPI( "federation_get_missing_events", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return GetMissingEvents(httpReq, request, rsAPI, vars["roomID"]) }, )).Methods(http.MethodPost) v1fedmux.Handle("/backfill/{roomID}", internal.MakeFedAPI( "federation_backfill", cfg.Matrix.ServerName, keys, wakeup, - func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest) util.JSONResponse { - vars, err := internal.URLDecodeMapValues(mux.Vars(httpReq)) - if err != nil { - return util.ErrorResponse(err) - } + func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return Backfill(httpReq, request, rsAPI, vars["roomID"], cfg) }, )).Methods(http.MethodGet) diff --git a/internal/basecomponent/base.go b/internal/basecomponent/base.go index 0fc95e824..beaf0e86d 100644 --- a/internal/basecomponent/base.go +++ b/internal/basecomponent/base.go @@ -103,7 +103,18 @@ func NewBaseDendrite(cfg *config.Dendrite, componentName string, enableHTTPAPIs })} } - httpmux := mux.NewRouter() + // Ideally we would only use SkipClean on routes which we know can allow '/' but due to + // https://github.com/gorilla/mux/issues/460 we have to attach this at the top router. + // When used in conjunction with UseEncodedPath() we get the behaviour we want when parsing + // path parameters: + // /foo/bar%2Fbaz == [foo, bar%2Fbaz] (from UseEncodedPath) + // /foo/bar%2F%2Fbaz == [foo, bar%2F%2Fbaz] (from SkipClean) + // In particular, rooms v3 event IDs are not urlsafe and can include '/' and because they + // are randomly generated it results in flakey tests. + // We need to be careful with media APIs if they read from a filesystem to make sure they + // are not inadvertently reading paths without cleaning, else this could introduce a + // directory traversal attack e.g /../../../etc/passwd + httpmux := mux.NewRouter().SkipClean(true) return &BaseDendrite{ componentName: componentName, diff --git a/internal/httpapi.go b/internal/httpapi.go index bacd1768a..991a98619 100644 --- a/internal/httpapi.go +++ b/internal/httpapi.go @@ -174,7 +174,7 @@ func MakeFedAPI( serverName gomatrixserverlib.ServerName, keyRing gomatrixserverlib.KeyRing, wakeup *FederationWakeups, - f func(*http.Request, *gomatrixserverlib.FederationRequest) util.JSONResponse, + f func(*http.Request, *gomatrixserverlib.FederationRequest, map[string]string) util.JSONResponse, ) http.Handler { h := func(req *http.Request) util.JSONResponse { fedReq, errResp := gomatrixserverlib.VerifyHTTPRequest( @@ -184,7 +184,12 @@ func MakeFedAPI( return errResp } go wakeup.Wakeup(req.Context(), fedReq.Origin()) - return f(req, fedReq) + vars, err := URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + + return f(req, fedReq, vars) } return MakeExternalAPI(metricsName, h) }