search

shaarli / Shaarli

The personal, minimalist, super-fast, database free, bookmarking service - community repo
https://shaarli.readthedocs.io/
Other
3.45k stars 296 forks source link

Do not use SHA-1 password hashing #94

Open nodiscc opened 9 years ago

nodiscc commented 9 years ago

As said on Debian mailing lists:

searching the codebase for mt_rand shows that you are using mt_rand salted SHA-1 for the password hashing. I would recommend switching to the current best practice of using scrypt (or bcrypt). http://pecl.php.net/package/scrypt

pikzen commented 9 years ago

Had we a huge database of passwords that may be leaked and then automatically cracked, I'd say yes. However, Shaarli stores a single user/pass/salt tuple. bcrypt's main interest is that it slows down bruteforce attempts, and is particularly effective when you want to prevent a massive cracking attempt. Swapping to bcrypt would probably increase the duration of an attack by a factor of 100. Which is still not a lot in the case of a single user.

I recommend setting this as low severity. This should be done one day, but is not really an important thing.

This would also require versioning somewhere in the config file, because there's no way to differentiate an SHA-1 from a bcrypt(), so updating config files would be rather... interesting.

mro commented 9 years ago

@pikzen agreed. KISS again.

Marsup commented 9 years ago

FWIW, bcrypt has a prefix, it's completely recognizable from a SHA1, and it's much safer, but I'd say by far more than a factor of 100 depending on the cost you set it on.

nodiscc commented 9 years ago

Sorry I was a bit hasty in closing this. Of course having more secure hashes would be great. This SO question explains a possible way to migrate the passwords to another hashing algorithm. I'm fine with scrypt, bcrypt, SHA-512...

aledeg commented 7 years ago

I think that it would be a good time to change that since we have now the proof that there are collision in SHA-1 hashing. See Google security blog for more information.

pikzen commented 7 years ago

Note: they were able to generate two files that hash to the same value using SHA-1. It is in no way a preimage attack, therefore that is not a threat model Shaarli faces. Unless some people want to craft two passwords that allow you to login to the same Shaarli ¯_(ツ)_/¯

Nevertheless, going away from iterative hashing functions is a good plan, as attacks get more sophisticated.

virtualtam commented 7 years ago

Tracked on https://github.com/shaarli/Shaarli/issues/667#issuecomment-282499299 , I've added links regarding the attack and current discussions around Git's internals