matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.83k stars 2.13k forks source link

Setting room history to anyone exposes users to search #9202

Open t2d opened 3 years ago

t2d commented 3 years ago

Description

Setting history in a private room to "Anyone" exposes users to search although search_all_users: false .

Steps to reproduce

Resetting the history to "Members only (since the point in time of selecting this option)" restores the original behavior.

Done with Element 1.7.17.

Version information

Settings

#user_directory:
#  enabled: true
#  search_all_users: false

or

user_directory:
  enabled: true
  search_all_users: false
anoadragon453 commented 3 years ago

If search_all_users is false, then one is only able to see other users if:

A room that is set to world-readable (history visibility granted to anyone, not only those that are in the room), effectively makes that room public. Thus any users in there become visible to all others in user directory queries. This functionality was introduced in https://github.com/matrix-org/synapse/pull/2723.

Whether we retrospectively think that it's correct to have users in world-readable rooms included in all user directory searches, even with search_all_users turned off, is a matter of discussion.

t2d commented 3 years ago

I think this is at least a UX bug.

a) I didn't select "Make this room public" during creation. b) Other settings, implying a private room, don't change. Just check the section "Who can access this room?" directly above

Screenshot from 2021-02-01 16-29-54

c) I saw security-minded users fuck this up in the real world.

richvdh commented 3 years ago

A note for posterity: one of the reasons that we made users in peekable rooms visible in the directory was so that the directory had some content on a new server. A world-readable room is public anyway, so the users are publicly visible anyway. However, this may or may not have been a valid decision.

This feels like UX problems:

These are product decisions - @callahad to follow up with @nadonomy.

dkasak commented 3 years ago

It seems to me that it would make more sense to split this out into a separate and prominent "public room" vs "private room" toggle.

The "public room" state could have an explanation along the lines of "Anyone can read messages sent to this room regardless of whether they are a member or not. The room members are publicly searchable."

The "private room" setting could expose (or enable) the "Who can read history?" dialog, but this time without the "Anyone" option and reformulated as "How far into the past can a new room member see?" or similar.

I think the current situation is complicated further by the fact that there is no way to retroactively change the visibility of historical messages. I know that it says that the setting will not affect the visibility of existing history, but I needed exactly that on a few occasions and noticed the urge in myself to reach for the "Anyone" setting to try to accomplish this because that's the only setting that doesn't mention a limit.

richvdh commented 3 years ago

It seems to me that it would make more sense to split this out into a separate and prominent "public room" vs "private room" toggle.

Personally, I am not a fan of the terms "public" and "private room" because it's entirely possible for a room to be "private" on one axis and "public" on another. See https://github.com/matrix-org/matrix-doc/issues/2612.

callahad commented 3 years ago

It sounds like the concern here is unintentionally getting into a "fishbowl" situation: only invited people can participate, but everyone can observe. Which is a valid thing to want, but our UI doesn't make it clear when you've configured something in that way.

dkasak commented 3 years ago

Personally, I am not a fan of the terms "public" and "private room" because it's entirely possible for a room to be "private" on one axis and "public" on another. See matrix-org/matrix-doc#2612.

That's a fair point to raise, the terminology indeed seems imprecise here. My point was more that the value "Anyone" definitely seems out of place under history visibility and deserves a separate, more prominent setting.

From the POV of matrix-org/matrix-spec#633, we might differentiate between the world-readable and world-joinable aspects. callahad's fishbowl situation is a room which is world-readable, but not world-joinable. Another separate aspect could be whether the room is world-advertised, that is whether it is announced in the room directory (/publicRooms).

rltas commented 1 year ago

Whether we retrospectively think that it's correct to have users in world-readable rooms included in all user directory searches, even with search_all_users turned off, is a matter of discussion.

We just ran into a privacy issue because of this and it should at least by properly phrased in the docs. "If false, search results will only contain users visible in public rooms and users sharing a room with the requester. Defaults to false." A room with a world-readable history isn't public, there's a clear distinction between m.room.join_rules and m.room.history_visibility. "If false, search results will only contain users visible in public (join rules) or world-readable (history visibility) rooms and users sharing a room with the requester. Defaults to false." Something like that... This is especially unexpected when limit_profile_requests_to_users_who_share_rooms is true.