docker-library / memcached

Docker Official Image packaging for memcached
http://memcached.org/
BSD 3-Clause "New" or "Revised" License
131 stars 99 forks source link

The image should define the USER ID #80

Closed camilamacedo86 closed 9 months ago

camilamacedo86 commented 2 years ago

What is the problem?

The Memcached image does not define the USER ID. Therefore, it is not a good practice and brings complexities to produce solutions that would be distributed to work on Openshift/OKD and K8s with this one in the latest version where the Pod Security Standards and the option to enforce the restricted policy via label be introduced.

Why it is not a good practice?

From Docker Docs

Note that when specifying a group for the user, the user will have only the specified group membership. Any other configured group memberships will be ignored.

When the user doesn’t have a primary group then the image (or the next instructions) will be run with the root group.

From: https://docs.docker.com/engine/reference/builder/#user

NOTE: Therefore because the container user is always a member of the root group, the container user can read and write these files.

From Dockefile best practices

Screenshot 2022-07-12 at 00 10 10

https://docs.docker.com/develop/develop-images/dockerfile_best-practices/

Motivations to ask for this change

On Kubernetes (>= 1.24) you can check

If we create a Pod/Container to be admitted as restricted and does not violate the PodSecurity with the following security context config:

spec:
      securityContext:
        runAsNonRoot: true
        seccompProfile:
          type: RuntimeDefault
      ...
      containers:
      - name: controller-manager
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - ALL

The Pod/Container will not run and the error container has runAsNonRoot and image will run as root … will be raised.

On Openshift

To allow a workload to run as restricted(restricted-v2 SCC) we MUST leave the runAsUser field empty. Otherwise, we will only be able to run the Pod if it has access to the SCC nonroot-v2 or anyuid. Note that we cannot here define a specific userID because it will depend on the namespace. So, if we want to distribute an Operator using this image we do not know what is the ns where it will be installed and it begins to add complexities on top either if we are OK by running it with security context more permissve.

Therefore if we try a number like 1001 we will face issues like:

Leave the runAsUser field empty, or provide a value that falls within the specific user range for the namespace. provide a value in the ranges: [1000680000, 1000689999]

Then, you can also check its docs:

Lastly, the final USER declaration in the Dockerfile should specify the user ID (numeric value) and not the user name. This allows OpenShift Container Platform to validate the authority the image is attempting to run with and prevent running images

(and one is distinctly more user-friendly to read than the other). 😬

What do you mean with and one is distinctly more user-friendly to read than the other? The UID and GID are defined in the Dockerfile already. (as described above) It does not change its ux.

Proposed solution

To sort it out it, we just need to set the USER ID. Same value used on this already:

RUN addgroup -g 11211 memcache && adduser -D -u 11211 -G memcache memcache

https://github.com/docker-library/memcached/blob/master/alpine/Dockerfile#L3-L4 https://github.com/docker-library/memcached/blob/master/debian/Dockerfile#L3-L5

It is a very small request that will help a lot anyone that is trying to use this image to provide solutions for the latest K8s and OCP versions.

The PR https://github.com/docker-library/memcached/pull/79 is open and would be great if we could get this one merged and have a new release/tag from this image.

camilamacedo86 commented 2 years ago

c/c @tianon @ncopa @blar @michael-k

tianon commented 9 months ago

Closing given discussion in https://github.com/docker-library/memcached/pull/79