Skip to content

run as non privileged user for security #39

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eatwithforks
Copy link
Contributor

@mkowalski
Copy link
Contributor

Hi guys, this one would be useful for my use case. Any chance of getting it in? (ping @rpardini)

@rpardini
Copy link
Owner

So only question here is compatibility.
Imagine a longtime user of the proxy has their CA/cache dir mounted and accessed as root.
Then they pull a newer version that includes this...
How does this affect old caches?

@rpardini
Copy link
Owner

nginx.conf already had user nginx; since forever; maybe there's no reason to be concerned? Can @mkowalski maybe confirm?

@mkowalski
Copy link
Contributor

mkowalski commented Dec 8, 2020

@rpardini Sorry for the long delay...

No, the current schema does not work. Inside the container nginx is mapped to 100:101. When adding to the docker-compose manifest user: 100:101, the following failure happens

# docker logs -f docker-registry-proxy
/entrypoint.sh: line 12: /etc/nginx/resolvers.conf: Permission denied

I only mount the following

    volumes:
      - /var/lib/docker-registry-proxy/ca:/ca
      - /var/lib/docker-registry-proxy/data:/docker_mirror_cache

and both have permissions for 100:101. The error message is coming purely from the container itself.

@gw0
Copy link

gw0 commented Feb 16, 2021

My attempts at running docker-pregistry-proxy:0.6.2 as a less-privileged user/context in Kubernetes have failed. I am listing each securityContext configuration and what errors it produced:

securityContext:
  readOnlyRootFilesystem: true

$ kubectl logs -f docker-proxy-docker-registry-proxy-0
/entrypoint.sh: line 12: /etc/nginx/resolvers.conf: Read-only file system

Possible solution: The above issue could be fixed if the /etc/nginx/resolvers.conf is put on a read-write volume and a symlink is put in place. This volume could be either next to the CA or an empty new tmpfs volume.

securityContext:
  runAsNonRoot: true

$ kubectl logs -f docker-proxy-docker-registry-proxy-0
/entrypoint.sh: line 12: /etc/nginx/resolvers.conf: Permission denied
securityContext:
  runAsUser: 100
  runAsGroup: 101

$ kubectl logs -f docker-proxy-docker-registry-proxy-0
/entrypoint.sh: line 12: /etc/nginx/resolvers.conf: Permission denied

Possible solution: The above issues could probably be fixed if Dockerfile would do: touch /etc/nginx/resolvers.conf && chown nginx:nginx /etc/nginx/resolvers.conf (or chown 100:101 ...).

securityContext:
  capabilities:
    drop:
    - ALL

$ kubectl logs -f docker-proxy-docker-registry-proxy-0
...
Testing nginx config...
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
2021/02/16 08:45:53 [emerg] 66#66: chown("/docker_mirror_cache", 100) failed (1: Operation not permitted)
nginx: [emerg] chown("/docker_mirror_cache", 100) failed (1: Operation not permitted)
nginx: configuration file /etc/nginx/nginx.conf test failed

Possible solution: This chown issue can be solved if the volume /docker_mirror_cache is mounted as user/group nginx. In Kubernetes with: podSecurityContext: fsGroup: 101. But then another error appears:

nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
2021/02/16 09:22:59 [emerg] 66#66: mkdir() "/var/cache/nginx/client_temp" failed (13: Permission denied)
nginx: [emerg] mkdir() "/var/cache/nginx/client_temp" failed (13: Permission denied)
nginx: configuration file /etc/nginx/nginx.conf test failed

Possible solution: This mkdir issue could be solved if the Dockerfile would create the directory beforehand mkdir /var/cache/nginx/client_temp. But any such solution would be incompatible with readOnlyRootFilesystem, therefore it would be better to symlink this directory to the /docker_mirror_cache or similar.

@rpardini rpardini added the breaks_tests Breaks the Sanity Test check; definitely needs rework label Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks_tests Breaks the Sanity Test check; definitely needs rework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants