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

USER root vs USER 1000 #506

Closed
MallocArray opened this issue Apr 28, 2023 · 5 comments · Fixed by #521
Closed

USER root vs USER 1000 #506

MallocArray opened this issue Apr 28, 2023 · 5 comments · Fixed by #521
Labels
needs_triage New item that needs to be triaged

Comments

@MallocArray
Copy link

When using ansible-builder, including v3, the user is set as root in the container.

When I looked at the config for the Execution Environment for AWX, I see they add a step at the end to set USER 1000 which changes behavior inside of the container
https://github.com/ansible/awx-ee/blob/devel/execution-environment.yml

Is there a reason that ansible-builder runs as root and awx-ee runs as 1000? Is one more secure than the other or more "correct"? I would have expected similar behavior from both.

@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Apr 28, 2023
@nitzmahone
Copy link
Member

nitzmahone commented May 2, 2023

We're still debating this one as well- realistically it doesn't much matter in most environments today, as by default (to properly work under all container runtimes we support) we have to set permissions on things like /etc/passwd and other sensitive resources in such a way that the container user can basically do whatever they want inside the container anyway, no matter what the default UID is (ie, if they're not already UID 0 inside, it's trivial to become UID 0). In rootless envs, it makes no difference at all, and in managed rootful envs, there are usually multiple layers of containment anyway (including forcing the use of a random ephemeral UID at runtime anyway).

IIUC, the original default in the runner images that most EEs are based on was set to something non-root to comply with some vague security guidelines that are mostly meaningless in today's real-world environments. Maybe @shanemcd or someone else has more thoughts?

@shanemcd
Copy link
Member

shanemcd commented May 3, 2023

The reason awx-ee sets USER to 1000 is to work out-of-the-box on k8s clusters with security policies that force pods to run as a non-root user. This might be a good enough reason to do this in ansible-builder by default, as I think anyone trying to use EEs in hardened k8s clusters will run into this.

@nitzmahone
Copy link
Member

nitzmahone commented May 3, 2023

Right, but they don't typically ever run as the container default user in any k8s config I've seen (no matter if it's root or not)- without manual container/pod config to the contrary, they end up running as a completely ephemeral UID with primary GID 0, hence all the custom ENTRYPOINT goo and chgrp/chmod shenanigans during build and init to clean up after that since the various container runtime accommodations for it are non-standard. IIUC our own devtools container stuff tends to default to matching the host UID to make it easier for directory/file permissions on bind-mounted volumes. We go to a decent amount of trouble to make sure any GID 0 user can do the needful, and while I have no problem setting USER to something in the final container (and/or providing a first-class method and a default), I'm still unclear if it provides any actual benefit anywhere. Maybe so interactive users just screwing around in the container manually get the same limitations of an ephemeral UID vs "weird, it works when I run $whatever interactively (as default root) but fails in AWX/AAP (when run as a GID0 user)? Anyone who knows what they're doing can easily -u 0 if they need to mess with something else, I suppose...

🤷‍♂️

@shanemcd
Copy link
Member

shanemcd commented May 3, 2023

Right, but they don't typically ever run as the container default user in any k8s config I've seen (no matter if it's root or not)- without manual container/pod config to the contrary, they end up running as a completely ephemeral UID with primary GID 0

This is true for OCP and some other security-minded distributions. But unless something has changed recently, by default Kubernetes will let you run pods under whatever UID is configured in the image itself.

@nitzmahone
Copy link
Member

Bleh, I thought they'd changed the default to Restricted (or at least Baseline) with the PodSecurityAdmission stuff, but digging around, it looks like "not so much" - it's also not that simple anyway since the actual behavior can be influenced by RuntimeClass, sandboxing tech, etc.

I'm actually less swayed by the k8s argument (since there are so many variables involved, the use of any default is likely to fail for some common use case and require manual intervention)... I do feel like "interactive mucking about should probably (by default) behave similarly to a typical deployment" is a good ideal to strive for though. So we should probably just add an options entry for this and default it to 1000 or something else non-root.

Akasurde added a commit to Akasurde/ansible-builder that referenced this issue May 4, 2023
* Added options.user setting
* Updated documentation with an example
* Added unit tests

Fixes: ansible#506

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
nitzmahone pushed a commit that referenced this issue May 4, 2023
* Allow to set user value for the container image

* Added options.user setting
* Updated documentation with an example
* Added unit tests

Fixes: #506

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

* Updated indentation

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

* review comments

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

* Fix integration tests

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

---------

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_triage New item that needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants