decred
Implement admin password hashing using SHA-256
Per SEI CERT C Coding Standard, it is best practice not to store plain text passwords in memory or on disk. This was achieved by storing the bcrypt hash of the admin pass in a separate file, removing the provided password bytes from memory. Closes decred#281
Just wondering that this would impact auto recovery systems. If a system crashes it will require admin to manually start the service for a secure setup (Imagine a long Christmas break and the API being down for days). Why not just store the hash to a config file in disk after the first prompt ? This would provide almost the same level of security without the need to prompt the user for password on each startup (Hence boot scripts can be automated)
Hi @ukane-philemon. Definitely appreciate the PR but I have to agree with @degeri here. vspd is designed to be run as a system service which can be started/restarted non-interactively.
I notice in this PR you used the database to store the password hash, and that seems like a good idea. Why did you change it? A better solution could be to just store it in a new file in the datadir (eg. ~/.vspd/password.hash). That would make forgotten password recovery very easy - just delete the file and set a new password through the terminal. It would also allow admins to easily install the file using third party tools like ansible.
Also, in that PR you used bcrypt, and this one is using SHA-256. Why the change?
@degeri @jholdstock thank you for looking into this PR, it's my first time contributing here so I was concerned about what the team would accept hence my switch to sha256 as it is used in dcrdex, except dcrstakepool which use bcrypt. My initial PR used bcrypt and saved admin password hash to db. Operators can easily change the password (overwriting hash in db) without the need to delete a file in their datadir, by providing the new password via secure terminal or config. If it's ok, I can close this PR and submit the other?
@jholdstock @degeri I know projects have their preferences when it comes to hashing. SHA-256 or Bcrypt? Bcrypt was especially made for passwords but SHA-256 use less code to do what it does.
@jholdstock I resolved to using bcrypt in my recent commit. The bcrypt password hashing mechanism is the second choice after Argon2id, for password hashing/storage to achieve FIPS-140 compliance.
Security thoughts on bcrypt vs sha256 @degeri?
Personally I have no preference from a security standpoint, but I would rather use SHA because it does not introduce extra dependencies on golang.org/x libraries.
Yeah should not be too big of a difference. Lets go with SHA since its less dependencies
While I agree with the premise of this change, and the code looks acceptable, I'd like to hold off merging this PR for a while because release 1.1.0 is imminent (to coincide with dcrd/dcrwallet 1.7).
Changing vspd from a non-interactive to an interactive process is a very significant change to make so close to a release.
@jholdstock @degeri thanks for your feedback and review of this piece. I'm happy it's slated for v1.1.