Cockatrice: "Save password" feature stores in clear text

[woogerboy21]

When you use the "save password" check box the password for your account is stored in plain text.
Its a security risk.

[ctrlaltca]

Any crypto that you can use should be two-way, in order for cockatrice to be able to use the original password.
The only real solution would be to use a wallet or implement one inside cockatrice.
Of course you'll need to feed the wallet with a password, so to avoid having to write a password in cockatrice, you'll have to write a password in the wallet.
I'm thinking of alternatives..

[woogerboy21]

Just reporting the news on this one. If you haven't heard my stance about using your bank account password on cockatrice I can repeat it but in general its bad practice to store password in plain text form.

[woogerboy21]

One suggestion was to remove the functionality completely. Not sure I like that idea.
Maybe at the very least a warning bubble that explains that its stored in clear text as a band-aid? Not sure I like that one either.

[ctrlaltca]

Removing the functionality breaks autoconnect and is a regression in terms of usability.
Adding a warning bubble that explains that "anyone with access to your pc's data can actually read it, including your cockatrice's account password" is an idea.
A more complicated solution would be to implement an external auth service (oauth, ..) or a sign-on service using temporary tokens.. but it's a bit overkill.

[woogerboy21]

A bit? LoL

[Daenyth]

I think the only reasonable solution is to tie into the system's wallet solution. I'd accept patches on that.

Is the warning really needed? It seems to me to be fairly obvious that storing your password implies that things can read your password. I could do what pidgin does and just encrypt it but given that the encryption key would have to be hardcoded it's kind of useless

[woogerboy21]

Even a hard coded encryption key is better than no encryption. The main concern that was discussed about was simple access by other users on the same PC. Simply being able to read it in clear text is the problem pointed out.

[Daenyth]

on start:

• if password store setting is on
  
  
  
  • read encrypted key
  
  
  • if missing, read clear key
  
  
  • if present, write to encrypted and delete clear
  
  

[woogerboy21]

Are you wanting to maintain older client compatibility? If so you may want to simply make a new stored value that is encrypted and on the newer clients clear the unencrypted store value on startup.

[ZeldaZach]

Are you assuming the average user is stupid enough to use the same passwords for the same thing, including a client that's free to use?

[woogerboy21]

@ZeldaZach
I went down that path but the underlying bad habit still looms. Storing a password in clear text is bad practice.

[ZeldaZach]

Would a simple hash not work, or would it have to be an encoder/decoder?

[Daenyth]

You need to be able to decrypt it, so it has to be a two-way encryption.

[woogerboy21]

If were low balling it, how about a simple character shift :smile_cat:

[ZeldaZach]

Maybe we can base64 it a certain number of times? This password encryption really doesn't have to be too strong, just enough that a plain password isn't stored locally

[woogerboy21]

I like the idea of the base64.
Example Link:
http://karanbalkar.com/2014/02/base64-encoding-and-decoding-using-qt-5-framework/

[Daenyth]

I think at this point the correct options are:

• Give a more clear warning that we are storing the password (at a minimum)
• Potentially integrate with some kind of keyring (better but more work)

[ctrlaltca]

This nice project has come to my attention: https://github.com/frankosterfeld/qtkeychain/
A cross-platform API to store encrypted QSettings data into wallets; we could import it as a submodule and add support for it.

[tooomm]

For completeness (and because @woogerboy21 asked today), the passwords are stored in servers.ini

Also linking https://github.com/Cockatrice/Cockatrice/issues/60

[Vafthrudnir]

How about not storing the password at all, but a random hash instead? Kinda like how a "Remember me" cookie works:
The first time a user chooses the "Save password" option during login, after his/her credentials are verified the server could generate a random hash (token), and send it back to the client. This would be stored by the server among the user data, and by the client in the settings.
On subsequent logins, the server checks whether the user sent the password or the token. If either matches, the login is successful. If "Save password" was checked, a new token is generated, stored and sent back, otherwise it gets cleared on the server side. If the login failed due to wrong credentials, the token would also be cleared.
This way the token would be either be updated or cleared with every login attempt. And the password wouldn't be stored in any form, which is also nice. What do you think? Would this solution be sufficiently secure?

