Permission issue with rootless containers

[dkolepp]

I tried to use a docker_image hook on a RHEL7.7 system using podman with the podman-docker package installed [This setup allows for rootless containers]. The hook attempts to modify the file, but gets a "permission denied" error. From looking at the source code, I see that pre-commit is roughly trying to execute:

docker run -u $(id -u):$(id -g)  -v $(pwd):/src:rw,Z --workdir /src -it <ENTRY> <FILE>

The hook does try to run, but results in permission error.

If, however, I remove the -u option from the source code (locally, languages/docker.py, docker_cmd()), then the hook runs fine:

[asottile]

sounds like podman is doing an non-compliant thing

without -u the files become owned by root which isn't really ok

I'm inclined to close this as wontfix, thoughts?

[dkolepp]

No - actually it is quite compliant. Rootless containers map UID zero to a user namespace UID. This means that’s UID 0 in the container corresponds to UID of the person running the container. So, if the files are owned by the same user running the container, then there appears to be no problem.

…

On Mon, Dec 16, 2019 at 3:47 PM Anthony Sottile ***@***.***> wrote: sounds like podman is doing an non-compliant thing without -u the files become owned by root which isn't really ok I'm inclined to close this as wontfix, thoughts? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1243?email_source=notifications&email_token=ACNM5MKMHL2HIGFRVI4WJPTQY7SOJA5CNFSM4J3PEU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHABIDY#issuecomment-566236175>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNM5MLDWHGNSZXYKJNUZ7LQY7SOJANCNFSM4J3PEU4Q> .

[dkolepp]

Does pre-commit have a config file, that the library could read, to decide if it should leave out the “-u” option?

…

On Mon, Dec 16, 2019 at 5:13 PM Dan Kolepp ***@***.***> wrote: No - actually it is quite compliant. Rootless containers map UID zero to a user namespace UID. This means that’s UID 0 in the container corresponds to UID of the person running the container. So, if the files are owned by the same user running the container, then there appears to be no problem. On Mon, Dec 16, 2019 at 3:47 PM Anthony Sottile ***@***.***> wrote: > sounds like podman is doing an non-compliant thing > > without -u the files become owned by root which isn't really ok > > I'm inclined to close this as wontfix, thoughts? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#1243?email_source=notifications&email_token=ACNM5MKMHL2HIGFRVI4WJPTQY7SOJA5CNFSM4J3PEU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHABIDY#issuecomment-566236175>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACNM5MLDWHGNSZXYKJNUZ7LQY7SOJANCNFSM4J3PEU4Q> > . >

[asottile]

Right, that's not how docker works though, so if this is attempting to emulate docker's cli it is doing so without complying to docker's approach

[dkolepp]

What about if the local docker daemon is setup as so: https://docs.docker.com/engine/security/rootless/

Pretty sure the above approach by docker is using the same set of linux kernel features that podman is using. Rootless containers are a must for any one that is security-minded.

[asottile]

no idea, can you try it and report back?

[dkolepp]

Yep - I'll give it go!

[dkolepp]

Update: I installed docker 19.03 on Centos 7.7 using the instructions found here: https://docs.docker.com/engine/security/rootless/#prerequiresites

This allows the docker daemon to run as a non-root user. This mode allows the docker daemon to be installed in user space, and does not require privileges to install or run the docker daemon, as long as certain prerequisites are satisfied.

With this type of install, the exact same issue is observed as with (rootless) podman.

[asottile]

do you have a suggestion as to how to detect / avoid this then? and would you be willing to submit a PR addressing it?

[dkolepp]

I'm happy to submit a PR. Would like some ideas about what is "acceptable" for a contribution. As you say, ideally there is some way to detect rootless vs privileged, and then adjust the associated docker command accordingly.

If that's not the case (that there's not an easy way to detect rootless mode), are there other options available? Environment variables? user configuration file for the pre-commit executable itself?

[dkolepp]

Update: podman system info produces a YAML file that has a readable key for "rootless". Am going to check docker for this too...

[asottile]

ideally just detect and adjust, if that's not an option this is probably wontfix -- there is currently no configuration and I'd like to keep it that way

and perhaps document putting a docker executable on the PATH in this case which drops -u argument (since it's completely non-functional)

this does seem like a bug in docker though, --user seems completely nonfunctional in "rootless" 🤔 -- I wonder if this should be reported as well to their tracker(s)

[dkolepp]

This is intentional moving forward with containers - that all containers are set to UID 0 inside the container, and the container runtime takes care of security and sandoxing the "root" user of the container. By applying a context, you limit the privileges of the container to the context in which that container is run: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/

Also, docker has a similar interface: docker system info that provides a rootless-enabled flag...

[asottile]

I'm not sure how kubernetes design decisions are "this is how containers are from now on", could you elaborate? I'm afraid you're making unsubstantiated claims or projecting one project's decisions on the entire concept of containers.

[asottile]

@dkolepp did you have any interest in working on this?

[dkolepp]

@asottile - yes I do! I try and find some time to tackle it.

[ashiazed]

Hey folks. I stumbled on this today as well. I'm testing an environment with docker configured with namespacing and while finding rootless flag will work for @dkolepp it won't work for the user namespace remapping. It seems that in both events we are trying to undo the -u arg that is passed by default to pre-commit via the wrapper.

There is really no easy way to check for the "right" user to pass to docker, for rootless mode @dkolepp mentioned a possible flag in the docker info, for user namespacing the only thing I found was a reference to Docker Root Dir which lists the id as part of the path.

Wondering if there is an approach that's the other way, where we don't assume how someone has docker and permissions setup given that there are multiple ways of configuring docker and that things will change. Is there some way we can pass -u in manually to those users who need it? Is the entry wrapper capable of that?

[themr0c]

Hi, I am also using podman and installed podman-docker as well. I very much like the fact that with podman, the -u option becomes unnecessary.

I really would like to contribute on this issue, unfortunately my python development skills are not sufficient to provide an useful PR :(.

As a quick fix, I edited the installed file ~/.local/lib/python3.8/site-packages/pre_commit/languages/docker.py
#L81 and replaced

        return ('-u', f'{os.getuid()}:{os.getgid()}')

by:

        return ()

With that very ugly hack I can use docker_image hooks.

[webknjaz]

@themr0c does podman-docker just replace/alias the docker command with podman?

You can contribute adding something like

ver_cmd = 'docker', '--version'
if subprocess.check_output(ver_cmd).startswith(b'podman version '):
    return ()

right before try/except block in pre_commit.languages.docker.get_docker_user()

@asottile FTR this is the difference in the output:

$ podman --version
podman version 1.9.0

$ docker --version
Docker version 19.03.8, build afacb8b7f0

[themr0c]

The podman-docker package installs this shell script in /usr/bin/docker:

#!/usr/bin/sh
[ -f /etc/containers/nodocker ] || \
echo "Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg." >&2
exec /usr/bin/podman "$@"
``

[themr0c]

Podman supports the -u option. See containers/podman#6431

In verbose mode the pre-commit error is:

Error: cannot setuid: Invalid argument: OCI runtime error

Indeed:

$ podman run  antora/antora -v
2.3.2
$ podman run -u 105971 antora/antora
Error: cannot setuid: Invalid argument: OCI runtime error

I will investigate further to find which options would be suitable when using podman. I believe the -u flag becomes unnecessary, but I may miss more complex scenarii.

[webknjaz]

Probably need to add a check whether the current user is not root. Or if they have capabilities to set uid/gid.

[themr0c]

Looking at last comments in containers/podman#6431, my problem is that my user has id 105971, which is greater than the max of 65536, so the -u option fails. But, using podman, it is not necessary to change the user. So I guess not using the option when podman is detected is a wise choice. Am I getting right?

[hroncok]

Update: podman system info produces a YAML file that has a readable key for "rootless". Am going to check docker for this too...

Does docker has this option as well? I'll try to test, but I will use podman to install an old Fedora to install actual docker into it. That might blow up.

[hroncok]

Not really :/

$ docker system info
Client:
 Debug Mode: false

Server:
 Containers: 8
  Running: 0
  Paused: 0
  Stopped: 8
 Images: 2
 Server Version: 19.03.8
 Storage Driver: overlay2
  Backing Filesystem: <unknown>
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: journald
 Cgroup Driver: systemd
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: runc
 Default Runtime: runc
 Init Binary: /usr/libexec/docker/docker-init
 containerd version: 
 runc version: fbdbaf85ecbc0e077f336c03062710435607dbf1
 init version: 
 Security Options:
  seccomp
   Profile: default
  selinux
 Kernel Version: 5.6.14-300.fc32.x86_64
 Operating System: Fedora 32 (Thirty Two)
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 15.29GiB
 Name: carbon
 ID: IUGE:4L5B:VVTV:JGIA:IIJN:G7MG:TVGF:XBVW:YHF7:MYRK:L524:4HBK
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: true

[themr0c]

How to detect a rootless container daemon:

• rootless docker

Installed a rootless docker with:

curl -sSL https://get.docker.com/rootless | sh

Then:

$HOME/bin/docker system info| grep rootless
WARNING: No swap limit support
WARNING: No kernel memory limit support
WARNING: No kernel memory TCP limit support
WARNING: No oom kill disable support
WARNING: No cpu cfs quota support
WARNING: No cpu cfs period support
WARNING: No cpu shares support
  rootless

• podman

podman system info | grep rootless
  rootless: true

[themr0c]

A non rootless installation of docker doesn't have the rootless keyword at all,
A rootless installation of docker has the rootless,
Podman has: rootless: true

So if the command docker system info | grep rootless exit status is zero, we are in a rootless install.

[hroncok]

what about:

rootless = False
output = subprocess.check_output(('docker', 'system', 'info'), text=True)
for line in output.splitlines():
    # rootless docker has "rootless"
    # rootless podman has "rootless: true"
    if line.strip().startswith('rootless'):
        if not 'false' in line:
            rootless = True
        break

[themr0c]

The complete code would be like that?

def get_docker_user() -> Tuple[str, ...]:  # pragma: win32 no cover
    try:
        rootless = False
        output = subprocess.check_output(('docker', 'system', 'info'), text=True)
        for line in output.splitlines():
            # rootless docker has "rootless"
            # rootless podman has "rootless: true"
            if line.strip().startswith('rootless'):
                if not 'false' in line:
                    rootless = True
                break
        if not rootless:
            return ('-u', f'{os.getuid()}:{os.getgid()}')
    except AttributeError:
        return ()

And then it will need a test ...

[hroncok]

I don't understand the AttributeError check. Also, this function can actually return None if rootless is True. What about:

def get_docker_user():
    output = subprocess.check_output(('docker', 'system', 'info'), text=True)
    for line in output.splitlines():
        # rootless docker has "rootless"
        # rootless podman has "rootless: true"
        if line.strip().startswith('rootless'):
            if not 'false' in line:
                return ()  # no -u for rootless
            break
    return ('-u', f'{os.getuid()}:{os.getgid()}')

[themr0c]

Thank you @hroncok, I take yours :)

[themr0c]

@hroncok the AttributeError check was probably to overcome these errors: https://asottile.visualstudio.com/asottile/_build/results?buildId=3829&view=logs&j=ae542f9d-96d9-59d2-9a06-a2a85a17a811&t=6deecb4d-4f67-5e8d-1829-1f5cd5833176

[themr0c]

I will need even more help to write a test for the coverage: :/

py38 run-test: commands[2] | coverage report
pre_commit/languages/docker.py      64      3      8      1    92%   89->90, 90-92

But let's see first if the implementation is acceptable.

[aryx]

Why does pre-commit pass a -u with the current userid to docker_container?
We get some permission denied in the container because my locao userid has nothing to do with the user id in the container.
Wouldn't it be simpler to just not pass any -u?

[asottile]

because things can write and then you'd have to deal with root owned files on the host