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.

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
 }
 

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)
+			}
 		}
 	}
 

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")
+	}
+}