search

triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.35k stars 399 forks source link

Replace MD5 with SHA512 and BCrypt #824

Closed RoiEXLab closed 7 years ago

RoiEXLab commented 8 years ago

I noticed, that TripleA is still using MD5 checksums. According to this Stackoverflow question it shouldn't be too diffifcult to use SHA256 instead... The difficulty would be to change the servers passwords of course... But for new Users? Thoughts?

RoiEXLab commented 8 years ago

EDIT: #828 implements a unused SHA512 class...

DanVanAtta commented 8 years ago

A possibility for migration:

Then eventually we should stop seeing MD5s in the system.

RoiEXLab commented 8 years ago

The question is, should we use Salt or double hashing for the passwords? Or Both?

RoiEXLab commented 8 years ago

Or as mentioned in #828 we could use some hashing Algorithms especially made for passwords like bcrypt for a bonus security - would require a new library though

DanVanAtta commented 8 years ago

sha512 is an upgrade as is, bcrypt seems like gravy. Go for it if you want.

But, we need to be sure we can integrate this well. For example, the tripleawarclub DB might potentially be shared with the one used by lobby. Perhaps not, but IIRC there is some sharing there.

RoiEXLab commented 8 years ago

AFAIK the MD5 class is not only used for Passwords... My suggestion would be to replace the password hashing with bcrypt and all the other "no-need-for-absolute-secureness" stuff with SHA512 since it's faster than MD5 (especcially when users want to join the lobby with MAC adresses and stuff) Therefore added a dependecy to #828 and set the MD5Crypt class as Deprecated

When tripleA will be using the bcrypt preffered to MD5, I will probably remove the @Deprecated annontation or add a @SuppressWarnings("deprecation") for the fallback code...

DanVanAtta commented 7 years ago

See https://github.com/triplea-game/triplea/issues/1041 for further follow-up.