Add require_auth_for_room_directory option

[ara4n]

This specifies whether a server's /publicRooms API is available to the general public or not, or whether it requires the user to have an account on the server in question.

This lets people opt out of /publicRooms spiders.

[richvdh]

I'm not convinced this makes sense, and I am worried it is proposed as a knee-jerk reaction to a problem.

If your security relies on people not finding out about room aliases, you have a problem anyway. It's easy for someone to accidentally leak a permalink or something. If I don't want people joining my room, I set the permissions on it accordingly. Apart from anything else, there's nothing to stop people guessing the alias.

I think we should focus on making the join rules more powerful rather than worrying about the directory. Once we have that, we can exclude rooms with join restrictions from the directory.

The other reason I'm against this is that I don't think its presence would have prevented the problem at hand, which appears largely to have been due to a misunderstanding of the defaults. There is no reason to imagine that anyone would find this setting and set it; unless we are going to make it default to True (which feels like a mistake for other reasons), it's essentially pointless.

[andrewgdunn]

@richvdh I'd agree with you if there was another state (other than the three currently available)

• Only people who have been invited
• Anyone who knows the room's link, apart from guests
• Anyone who knows the room's link, including guests
• Anyone who knows the room's link, restricted to homeserver

[ara4n]

I agree that the right solution here is better join rules (i.e. to create a group and only let members of that group join the room), and that this is a partial solution.

However, there are clearly some servers out there which intend their directory (complete with avatar, topic and other sensitive info) to be private to the server. Given we have no alternative way of defining private room directories currently, I think it's still useful to have the ability for folks to lock down the room dir. And yes, it would have solved this particular problem, which was a spider which was walking everyone's /publicRooms.

[richvdh]

it would have solved this particular problem, which was a spider which was walking everyone's /publicRooms.

Only if the option had been enabled. I don't believe it would have been.

[andrewgdunn]

I'd venture you're thoughts on the concept of 'homeserver' don't necessarily represent the entire population. A listed by default with no reasonable access control over than invite is likely not what most homeserver operators anticipated.

[richvdh]

A listed by default with no reasonable access control over than invite is likely not what most homeserver operators anticipated.

This is my point: this PR does nothing to change the default behaviour, so if the concern is about HS operators being caught out by unexpected defaults, it doesn't help.

[richvdh]

I think this is the wrong place for this check. Specifically, it blocks local users from requesting the directory from remote servers. It does nothing to stop users on remote servers from requesting our directory, which uses a different endpoint, and is what I think you're after.

Try here:

synapse/synapse/federation/transport/server.py

Line 574 in b20602e

def on_GET(self, origin, content, query):

[richvdh]

oh, we probably need both checks.

[richvdh]

@ara4n what's the state of this? do you want me to pick it up?

[ara4n]

The state is that this keeps biting us badly - we have another instance where a homeserver admin intended their publicRooms to be private (given it leaked sensitive room topics and names and avatars) and was aghast to discover anyone on the 'net could query the directory even without having an account on it.

So someone (probably me) needs to fix the review feedback and get it merged. Reopening so it doesn't get lost.

This is the fix to #2419 and #1467 (which already had a fix in it :/)

[erikjohnston]

Since this PR doesn't help with people not realising up front that the room directory is public, its probably worth taking the time to add some UI to Riot to make that more obvious as well. For example, labelling it the "public directory" in the directory screen and having a short explanation on how to view other servers' directories, etc.

(In general, I'd prefer that we did work to support allowing using groups for these use cases where possible. Though that is more work.)

[ara4n]

just had another instance of a user not wanting his room directory to be public; he ended up applying auth on it in his LB to stop it being exposed to the internet via CS API.

To reiterate, the reason for this is to stop aliases, topics, avatars and membership counts from non-public rooms leaking out onto the internet.

The problem here is that we don't have a way to know when it's safe to serve up public rooms info to uses via SS API: just because a server presents auth headers doesn't mean we trust its users to see our list.

So perhaps a better PR here would be:

• Make an option to require auth to show publicRooms only to locally trusted users. This doesn't have to be enabled by default as long as it's clear in the config, so privacy-concerned admins can find and enable it if they want. This should also disable serving up publicRooms data over federation, given we can't trust anyone over federation.

I don't have bandwidth to clear up & fix this PR, plus I'd want a thumbs up on this approach from @erikjohnston and @richvdh first anyway.

[andrewgdunn]

It might seem like a silly suggestion, but maybe the idea of publicRooms is totally fine (adopted from XMPP MUCs?) but you need a concept of privateRooms. Right now the UX necessitates enumerating rooms on a HS as publicRooms for user discovery (but it translates into global discovery if federation is enabled).

This seems like a first class gotcha for people who desire to care about localized/contextual privacy within their HS. If E2E was enabled by default you'd not have to worry about "leak", but it seems like tidying up the concept around room listing would be more tractable than getting E2E perfect.

[richvdh]

• Make an option to require auth to show publicRooms only to locally trusted users. This doesn't have to be enabled by default as long as it's clear in the config, so privacy-concerned admins can find and enable it if they want. This should also disable serving up publicRooms data over federation, given we can't trust anyone over federation.

Ok, so this is #1467 I think. Note that it needs spec changes.

In any case this PR has bitrotted, so I'm going to close it. Let's take further discussion to matrix-org/matrix-spec-proposals#1467 / https://github.com/matrix-org/matrix-doc/issues/612.