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

Possibility to encrypt database password in config #15768

Closed
TylerMiller-TomTom opened this issue Apr 1, 2020 · 20 comments
Closed

Possibility to encrypt database password in config #15768

TylerMiller-TomTom opened this issue Apr 1, 2020 · 20 comments
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.

Comments

@TylerMiller-TomTom
Copy link

When I went in to look at the generated config.ini.php file I was a bit disturbed to find the superuser password sitting there unhashed/unencrypted. This to me is a security flaw, we would be deploying with Docker under Kubernetes and this effectively would allow anyone with Kubernetes access to exec into the container and obtain or change the superuser password. Has there been any discussion about this? Is there some option I'm missing to secure the password?

@tsteur
Copy link
Member

tsteur commented Apr 1, 2020

@warrentmills are you maybe on a very old version of Piwik? This was removed years ago. If it's still there, maybe you had some issues with the updater where it wasn't removed there eg because of missing write permissions.

@tsteur tsteur closed this as completed Apr 1, 2020
@tsteur
Copy link
Member

tsteur commented Apr 1, 2020

I recommend you remove it from the config manually.

@tsteur tsteur added the answered For when a question was asked and we referred to forum or answered it. label Apr 1, 2020
@TylerMiller-TomTom
Copy link
Author

I am using the latest version of the docker container. If it's been removed since a long time ago, how long ago was the docker container updated?

@tsteur
Copy link
Member

tsteur commented Apr 1, 2020

I don't know which docker container you are using. If there is a git repository for that container, I recommend you ask there. Sorry I can't be of better help.

@TylerMiller-TomTom
Copy link
Author

Specifically I'm using 3-apache which says was updated 8 hours ago, and I pulled the image more recently than that.

@tsteur
Copy link
Member

tsteur commented Apr 1, 2020

It was removed 6 years ago as part of https://matomo.org/changelog/piwik-2-1-0/

@tsteur
Copy link
Member

tsteur commented Apr 1, 2020

@warrentmills sorry I don't know what 3-apache is and what exactly they do.

@TylerMiller-TomTom
Copy link
Author

Alright. It's am image on the Matomo official images under https://github.com/matomo-org/docker so it's not third party. I figured since it's a recent official version that this would be an issue with Matomo itself and not specifically the container.

@TylerMiller-TomTom
Copy link
Author

Sorry to post again, but I presented some incorrect information. The password that was stored in plain text was not the superuser password, but was instead the database password. The confusion was because I had the database user/pass the same as the super user for Matomo. So I now understand the need to store an obtainable password for connecting to the database, but it could still be encrypted instead of being plain text or it could be read from an environment variable if not present in the config file. I removed it from the config as you said may work, and I got an error saying unable to log into database.

I know there was a ticket to support setting the config values from environment variables, but I'm referring to reading from the environment variable if the config value doesn't exist, rather than writing the value of an environment variable to the config.

@TylerMiller-TomTom
Copy link
Author

Also as a note, I've never understood the rush to close an issue immediately after providing a response rather than waiting to check if the response actually addressed the problem.

@tsteur
Copy link
Member

tsteur commented Apr 2, 2020

@warrentmills we're closing issues when they are quite clear (as it was in this case) and we're then always happy to reopen should it be needed. We have 1700 open issues and it's other wise hard to keep track of all the issues when people often don't respond anymore after a while etc as it means going again through hundreds of issues looking if it can now be closed.

If you prefer reading it from an environment variable check this out: https://plugins.matomo.org/EnvironmentVariables

Encrypting it is not much of a point because the app needs to be able to decrypt it so anyone who has access to the server (or that file) would very likely also be able to decrypt it. I'll reopen the issue and mark it as an enhancement that someone could potentially configure a path to a private key so we could decrypt the DB password eg using openssl_private_decrypt. It kind of means though you'd need to have the private key on that same server in order for Matomo to decrypt it and anyone with access to the server would be able to do decrypt it. Also Matomo itself would need read access to that file so it would basically only help should the config file be accessible in plain text through the web because PHP is disabled or so. In all other cases where user is able to read the config file, would likely also be able to read that private key.

@tsteur tsteur reopened this Apr 2, 2020
@tsteur tsteur added c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. and removed answered For when a question was asked and we referred to forum or answered it. labels Apr 2, 2020
@tsteur tsteur changed the title Password stored as plaintext in config Possibility to encrypt database password in config Apr 2, 2020
@TylerMiller-TomTom
Copy link
Author

Thank you, but I disagree that it was clear, the solution you proposed didn't work, and the conclusion you drew that I was using an old version was incorrect, thus it was apparently less clear than it seemed at first.

I figure I should mention the possibility of using a bot, there are bots out there that allow you to have issues close themselves after a certain amount of time with no response. I didn't know if you had considered using that at some point as it could help make the job you mentioned a bit easier.

I know it's practically inevitable without introducing another hop to another machine on the way to the database, to completely secure a password, but the harder it is for a potential hacker to get at the better. Our instance of Matomo would be containerized so getting access to the running container and destroying it poses little threat as we'd just spin a new one up, the database would have backups and so data deletion wouldn't be a major concern, data reading on the other hand could be potentially. I've just never seen a program this used to store a password plainly readable, even if it's stored in the code it at least forces the potential hacker to scour through code to find the key. Knowing it's the database password and not the superuser password does make this less of an issue to me at this point, but I do feel that some form of solution for this is important. I may at some point even try contributing something.

