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

Passwords not autosaving #8303

Closed
KyleMoran138 opened this issue Sep 1, 2020 · 7 comments
Closed

Passwords not autosaving #8303

KyleMoran138 opened this issue Sep 1, 2020 · 7 comments

Comments

@KyleMoran138
Copy link

@KyleMoran138 KyleMoran138 commented Sep 1, 2020

Version of OpenTTD 1.10.3

Expected result

When the dedicated server has completed an autosave, upon restart all companies should then be loaded in with their passwords

Actual result

When the dedicated server has completed an autosave, upon restart all companies are loaded in without their company passwords

Steps to reproduce

  1. Start server
  2. Connect as client
  3. Create company and set a password
  4. Wait until the server restarts
  5. Disconnect client and stop the server
  6. Start the server
  7. Connect to server as created company
  8. Observe company does not require password
@glx22
Copy link
Contributor

@glx22 glx22 commented Sep 1, 2020

Not saving passwords is intended.

@KyleMoran138
Copy link
Author

@KyleMoran138 KyleMoran138 commented Sep 1, 2020

Oh, interesting.
Why is that? I have to restart my server host sometimes and wiping all my friends passwords is kind of not great.

@James103
Copy link
Contributor

@James103 James103 commented Sep 1, 2020

I think it's a possible security risk. Someone could download the save game off of the server (using their local client), extract the passwords out of it, and then if they're quick enough, use them to join companies and do evil things that way.

@KyleMoran138
Copy link
Author

@KyleMoran138 KyleMoran138 commented Sep 1, 2020

Oh, that's fair.
Would it be possible to save the passwords in a separate file on the server host?
Then the clients could just authenticate against that and the passwords wouldn't be downloaded in the autosave

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Sep 1, 2020

Yes, some servers may want to publish the current game state as a download in some form, and if the passwords were saved either directly in the savegame file, or as a separate file next to it, there would be a large risk of the passwords being leaked. While everyone ought to understand that OpenTTD company passwords are not secure, you never know what kind of personal information someone might use as a password, so better safe than sorry.

The real solution, which would require a ton of new work and infrastructure, would be a way of having persistent client identities, so a client can prove who the player is to the server in some way, and the server can then grant the client something in the game (like control of a company).
If that was in place, the server would be able to store a list of player fingerprints that have access to each company instead of using company passwords, and that would be secure.

@KyleMoran138
Copy link
Author

@KyleMoran138 KyleMoran138 commented Sep 13, 2020

I'm not 100% familiar with any of this code. I cloned the project and tried to read through to gain some understanding, but that didn't work out.

But I'm curious why there'd need to be persistent client identities. I understand why we can't save passwords in the savegame file. But to create an extra step between the server password and joining a company feels like it could be done away from the rest of the application. It's just shifting the storage of the password and then checking against it when someone joins a company. No complicated fingerprinting. Just enter your password like you do today. If it's set up that way then peoples companies aren't unprotected in the case of a server reboot. The fingerprinting isn't a bad idea, but it does "require a ton of new work and infrastructure"

Like I said, I have no context about the project and I'm still trying to learn. But My curiosity has gotten the best of me :)
Thanks for time 👍 Sorry if my idea doesn't make sense

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 1, 2021

Your idea does make sense, just needs a bit more nuance in order for it to work :) Storing passwords of other people locally is a huge security risk, and not one we should encourage :)

But fear not, we have been looking into this. It resulted in a draft of an addition we are considering for OpenTTD: #8420

This should solve your issue too :)

For now I am going to close this issue if you don't mind, as this is not a bug (but working as intended). Hopefully soon, but not likely in our next (1.11) release, we have resolved this issue for you :)

@TrueBrain TrueBrain closed this Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants