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

Address default permissions with webuser or www-data users #253

Closed
jaydrogers opened this issue Dec 5, 2023 Discussed in #252 · 16 comments · Fixed by #311
Closed

Address default permissions with webuser or www-data users #253

jaydrogers opened this issue Dec 5, 2023 Discussed in #252 · 16 comments · Fixed by #311
Assignees
Labels
⚡️ Enhancement Items that are new features requested to be added. 🙏 Help Wanted Issues that specifically could use some help from the community 👨‍🔬 Release: Pre-release Items that relate to a release that isn't considered stable.

Comments

@jaydrogers
Copy link
Member

Discussed in #252

Originally posted by jaydrogers December 5, 2023

Background

  • In the v2 of the PHP images, webuser was added with an ID of 9999
  • In the v3 Beta, no such user is added. It defaults to www-data, but this user ID can change between Debian & Alpine

Request for comment

  1. Should we add a default user like webuser back? Or should we stick with www-data?
  2. Is there concern about the UID changing between operating systems?

Next steps

At Server Side Up, we're working on other projects that will likely run into some issues relating to this discussion. As we advance on those projects, we will update this discussion as well. Please chime in with your own experiences too.

@jaydrogers jaydrogers added the 👨‍🔬 Release: Pre-release Items that relate to a release that isn't considered stable. label Dec 5, 2023
@jaydrogers jaydrogers self-assigned this Dec 5, 2023
@jaydrogers jaydrogers added ⚡️ Enhancement Items that are new features requested to be added. 🙏 Help Wanted Issues that specifically could use some help from the community labels Dec 5, 2023
@dreadfullyposh
Copy link

Personally I don't care if a webuser is added or if www-data is used, and it doesn't bother me that the default user ID might change between Debian and Alpine.

What is important for me is the ability to remap the UID and GID of whatever user is running PHP-FPM. I frequently run containers from multiple users that do not have root/sudo access, so having this control is critical to ensuring those users have permissions to directories/files that are bind-mounted into the container.

@jeremynz21
Copy link

Personally I would prefer to use the defaults. I had been trying to switch our Silverstripe sites which are using default PHP images to use the serversideup v2 images and it created a bit of a headache trying to change the user or convert mounted file system permissions.

Alternatively if the default was changed, an option to revert back to defaults or set the PHP user via an env variable would be handy.

Love the work you doing!

@sneycampos
Copy link

i prefer to use the defaults too with the ability to change the UID/GID

@shinsenter
Copy link

shinsenter commented Dec 22, 2023

I believe that regardless of whether you use webuser, www-data or any custom user:group or UID:GID values, problems may arise if a user run your docker image along with other containers and share directories.

Therefore, it’s best to use the default user:group of the base OS and allow users to modify it according to their project setup. Avoid hard-coding UID:GID unless you’re certain that processes in your container don’t share any data with other containers. That’s my personal perspective when initializing Docker containers.

@jaydrogers
Copy link
Member Author

Thanks for your feedback everyone. I am leaning to NOT add a webuser like I did before and just use the system defaults of www-data.

👉 My new, but related question

  • What about adding shell privileges to www-data?

This is why I created webuser before, so I could have a dedicated user.

Use case

It's nice to have shell when I need to run php artisan commands as the app user. Here you can see we currently do not have this in the beta.

image

Is adding a shell to www-data a good idea? Do you feel it is secure to do this? Thoughts?

@shinsenter
Copy link

@jaydrogers

Think of www-data like a cafeteria worker at school. Their job is just to serve food in the cafeteria - they don't need access to other parts of the school. Giving www-data a shell would be like giving the cafeteria worker keys to the whole school!

Even if the cafeteria worker is honest, now a bad guy could trick or bribe them to get the keys and access to the whole school. Or if the cafeteria worker makes a mistake, the bad guy could get the keys and make a copy.

The www-data user is similar - its job is just to show web pages. It doesn't need access to run programs or files on the server. Giving it a shell lets bad guys access the whole server if they compromise a web app.

It's safer to give users and programs only the access they absolutely have to have. That way there's less risk if one gets compromised. The cafeteria worker doesn't need keys to the chem lab, and www-data doesn't need a shell. Limiting access limits damage if something goes wrong!

