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

Update the logme function to use password_verify rather than compare md5 strings #11670

Open
t-pearce opened this issue May 8, 2017 · 5 comments
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github.

Comments

@t-pearce
Copy link

t-pearce commented May 8, 2017

The standard accounting login system seems to function correctly with password_verify. It doesn't make much sense to have the logme function work on MD5. This is especially an issue because MD5 is cryptographically insecure.

@tsteur
Copy link
Member

tsteur commented May 8, 2017

We definitely need to change this eventually. Ideally we'd include eg oauth or something but passing md5 hash is not really a solution. Especially since you basically need to have the raw password to do this. To use password_verify we'd need the raw password which is not really a solution either or do you mean passing the password hash instead? Oauth wouldn't be too much work to implement for us but of course adds some overhead for the user but be more secure as it is now.

@t-pearce
Copy link
Author

t-pearce commented May 8, 2017

Needing the raw password at the time of authentication isn't a bad thing; you ask the user to log in to a CMS, the CMS hashes the password, does its own login, and redirects to the logme function. At least in this use case, the logme function would work out-of-the-box, without requiring the server to log the MD5 hash somewhere in their CMS and somehow redirect that to the logme function.

To be honest, I tried to find a solution to this myself; I dug through the source code to try and find where the MD5 hash comparison was being done, to see if I could alter it to use password_verify. However, I'm very unfamiliar with Piwik's source, and accounting security is paramount so I'm wary of doing anything too cavalier with it.

@tsteur
Copy link
Member

tsteur commented May 8, 2017

Needing the raw password at the time of authentication isn't a bad thing

yes that's true. The thing is you don't want to transfer the raw password over the network via URL parameter in a GET request which may appear in access logs etc. and possibly used over HTTP is no good :)

@t-pearce
Copy link
Author

t-pearce commented May 8, 2017

Yeah, perhaps some kinda cURL interface so it can be done via POST?

Is there any other method of remote authentication? My usage of Piwik somewhat requires a single login process, having the user log in to my CMS and then into Piwik isn't really acceptable. Password requirements aren't an issue, this is a local piwik install and I've got them set up to share passwords.

@mattab mattab added c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. answered For when a question was asked and we referred to forum or answered it. and removed answered For when a question was asked and we referred to forum or answered it. labels Aug 30, 2018
@mattab
Copy link
Member

mattab commented Feb 26, 2021

A customer is asking about the logme feature and the security implications.

Btw I added this (bold part) in the FAQ as it seems it should work to POST data (and more safe)

Important: we recommend to make this request over https (SSL) in order to keep the password hash secure, and we also recommend to POST the password and login URL parameters (instead of sending it as GET parameters, which may be visible in browser history and web server access logs).

Would be valuable to have oAuth for this or otherwise use a secure hash for the passwords for this logme feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github.
Projects
None yet
Development

No branches or pull requests

4 participants