From 088ad1dd21a06ad3c4ae2db2a73936ed3e0d809e Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 14 Oct 2022 09:14:54 +0200 Subject: [PATCH] Fix `outliers whose auth_events are in a different room are correctly rejected` (#2791) Fixes `outliers whose auth_events are in a different room are correctly rejected`, by validating that auth events are all from the same room and not using rejected events for event auth. --- go.mod | 2 +- go.sum | 4 +- roomserver/internal/helpers/auth.go | 20 +++++- roomserver/internal/input/input_events.go | 31 ++++++--- .../internal/input/input_events_test.go | 63 +++++++++++++++++++ sytest-whitelist | 3 +- test/event.go | 7 +++ test/room.go | 9 ++- 8 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 roomserver/internal/input/input_events_test.go diff --git a/go.mod b/go.mod index eefad89e..23d5655d 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/matrix-org/dugong v0.0.0-20210921133753-66e6b1c67e2e github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91 github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 - github.com/matrix-org/gomatrixserverlib v0.0.0-20221011115330-49fa704b9a64 + github.com/matrix-org/gomatrixserverlib v0.0.0-20221014061925-a132619fa241 github.com/matrix-org/pinecone v0.0.0-20220929155234-2ce51dd4a42c github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 github.com/mattn/go-sqlite3 v1.14.15 diff --git a/go.sum b/go.sum index 0d08ac69..a1069da3 100644 --- a/go.sum +++ b/go.sum @@ -384,8 +384,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91 h1:s7fexw github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91/go.mod h1:e+cg2q7C7yE5QnAXgzo512tgFh1RbQLC0+jozuegKgo= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= -github.com/matrix-org/gomatrixserverlib v0.0.0-20221011115330-49fa704b9a64 h1:QJmfAPC3P0ZHJzYD/QtbNc5EztKlK1ipRWP5SO/m4jw= -github.com/matrix-org/gomatrixserverlib v0.0.0-20221011115330-49fa704b9a64/go.mod h1:Mtifyr8q8htcBeugvlDnkBcNUy5LO8OzUoplAf1+mb4= +github.com/matrix-org/gomatrixserverlib v0.0.0-20221014061925-a132619fa241 h1:e5o68MWeU7wjTvvNKmVo655oCYesoNRoPeBb1Xfz54g= +github.com/matrix-org/gomatrixserverlib v0.0.0-20221014061925-a132619fa241/go.mod h1:Mtifyr8q8htcBeugvlDnkBcNUy5LO8OzUoplAf1+mb4= github.com/matrix-org/pinecone v0.0.0-20220929155234-2ce51dd4a42c h1:iCHLYwwlPsf4TYFrvhKdhQoAM2lXzcmDZYqwBNWcnVk= github.com/matrix-org/pinecone v0.0.0-20220929155234-2ce51dd4a42c/go.mod h1:K0N1ixHQxXoCyqolDqVxPM3ArrDtcMs8yegOx2Lfv9k= github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 h1:eCEHXWDv9Rm335MSuB49mFUK44bwZPFSDde3ORE3syk= diff --git a/roomserver/internal/helpers/auth.go b/roomserver/internal/helpers/auth.go index 935a045d..03d8bca0 100644 --- a/roomserver/internal/helpers/auth.go +++ b/roomserver/internal/helpers/auth.go @@ -19,10 +19,11 @@ import ( "fmt" "sort" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/dendrite/roomserver/state" "github.com/matrix-org/dendrite/roomserver/storage" "github.com/matrix-org/dendrite/roomserver/types" - "github.com/matrix-org/gomatrixserverlib" ) // CheckForSoftFail returns true if the event should be soft-failed @@ -129,6 +130,12 @@ type authEvents struct { stateKeyNIDMap map[string]types.EventStateKeyNID state stateEntryMap events EventMap + valid bool +} + +// Valid verifies that all auth events are from the same room. +func (ae *authEvents) Valid() bool { + return ae.valid } // Create implements gomatrixserverlib.AuthEventProvider @@ -197,6 +204,7 @@ func loadAuthEvents( needed gomatrixserverlib.StateNeeded, state []types.StateEntry, ) (result authEvents, err error) { + result.valid = true // Look up the numeric IDs for the state keys needed for auth. var neededStateKeys []string neededStateKeys = append(neededStateKeys, needed.Member...) @@ -218,6 +226,16 @@ func loadAuthEvents( if result.events, err = db.Events(ctx, eventNIDs); err != nil { return } + roomID := "" + for _, ev := range result.events { + if roomID == "" { + roomID = ev.RoomID() + } + if ev.RoomID() != roomID { + result.valid = false + break + } + } return } diff --git a/roomserver/internal/input/input_events.go b/roomserver/internal/input/input_events.go index d1b6bc73..60160e8e 100644 --- a/roomserver/internal/input/input_events.go +++ b/roomserver/internal/input/input_events.go @@ -19,9 +19,16 @@ package input import ( "context" "database/sql" + "errors" "fmt" "time" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/opentracing/opentracing-go" + "github.com/prometheus/client_golang/prometheus" + "github.com/sirupsen/logrus" + fedapi "github.com/matrix-org/dendrite/federationapi/api" "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/internal/eventutil" @@ -31,11 +38,6 @@ import ( "github.com/matrix-org/dendrite/roomserver/internal/helpers" "github.com/matrix-org/dendrite/roomserver/state" "github.com/matrix-org/dendrite/roomserver/types" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" - "github.com/opentracing/opentracing-go" - "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" ) // TODO: Does this value make sense? @@ -196,7 +198,7 @@ func (r *Inputer) processRoomEvent( isRejected := false authEvents := gomatrixserverlib.NewAuthEvents(nil) knownEvents := map[string]*types.Event{} - if err = r.fetchAuthEvents(ctx, logger, headered, &authEvents, knownEvents, serverRes.ServerNames); err != nil { + if err = r.fetchAuthEvents(ctx, logger, roomInfo, headered, &authEvents, knownEvents, serverRes.ServerNames); err != nil { return fmt.Errorf("r.fetchAuthEvents: %w", err) } @@ -336,7 +338,7 @@ func (r *Inputer) processRoomEvent( // doesn't have any associated state to store and we don't need to // notify anyone about it. if input.Kind == api.KindOutlier { - logger.Debug("Stored outlier") + logger.WithField("rejected", isRejected).Debug("Stored outlier") hooks.Run(hooks.KindNewEventPersisted, headered) return nil } @@ -536,6 +538,7 @@ func (r *Inputer) processStateBefore( func (r *Inputer) fetchAuthEvents( ctx context.Context, logger *logrus.Entry, + roomInfo *types.RoomInfo, event *gomatrixserverlib.HeaderedEvent, auth *gomatrixserverlib.AuthEvents, known map[string]*types.Event, @@ -557,9 +560,19 @@ func (r *Inputer) fetchAuthEvents( continue } ev := authEvents[0] + + isRejected := false + if roomInfo != nil { + isRejected, err = r.DB.IsEventRejected(ctx, roomInfo.RoomNID, ev.EventID()) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("r.DB.IsEventRejected failed: %w", err) + } + } known[authEventID] = &ev // don't take the pointer of the iterated event - if err = auth.AddEvent(ev.Event); err != nil { - return fmt.Errorf("auth.AddEvent: %w", err) + if !isRejected { + if err = auth.AddEvent(ev.Event); err != nil { + return fmt.Errorf("auth.AddEvent: %w", err) + } } } diff --git a/roomserver/internal/input/input_events_test.go b/roomserver/internal/input/input_events_test.go new file mode 100644 index 00000000..818e7715 --- /dev/null +++ b/roomserver/internal/input/input_events_test.go @@ -0,0 +1,63 @@ +package input + +import ( + "testing" + + "github.com/matrix-org/gomatrixserverlib" + + "github.com/matrix-org/dendrite/test" +) + +func Test_EventAuth(t *testing.T) { + alice := test.NewUser(t) + bob := test.NewUser(t) + + // create two rooms, so we can craft "illegal" auth events + room1 := test.NewRoom(t, alice) + room2 := test.NewRoom(t, alice, test.RoomPreset(test.PresetPublicChat)) + + authEventIDs := make([]string, 0, 4) + authEvents := []*gomatrixserverlib.Event{} + + // Add the legal auth events from room2 + for _, x := range room2.Events() { + if x.Type() == gomatrixserverlib.MRoomCreate { + authEventIDs = append(authEventIDs, x.EventID()) + authEvents = append(authEvents, x.Event) + } + if x.Type() == gomatrixserverlib.MRoomPowerLevels { + authEventIDs = append(authEventIDs, x.EventID()) + authEvents = append(authEvents, x.Event) + } + if x.Type() == gomatrixserverlib.MRoomJoinRules { + authEventIDs = append(authEventIDs, x.EventID()) + authEvents = append(authEvents, x.Event) + } + } + + // Add the illegal auth event from room1 (rooms are different) + for _, x := range room1.Events() { + if x.Type() == gomatrixserverlib.MRoomMember { + authEventIDs = append(authEventIDs, x.EventID()) + authEvents = append(authEvents, x.Event) + } + } + + // Craft the illegal join event, with auth events from different rooms + ev := room2.CreateEvent(t, bob, "m.room.member", map[string]interface{}{ + "membership": "join", + }, test.WithStateKey(bob.ID), test.WithAuthIDs(authEventIDs)) + + // Add the auth events to the allower + allower := gomatrixserverlib.NewAuthEvents(nil) + for _, a := range authEvents { + if err := allower.AddEvent(a); err != nil { + t.Fatalf("allower.AddEvent failed: %v", err) + } + } + + // Finally check that the event is NOT allowed + if err := gomatrixserverlib.Allowed(ev.Event, &allower); err == nil { + t.Fatalf("event should not be allowed, but it was") + } +} diff --git a/sytest-whitelist b/sytest-whitelist index a3218ed7..2bd8b940 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -743,4 +743,5 @@ User joining then leaving public room appears and dissappears from directory User in remote room doesn't appear in user directory after server left room User in shared private room does appear in user directory until leave Existing members see new member's presence -Inbound federation can return missing events for joined visibility \ No newline at end of file +Inbound federation can return missing events for joined visibility +outliers whose auth_events are in a different room are correctly rejected \ No newline at end of file diff --git a/test/event.go b/test/event.go index 73fc656b..0c7bf435 100644 --- a/test/event.go +++ b/test/event.go @@ -30,6 +30,7 @@ type eventMods struct { unsigned interface{} keyID gomatrixserverlib.KeyID privKey ed25519.PrivateKey + authEvents []string } type eventModifier func(e *eventMods) @@ -52,6 +53,12 @@ func WithUnsigned(unsigned interface{}) eventModifier { } } +func WithAuthIDs(evs []string) eventModifier { + return func(e *eventMods) { + e.authEvents = evs + } +} + func WithKeyID(keyID gomatrixserverlib.KeyID) eventModifier { return func(e *eventMods) { e.keyID = keyID diff --git a/test/room.go b/test/room.go index 94eb51bb..4328bf84 100644 --- a/test/room.go +++ b/test/room.go @@ -21,8 +21,9 @@ import ( "testing" "time" - "github.com/matrix-org/dendrite/internal/eventutil" "github.com/matrix-org/gomatrixserverlib" + + "github.com/matrix-org/dendrite/internal/eventutil" ) type Preset int @@ -174,11 +175,17 @@ func (r *Room) CreateEvent(t *testing.T, creator *User, eventType string, conten if err != nil { t.Fatalf("CreateEvent[%s]: failed to StateNeededForEventBuilder: %s", eventType, err) } + refs, err := eventsNeeded.AuthEventReferences(&r.authEvents) if err != nil { t.Fatalf("CreateEvent[%s]: failed to AuthEventReferences: %s", eventType, err) } builder.AuthEvents = refs + + if len(mod.authEvents) > 0 { + builder.AuthEvents = mod.authEvents + } + ev, err := builder.Build( mod.originServerTS, mod.origin, mod.keyID, mod.privKey, r.Version,