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

WORKDIR does not honor USER's uid&gid when creating the directory #2323

Closed
petr-motejlek opened this issue Apr 25, 2020 · 21 comments
Closed
Labels
buildkit Good First Issue This issue would be a good issue for a first time contributor to undertake. locked - please file new issue/PR

Comments

@petr-motejlek
Copy link

Description

It seems that when Buildah's WORKDIR creates directories, it does not honor the current USER. Both docker build and DOCKER_BUILDKIT=1 docker build with the same Dockerfile will honor it.

Steps to reproduce the issue:

FROM ubuntu:latest
RUN useradd -r -s /bin/false -m -d /opt/my_project my_project
USER my_project
WORKDIR /opt/my_project/src
RUN id && ls -la && mkdir this_fails

Describe the results you received:
With docker buildx:

docker build --no-cache --progress=plain .
#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 32B done
#2 DONE 0.0s

#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#3 [internal] load metadata for docker.io/library/ubuntu:latest
#3 DONE 0.6s

#4 [1/4] FROM docker.io/library/ubuntu:latest@sha256:747d2dbbaaee995098c979...
#4 CACHED

#5 [2/4] RUN useradd -r -s /bin/false -m -d /opt/my_project my_project
#5 DONE 1.2s

#6 [3/4] WORKDIR /opt/my_project/src
#6 DONE 0.1s

#7 [4/4] RUN id && ls -la && mkdir this_fails
#7 0.967 uid=999(my_project) gid=999(my_project) groups=999(my_project)
#7 0.967 total 8
#7 0.967 drwxr-xr-x 2 my_project my_project 4096 Apr 25 12:32 .
#7 0.967 drwxr-xr-x 1 my_project my_project 4096 Apr 25 12:32 ..
#7 DONE 1.1s

#8 exporting to image
#8 exporting layers 0.1s done
#8 writing image sha256:b06c6f73926a43dcb22c389b2974244b7aac47f8444c300aa27d37d2b21bd00c done
#8 DONE 0.2s

Notice #7 0.967 drwxr-xr-x 2 my_project my_project 4096 Apr 25 12:32 .

With Buildah:

$ docker run -it --rm -v "$(pwd)":"$(pwd)":ro -w "$(pwd)" --privileged --net=host --security-opt label=disable --security-opt seccomp=unconfined --device /dev/fuse:rw quay.io/buildah/stable buildah bud -f prdel/Dockerfile prdel
STEP 1: FROM ubuntu:latest
Getting image source signatures
Copying blob d51af753c3d3 done
Copying blob fc878cd0a91c done
Copying blob fee5db0ff82f done
Copying blob 6154df8ff988 done
Copying config 1d622ef86b done
Writing manifest to image destination
Storing signatures
STEP 2: RUN useradd -r -s /bin/false -m -d /opt/my_project my_project
STEP 3: USER my_project
STEP 4: WORKDIR /opt/my_project/src
STEP 5: RUN id && ls -la && mkdir this_fails
uid=999(my_project) gid=999(my_project) groups=999(my_project)
total 8
drwxr-xr-x 2 root       root       4096 Apr 25 12:32 .
drwxr-xr-x 3 my_project my_project 4096 Apr 25 12:32 ..
mkdir: cannot create directory 'this_fails': Permission denied
subprocess exited with status 1
subprocess exited with status 1
                               error building at STEP "RUN id && ls -la && mkdir this_fails": exit status 1

Notice drwxr-xr-x 2 root root 4096 Apr 25 12:32 .

Describe the results you expected:
I would expect the directory to be owned by the user my_project and group my_project.

Output of rpm -q buildah or apt list buildah:
N/A

Output of buildah version:

Version:         1.14.3
Go Version:      go1.13.6
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:
image Version:   5.2.1
Git Commit:
Built:           Thu Jan  1 00:00:00 1970
OS/Arch:         linux/amd64
@petr-motejlek
Copy link
Author

This is also reproducible with the most recent Buildah. The behavior is exactly the same.

Version:         1.15.0-dev
Go Version:      go1.13.6
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:  v0.7.2-0.20190904153231-83439463f784
image Version:   5.2.1
Git Commit:      002dffb8
Built:           Sat Feb 22 10:27:15 2020
OS/Arch:         linux/amd64

