alexanderankin / contest-base

A database for electoral contests
0 stars 1 forks source link

Password Hashing is Insecure #2

Open zaclittleberry opened 5 years ago

zaclittleberry commented 5 years ago

md5 is not suitable for any security purposes, especially for passwords. if there's any user info in the db there now it should be expunged, and reset once it has been modified to be stored securely. I suggest using a separate identity provider if possible, such as auth0 or keycloak if you can manage it. If you do decide to keep user passwords in the database you can see how other projects do it, like opencollective: https://github.com/opencollective/opencollective-api/blob/master/server/models/User.js

alexanderankin commented 5 years ago

It seems they are using the model where they have a separate bcrypt salt for each user. i figured that since 25% of the web runs on wordpress then their approach (my current implementation) is fine, but i suppose this isnt too much extra work. for a solo project, i can let my own standards dip but appreciate the accountability and will implement the suggested approach.

zaclittleberry commented 5 years ago

Wordpress' current implementation uses phpass which uses bcrypt and only has md5 for backwards compatibility, but will also re-hash the md5 to a better hash if possible. This post contains the relevant code and goes over the functions called for implementation a bit at the bottom: http://www.kvcodes.com/2016/09/wordpress-password-hash-generator/

I am unsure if wordpress does per-user salts (I don't feel like reading through more wp code or db inspecting :P), but of course that would be the more secure method.