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

Is it necessary to run it as root? #52

Closed
vpal opened this issue Sep 20, 2016 · 5 comments
Closed

Is it necessary to run it as root? #52

vpal opened this issue Sep 20, 2016 · 5 comments
Assignees

Comments

@vpal
Copy link

vpal commented Sep 20, 2016

Greetings,

I just needed phpmyadmin for one of our projects and I looked into the image and I don't understand a couple of concepts here.

1.) Why does anything in this image run as root (tried both latest and 4.6.4-1 from docker hub and although they are different both run php processes as root).
2.) Let's presume this is changed and the processes in the container are ran as UID 1000, in that case why is the /www/ directory writable by user 1000.

I think running web applications as root even in a docker container is very insecure as it highly increases the attack surface despite not being as bad as running them on the host.
Also it is I think a common best practice in web operation to make sure that an application is not able to write it's own code.

I don't think there is any need in the docker world to run things on privileged ports like 80 so I don't think that should prevent the process to be ran as a non root user that is not able to write anything except what it needs to.

Thanks a lot in advance.

@nijel nijel added the question label Sep 20, 2016
@nijel
Copy link
Contributor

nijel commented Sep 20, 2016

All what runs as root should supervisor and master processes since #48 has been merged (processes are started as root, but setuid to unprivileged users). The PHP could be probably started as not privileged user directly, I will give it a try. The nginx has to start as root for binding port 80.

The discussion on used port has been in #18, so feel free to continue there if you have something to add to it.

@vpal
Copy link
Author

vpal commented Sep 20, 2016

Thanks for the explanation and putting efforts into this.
Now I see that the php subprocesses would then run as nobody so the webroot is not writable by the web processes itself. This sounds much better.
Maybe an explicit chown + chmod in the Dockerfile for the extracted files would make sense to not rely on the ownership and permissions that are coming out from the archive would increase robustness and security.

Thanks

@nijel
Copy link
Contributor

nijel commented Sep 20, 2016

Now even PHP FPM master process is run as nobody (#53).

You mean chown to root:root? I thought that's implicit in Docker, but I might be wrong...

@vpal
Copy link
Author

vpal commented Sep 20, 2016

Here: https://github.com/phpmyadmin/docker/blob/master/Dockerfile
phpMyAdmin.tar.gz is extracted and moved into place and the ownership permissions are inherited from the archive itself.
if you enter the running container and list the /www directory you will see permissions like this:
-rw-r--r-- 1 1000 1000
This is basically okay, but these permissions are inherited from phpMyAdmin.tar.gz and can change any time. It can also make the image fail to start or even inherit permissions like 777, so it is much more robust to adjust those after unpacking the archive as you can't really rely on what permissions the files in the archive have.
Maybe the most secure way with this setup would be to do something like this after the files are in place:

chown -R root:nobody /www
find /www -type d -exec chmod 750 {} \;
find /www -type f -exec chmod 640 {} \;
In this case only nobody and root could read the code.

nijel added a commit that referenced this issue Sep 20, 2016
Issue #52

Signed-off-by: Michal Čihař <[email protected]>
@nijel
Copy link
Contributor

nijel commented Sep 20, 2016

All future phpMyAdmin tarballs will be root:root, see phpmyadmin/phpmyadmin#12560, but it's probably still good idea to change it like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants