@natejgardner opened this Issue on August 16th 2017

I was just browsing the Piwik FAQ on resetting passwords when I noticed Piwik uses MD5. MD5 is quite vulnerable... Could you please hash passwords using a secure algorithm like Scrypt? See this StackExchange post for details.

@Findus23 commented on August 16th 2017 Member

What FAQ entry are you referring to?
This one (https://piwik.org/faq/how-to/faq_191/) mentiones that the password is stored with the new and secure password_hash function (which uses `crypt) since Piwik 3.0.

echo password_hash(md5("changeMe"), PASSWORD_DEFAULT)

I am not sure why the md5-hash is calculated before as it seems useless, but this shouldn't lower the security.

@natejgardner commented on August 16th 2017

That makes sense, thanks!

@lanwen commented on February 19th 2021

@tsteur md5 has a very low entropy and lowers the security, could we reopen the issue and stop using md5?

@Findus23 commented on February 19th 2021 Member

Hi @lanwen,

MD5 hasn't been used anywhere near passwords since Matomo 3.0, so there should be nothing that needs to be changed.

If you are referring to something else, please expand on that.

Update and minor correction: I check again and indeed Matomo does use md5 in the password process, but just as a first step before passing it to password_hash() which should not affect security.

@lanwen commented on February 19th 2021

thanks for the prompt reply, however combination of different hashing algorithms usually leads to reduced security of the resulting hash.

https://crypto.stackexchange.com/questions/44178/why-not-combine-hashing-algorithms

even if it doesn't affect the security, why to complicate the process?

@Findus23 commented on February 19th 2021 Member

Hi,

You are right that simply using password_hash() directly would be better. But there is one good reason why the code looks as it is and why one can't really change it:

An ancient version of Matomo (<3.x) used md5 directly to hash passwords. This wasn't great, but at the time password_hash also didn't exit yet.

But if people update from the old version of Matomo to a recent one, Matomo can't upgrade the passwords as it only knows the hashes. But it can hash every md5-password that is stored with password_hash and then store every new password with password_hash(md5()). This way both old and new passwords are reasonably secure without the user needing to do anything. (which is what Matomo does now)
If Matomo now dropped the md5 people with old hashes stored, couldn't login anymore.

One could argue that Matomo could update the hash during the login of a user (as then the password is available and can be hashed properly again).
This would work conceptually similar to https://docs.djangoproject.com/en/3.0/topics/auth/passwords/#password-upgrading-without-requiring-a-login, but would mean adding quite a bit of code to both support the old and new hash, which might introduce new bugs.
And the code supporting the old hash could also never be deleted as it would be needed for a user that logs in for the first time since upgrading from an old version.

@lanwen commented on February 19th 2021

Thank you @Findus23,

I could understand the complexity of supporting for the multiple algorithms, however it would always be the case, when new algorithms developed. In Spring Framework that was addressed quite simple - via the delegated password hasher, which just embeds some prefix to a hash to show - which kind of hasher was used for real. Then a task of maintaining the dictionary of prefix to hasher becomes not something really huge.

https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/crypto/password/DelegatingPasswordEncoder.html

so in your case it could be empty for the legacy, and some prefix - for the new, updated hash. Then you could keep the legacy as internal thing for backward compatibility, but gracefully add new algorithm and settings for new logins and advertise that in the docs.

This Issue was closed on August 16th 2017
Powered by GitHub Issue Mirror