Skip to content

Adding USER id to avoid run as root in OpenShift#45376

Closed
josgonza-rh wants to merge 1 commit intoelastic:masterfrom
josgonza-rh:master
Closed

Adding USER id to avoid run as root in OpenShift#45376
josgonza-rh wants to merge 1 commit intoelastic:masterfrom
josgonza-rh:master

Conversation

@josgonza-rh
Copy link
Copy Markdown

@josgonza-rh josgonza-rh commented Aug 9, 2019

This change is needed because: "If the image does not specify a USER, it inherits the USER from the parent image" [1]

[1] OpenShift Guidelines

This change is needed because: "If the image does not specify a USER, it inherits the USER from the parent image" [1]

[1](https://docs.openshift.com/container-platform/3.11/creating_images/guidelines.html)
@cbuescher cbuescher added the :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts label Aug 9, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

@dliappis
Copy link
Copy Markdown
Contributor

dliappis commented Aug 22, 2019

Thanks for the PR @josgonza-rh . The reason why we haven't been setting it (some historical info here, originating from this issue) is that AFAIU as per the guidelines you linked:

By default, OKD runs containers using an arbitrarily assigned user ID. This provides additional security against processes escaping the container due to a container engine vulnerability and thereby achieving escalated permissions on the host node.

i.e. USER will anyway be overridden to some randomized value.

Could you share more light into how the arbitrary UID gets applied and the need to specify USER? Judging by the -rh suffix in your GH username perhaps you are most involved with OpenShift.

@josgonza-rh
Copy link
Copy Markdown
Author

Hi @dliappis,

It is the same for OKD and OpenShift .. and it is all explained in the OpenShift Guidelines mentioned before.

The key is (in the same point you extract the info Support Arbitrary User IDs):

If the image does not specify a USER, it inherits the USER from the parent image

That means that if your image inherints from other, in this case from centos:7, and the user of that parent image is root, your container will run as root and if you don't use the SCC anyuid or the privileged it will fail to deploy in an OpenShift cluster.

In our case it won't work even if you change the SCC just because you're forcing your pod to run as not root (pod.spec.securityContext.runAsNonRoot: true).

The only important thing is to set the USER with a numeric value, no matters whats, as it is true that OpenShift will anyway override it to some randomized value.

At the end of the point Support Arbitrary User IDs:

Lastly, the final USER declaration in the Dockerfile should specify the user ID (numeric value) and not the user name. This allows OKD to validate the authority the image is attempting to run with and prevent running images that are trying to run as root, because running containers as a privileged user exposes potential security holes. If the image does not specify a USER, it inherits the USER from the parent image.

@dliappis
Copy link
Copy Markdown
Contributor

Many thanks for the explanation @josgonza-rh . Your reasoning makes sense.

Regarding:

In our case it won't work even if you change the SCC just because you're forcing your pod to run as not root (pod.spec.securityContext.runAsNonRoot: true).

I guess you are referring to this property specified in the Elastic Cloud on Kubernetes project?

At any rate one issue with this PR is that we'll need to change things in the current Entrypoint script. As you can see here, here and here there are certain actions taken if user is root. Some of them could be simplified by assuming USER defaults to 1000, but we'd e.g. break the feature TAKE_FILE_OWNERSHIP described in our docs, item 1.

Would it be ok to ask you to actually close this PR and open an issue instead (you can ping me there) copying over the argumentation you just presented, so that we can discuss any potential for breaking changes and come up with a PR that addresses the OpenShift/OKD fix plus any necessary changes in the Entrypoint and docs? Thanks again for raising this.

@josgonza-rh
Copy link
Copy Markdown
Author

Hi @dliappis,

About

I guess you are referring to this property specified in the Elastic Cloud on Kubernetes project?

Right:

KIND:     Pod
VERSION:  v1

FIELD:    runAsNonRoot <boolean>

DESCRIPTION:
     Indicates that the container must run as a non-root user. If true, the
     Kubelet will validate the image at runtime to ensure that it does not run
     as UID 0 (root) and fail to start the container if it does. If unset or
     false, no such validation will be performed. May also be set in
     SecurityContext. If set in both SecurityContext and PodSecurityContext, the
     value specified in SecurityContext takes precedence.

About

but we'd e.g. break the feature TAKE_FILE_OWNERSHIP described in our docs, item 1.

You can resolve of this problems following the OpenShift Guidelines, concretely the solution described in the point mentioned above: Support Arbitrary User IDs

  1. In the Dockerfile:

    RUN chgrp -R 0 /some/directory && \
    chmod -R g=u /some/directory
    
    ... ommited ...
    
    RUN chmod g=u /etc/passwd
    ENTRYPOINT [ "uid_entrypoint.sh" ]
    USER 1001
    
  2. uid_entrypoint.sh:

    if ! whoami &> /dev/null; then
      if [ -w /etc/passwd ]; then
        echo "${USER_NAME:-default}:x:$(id -u):0:${USER_NAME:-default} user:${HOME}:/sbin/nologin" >> /etc/passwd
      fi
    fi
    exec "$@"
    

If you still don't feel confidence with the steps described in the OpenShift Guidelines you can close this PR or open the issue by yourself without problem.

@dliappis
Copy link
Copy Markdown
Contributor

@josgonza-rh Thanks the followup.

If you still don't feel confidence with the steps described in the OpenShift Guidelines you can close this PR or open the issue by yourself without problem.

It's not a matter of lack of confidence; in fact we have used quite a bunch of the steps you mentioned in the Dockerfile and a few places in the entrypoint. We'll need to modify the entrypoint to work properly with the USER 1000 change that this PR brings, possibly adjust docs if e.g. we need to deprecate an existing option, decide which version to push it to, possibly require additional backward compatibility tests to name a few.

If you don't have the time to address those in this PR (all of this can be quite laborious) I think it's best that I open an issue summarizing all the discussion here and work on a PR -- that will of course acknowledge your contribution -- that will have everything needed.

Is Minishift a good environment to use for testing image changes for compatibility with OpenShift?

@josgonza-rh
Copy link
Copy Markdown
Author

@dliappis OK, I'm agree with you (this PR is not valid as it needs more changes and open an issue fits perfect for this case). It's true that I don't have so much time to address the complete issue but of course I can help you to face it.

Of course Minishift is a good environment to test all that kind of stuffs (and learn how OpenShift works). The Minishift's GitHub Site has a lot of info and links that will help you.

@dliappis
Copy link
Copy Markdown
Contributor

@josgonza-rh Many thanks for your help. We truly appreciate it. I will ping you directly on the new issue and kindly ask for your help reviewing the PR if you have the time.

@josgonza-rh
Copy link
Copy Markdown
Author

@dliappis sure ;)

@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts feedback_needed Team:Delivery Meta label for Delivery team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants