Users that knock on a room with a shared history visibility and are subsequently kicked are able to view all previous events

[zamanzamzz]

Description

I created a room (room version 10) as a test user with the following initial state:

• history_visibility: shared
• join_rule: knock

I'm observing that if another user knocks on the room and that admin test user kicks them, the kicked user is still able to fetch all of the previous events of the room using the /_matrix/client/v3/rooms/{roomId}/messages GET endpoint.

This seems like unexpected behaviour to me and a major risk to the privacy/confidentiality of the users in the room.

The Client-Server API Spec mentions this for shared room history visibility:

"Previous events are always accessible to newly joined members. All events in the room are accessible, even those sent when the member was not a part of the room."

I have a feeling that room members with membership set as knock are mistakenly being considered as newly joined members in this case.

Steps to reproduce

• the following steps refer to user 1 (room creator and admin) and user 2 (knocker)
• create a room as user 1 with the following properties:
  • history_visibility: shared
  • join_rule: knock
  • room_version: 10
• send a few test messages as user 1 in the room
• knock on the room as user 2
• kick user 2 as user 1
• access previous events in the room as user 2 using the /_matrix/client/v3/rooms/{roomId}/messages GET endpoint
• observe that the test messages sent as user 1 are accessible through this endpoint by user 2

Homeserver

Local test homeserver

Synapse Version

1.68.0

Installation Method

Docker (matrixdotorg/synapse)

Platform

Official Docker image running in a container on Manjaro Linux

Relevant log output

[Edit 2022-09-30 11:41 UTC by dmr: redacted logs which contained sensitive information.]

Anything else that would be useful to know?

No response

[DMRobertson]

Please note that your log output contains access tokens (syt_...). I have edited the description to redact your logs, but you should revoke those access tokens immediately by logging out the corresponding devices. You should also check the logs for other sensitive information or PII.

Assuming that user2's membership of the room is knock -> leave I agree this is a bug and a serious one at that, thank you for spotting and reporting.

[reivilibre]

Reckon it's down to this:

synapse/synapse/api/auth.py

Line 113 in eaed4e6

if allow_departed_users and membership == Membership.LEAVE:

Since the user has a leave membership, they are considered departed despite never being in the room. Later on, this means we call _filter_events_for_client with is_peeking = False.

[DMRobertson]

Couldn't see any obviously related open issues. Maybe #3515 but I think that's a stretch.

[zamanzamzz]

Please note that your log output contains access tokens (syt_...). I have edited the description to redact your logs, but you should revoke those access tokens immediately by logging out the corresponding devices. You should also check the logs for other sensitive information or PII.

Assuming that user2's membership of the room is knock -> leave I agree this is a bug and a serious one at that, thank you for spotting and reporting.

These are logs from a locally running test homeserver (that is not federating) that I'm using for development where I flush the database regularly as part of the development workflow. So the access tokens and other sensitive data shown in the logs aren't really an issue but I don't think the logs are that helpful so it should be okay I guess. I probably should've pointed that out originally though.

[DMRobertson]

No worries; I merely wanted to err on the side of caution!

[zamanzamzz]

I have started working on a fix on #14067 and I believe the /_matrix/client/v3/rooms/{roomId}/messages GET endpoint behaves as expected now.

However, I noticed that the shared history is still sent on an initial sync with an include_leave=true filter to users whose membership changed from knock to leave. I'll try and take a look at that as well.