[Daenyth]

I'll have to think about it. On first blush it seems reasonable. Can you
point to any existing security literature about this method?

On Thu, Mar 8, 2018, 8:46 AM David Szabo [e-mail] wrote:

How about not storing the password at all, but a random hash instead?
Kinda like how a "Remember me" cookie works:
The first time a user chooses the "Save password" option during login,
after his/her credentials are verified the server could generate a random
hash (token), and send it back to the client. This would be stored by the
server among the user data, and by the client in the settings.
On subsequent logins, the server checks whether the user sent the password
or the token. If either matches, the login is successful. If "Save
password" was checked, a new token is generated, stored and sent back,
otherwise it gets cleared on the server side. If the login failed due to
wrong credentials, the token would also gets to be cleared.
This way the token would be either be updated or cleared with every login
attempt. Do you think this solution would be sufficiently secure?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/Cockatrice/Cockatrice/issues/344#issuecomment-371490624,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA5NDOMWSAwV-eztL2bNZdNQisIuI2Pks5tcTYdgaJpZM4CqGNg
.

[Vafthrudnir]

After some research I found this link, which discusses an advanced version of the method I described, and also references the original author.

[tooomm]

I didn't study your link @Vafthrudnir, but just from our explanation it sounds possible to simply copy the token from one's settings file and paste it into your own to log in.
So instead the pw in clear text, you would "spy" his token in clear text to login successfully...

It should still be an improvement, because you would realize on next login that your token isn't valid anymore and something is wrong. But that might be too late.
While it limits damage it doesn't fix the problem itself completely.

Would it be possible to salt/combine such a hash with ones client id which is also tied to his hardware for example? The server knows about last used client id already.

[Vafthrudnir]

I haven't checked our login implementation yet, but if a unique client id is sent and stored on the server right now, then it would be quite easy to bind it to the token, and use the username-token-clientID-seriesID quartet on logins. That would add an additional layer of protection.
Although this isn't circumventable either: if someone modifies their own client to show the victims ID after stealing their token, or uses the same computer as the victim, there will be no way to detect it. Basically the whole feature is a security risk, and after a point there really isn't much we can do to protect the user.

[skwerlman]

it's not like there's a huge incentive to steal cockatrice tokens, so as long as we make it difficult, it's probably good enough.

ofc, if we wanted to really overengineer this bit, we could look at how chromium uses system crypto APIs to encrypt saved passwords

[Vafthrudnir]

AFAIK chromium uses gnome keyring or kwallet to store passwords, while Firefox just encrypts them using a master password. Chrome on windows uses the DPAPI.

These are all nice solutions, but I agree, we don't need to overengineer this - just don't store the user's password in plain text, and inform them if we can about a previous login they might be unaware of.
To achieve this I think using the login token would be enough.

[FluxIX]

If you wanted to save the password on the machine directly, encrypt it with a master password, but instead of hard-coding the master password, look for the existence of a key file or key value (if you wanted to store the key in a configuration file) and if the key is not found, create one using the current system state.

[papb]

the suggestion by @Vafthrudnir is immensely better than storing the password in plain text. Of course it's not perfect, but it's better:

• If a user uses the same password on cockatrice and their email (which they definitely shouldn't in the first place), at least their email is safe. This is an immense advantage.
• If a user uses different but similar passwords on cockatrice and their email (which they also shouldn't but is definitely a lesser sin than using identical passwords), having the cockatrice password in plaintext makes it much, much easier to brute-force their email password. This is also terrible.
• If a user's password is not random and instead contains some 'personal secret' such as i_hate_javascript, at least their secret is not revealed.
• If a user's password is awful such as 123456 at least no one will know immediately that the user is terrible with security practices and they will not become a target for a trivial dictionary attack on their email.
• There are many other information leaks that can happen from a plaintext password, maybe the user uses their phone numer as password, so now anyone knows their phone number, etc.

[ZeldaZach]

While I don't have a solution yet, this still needs to be addressed