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.

Sign up for GitHub

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

Fix Permissions of config.secret.inc.php #432

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Fix Permissions of config.secret.inc.php #432

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ if [[ "$1" == apache2* ]] || [ "$1" == php-fpm ]; then
\$cfg['blowfish_secret'] = '$(tr -dc 'a-zA-Z0-9~!@#$%^&*_()+}{?></";.,[]=-' < /dev/urandom | fold -w 32 | head -n 1)';
EOT
fi
chgrp www-data /etc/phpmyadmin/config.secret.inc.php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be moved to the above if when the file is created, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. testing it, the docker daemon resets the group each start. so it needs to be done each time the container starts.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@williamdes What do you mean? Can we add this owner change to close a 5 years old issue? Don't get me wrong, but this will help a lot of people :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I am very interested too in closing this old bug
What bugs me is why can we not change the file owner at build time or create file time instead of repeatingly changing the group
In rootless mode this makes no sense to me: how a non root user would change the group of a file root owned??

Copy link
Contributor Author

@obel1x obel1x Jan 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I am very interested too in closing this old bug What bugs me is why can we not change the file owner at build time or create file time instead of repeatingly changing the group In rootless mode this makes no sense to me: how a non root user would change the group of a file root owned??

Security is exactly the point, not to change the file to be owned by www- user. In docker rootless mode, the file will be created inside the Container as running user/group, which would be root/root. This is safe, but cannot be read by phpmyadmin, as the user+group of apache would be www.
Setting the user to be www would mean giving the apache full rights to do whatever apache wants with that file, ALSO if the user would have NO RIGHTS TO WRITE to it (this is a often missunderstood feature of Linux).
The only way without using acls to give someone right to read a file without having the possibility to change it, is to not make the user own that file.
Also currently docker is a bit lazy of what groups and permissions are, so unfortunatelly it resets the group every time the container starts.
I don't see another way, if someone does, we could use it. But by now, rootless is only not working because of that.
So i would suppose to take that fix and make rootless work and after that if there are further problems - investigate seperately. only my two cents :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@J0WI do you have knowledge of other techniques?

Copy link
Contributor

@J0WI J0WI Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a common pattern:

If you mount the config anyway, you can mount it read-only: https://docs.docker.com/storage/bind-mounts/#use-a-read-only-bind-mount

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, there is no mount an no config file. The file is part of running application, created inside the container itself at first start, giving an unique secret key for each instance.

This could be added, to maybe improve fpm/alpine compatibility in the future, but currently I don't see fpm images with phpmyadmin:
if [[ "$1" == apache2* ]] || [ "$1" = 'php-fpm' ]; then
...
group="${APACHE_RUN_GROUP:-www-data}"
...

As written above, user should stay root, group should he changed inside the container, so that for the host (running rootless, having umask 0007) the file will remain only read/writeable to the docker user and in the container file will only be changeable by root and only readable by Apache.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you define the umask ?
Reading moby/moby#19189 it seems to not be supported by Docker.

Also, please see: #187 (comment)

Copy link
Contributor Author

@obel1x obel1x Apr 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you define the umask ? Reading moby/moby#19189 it seems to not be supported by Docker.

Well yes it is not by Docker itself. For my testing i have host facl set to:

# file: .
# owner: docker
# group: docker
user::rwx
group::--x
other::---
default:user::rwx
default:group::rwx
default:other::---

While those x are X more precisely (which makes only directories X, not files), making the umask 0007 effectively.
My service has no umask set in docker.service, only my user has a ~./bashrc. Looks like docker will use the facl ones even in container itself.
So /etc/phpmyadmin/config.secret.inc.php will get 660 when created in entrypoint which is basically the wanted - while secure - default for new files as new files are usually created by the service that needs the file.

Only that file is created by root (in entrypoint) and should be readable by www-data later.

In rootless mode this makes no sense to me: how a non root user would change the group of a file root owned??

We are in entrypoint.sh. Files created here will get root:root by default and it seems that after new start of the container the group is reset again. So the (running) root- user may give the file the right to be readable by the www-data user of the service.

Also after reading #187 (comment) i don't see any reason for not doing this pull.


if [ ! -f /etc/phpmyadmin/config.user.inc.php ]; then
touch /etc/phpmyadmin/config.user.inc.php
Expand Down
Loading