@TomSweeneyRedHat
Copy link
Member

TomSweeneyRedHat commented Apr 25, 2020

@petr-motejlek I don't have quick access to test, will do so on Monday. If by chance you've time this weekend, could you try running this as root and let me know if this has the same issue then? My first suspicion is the specified WORKDIR isn't get the right privs set in rootless, but just a quick guess.

@petr-motejlek
Copy link
Author

@TomSweeneyRedHat ,

certainly. Do you mean "as root" as in "docker run ... sudo buildah bud" ?

If so, then the result is the same :(

$ docker run -it --rm -v "$(pwd)":"$(pwd)":ro -w "$(pwd)" --privileged --net=host --security-opt label=disable --security-opt seccomp=unconfined --device /dev/fuse:rw quay.io/buildah/upstream:master sudo buildah bud -f prdel/Dockerfile prdel
STEP 1: FROM ubuntu:latest
Getting image source signatures
Copying blob fc878cd0a91c done
Copying blob d51af753c3d3 done
Copying blob 6154df8ff988 done
Copying blob fee5db0ff82f done
Copying config 1d622ef86b done
Writing manifest to image destination
Storing signatures
STEP 2: RUN useradd -r -s /bin/false -m -d /opt/my_project my_project
STEP 3: USER my_project
STEP 4: WORKDIR /opt/my_project/src
STEP 5: RUN id && ls -la && mkdir this_fails
uid=999(my_project) gid=999(my_project) groups=999(my_project)
total 8
drwxr-xr-x 2 root       root       4096 Apr 25 18:05 .
drwxr-xr-x 3 my_project my_project 4096 Apr 25 18:05 ..
mkdir: cannot create directory 'this_fails': Permission denied
error building at STEP "RUN id && ls -la && mkdir this_fails": error while running runtime: exit status 1

@rhatdan rhatdan added the Good First Issue This issue would be a good issue for a first time contributor to undertake. label Apr 27, 2020
@rhatdan
Copy link
Member

rhatdan commented Apr 27, 2020

Nice work diagnosing,

@TomSweeneyRedHat
Copy link
Member

Thanks @petr-motejlek . I think @rhatdan has a fix in progress with #2332

@rhatdan
Copy link
Member

rhatdan commented Apr 27, 2020

Nope not fixing this issue.

@nalind
Copy link
Member

nalind commented Apr 28, 2020

Hmm. When I try this with docker build from the moby-engine-18.09.7-5.ce.git2d0083d.fc30 package, the build shows /opt/my_project owned by the "my_project" user, but /opt/my_project/src is created owned by "root". When I try it with DOCKER_BUILDKIT=1, it seems to hang at the useradd step.

Which version are you using? I'm wondering if this is a behavior that changed at some point.

@rhatdan
Copy link
Member

rhatdan commented Apr 29, 2020

I am on cgroups V2 right now so can' t easily test right now. But the original code was attempting to do what we are doing now.

@rhatdan
Copy link
Member

rhatdan commented Apr 29, 2020

Ok I just tried this out on Docker, and it does not change the label of the workdir

@rhatdan rhatdan closed this as completed Apr 29, 2020
@rhatdan
Copy link
Member

rhatdan commented Apr 29, 2020

@petr-motejlek Just checked on moby(docker) and the workdir gets setup as root there just like Podman.

cat /tmp/Dockerfile 
FROM alpine
USER bin
WORKDIR /home/workdir
RUN ls -l /home
RUN touch file
RUN ls -l /home/workdir/file
# docker build /tmp/
Sending build context to Docker daemon   7.68kB
Step 1/6 : FROM alpine
 ---> f70734b6a266
Step 2/6 : USER bin
 ---> Using cache
 ---> 42a3ae6a04e2
Step 3/6 : WORKDIR /home/workdir
 ---> Using cache
 ---> 129786937ecd
Step 4/6 : RUN ls -l /home
 ---> Using cache
 ---> 0bb152dac2bc
Step 5/6 : RUN touch file
 ---> Running in 65e9579e9a3a
touch: file: Permission denied
The command '/bin/sh -c touch file' returned a non-zero code: 1
# docker -v
Docker version 19.03.8, build afacb8b

@petr-motejlek
Copy link
Author

petr-motejlek commented May 7, 2020

Argh. This is just too mind-blowing.

So, plain Docker indeed seems to create the workdir owned by ROOT. Yet, when I use DOCKER_BUILDKIT (which uses the BuildKit engine that comes with the Docker CLI), it will use USER.

What moby(docker) did you use to test this (you said it's not setting the ownership there either)? (I am using docker-buildx, currently at version 0.4.1, released 6 days ago, and with that one, I can see the expected ownership (my_project)).

I assume, even though this behavior might not be desired, you won't want to change buildah's code, unless upstream changes, huh?

@petr-motejlek
Copy link
Author

I ask, because this very well might be something that Docker does not implement (anymore), yet BuildKit does.

@TomSweeneyRedHat
Copy link
Member

and I should have reopened this prior.

@TomSweeneyRedHat
Copy link
Member

I used the latest upstream client of Docker that I could get. I think it was 1.18+, however, that was for when the ARG was declared above the FROM and then referenced again after the FROM. I've a suspicion something is off with the chown due to a recent change to stage handling, but I'd not yet run it down.

@TomSweeneyRedHat
Copy link
Member

Arg, too many issues, I meant to reopen #2192.

@rhatdan
Copy link
Member

rhatdan commented May 8, 2020

I actually tested against
docker-ce-19.03.7-3.fc31.x86_64

I guess it is arguable, what is the correct behaviour. If you open a issue against upstream Docker to change the default, it would make it easier for us to follow.

But if you have good arguments about what it should be then I am willing to push for it again.

@nalind @vrothberg WDYT?

@petr-motejlek
Copy link
Author

I think this heavily depends on what is the generally expected usage for WORKDIR.

I think the simplest case why one would use it is to affect the working directory of the image I am creating (parallel to what USER, EXPOSE, VOLUME, ... do). In this case, I would not expect it to create the directory at all. USER does not create a user. EXPOSE does not actually listen on any ports, it's just metadata. So is VOLUME, technically.

The expanded case of using it in the middle of a Dockerfile as an alternative to mkdir -p ... && cd ... is a bit weird, honestly. But it exists, and when you ask me, I would expect it to honor the currently set USER and try to use that user to create the directory (the creation should fail IMHO if the user does not have proper permissions).

So, honestly, my number 1 would be: only use WORKDIR as metadata indicator. Since we are already pass that, I think it would be better for it to honor USER. Reason?

Well, think about a Dockerfile like this:

FROM nginx:18.04
ARG user=nginx
USER $USER
WORKDIR /var/tmp/nginx
COPY --chown=$USER ./www ./
...

It's obviously incomplete, and nginx might not be the best example, but there are cases where I, in theory, have no need to use RUN at all. In this one, I switch user (the nginx image already provides the user), then I want to create a temporary directory where I'll copy the files (expecting to also add the necessary configuration to use this directory to serve the files from, but that's not in the example). Because I don't like to repeat myself, I do not use COPY (with an absolute path as target) first and then switch using WORKDIR, as that would require an ARG or ENV variable to keep track of the path (in here, I can use ./ as the COPY target).

The way things are, in this example the nginx user won't actually be able to access the working directory.


Strike that. Writing this, I realized, WORKDIR might actually instead need --chown, similarly to how COPY has it. COPY will also by default create any files and directories owned by root. Only when you use --chown will it change the ownership. I think this might make more sense as a proposal for Docker, rather than changing the way it works altogether. This would even be a backwards-compatible change.

I'll link the ticket for Docker, when I create it.

@petr-motejlek
Copy link
Author

Turns out there's already a request for this @ moby/moby#36677

@petr-motejlek
Copy link
Author

And there's also moby/moby#34819 (related) about implementing --chmod for COPY, but the discussion there seems to indicate that's never happening, so one has to wonder how likely is it to get --chown for WORKDIR :)

@rhatdan
Copy link
Member

rhatdan commented May 15, 2020

Especially where the issue is two years old.

@rhatdan
Copy link
Member

rhatdan commented May 15, 2020

Personally I would prefer the --chown, since that would not break existing users.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
buildkit Good First Issue This issue would be a good issue for a first time contributor to undertake. locked - please file new issue/PR
Projects
None yet
Development

No branches or pull requests

4 participants