From 279044cd90722ba98b018b0aa277285113454822 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Fri, 2 Oct 2020 17:08:13 +0100 Subject: [PATCH] Add history visibility guards (#1470) * Add history visibility guards Default to 'joined' visibility to avoid leaking events, until we get around to implementing history visibility completely. Related #617 * Don't apply his vis checks on shared rooms * Fix order of checks * Linting and remove another misleading check * Update whitelist --- syncapi/routing/messages.go | 93 +++++++++++++++++++++++++++- syncapi/routing/routing.go | 2 +- syncapi/storage/shared/syncserver.go | 27 ++++++++ syncapi/storage/storage_test.go | 1 + sytest-whitelist | 10 +-- 5 files changed, 124 insertions(+), 9 deletions(-) diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go index 6447e5d5..9c6c6a80 100644 --- a/syncapi/routing/messages.go +++ b/syncapi/routing/messages.go @@ -26,6 +26,7 @@ import ( "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/syncapi/storage" "github.com/matrix-org/dendrite/syncapi/types" + userapi "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" "github.com/sirupsen/logrus" @@ -41,6 +42,7 @@ type messagesReq struct { from *types.TopologyToken to *types.TopologyToken fromStream *types.StreamingToken + device *userapi.Device wasToProvided bool limit int backwardOrdering bool @@ -58,7 +60,7 @@ const defaultMessagesLimit = 10 // client-server API. // See: https://matrix.org/docs/spec/client_server/latest.html#get-matrix-client-r0-rooms-roomid-messages func OnIncomingMessagesRequest( - req *http.Request, db storage.Database, roomID string, + req *http.Request, db storage.Database, roomID string, device *userapi.Device, federation *gomatrixserverlib.FederationClient, rsAPI api.RoomserverInternalAPI, cfg *config.SyncAPI, @@ -151,6 +153,7 @@ func OnIncomingMessagesRequest( wasToProvided: wasToProvided, limit: limit, backwardOrdering: backwardOrdering, + device: device, } clientEvents, start, end, err := mReq.retrieveEvents() @@ -238,6 +241,10 @@ func (r *messagesReq) retrieveEvents() ( } events = reversed(events) } + events = r.filterHistoryVisible(events) + if len(events) == 0 { + return []gomatrixserverlib.ClientEvent{}, *r.from, *r.to, nil + } // Convert all of the events into client events. clientEvents = gomatrixserverlib.HeaderedToClientEvents(events, gomatrixserverlib.FormatAll) @@ -252,6 +259,90 @@ func (r *messagesReq) retrieveEvents() ( return clientEvents, start, end, err } +// nolint:gocyclo +func (r *messagesReq) filterHistoryVisible(events []gomatrixserverlib.HeaderedEvent) []gomatrixserverlib.HeaderedEvent { + // TODO FIXME: We don't fully implement history visibility yet. To avoid leaking events which the + // user shouldn't see, we check the recent events and remove any prior to the join event of the user + // which is equiv to history_visibility: joined + joinEventIndex := -1 + for i, ev := range events { + if ev.Type() == gomatrixserverlib.MRoomMember && ev.StateKeyEquals(r.device.UserID) { + membership, _ := ev.Membership() + if membership == "join" { + joinEventIndex = i + break + } + } + } + + var result []gomatrixserverlib.HeaderedEvent + var eventsToCheck []gomatrixserverlib.HeaderedEvent + if joinEventIndex != -1 { + if r.backwardOrdering { + result = events[:joinEventIndex+1] + eventsToCheck = append(eventsToCheck, result[0]) + } else { + result = events[joinEventIndex:] + eventsToCheck = append(eventsToCheck, result[len(result)-1]) + } + } else { + eventsToCheck = []gomatrixserverlib.HeaderedEvent{events[0], events[len(events)-1]} + result = events + } + // make sure the user was in the room for both the earliest and latest events, we need this because + // some backpagination results will not have the join event (e.g if they hit /messages at the join event itself) + wasJoined := true + for _, ev := range eventsToCheck { + var queryRes api.QueryStateAfterEventsResponse + err := r.rsAPI.QueryStateAfterEvents(r.ctx, &api.QueryStateAfterEventsRequest{ + RoomID: ev.RoomID(), + PrevEventIDs: ev.PrevEventIDs(), + StateToFetch: []gomatrixserverlib.StateKeyTuple{ + {EventType: gomatrixserverlib.MRoomMember, StateKey: r.device.UserID}, + {EventType: gomatrixserverlib.MRoomHistoryVisibility, StateKey: ""}, + }, + }, &queryRes) + if err != nil { + wasJoined = false + break + } + var hisVisEvent, membershipEvent *gomatrixserverlib.HeaderedEvent + for i := range queryRes.StateEvents { + switch queryRes.StateEvents[i].Type() { + case gomatrixserverlib.MRoomMember: + membershipEvent = &queryRes.StateEvents[i] + case gomatrixserverlib.MRoomHistoryVisibility: + hisVisEvent = &queryRes.StateEvents[i] + } + } + if hisVisEvent == nil { + return events // apply no filtering as it defaults to Shared. + } + hisVis, _ := hisVisEvent.HistoryVisibility() + if hisVis == "shared" { + return events // apply no filtering + } + if membershipEvent == nil { + wasJoined = false + break + } + membership, err := membershipEvent.Membership() + if err != nil { + wasJoined = false + break + } + if membership != "join" { + wasJoined = false + break + } + } + if !wasJoined { + util.GetLogger(r.ctx).WithField("num_events", len(events)).Warnf("%s was not joined to room during these events, omitting them", r.device.UserID) + return []gomatrixserverlib.HeaderedEvent{} + } + return result +} + func (r *messagesReq) getStartEnd(events []gomatrixserverlib.HeaderedEvent) (start, end types.TopologyToken, err error) { start, err = r.db.EventPositionInTopology( r.ctx, events[0].EventID(), diff --git a/syncapi/routing/routing.go b/syncapi/routing/routing.go index f42679c6..141eec79 100644 --- a/syncapi/routing/routing.go +++ b/syncapi/routing/routing.go @@ -51,7 +51,7 @@ func Setup( if err != nil { return util.ErrorResponse(err) } - return OnIncomingMessagesRequest(req, syncDB, vars["roomID"], federation, rsAPI, cfg) + return OnIncomingMessagesRequest(req, syncDB, vars["roomID"], device, federation, rsAPI, cfg) })).Methods(http.MethodGet, http.MethodOptions) r0mux.Handle("/user/{userId}/filter", diff --git a/syncapi/storage/shared/syncserver.go b/syncapi/storage/shared/syncserver.go index edb51b34..b9f21913 100644 --- a/syncapi/storage/shared/syncserver.go +++ b/syncapi/storage/shared/syncserver.go @@ -769,6 +769,33 @@ func (d *Database) getJoinResponseForCompleteSync( return } + // TODO FIXME: We don't fully implement history visibility yet. To avoid leaking events which the + // user shouldn't see, we check the recent events and remove any prior to the join event of the user + // which is equiv to history_visibility: joined + joinEventIndex := -1 + for i := len(recentStreamEvents) - 1; i >= 0; i-- { + ev := recentStreamEvents[i] + if ev.Type() == gomatrixserverlib.MRoomMember && ev.StateKeyEquals(device.UserID) { + membership, _ := ev.Membership() + if membership == "join" { + joinEventIndex = i + if i > 0 { + // the create event happens before the first join, so we should cut it at that point instead + if recentStreamEvents[i-1].Type() == gomatrixserverlib.MRoomCreate && recentStreamEvents[i-1].StateKeyEquals("") { + joinEventIndex = i - 1 + break + } + } + break + } + } + } + if joinEventIndex != -1 { + // cut all events earlier than the join (but not the join itself) + recentStreamEvents = recentStreamEvents[joinEventIndex:] + limited = false // so clients know not to try to backpaginate + } + // Retrieve the backward topology position, i.e. the position of the // oldest event in the room's topology. var prevBatchStr string diff --git a/syncapi/storage/storage_test.go b/syncapi/storage/storage_test.go index 8f16642f..2869ac5d 100644 --- a/syncapi/storage/storage_test.go +++ b/syncapi/storage/storage_test.go @@ -689,6 +689,7 @@ func assertInvitedToRooms(t *testing.T, res *types.Response, roomIDs []string) { } func assertEventsEqual(t *testing.T, msg string, checkRoomID bool, gots []gomatrixserverlib.ClientEvent, wants []gomatrixserverlib.HeaderedEvent) { + t.Helper() if len(gots) != len(wants) { t.Fatalf("%s response returned %d events, want %d", msg, len(gots), len(wants)) } diff --git a/sytest-whitelist b/sytest-whitelist index df71e275..eaaeea00 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -90,8 +90,6 @@ Real non-joined users can get state for world_readable rooms Real non-joined users can get individual state for world_readable rooms #Real non-joined users can get individual state for world_readable rooms after leaving Real non-joined users cannot send messages to guest_access rooms if not joined -Real users can sync from world_readable guest_access rooms if joined -Real users can sync from default guest_access rooms if joined Can't forget room you're still in Can get rooms/{roomId}/members Can create filter @@ -236,13 +234,11 @@ Outbound federation can query v2 /send_join Inbound federation can receive v2 /send_join Message history can be paginated Getting messages going forward is limited for a departed room (SPEC-216) -m.room.history_visibility == "world_readable" allows/forbids appropriately for Real users Backfill works correctly with history visibility set to joined Guest user cannot call /events globally Guest users can join guest_access rooms Guest user can set display names Guest user cannot upgrade other users -m.room.history_visibility == "world_readable" allows/forbids appropriately for Guest users Guest non-joined user cannot call /events on shared room Guest non-joined user cannot call /events on invited room Guest non-joined user cannot call /events on joined room @@ -252,8 +248,6 @@ Guest non-joined users can get individual state for world_readable rooms Guest non-joined users cannot room initalSync for non-world_readable rooms Guest non-joined users can get individual state for world_readable rooms after leaving Guest non-joined users cannot send messages to guest_access rooms if not joined -Guest users can sync from world_readable guest_access rooms if joined -Guest users can sync from default guest_access rooms if joined Real non-joined users cannot room initalSync for non-world_readable rooms Push rules come down in an initial /sync Regular users can add and delete aliases in the default room configuration @@ -478,4 +472,6 @@ Federation key API can act as a notary server via a GET request Inbound /make_join rejects attempts to join rooms where all users have left Inbound federation rejects invites which include invalid JSON for room version 6 Inbound federation rejects invite rejections which include invalid JSON for room version 6 -GET /capabilities is present and well formed for registered user \ No newline at end of file +GET /capabilities is present and well formed for registered user +m.room.history_visibility == "joined" allows/forbids appropriately for Guest users +m.room.history_visibility == "joined" allows/forbids appropriately for Real users \ No newline at end of file