@jaydrogers
Copy link
Member Author

Thanks @shinsenter! I greatly appreciate your feedback. You and I are definitely on the same page. I highly respect your opinion and I love the analogy too 😃

So in finding the balance of security and user experience:

  • What is the most secure approach in running php artisan commands as www-data if we keep using the default operating system user?

With PHP requiring root to bring up services for things like fpm-nginx, it turns into a Docker permission nightmare 😆

@shinsenter
Copy link

shinsenter commented Jan 10, 2024

@jaydrogers

One of approaches for running php artisan commands as www-data is setting the setuid bit on the artisan file so it always runs as www-data. chmod u+s /path/to/artisan. (I have not tested).

Setting the setuid bit on artisan could work, but is not recommended as it allows anyone to run artisan commands as www-data.

This is less flexible but simple.

@jaydrogers
Copy link
Member Author

Although this solution would work, this requires a lot of special "know-how" for Laravel developers. I don't think this would be easy enough for most users to have an easy development experience.

This is why I was thinking of adding a shell to www-data. It's basically what Forge does on their servers.

Having a sudo policy was another thing I was thinking of, but that might add a lot of weight to the docker images.

I appreciate any other ideas/feedback if you have any!

@hughbris
Copy link

This is why I was thinking of adding a shell to www-data.

I'm confused, probably because of the plethora of new releases lol. I am deriving an image from php:beta-8.3-unit and I can open an interactive terminal as www-data, run one-off commands using docker exec -u www-data etc. Doesn't that mean you've added "shell privileges"?

In any case, this capability is required for building a flexible PHP stack for more reasons than running artisan. Scripts and permissions are often quite important when setting up a framework.

@shdehnavi
Copy link

@jaydrogers As you mentioned, the following command is not working ...
command: ["su", "www-data", "-c", "php artisan schedule:work"]

So, how i can run the horizon or task scheduler?

@AlejandroAkbal
Copy link

@jaydrogers As you mentioned, the following command is not working ... command: ["su", "www-data", "-c", "php artisan schedule:work"]

So, how i can run the horizon or task scheduler?

I got it working by implementing this in my dockerfile #287

@jaydrogers
Copy link
Member Author

💪 Big Update

We have a PR ready for testing that will dramatically improve container security and developer experience:

📖 Long story, short

  • We will be sticking with the native UID/GID of www-data (33:33 for Debian and 82:82 for Alpine)
  • There will NOT be any shell privileges for www-data by default

🙏 Please help test this

Everything is documented on the PR on how to test:

  • We have pre-built images for you to test (see the PR for more details)
  • We also dramatically improved the documentation with the containers moving to be unprivileged by default (get the preview URL from the PR)

There is also a significant improvement with running Laravel Queues, Schedulers, etc in the PR's Preview Documentation site.

We will merge this big change soon if we get good feedback 😅🚀

Check the PR for more details →

@jaydrogers jaydrogers linked a pull request Apr 18, 2024 that will close this issue
@jaydrogers
Copy link
Member Author

A fix has been released 🥳

These changes are now live in our beta images! Learn more about the release here:

Monitor this CI/CD job for the exact moment these changes will be live (takes about an hour to build):

@shdehnavi

This comment was marked as resolved.

@shdehnavi
Copy link

@jaydrogers
Thanks, everything works like a charm in the development stage by setting UID and GID and changing the users.
But unfortunately i got this error in production when i don't want to make the same UID and GID and i want to run as www-data for security purposes:
image

My Dockerfile:

FROM serversideup/php:beta-8.3-fpm-nginx

USER root

ENV PHP_POST_MAX_SIZE=50M
ENV PHP_UPLOAD_MAX_FILE_SIZE=50M

RUN docker-php-serversideup-dep-install-debian "procps"

RUN install-php-extensions pgsql intl gd

USER www-data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡️ Enhancement Items that are new features requested to be added. 🙏 Help Wanted Issues that specifically could use some help from the community 👨‍🔬 Release: Pre-release Items that relate to a release that isn't considered stable.
Projects
Development

Successfully merging a pull request may close this issue.

8 participants