search

OuterDeepSpace / OuterDeepSpace

GNU General Public License v2.0
0 stars 1 forks source link

Better Password Hashing #7

Closed Ragora closed 6 years ago

Ragora commented 8 years ago

Passwords are hashed using MD5, and probably without a salt as well. We should use a non-broken hash algorithm and make it use salts.

ghost commented 8 years ago

The python implementations of bcrypt and scrypt might be appropriate here.

Ragora commented 8 years ago

I remember using py-bcrypt for my Muck programming before. It's pretty useful for that task.

atmafox commented 8 years ago

What's the time involvement for this? I understand it would render a password change necessary for all users and the hope is that we will not be on the legacy code for any longer than we have to. I support doing this, if it can be done in less than thirty minutes. Using existing libraries would indicate as much.

ghost commented 8 years ago

(Okay. Apparently this site-browser combo does very weird things with certain key combinations. Let's try this comment again :P)

It's possible for this kind of transition to be smooth and transparent to users, rather than disruptive. Exactly what's involved would differ a bit depending on whether it's the client or the server doing the hashing; I'll admit I haven't peeked too deeply at that just yet. What I'm thinking...

If the server is doing it (bad from a "direct password transmission?!" standpoint): bcrypt/scrypt/whatever is implemented on the server and made into the primary hashing mechanism. The next time a user with an MD5 pass on record logs in, their pass is first hashed to MD5 and checked for validity, then fed into whatever the new scheme is, with the result becoming the new digest on file.

If the client is doing it: bcrypt/scrypt/whatever is implemented on the server and made into the primary hashing mechanism. In addition, the login scheme is modified so that the client dispatches both an MD5 hash and secure hash of their password. The MD5 hash would be compared to what was on file for legacy accounts, and if it matched, the secure hash the client also supplied would be dropped in to replace it. Once no accounts with MD5 hashes for password storage remain, we could strip the transition features from the client and server.

Of course, this early into development, it might be better simply to make this the top priority and do a single password reset while the userbase is still small.

Ragora commented 8 years ago

Server/client (looks like code is a mishmash, despite being a "server" lib) is using the MD5 in the following locations:

https://github.com/OuterDeepSpace/OuterDeepSpace/blob/master/libs/server/ige/GameMngr.py#L58

https://github.com/OuterDeepSpace/OuterDeepSpace/blob/master/libs/server/ige/LimitedClientMngr.py#L95 (and below as well)

https://github.com/OuterDeepSpace/OuterDeepSpace/blob/master/libs/server/ige/ClientMngr.py#L128 (and below as well)

https://github.com/OuterDeepSpace/OuterDeepSpace/blob/master/libs/server/igeclient/IClient.py#L74