Anyways, thanks for the support so far.

@toredash
Copy link
Contributor

toredash commented Apr 6, 2020

I've just never seen a program this used to store a password plainly readable, even if it's stored in the code it at least forces the potential hacker to scour through code to find the key.

How do you suggest this be solved ? I I've never seen this "solved", at least in PHP. The process that connects to the database must decrypt the password somehow (if encrypted), the process needs access to the decryption key, how do you store/protect that key ? How does other programs store credentials ? Please provide examples and suggestions.

Even if you loaded the password to memory and delete the ENV/file containing the password, the password would still be available in-memory for a process to read.

Knowing it's the database password and not the superuser password does make this less of an issue to me at this point,

I would disagree, as it is easier to delete all the data via the database user (DROP DATABASE matomo;) than the superuser user (many API calls)

@Findus23
Copy link
Member

Findus23 commented Apr 6, 2020

I've just never seen a program this used to store a password plainly readable, even if it's stored in the code it at least forces the potential hacker to scour through code to find the key.

This is done by all major PHP applications I know (wordpress, moodle, nextcloud, mediawiki, everything built with laravel, etc.) and also for all non-php web applications I know.

Anything you do to the password can easily be undone as it also has to be undone by the Matomo PHP process. So at most it is a minor inconvenience to the attacker.

@toredash
Copy link
Contributor

toredash commented Apr 6, 2020

I've just never seen a program this used to store a password plainly readable, even if it's stored in the code it at least forces the potential hacker to scour through code to find the key.

This is done by all major PHP applications I know (wordpress, moodle, nextcloud, mediawiki, everything built with laravel, etc.) and also for all non-php web applications I know.

I checked wordpress, and they do the same as Matomo: https://github.com/WordPress/WordPress/blob/master/wp-config-sample.php

@warrentmills if you know of a better way please share, afaik passwords has been managed like this for PHP since it's beginning

@TylerMiller-TomTom
Copy link
Author

I would disagree, as it is easier to delete all the data via the database user (DROP DATABASE matomo;) than the superuser user (many API calls)

This is true, however database backups on many cloud hosting providers are far cheaper if not provided for free than backups for general storage as would be used for the configuration of the site. If a user drops the table we could restore a backup with minimal impact.

As for a solution I get that there is really none. The program needs to get the database password somehow. I do just feel that the only option is security through obscurity, not naming the field "database.password", encrypting it with some key just to make it less obvious. It's just a small inconvenience but may fool people with less experience.

@toredash
Copy link
Contributor

toredash commented Apr 6, 2020

As for a solution I get that there is really none. The program needs to get the database password somehow. I do just feel that the only option is security through obscurity, not naming the field "database.password", encrypting it with some key just to make it less obvious. It's just a small inconvenience but may fool people with less experience.

You said initially that you haven't seen this approach before, now there isn't a solution. So I'm not sure what you want the community to consider.

I'm not seen a suggestion or resources that indicates that what your asking for is possible. If anything, it would have to be renaming the variables to obscure it?

Have you considered the plugin mentioned earlier, that allows you setting config variables via environment variables?

@TylerMiller-TomTom
Copy link
Author

You said initially that you haven't seen this approach before, now there isn't a solution. So I'm not sure what you want the community to consider.

I said I hadn't seen them stored as plain text, I said nothing about having seen systems that perfectly protect passwords. Even if it's simple to undo I've always seen passwords encrypted or encoded in some way, that's all. I haven't worked particularly much with PHP applications so if this is a norm in PHP then so be it, I merely hadn't seen it myself.

I'm not seen a suggestion or resources that indicates that what your asking for is possible. If anything, it would have to be renaming the variables to obscure it?

The variable is called password under the database section, making it very obvious it's the database password. Not saying this needs to be done, but changing the name of the variable in the config and having it not under database along with being encoded as something less obvious as a password, such as pretending to be an array of integers where each integer is comprised of the bytes of 2-4 of the characters depending on string encoding. While not difficult to undo, people might not be looking for that unless they look through the code. This is just an example, I don't expect this example to be something you do, but that's what I mean by obscurity. Another simpler example would be to call it something like 'token' and encrypt it with a key and encode it as hex, again doesn't make it hard to get at, but can be less obvious. Obviously there's then performance considerations to make as well. I'm not saying you have to do anything in particular or do anything at all. I was just bringing it up to cause conversation mostly.

I've looked at the plugin and am still considering it.

@mattab
Copy link
Member

mattab commented Apr 8, 2020

Realistically we won't implement encryption of the password for reasons noted above by a few people. Closing.

@mattab mattab closed this as completed Apr 8, 2020
@mattab mattab added the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Apr 8, 2020
@TylerMiller-TomTom
Copy link
Author

TylerMiller-TomTom commented Apr 8, 2020

"It's being discussed in [references this issue]", then within the same minute closes the issue it's "being discussed" in.

"for reasons noted above", no one here said reasons it "shouldn't" be encrypted, merely stated they didn't see an advantage to it. "Realistically" it takes minutes to add some encryption and to make it opt-in, the rejection isn't because of it being unrealistic, it's because everyone here considers it of no concern to them, not considering potential company policy of people who may want to use your software. In the end it's your decision obviously, however this has not given me the best impression so far of things.

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. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

No branches or pull requests

5 participants