Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Option not to disclose server information when password-protected #7268

Closed
Berbe opened this issue Feb 23, 2019 · 14 comments · Fixed by #9475
Closed

Suggestion: Option not to disclose server information when password-protected #7268

Berbe opened this issue Feb 23, 2019 · 14 comments · Fixed by #9475

Comments

@Berbe
Copy link
Contributor

@Berbe Berbe commented Feb 23, 2019

When a server is password-protected, information about it (list of companies, NewGRF extensions, etc.) are still available through queries.

It would be nice if an option in the server configuration allowed to avoid sending that information through queries, thus non giving away information to not-authenticated clients.

@planetmaker
Copy link
Contributor

@planetmaker planetmaker commented Feb 23, 2019

That is not possible nor desirable. Those information are what help players decide where to play. Additionally: OpenTTD can only connect with those information being available, especially NewGRFs, thus needs them in order to start connecting as any player-side connection means to load the map.

If you are concerned about those information: do not announce your server to OpenTTD's master server - then it simply will not be listed and no information disclosed. Give interested players your IP and port.

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Feb 23, 2019

The NewGRF data is part of the game announce, but the companies list is not. It could certainly be possible to add in a mode where you always connect to the server as spectator, and don't receive any details not in the game announce packet before the password is validated.

@Berbe
Copy link
Contributor Author

@Berbe Berbe commented Feb 24, 2019

The reasoning for abruptly closing that suggestion is flawed: there is a confusion between availability & security.

Publishing a game to a master list helps matchmaking, allowing users to find a game without having to resort to manually enter IP/Port. That helps availability.
If the master list was considered a security mechanism, why are there passwords at all? Why is there even a single game published on the master list being password-protected? I see tons of them. And it does not feel wrong.
Another example: making sure your webpage is not findable on a search engine is not a security feature: it's a lack of availability.

Rather, security builds on top of availability: knowing the location of a resource does not mean you can access it. There are many locations asking a password to even connect.
Furthermore, being able to connect to a webpage does not mean you can do admin actions over there. There usually is authentication to enable such options.
You clearly see decoupling beween accessibility & security in those webpage examples, if that helps understanding. Thus, passwords exist, for good reasons.

Now, I am merely wishing to take an even cleaner approach: if your server is password-protected, gathering information about companies might reveal information not meant for outsiders.
The trade-off might be NewGRFs, as you said they were required for initial connection: that's fine, as those are generic, non-customized piece of information, unrelated to players' choices or identity.

Could this suggestion be reopened for proper discussion/evaluation before any administrative action is taken?

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Feb 24, 2019

What a nice form of reasoning @Berbe :) I like your reply.

This possibly would require some work, as there are some assumptions currently that this information is freely available. So I guess that brings us to the more important question: what is the idea behind the request?

Your request basically is a solution, but that makes me wonder: why would it be an issue for others to see what your server is doing without the correct password. This is a game, my thinking goes, so what is there to "hide". Don't get me wrong, this is an honest question. Could you elaborate on your use case?

Tnx!

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Feb 24, 2019

The specific data sent in the current network game announce are:

  • GRFConfig, contains the GRF ID and MD5-sum of the NewGRFs used, but not the names. If the game uses a NewGRF that isn't publicly available I don't think you would be able to find its name.
  • Current game date
  • Game start date
  • Max number of companies allowed
  • Number of active (started) companies
  • Max number of spectators allowed
  • Name of the server
  • Game version used
  • Spoken language on the server
  • Password protected yes/no
  • Max number of simultaneous clients
  • Current number of clients
  • Current number of spectators
  • Name of the map
  • Width and height of the map
  • Terrain type for the map (temperate, arctic, etc)
  • Dedicated server yes/no

@Berbe
Copy link
Contributor Author

@Berbe Berbe commented Feb 24, 2019

Thanks @TrueBrain for taking the time of reading my answer. I indeed tried to organize my ideas before posting.
Those are actually pretty old misconceptions, and numerous people fight over those every day, for several decades already...

The thing that hit me the most is the companies list, as usually those are named afer creator's nickname, as it is their default name, thus revealing (a part of) players list.
As a general rule, user-induced content shall be kept private if you decided your server was private. Notions like semi-private, or semi-public, are not a thing to me: everything not private shall be considered public.

I acknowledge you emphasize it might be a tons of work, which is a valid argument, yet different debate. I respect that, though, and would totally understand if, as a result, it would be considered low-priority.

However, since my arguments were valid, this issue has been (and remains) closed for now no reason. Would it be possible to reopen it?

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Feb 24, 2019

Not sure if it is a wanted solution, but what we can do, that with a settings set, a protected server doesn't send all the information. I would suggest it blanks out fields like:

