-
Notifications
You must be signed in to change notification settings - Fork 80
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
Noobaa Account: Replace bcrypt password hashing by argon2 #8213
base: master
Are you sure you want to change the base?
Conversation
5a86b22
to
9433070
Compare
9433070
to
1d503b1
Compare
1 - I need to update the description of this PR based on our new approach in which we have just removed the bcrypt code. |
@aspandey is there a specific need to use argon? I see that in recent years there is already support for password hashing natively in the nodejs crypto module, should we evaluate using it? - https://stackoverflow.com/questions/62908969/password-hashing-in-nodejs-using-built-in-crypto/67038052#67038052 |
1d503b1
to
3b46af7
Compare
Hey @guymguym I have read the difference between argon/bcrypt and node.js module...at first looks like argon is the better approach to use. However, I am thinking of implementing node.js password encrypting module and will send it as different PR. |
As bcrypt is not under active maintenance, we need to replace it with argon2 hashing module. Signed-off-by: Ashish Pandey <[email protected]>
3b46af7
to
afe01f0
Compare
PR with node.js crypto module - #8252 |
This PR had no activity for too long - it will now be labeled stale. Update it to prevent it from getting closed. |
As bcrypt is not under active maintenance, we need to replace it with argon2 hashing module.
Explain the changes
We need to move to argon password hashing from bcrypt. bcrypt is not maintained very well which could cause issue in future.
This task will require two types of changes broadly.
1 - Modify the code so that whenever an account or password is created for an account, it should create a hash using argon. This change is straight forward. Wherever we are using bcrypt we have to replace it with argon2. Just a small difference in the way arguments are passed.
2 - The critical thing is to handle update. What happens when a user updates to this latest version.
All the previous passwords have been stored as bcrypt hash.
We can not write a upgrade script to modify these hashed password from bcrypt to argon.
To hash a password using argon we need original password in plain text, which we can not get just from bcrypt hash . It has to be coming from user.
This we can do in following ways -
As soon as user does upgrade, we need to ask users to reset password. Whenever a user resets password a new password hash will be generated by argon and it will replace the bcrypt password stored in database. Although, previous password will be authenticated by bcrypt first.
For this we have to keep bcrypt module inside the code for one release and once users are shifted to argon we can remove the dead code in next release.