Skip to content
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

MD5 is used to hash passwords #11962

Closed
natejgardner opened this issue Aug 16, 2017 · 7 comments
Closed

MD5 is used to hash passwords #11962

natejgardner opened this issue Aug 16, 2017 · 7 comments
Labels
answered For when a question was asked and we referred to forum or answered it.

Comments

@natejgardner
Copy link

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
Copy link
Member

Findus23 commented Aug 16, 2017

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
Copy link
Author

That makes sense, thanks!

@Findus23 Findus23 added the answered For when a question was asked and we referred to forum or answered it. label Aug 16, 2017
@tsteur tsteur closed this as completed Aug 16, 2017
@lanwen
Copy link

lanwen commented Feb 19, 2021

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

@Findus23
Copy link
Member

Findus23 commented Feb 19, 2021

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
Copy link

lanwen commented Feb 19, 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
Copy link
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
Copy link

lanwen commented Feb 19, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered For when a question was asked and we referred to forum or answered it.
Projects
None yet
Development

No branches or pull requests

4 participants