Soft-fail (#1364)

* Initial work on soft-fail

* Fix state block retrieval

* Copy-pasta QueryLatestEventsAndState code

* Fix state lookup

* Clean up

* Fix up failing sytest

* Linting

* Update previous events SQLite insert query

* Update SQLite InsertPreviousEvent properly

* Hopefully fix the event references updates

Co-authored-by: Kegan Dougal <kegan@matrix.org>

roomserver/api/wrapper.go

@@ -122,15 +122,7 @@ func SendEventWithRewrite(
 		// We will handle an event as if it's an outlier if one of the
 		// following conditions is true:
 		storeAsOutlier := false
-		if authOrStateEvent.Type() == event.Type() && *authOrStateEvent.StateKey() == *event.StateKey() {
-			// The event is a state event but the input event is going to
-			// replace it, therefore it can't be added to the state or we'll
-			// get duplicate state keys in the state block. We'll send it
-			// as an outlier because we don't know if something will be
-			// referring to it as an auth event, but need it to be stored
-			// just in case.
-			storeAsOutlier = true
-		} else if _, ok := isCurrentState[authOrStateEvent.EventID()]; !ok {
+		if _, ok := isCurrentState[authOrStateEvent.EventID()]; !ok {
 			// The event is an auth event and isn't a part of the state set.
 			// We'll send it as an outlier because we need it to be stored
 			// in case something is referring to it as an auth event.

roomserver/internal/helpers/auth.go

@@ -16,13 +16,78 @@ package helpers
 
 import (
 	"context"
+	"fmt"
 	"sort"
 
+	"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
+// and false otherwise. The return error value should be checked before
+// the soft-fail bool.
+func CheckForSoftFail(
+	ctx context.Context,
+	db storage.Database,
+	event gomatrixserverlib.HeaderedEvent,
+	stateEventIDs []string,
+) (bool, error) {
+	rewritesState := len(stateEventIDs) > 1
+
+	var authStateEntries []types.StateEntry
+	var err error
+	if rewritesState {
+		authStateEntries, err = db.StateEntriesForEventIDs(ctx, stateEventIDs)
+		if err != nil {
+			return true, fmt.Errorf("StateEntriesForEventIDs failed: %w", err)
+		}
+	} else {
+		// Work out if the room exists.
+		var roomInfo *types.RoomInfo
+		roomInfo, err = db.RoomInfo(ctx, event.RoomID())
+		if err != nil {
+			return false, fmt.Errorf("db.RoomNID: %w", err)
+		}
+		if roomInfo == nil || roomInfo.IsStub {
+			return false, nil
+		}
+
+		// Then get the state entries for the current state snapshot.
+		// We'll use this to check if the event is allowed right now.
+		roomState := state.NewStateResolution(db, *roomInfo)
+		authStateEntries, err = roomState.LoadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID)
+		if err != nil {
+			return true, fmt.Errorf("roomState.LoadStateAtSnapshot: %w", err)
+		}
+	}
+
+	// As a special case, it's possible that the room will have no
+	// state because we haven't received a m.room.create event yet.
+	// If we're now processing the first create event then never
+	// soft-fail it.
+	if len(authStateEntries) == 0 && event.Type() == gomatrixserverlib.MRoomCreate {
+		return false, nil
+	}
+
+	// Work out which of the state events we actually need.
+	stateNeeded := gomatrixserverlib.StateNeededForAuth([]gomatrixserverlib.Event{event.Unwrap()})
+
+	// Load the actual auth events from the database.
+	authEvents, err := loadAuthEvents(ctx, db, stateNeeded, authStateEntries)
+	if err != nil {
+		return true, fmt.Errorf("loadAuthEvents: %w", err)
+	}
+
+	// Check if the event is allowed.
+	if err = gomatrixserverlib.Allowed(event.Event, &authEvents); err != nil {
+		// return true, nil
+		return true, fmt.Errorf("gomatrixserverlib.Allowed: %w", err)
+	}
+	return false, nil
+}
+
 // CheckAuthEvents checks that the event passes authentication checks
 // Returns the numeric IDs for the auth events.
 func CheckAuthEvents(

roomserver/internal/input/input_events.go

@@ -53,6 +53,20 @@ func (r *Inputer) processRoomEvent(
 		isRejected = true
 	}
 
+	var softfail bool
+	if input.Kind == api.KindBackfill || input.Kind == api.KindNew {
+		// Check that the event passes authentication checks based on the
+		// current room state.
+		softfail, err = helpers.CheckForSoftFail(ctx, r.DB, headered, input.StateEventIDs)
+		if err != nil {
+			logrus.WithFields(logrus.Fields{
+				"event_id": event.EventID(),
+				"type":     event.Type(),
+				"room":     event.RoomID(),
+			}).WithError(err).Info("Error authing soft-failed event")
+		}
+	}
+
 	// If we don't have a transaction ID then get one.
 	if input.TransactionID != nil {
 		tdID := input.TransactionID
@@ -88,6 +102,7 @@ func (r *Inputer) processRoomEvent(
 			"event_id": event.EventID(),
 			"type":     event.Type(),
 			"room":     event.RoomID(),
+			"sender":   event.Sender(),
 		}).Debug("Stored outlier")
 		return event.EventID(), nil
 	}
@@ -110,11 +125,13 @@ func (r *Inputer) processRoomEvent(
 	}
 
 	// We stop here if the event is rejected: We've stored it but won't update forward extremities or notify anyone about it.
-	if isRejected {
+	if isRejected || softfail {
 		logrus.WithFields(logrus.Fields{
 			"event_id":  event.EventID(),
 			"type":      event.Type(),
 			"room":      event.RoomID(),
+			"soft_fail": softfail,
+			"sender":    event.Sender(),
 		}).Debug("Stored rejected event")
 		return event.EventID(), rejectionErr
 	}

roomserver/storage/sqlite3/previous_events_table.go

@@ -18,6 +18,8 @@ package sqlite3
 import (
 	"context"
 	"database/sql"
+	"fmt"
+	"strings"
 
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/roomserver/storage/shared"
@@ -25,10 +27,15 @@ import (
 	"github.com/matrix-org/dendrite/roomserver/types"
 )
 
+// TODO: previous_reference_sha256 was NOT NULL before but it broke sytest because
+// sytest sends no SHA256 sums in the prev_events references in the soft-fail tests.
+// In Postgres an empty BYTEA field is not NULL so it's fine there. In SQLite it
+// seems to care that it's empty and therefore hits a NOT NULL constraint on insert.
+// We should really work out what the right thing to do here is.
 const previousEventSchema = `
   CREATE TABLE IF NOT EXISTS roomserver_previous_events (
     previous_event_id TEXT NOT NULL,
-    previous_reference_sha256 BLOB NOT NULL,
+    previous_reference_sha256 BLOB,
     event_nids TEXT NOT NULL,
     UNIQUE (previous_event_id, previous_reference_sha256)
   );
@@ -45,6 +52,11 @@ const insertPreviousEventSQL = `
 	  VALUES ($1, $2, $3)
 `
 
+const selectPreviousEventNIDsSQL = `
+	SELECT event_nids FROM roomserver_previous_events
+	  WHERE previous_event_id = $1 AND previous_reference_sha256 = $2
+`
+
 // Check if the event is referenced by another event in the table.
 // This should only be done while holding a "FOR UPDATE" lock on the row in the rooms table for this room.
 const selectPreviousEventExistsSQL = `
@@ -55,6 +67,7 @@ const selectPreviousEventExistsSQL = `
 type previousEventStatements struct {
 	db                            *sql.DB
 	insertPreviousEventStmt       *sql.Stmt
+	selectPreviousEventNIDsStmt   *sql.Stmt
 	selectPreviousEventExistsStmt *sql.Stmt
 }
 
@@ -69,6 +82,7 @@ func NewSqlitePrevEventsTable(db *sql.DB) (tables.PreviousEvents, error) {
 
 	return s, shared.StatementList{
 		{&s.insertPreviousEventStmt, insertPreviousEventSQL},
+		{&s.selectPreviousEventNIDsStmt, selectPreviousEventNIDsSQL},
 		{&s.selectPreviousEventExistsStmt, selectPreviousEventExistsSQL},
 	}.Prepare(db)
 }
@@ -80,9 +94,28 @@ func (s *previousEventStatements) InsertPreviousEvent(
 	previousEventReferenceSHA256 []byte,
 	eventNID types.EventNID,
 ) error {
-	stmt := sqlutil.TxStmt(txn, s.insertPreviousEventStmt)
-	_, err := stmt.ExecContext(
-		ctx, previousEventID, previousEventReferenceSHA256, int64(eventNID),
+	var eventNIDs string
+	eventNIDAsString := fmt.Sprintf("%d", eventNID)
+	selectStmt := sqlutil.TxStmt(txn, s.selectPreviousEventExistsStmt)
+	err := selectStmt.QueryRowContext(ctx, previousEventID, previousEventReferenceSHA256).Scan(&eventNIDs)
+	if err != sql.ErrNoRows {
+		return fmt.Errorf("selectStmt.QueryRowContext.Scan: %w", err)
+	}
+	var nids []string
+	if eventNIDs != "" {
+		nids = strings.Split(eventNIDs, ",")
+		for _, nid := range nids {
+			if nid == eventNIDAsString {
+				return nil
+			}
+		}
+		eventNIDs = strings.Join(append(nids, eventNIDAsString), ",")
+	} else {
+		eventNIDs = eventNIDAsString
+	}
+	insertStmt := sqlutil.TxStmt(txn, s.insertPreviousEventStmt)
+	_, err = insertStmt.ExecContext(
+		ctx, previousEventID, previousEventReferenceSHA256, eventNIDs,
 	)
 	return err
 }

sytest-blacklist

@@ -53,6 +53,3 @@ Outbound federation requests missing prev_events and then asks for /state_ids an
 
 # We don't implement lazy membership loading yet.
 The only membership state included in a gapped incremental sync is for senders in the timeline
-
-# flakey since implementing rejected events
-Inbound federation correctly soft fails events

sytest-whitelist

@@ -473,3 +473,5 @@ Local users can peek by room alias
 Peeked rooms only turn up in the sync for the device who peeked them
 Room state at a rejected message event is the same as its predecessor
 Room state at a rejected state event is the same as its predecessor
+Inbound federation correctly soft fails events
+Inbound federation accepts a second soft-failed event