-
-
Notifications
You must be signed in to change notification settings - Fork 972
RFC: Feature: Public key company authorization #8391
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
TBH, I'd be in favour of removing "passwords" as they are currently entirely and only using this system.
Oh, and since I actually thought to look - libhydrogen (ISC licenced) is GPL compatible - https://www.gnu.org/licenses/license-list.en.html#ISC
src/lang/english.txt
Outdated
@@ -1749,6 +1749,9 @@ STR_CONFIG_SETTING_REVERSE_AT_SIGNALS_HELPTEXT :Allow trains to | |||
|
|||
STR_CONFIG_SETTING_QUERY_CAPTION :{WHITE}Change setting value | |||
|
|||
STR_CONFIG_SETTING_COMPANY_AUTOPROTECT :Automatically protect new companies with pubkey | |||
STR_CONFIG_SETTING_COMPANY_AUTOPROTECT_HELPTEXT :Automatically protect new companies with cryptographic public key, allowing you to enter your companies without password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to ... "soften" the language here for sure :D But that will come when we do a proper review :)
cd5c500
to
81e15ce
Compare
Pubkey might used for server auth(like OpenRCT2). |
Basically, we need a plan where we want to head with this. This is a bit unusual for OpenTTD, as mostly we just do stuff and see where we end up :P But as we are touching network protocol and the way servers operate, this requires some planning ahead. As many of the things you mention are valid, but are also point-solutions, which currently don't fit in a bigger picture (as that bigger picture is lacking). Making a plan does not mean we have to stick to it, of course, but it gives a direction to aim and to evaluate extensions to this idea. For now I think this PR should remain as small as possible, with just some basic proof-of-value code to show what this can do. The plan can extend on that, and later PRs can start implementing that plan. As example, whether we need an "anonymous mode" strongly depends on how we want to use this. Maybe we find out that, because of the plan, a client should always generate an unique public/secret key pair for every unique server it connects to. Or maybe per cluster. But to evaluate this, we need a bigger vision :) If you feel up for it, please draft up a plan (in a gist or something; and I mean text here, not code). It should loosely define an end-state, how we see the multiplayer eco-system work. This allows server-operators (and players) to contribute to the plan, before we make choices :) Let me know if you feel up for it! |
The above request of mine has been done and resulted in a Discussion page: #8420 |
Assuming this PR will represent Phase 1 of the above discussion (and this is not a black&white thing of course), already a few requests for this PR (of course Phase 1 can change based on feedback, but I doubt it will change heavily):
And of course it might be good to scan if we can make changes already that make other phases easier to implement. Owh, and in case this wasn't clear yet: I really like what you have done here :D Now we all have to do is bring it home ;) Cheers! |
Some commit splitting, pubkey based rcon, pubkey based admin port, keymaterial stored in |
While there's still some work in this PR, I meanwhile realized that security model achieved by NEED_KEYAUTH/KEYAUTH packets on unencrypted connection is pretty weak. Discussion here: https://gist.github.com/Milek7/8482c4e87947f4748aea5d96b3e89b80 |
This PR serves more as RFC, as there are many things left to consider which could change implementation significantly
Libhydrogen is included in-tree as cryptography library.
As we don't need full-blown complex TLS crypto library, Libsodium is usually good choice. However, it is rather big library, which would add another external dependency, grow code size unnecessarily (especially important eg. with Emscripten), etc. Thus libhydrogen is smaller alternative, which could be included directly in-tree avoiding any extra dependencies. Monocypher was also considered but I decided against it as it requires random bytes to be supplied by application, which would require additional platform-specific code on our side.
How this is implemented now:
For now in this PR only single pubkey is stored for a company. It works as alternative access method to password, so you can join with matching pubkey OR password. As I think storing multiple pubkeys should be ultimately implemented, GUI changes for current solution are rudimentary.
In
NetworkStartUp
, keypair is read fromBASE_DIR/keypair
in format ofpublickey,secretkey
(keys are hex-encoded). If file cannot be parsed or keypair is invalid, new keypair is generated and saved to this file.Always during joining to server, after NewGRF check and possible game password check server sends
PACKET_SERVER_NEED_KEYAUTH
packet containing challenge with 16 random bytes. Client then replies withPACKET_CLIENT_KEYAUTH
containing its own public key and signature for given challenge. Server verifies challenge and stores client pubkey inClientInfo
, or disconnects client when signature is invalid.Now, if client tries to join company with matching pubkey, access is allowed without any passwords.
Client can protect company with pubkey sending
PACKET_CLIENT_PROTECT_COMPANY
, containing bool indicating whether to protect or unprotect. As for now only single pubkey is stored, it always stores pubkey of client who sent the packet.Autoprotecting on company creation can be enabled by
network.company_autoprotect
setting (enabled by default).As company is indicated to clients as 'protected' whenever there is any protection on the company, and there is no way to know whether pubkey will succeed, clients now always try to join after clicking on
Password
button without password. This is harmless, except of short password prompt flicker in case when pubkey auth succeeds.Protecting/unprotecting company with pubkeys manually is done in password setting GUI through two additional buttons.
Implications and further changes
Next step would be adding ability to store multiple allowed public keys for a company, and add necessary GUI for managing that. Keys could be also now stored in savegames, as there is no risk that user entered sensitive data there. (contrary to passwords)
Now players can be uniquely identified by servers. Whether this is desired, or exactly the opposite is up to debate. While I think this is nice feature, I guess it might raise privacy concerns. Currently in this PR pubkeys are not shared to other clients, but if we allow multiple keys in company it would be useful, as client could store some sort of "friends" list, consisting of pubkey and last known readable name. You could then grant access to your company to a friend, even if he's offline at the moment.
In future, masterserver could be also extended to share list of connected clients pubkeys, so we could have an icon on server list indicating "some friends are playing there".
If pubkeys won't be shared with other clients, then these two features I mentioned won't be possible, so I think sharing keys is preferable even if it makes privacy concerns even bigger, as clients could be also uniquely tracked by other clients. To alleviate that, I propose adding optional 'privacy' mode, which won't store one keypair, but random secret material from which ephemeral pubkeys are generated based on some server characteristics (server address, or something like that, TBD). That would break friends list features, but basic pubkey authorization will still work.