In CLIENT_FIND_SERVER:

  • Name of the map

In CLIENT_DETAIL_INFO:

  • Company names

It is a bit of a hack; a proper fix needs a lot more rework of the protocol etc.

The reason I did not reopen this yet, is that I am on the fence: we tend to close suggestions nobody would implement in the next year (as many open suggestions doesn't help contributions to a project). But I like 'good-first-issues' around, so we could do that.

For now I will reopen it; possibly @nielsmh has some insights if a solution like I suggest works, or that we just need to rework the protocol after all (we have some more things we like to fix anyway).

EDIT: the more I think about my own idea, the more I hate it. You would see the company list before joining, but you don't know which company. So you have to guess if you want to join a friend. Guess the flow is just wrong, and the password should be asked before things like NewGRFs are validated. That too, is an option from what I can see.

@TrueBrain TrueBrain reopened this Feb 24, 2019
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Feb 24, 2019

Poking some more in the code, my last comment is bananas.

There are 2 ways to get information about companies and clients:

  • via UDP, CLIENT_DETAIL_INFO. For this the above suggestion works fine.
  • via TCP, when you join a company. For this the above suggestion sucks, as it also robs you from knowing what the server is doing before you join. Joining your friends is just impossible in that scenario.

But that brought me to another observation: the moment the password is asked. We currently first validate if the NewGRFs are compatible and stuff, before we ask for the password.

Possibly a much better suggestion is to move where the password is asked, to before the client info is requested, and NewGRFs are validated. From what I can tell, the server initiates the request, so possibly this is a very simple solution.

Still looking into the code ... has been years since I did that :P

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Feb 24, 2019

The reasoning is good, but what's the value of protecting this information?

I appreciate it's just applying good basic security principles, but another principle of security is applying controls proportionate to the risk. What's the potential loss of exposing, e.g. company list or newgrfs? For example, in terms of threat to end user privacy, or providing information to compromise the host?

I am +/-0 to this, just curious about the security and privacy stance.

@Berbe
Copy link
Contributor Author

@Berbe Berbe commented Feb 25, 2019

@andythenorth I do not find any other piece of information to add other than what I already said, thus I won't babble in lengths for nothing. Unless I realize something is missing, you'll find my thoughs in previous messages.

The baseline is: what is not private is due to be considered public. I, for one, value privacy and on a private, protected, server I host, I expect that any information on its users (not merely myself) to be jealously withdrawn from public eyes, as I want to be able to assure that to people visiting my lands.
From that general assumption, I do not want player/company names to be publicly visible if I chose so. Currently, there is no way to do that.

I already babbled too much... Don't push me 😆

@bennyman123abc
Copy link

@bennyman123abc bennyman123abc commented Apr 23, 2019

This would require a bit of a rewrite of the server module to complete. The server doesn't ask for a password until the client tries to spectate the game, create a company, or join a company. Other information is sent to the client before the password request is.

If someone else would like to try this, feel free I suppose. However, I don't see the cons to outweigh the pros in this particular issue.

@rpwjanzen
Copy link

@rpwjanzen rpwjanzen commented Mar 26, 2021

@TrueBrain Can you clarify the requirements for this issue? What feature would need to be implemented for it to be considered done? I was thinking of picking it up. Is it still considered a "good first issue"?

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Mar 27, 2021

Oof, this is 2 years old. Can I remember what my thinking was here :D

Not sure if "good first issue" still applies, but if I read this correct, this mostly needs a design or idea how to approach this problem. Coding-wise, it is most likely not difficult.

For example, one of the idea was to change the order of packets, that you cannot get server info before you authenticated. This seems reasonable to me on first sight, but I have not evaluated the consequences of that yet.

So honestly, I do not know the answer to your question. "possibly"? :D To give a more solid answer, I would need to dive into this myself, and I am short on time for that at the moment. Sorry .. not helpful :( You can just give it a spin if you like, and we can see what happens :)

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Aug 14, 2021

There has been several changes to how multiplayer works, to address most of this ticket.

  1. servers can now be "invite only" and not reachable via a public IP. This means that unless you know it exists, you cannot scrape it for information.
  2. the biggest amount of personal information could be found in the COMPANY_INFO packet, which you could query before joining. This has now been removed from the game.
  3. mapname has been removed from the game-info, meaning the name of your savegame is no longer shared. As is the language field.

The only thing not addressed which was mentioned in this ticket, is for public servers. The NewGRFs they use are announced, and I think that is okay. If you don't want others to see what NewGRFs you play, you shouldn't have a public server. The way OpenTTD works, this is also not really solvable in another way.

As such, I think this ticket finally has been implemented. Honestly, it was never a goal, but we made a lot of multiplayer changes that so happened to also address this problem :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants