From 58af9a2712ca18bc43d37d5d8de62bf85afe55db Mon Sep 17 00:00:00 2001
From: Till <2353100+S7evinK@users.noreply.github.com>
Date: Tue, 24 Oct 2023 14:47:30 +0200
Subject: [PATCH] Check if event is rejected before using it for auth (#421)

To be even more spec compliant, this adds a `IsRejected` type/function
when resolving state.
(Part 2 of https://github.com/matrix-org/gomatrixserverlib/pull/419)
---
 stateresolution.go        |  3 +-
 stateresolution_test.go   |  2 +-
 stateresolutionv2.go      | 12 ++++++-
 stateresolutionv2_test.go | 67 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/stateresolution.go b/stateresolution.go
index 767a29ef..d629455d 100644
--- a/stateresolution.go
+++ b/stateresolution.go
@@ -331,6 +331,7 @@ func ResolveConflicts(
 	events []PDU,
 	authEvents []PDU,
 	userIDForSender spec.UserIDForSender,
+	isRejectedFn IsRejected,
 ) ([]PDU, error) {
 	type stateKeyTuple struct {
 		Type     string
@@ -389,7 +390,7 @@ func ResolveConflicts(
 		resolved = ResolveStateConflicts(conflicted, authEvents, userIDForSender)
 		resolved = append(resolved, notConflicted...)
 	case StateResV2:
-		resolved = ResolveStateConflictsV2(conflicted, notConflicted, authEvents, userIDForSender)
+		resolved = ResolveStateConflictsV2(conflicted, notConflicted, authEvents, userIDForSender, isRejectedFn)
 	default:
 		return nil, fmt.Errorf("unsupported state resolution algorithm %v", stateResAlgo)
 	}
diff --git a/stateresolution_test.go b/stateresolution_test.go
index 8ec81b3c..8c361167 100644
--- a/stateresolution_test.go
+++ b/stateresolution_test.go
@@ -48,7 +48,7 @@ func TestStateResV1(t *testing.T) {
 
 	authEvents := []PDU{a1, a2, a3}
 
-	resolved, err := ResolveConflicts(RoomVersionV1, conflicted, authEvents, UserIDForSenderTest)
+	resolved, err := ResolveConflicts(RoomVersionV1, conflicted, authEvents, UserIDForSenderTest, isRejectedTest)
 	if err != nil {
 		t.Fatalf("failed to resolve conflicts: %s", err)
 	}
diff --git a/stateresolutionv2.go b/stateresolutionv2.go
index 4aadc1f5..8ff346cd 100644
--- a/stateresolutionv2.go
+++ b/stateresolutionv2.go
@@ -46,15 +46,20 @@ type stateResolverV2 struct {
 	resolvedMembers           map[spec.SenderID]PDU         // Resolved member events
 	resolvedOthers            map[StateKeyTuple]PDU         // Resolved other events
 	result                    []PDU                         // Final list of resolved events
+	isRejectedFn              IsRejected                    // Check if the given eventID is rejected
 }
 
+// IsRejected should return if the given eventID is rejected or not.
+type IsRejected func(eventID string) bool
+
 // ResolveStateConflicts takes a list of state events with conflicting state
 // keys and works out which event should be used for each state event. This
 // function returns the resolved state, including unconflicted state events.
 func ResolveStateConflictsV2(
-	conflicted, unconflicted []PDU,
+	conflicted, unconflicted,
 	authEvents []PDU,
 	userIDForSender spec.UserIDForSender,
+	isRejectedFn IsRejected,
 ) []PDU {
 	// Prepare the state resolver.
 	conflictedControlEvents := make([]PDU, 0, len(conflicted))
@@ -69,6 +74,7 @@ func ResolveStateConflictsV2(
 		resolvedMembers:           make(map[spec.SenderID]PDU, len(conflicted)),
 		resolvedOthers:            make(map[StateKeyTuple]PDU, len(conflicted)),
 		result:                    make([]PDU, 0, len(conflicted)+len(unconflicted)),
+		isRejectedFn:              isRejectedFn,
 	}
 	var roomID *spec.RoomID
 	if len(conflicted) > 0 {
@@ -459,6 +465,10 @@ func (r *stateResolverV2) authAndApplyEvents(events []PDU) {
 						continue
 					}
 					if authEv.Type() == spec.MRoomMember && authEv.StateKeyEquals(needed) {
+						// Don't use rejected events for auth
+						if r.isRejectedFn(authEventID) {
+							continue
+						}
 						_ = r.authProvider.AddEvent(authEv)
 					}
 				}
diff --git a/stateresolutionv2_test.go b/stateresolutionv2_test.go
index ecd34be4..9e9f8cf3 100644
--- a/stateresolutionv2_test.go
+++ b/stateresolutionv2_test.go
@@ -426,6 +426,8 @@ func TestReverseTopologicalEventSorting(t *testing.T) {
 	}
 }
 
+func isRejectedTest(_ string) bool { return false }
+
 func TestStateResolutionOtherEventDoesntOverpowerPowerEvent(t *testing.T) {
 	eventJSONs := []string{
 		/* create event            */ `{"auth_events":[],"content":{"creator":"@anon-20220512_124253-1:localhost:8800","room_version":"6"},"depth":1,"hashes":{"sha256":"ej3MHt4EnQemwqnfLhgwN6RBArYc5JnWcZt1PI3m4hE"},"origin":"localhost:8800","origin_server_ts":1652359375504,"prev_events":[],"prev_state":[],"room_id":"!3CHu7khd0phWyTm5:localhost:8800","sender":"@anon-20220512_124253-1:localhost:8800","signatures":{"localhost:8800":{"ed25519:rhNBRg":"7Pu9f39yDWJtl8msrnz+sPSBEA2jOJ4tJsZ1Zb6Bi+vZQMzMWwT/U6GZipxQqaeJr0TpVMa7zq/YhivArRRbAA"}},"state_key":"","type":"m.room.create"}`,
@@ -463,6 +465,7 @@ func TestStateResolutionOtherEventDoesntOverpowerPowerEvent(t *testing.T) {
 		unconflicted, // unconflicted set
 		events,       // full auth set
 		UserIDForSenderTest,
+		isRejectedTest,
 	)
 	t.Log("Resolved:")
 	for k, v := range result {
@@ -489,6 +492,7 @@ func runStateResolutionV2(t *testing.T, additional []PDU, expected []string) {
 		unconflicted, // unconflicted set
 		input,        // full auth set
 		UserIDForSenderTest,
+		isRejectedTest,
 	)
 
 	t.Log("Result:")
@@ -571,7 +575,68 @@ func TestStateReset(t *testing.T) {
 		"$IaXTO8US6DmRrPexxTucUaMoc7M0uwvMrOzWoFSsy5o": jr2Ev,
 	}
 
-	resolved := ResolveStateConflictsV2(conflicted, unconflicted, authEvents, UserIDForSenderTest)
+	resolved := ResolveStateConflictsV2(conflicted, unconflicted, authEvents, UserIDForSenderTest, isRejectedTest)
+	unexpectedEvents := make(map[string]PDU)
+	for _, resolvedEv := range resolved {
+		if _, found := expectedEvents[resolvedEv.EventID()]; !found {
+			unexpectedEvents[resolvedEv.EventID()] = resolvedEv
+			continue
+		}
+		delete(expectedEvents, resolvedEv.EventID())
+	}
+
+	if len(expectedEvents) > 0 {
+		t.Error("Expected event missing after state resolution:")
+		for evID, ev := range expectedEvents {
+			t.Errorf("\t%s: %s", evID, ev.JSON())
+		}
+	}
+
+	if len(unexpectedEvents) > 0 {
+		t.Error("Unexpected events after state resolution:")
+		for evID, ev := range unexpectedEvents {
+			t.Errorf("\t%s: %s", evID, ev.JSON())
+		}
+	}
+}
+
+// Basically copy and paste of TestStateReset, with the difference
+// that the join event of Bob (bobJoinEv) is rejected when authing
+// the name change event. This results in Bob not being in the room.
+func TestStateResetWithRejectedJoin(t *testing.T) {
+	// NOTE: The following events are taken from a Dendrite UT.
+	createEv := mustParseEvent(t, []byte(`{"auth_events":[],"content":{"creator":"@1:test","room_version":"9"},"depth":1,"hashes":{"sha256":"OZriBeMNVoymY/JqjM3Ee6oKSfoWskCiy48dUq5crR8"},"origin":"test","origin_server_ts":1697134517143,"prev_events":[],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"eyVzDAWjFtEaDtcYD2aLOjwxYegJSRJTEg5eRkksL3rNJUB0nRim2iGGCznjNzTg3V84K4bmuIs41aR7A2TBBQ"}},"state_key":"","type":"m.room.create"}`))
+	aliceJoinEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg"],"content":{"membership":"join"},"depth":2,"hashes":{"sha256":"xktNmFYn936RCil8B5h5Jfb+BFuyCKfUsOHCU92KHbE"},"origin":"test","origin_server_ts":1697134517143,"prev_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg"],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"cBaNa3UDzDE/TEH0ZmDciJj7aa5XOv8Ze1F+YGxea/TI86ivD6ULkylEl9+52A3kNC1/k8u7d7VZTDEMA/YZCw"}},"state_key":"@1:test","type":"m.room.member"}`))
+	plEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc"],"content":{"ban":50,"events":{"m.room.avatar":50,"m.room.canonical_alias":50,"m.room.encryption":100,"m.room.history_visibility":100,"m.room.name":50,"m.room.power_levels":100,"m.room.server_acl":100,"m.room.tombstone":100},"events_default":0,"invite":0,"kick":50,"notifications":{"room":50},"redact":50,"state_default":50,"users":{"@1:test":100},"users_default":0},"depth":3,"hashes":{"sha256":"Tg8kVJh7Pam9Q9rkMa2qoPzkQ2febdBLGiB2dB+6aqQ"},"origin":"test","origin_server_ts":1697134517143,"prev_events":["$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc"],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"1BKFKdklWUkxeKn8+9lGUVRNYSTscFwP0JR6KPH/KvuqOdOOl896mIJ3lp9iLrrHFYEOP0+Tl/gWWjY0r4zoBA"}},"state_key":"","type":"m.room.power_levels"}`))
+	jrEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0","$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc"],"content":{"join_rule":"public"},"depth":4,"hashes":{"sha256":"he+A0e/+4282sOZ0E6eXAGrQ7b2wAHfr4E/qMX2Sosg"},"origin":"test","origin_server_ts":1697134517144,"prev_events":["$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0"],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"tnX+YOaNhWipRTAsnXtxDeT0OLGQRhQN/cWF4cjLj92zjfyvGPIgoMXBpdFojq+0TiCsTPx673aWiYKFGap3AA"}},"state_key":"","type":"m.room.join_rules"}`))
+	hisVisEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0","$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc"],"content":{"history_visibility":"shared"},"depth":5,"hashes":{"sha256":"keUaZvadB9S775FT2/WOog+XwqcquUMDLlsKjtx7HYI"},"origin":"test","origin_server_ts":1697134517144,"prev_events":["$ZDzKFnVFil6ea2QoMa5wpFW_RJe5kTEv2ZD4jctEWM4"],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"OX1cA63VvYy/xli+kzJqAy/0f20RKMxn8khqvWYry3bRQqXRN2Z+er3wMFo6dego9e37l1b4UWXnoFvAUYbaDA"}},"state_key":"","type":"m.room.history_visibility"}`))
+	bobJoinEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$ZDzKFnVFil6ea2QoMa5wpFW_RJe5kTEv2ZD4jctEWM4","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0"],"content":{"membership":"join"},"depth":6,"hashes":{"sha256":"eMAArXQGPaJMbU22Bvgqzks1zLMiQTGXI28eT4CsEaM"},"origin":"test","origin_server_ts":1697134517145,"prev_events":["$KvMXxqhECWclFe58hgxr_s26ytJ57olFSMv2uVjbtSo"],"prev_state":[],"room_id":"!2:test","sender":"@2:test","signatures":{"test":{"ed25519:test":"TT+NLaNGzJbEe2B9AZ1rPUX/Af3S7rHpZK2Vqv3ioxrH7pzs7nDSs2+1G9+yxxjqLtybi1QuFbyGKAb2J/DmDA"}},"state_key":"@2:test","type":"m.room.member"}`))
+	charlieJoinEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$ZDzKFnVFil6ea2QoMa5wpFW_RJe5kTEv2ZD4jctEWM4","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0"],"content":{"membership":"join"},"depth":7,"hashes":{"sha256":"lsD2HYq8ovMhfTznU4RH2vk/cZTa8nwQVM8LYyCTzZs"},"origin":"test","origin_server_ts":1697134517145,"prev_events":["$AbMj-EZa-blPfjNYj9dPUXAQM5oLxcnPdoUvtTVI-hs"],"prev_state":[],"room_id":"!2:test","sender":"@3:test","signatures":{"test":{"ed25519:test":"5wOjA9IZkN28/yd4bJl8elnsxGR9QjDzKhSxDq/Js24hnqErV0DRB4luETZZDTWdCJJPRy5q7mIOoCTr6Zv/DQ"}},"state_key":"@3:test","type":"m.room.member"}`))
+	bobNameEv := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$ZDzKFnVFil6ea2QoMa5wpFW_RJe5kTEv2ZD4jctEWM4","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0","$AbMj-EZa-blPfjNYj9dPUXAQM5oLxcnPdoUvtTVI-hs"],"content":{"displayname":"Bob!","membership":"join"},"depth":10,"hashes":{"sha256":"GKm317RAdHnrAnESQRocRS5DhSJ756/3UjxBskjKEz4"},"origin":"test","origin_server_ts":1697134517386,"prev_events":["$sd5vMK06VQ28CEnyUfZuJbiD-HXSsQXAS2-6Mm906qk"],"prev_state":[],"room_id":"!2:test","sender":"@2:test","signatures":{"test":{"ed25519:test":"oXQalkq0zQ28+KFueHLTD+hXu2oq4/MgN9w2jThn79IICDl+RDp0svYzqaYRahwbBDpTjPvWXjjiN6oJ4Z53Bw"}},"state_key":"@2:test","type":"m.room.member"}`))
+	jr2Ev := mustParseEvent(t, []byte(`{"auth_events":["$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg","$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0","$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc"],"content":{"join_rule":"invite"},"depth":11,"hashes":{"sha256":"HtEx3O2ORo/V5Q3FHQVuPi8AvXts5tcMV5BbmsHcm+E"},"origin":"test","origin_server_ts":1697134517410,"prev_events":["$7w83ropQafHxxgMcJFWgynFXN_f1L-DgVHUPYC8KYVY"],"prev_state":[],"room_id":"!2:test","sender":"@1:test","signatures":{"test":{"ed25519:test":"5euVd5N/01mjrYQoNLlSnQWqonb2DUPSwQ4ZQz0A34fQzV3z+QjsBaIByYh2ZwkhFHnmJUvC8rUH8+JSv28NAA"}},"state_key":"","type":"m.room.join_rules"}`))
+
+	// conflicted/unconflicted as calculated by Dendrite
+	conflicted := []PDU{bobJoinEv, bobNameEv}
+	unconflicted := []PDU{createEv, aliceJoinEv, plEv, jrEv, hisVisEv, charlieJoinEv, jr2Ev}
+	authEvents := append(unconflicted, conflicted...)
+
+	// The events we expect after state resolution
+	expectedEvents := map[string]PDU{
+		"$0B4FVZWbziXiuaBZyVerHDfBs40toK4FhoT1DNLs_tg": createEv,
+		"$pha7iGLaAXqkf_GAwBhPtdyjM0DF4qxiAbbc-zGJbRc": aliceJoinEv,
+		"$B6yTeN_9fWhp5duir471Ac-OSC9BlsHnlRcbVpfmOH0": plEv,
+		"$KvMXxqhECWclFe58hgxr_s26ytJ57olFSMv2uVjbtSo": hisVisEv,
+		"$9IhOwrJzpmcAKaq05_MmKq7mqBUZENVoFOYz1_GQgX4": charlieJoinEv,
+		"$IaXTO8US6DmRrPexxTucUaMoc7M0uwvMrOzWoFSsy5o": jr2Ev,
+	}
+
+	// The join event of Bob is rejected, so don't use it to auth the name change event
+	// This results in Bob not being in the room.
+	var isRejected = func(eventID string) bool {
+		return true
+	}
+
+	resolved := ResolveStateConflictsV2(conflicted, unconflicted, authEvents, UserIDForSenderTest, isRejected)
 	unexpectedEvents := make(map[string]PDU)
 	for _, resolvedEv := range resolved {
 		if _, found := expectedEvents[resolvedEv.EventID()]; !found {