diff --git a/sync3/caches/user.go b/sync3/caches/user.go index 5cdd0f1..d05fdb7 100644 --- a/sync3/caches/user.go +++ b/sync3/caches/user.go @@ -26,6 +26,10 @@ type TransactionIDFetcher interface { TransactionIDForEvents(userID, deviceID string, eventIDs []string) (eventIDToTxnID map[string]string) } +type JoinChecker interface { + IsUserJoined(userID, roomID string) bool +} + // UserRoomData describes a single room from the perspective of particular user. // It is primarily used in two places: // - in the caches.UserCache, to hold the latest version of user-specific data; and @@ -194,11 +198,12 @@ type UserCache struct { store UserCacheStore globalCache *GlobalCache txnIDs TransactionIDFetcher + joinChecker JoinChecker ignoredUsers map[string]struct{} ignoredUsersMu *sync.RWMutex } -func NewUserCache(userID string, globalCache *GlobalCache, store UserCacheStore, txnIDs TransactionIDFetcher) *UserCache { +func NewUserCache(userID string, globalCache *GlobalCache, store UserCacheStore, txnIDs TransactionIDFetcher, joinChecker JoinChecker) *UserCache { // see SyncLiveHandler.userCache for the initialisation proper, which works by // firing off a bunch of OnBlahBlah callbacks. uc := &UserCache{ @@ -210,6 +215,7 @@ func NewUserCache(userID string, globalCache *GlobalCache, store UserCacheStore, store: store, globalCache: globalCache, txnIDs: txnIDs, + joinChecker: joinChecker, ignoredUsers: make(map[string]struct{}), ignoredUsersMu: &sync.RWMutex{}, } @@ -572,12 +578,14 @@ func (c *UserCache) OnSpaceUpdate(ctx context.Context, parentRoomID, childRoomID c.roomToDataMu.Unlock() // now we need to notify connections for the _child_ - roomUpdate := &RoomEventUpdate{ - RoomUpdate: c.newRoomUpdate(ctx, childRoomID), - EventData: eventData, + // but only if they are allowed to see the child event (i.e they are joined to the child room) + if c.joinChecker.IsUserJoined(c.UserID, childRoomID) { + roomUpdate := &RoomEventUpdate{ + RoomUpdate: c.newRoomUpdate(ctx, childRoomID), + EventData: eventData, + } + c.emitOnRoomUpdate(ctx, roomUpdate) } - - c.emitOnRoomUpdate(ctx, roomUpdate) } func (c *UserCache) OnNewEvent(ctx context.Context, eventData *EventData) { diff --git a/sync3/caches/user_test.go b/sync3/caches/user_test.go index 5809317..04b572c 100644 --- a/sync3/caches/user_test.go +++ b/sync3/caches/user_test.go @@ -10,6 +10,12 @@ import ( "github.com/matrix-org/sliding-sync/sync3/caches" ) +type joinChecker struct{} + +func (c *joinChecker) IsUserJoined(userID, roomID string) bool { + return true +} + type txnIDFetcher struct { data map[string]string } @@ -82,7 +88,7 @@ func TestAnnotateWithTransactionIDs(t *testing.T) { fetcher := &txnIDFetcher{ data: tc.eventIDToTxnIDs, } - uc := caches.NewUserCache(userID, nil, nil, fetcher) + uc := caches.NewUserCache(userID, nil, nil, fetcher, &joinChecker{}) got := uc.AnnotateWithTransactionIDs(context.Background(), userID, "DEVICE", convertIDToEventStub(userID, tc.roomIDToEvents)) want := convertIDTxnToEventStub(userID, tc.wantRoomIDToEvents) if !reflect.DeepEqual(got, want) { diff --git a/sync3/handler/connstate_test.go b/sync3/handler/connstate_test.go index c5b4f11..0515431 100644 --- a/sync3/handler/connstate_test.go +++ b/sync3/handler/connstate_test.go @@ -18,6 +18,12 @@ import ( "github.com/matrix-org/sliding-sync/testutils" ) +type joinChecker struct{} + +func (c *joinChecker) IsUserJoined(userID, roomID string) bool { + return true +} + type NopExtensionHandler struct{} func (h *NopExtensionHandler) Handle(ctx context.Context, req extensions.Request, extCtx extensions.Context) (res extensions.Response) { @@ -106,7 +112,7 @@ func TestConnStateInitial(t *testing.T) { roomC.RoomID: {NID: 780, Timestamp: 789}, }, nil, nil } - userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}) + userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}, &joinChecker{}) dispatcher.Register(context.Background(), userCache.UserID, userCache) dispatcher.Register(context.Background(), sync3.DispatcherAllUsers, globalCache) userCache.LazyLoadTimelinesOverride = func(loadPos int64, roomIDs []string, maxTimelineEvents int) map[string]state.LatestEvents { @@ -279,7 +285,7 @@ func TestConnStateMultipleRanges(t *testing.T) { } return 1, roomMetadata, joinTimings, nil, nil } - userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}) + userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}, &joinChecker{}) userCache.LazyLoadTimelinesOverride = mockLazyRoomOverride dispatcher.Register(context.Background(), userCache.UserID, userCache) dispatcher.Register(context.Background(), sync3.DispatcherAllUsers, globalCache) @@ -458,7 +464,7 @@ func TestBumpToOutsideRange(t *testing.T) { }, nil, nil } - userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}) + userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}, &joinChecker{}) userCache.LazyLoadTimelinesOverride = mockLazyRoomOverride dispatcher.Register(context.Background(), userCache.UserID, userCache) dispatcher.Register(context.Background(), sync3.DispatcherAllUsers, globalCache) @@ -561,7 +567,7 @@ func TestConnStateRoomSubscriptions(t *testing.T) { roomD.RoomID: {NID: 4, Timestamp: 4}, }, nil, nil } - userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}) + userCache := caches.NewUserCache(userID, globalCache, &NopUserCacheStore{}, &NopTransactionFetcher{}, &joinChecker{}) userCache.LazyLoadTimelinesOverride = func(loadPos int64, roomIDs []string, maxTimelineEvents int) map[string]state.LatestEvents { result := make(map[string]state.LatestEvents) for _, roomID := range roomIDs { diff --git a/sync3/handler/handler.go b/sync3/handler/handler.go index 127dedd..01e36b9 100644 --- a/sync3/handler/handler.go +++ b/sync3/handler/handler.go @@ -528,7 +528,7 @@ func (h *SyncLiveHandler) userCache(userID string) (*caches.UserCache, error) { if ok { return c.(*caches.UserCache), nil } - uc := caches.NewUserCache(userID, h.GlobalCache, h.Storage, h) + uc := caches.NewUserCache(userID, h.GlobalCache, h.Storage, h, h.Dispatcher) // select all non-zero highlight or notif counts and set them, as this is less costly than looping every room/user pair err := h.Storage.UnreadTable.SelectAllNonZeroCountsForUser(userID, func(roomID string, highlightCount, notificationCount int) { uc.OnUnreadCounts(context.Background(), roomID, &highlightCount, ¬ificationCount) diff --git a/tests-e2e/security_test.go b/tests-e2e/security_test.go index d762c78..4e84617 100644 --- a/tests-e2e/security_test.go +++ b/tests-e2e/security_test.go @@ -324,3 +324,88 @@ func TestSecuritySpaceMetadataLeak(t *testing.T) { }) m.MatchResponse(t, res, m.MatchNoV3Ops(), m.MatchRoomSubscriptionsStrict(nil)) } + +// Test that adding a child room to a space does not leak global room metadata about that +// child room to users in the parent space. This information isn't strictly confidential as +// the /rooms/{roomId}/hierarchy endpoint will include such metadata (room name, avatar, join count, etc) +// because the user is part of the parent space. There isn't an attack vector here, but repro steps: +// - Alice and Bob are in a parent space. +// - Bob has a poller on SS running. +// - Alice is live streaming from SS. +// - Bob creates a child room in that space, and sends both the m.space.parent in the child room AND +// the m.space.child in the parent space. +// - Ensure that no information about the child room comes down Alice's connection. +func TestSecuritySpaceChildMetadataLeakFromParent(t *testing.T) { + alice := registerNewUser(t) + bob := registerNewUser(t) + parentName := "The Parent Room Name" + childName := "The Child Room Name" + + // Alice and Bob are in a parent space. + parentSpace := bob.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": parentName, + "creation_content": map[string]string{ + "type": "m.space", + }, + }) + alice.MustJoinRoom(t, parentSpace, []string{"hs1"}) + + // Bob has a poller on SS running. + bobRes := bob.SlidingSync(t, sync3.Request{}) + + // Alice is live streaming from SS. + aliceRes := alice.SlidingSync(t, sync3.Request{ + Lists: map[string]sync3.RequestList{ + "a": { + Ranges: [][2]int64{{0, 20}}, + }, + }, + }) + m.MatchResponse(t, aliceRes, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{ + parentSpace: { + m.MatchJoinCount(2), + m.MatchRoomName(parentName), + }, + })) + + // Bob creates a child room in that space, and sends both the m.space.parent in the child room AND + // the m.space.child in the parent space. + childRoom := bob.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": childName, + "initial_state": []map[string]interface{}{ + { + "type": "m.space.parent", + "state_key": parentSpace, + "content": map[string]interface{}{ + "canonical": true, + "via": []string{"hs1"}, + }, + }, + }, + }) + chlidEventID := bob.SendEventSynced(t, parentSpace, b.Event{ + Type: "m.space.child", + StateKey: ptr(childRoom), + Content: map[string]interface{}{ + "via": []string{"hs1"}, + }, + }) + // wait for SS to process it + bob.SlidingSyncUntilEventID(t, bobRes.Pos, parentSpace, chlidEventID) + + // Ensure that no information about the child room comes down Alice's connection. + aliceRes = alice.SlidingSync(t, sync3.Request{}, WithPos(aliceRes.Pos)) + m.MatchResponse(t, aliceRes, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{ + parentSpace: { + MatchRoomTimeline([]Event{{ + Type: "m.space.child", + StateKey: ptr(childRoom), + Content: map[string]interface{}{ + "via": []interface{}{"hs1"}, + }, + }}), + }, + })) +}