buildkit icon indicating copy to clipboard operation
buildkit copied to clipboard

RUN --mount=type=cache should inherit ownership/permissions from mountpoint

Open thaJeztah opened this issue 6 years ago • 7 comments

When using RUN --mount=type=cache, the uid/gid of the mount defaults to 0:0 (root), even if the mountpoint is an existing directory with different permissions.

While it's possible to manually override the uid/gid for the mount, I think it would make sense to default to the permissions of the mountpoint (if exists), and otherwise fall back to the current behaviour (root).

Example / reproduction steps:

# syntax=docker/dockerfile:1.1.3-experimental

FROM busybox

ARG UID=1000
ARG GID=1000

RUN addgroup -g ${GID} foo
RUN adduser -D -u ${UID} -G foo foo
RUN mkdir -p /mycache && chown -R "${UID}:${GID}" /mycache

# this works (as we're root):
RUN mkdir -p /mycache/root-nomount/bar

# this works (we're still root):
RUN --mount=type=cache,target=/mycache \
        mkdir -p /mycache/root-withmount/bar

USER ${UID}:${GID}

# without `--mount`, this works, because `/mycache` was created with
# the correct permissions:
RUN mkdir -p /mycache/foo-nomount/bar

# with `--mount`, this works if we manually set the uid/gid, but this does not
# work well (currently) with dynamically set UID/GID
RUN --mount=type=cache,target=/mycache,uid=1000,gid=1000 \
        mkdir -p /mycache/foo-withmount-manually/bar

# but this fails:
#
# > [stage-0 9/9] RUN --mount=type=cache,target=/mycache         mkdir -p /mycache/foo-withmount/bar:
#15 0.607 mkdir: can't create directory '/mycache/foo-withmount/': Permission denied
RUN --mount=type=cache,target=/mycache \
        mkdir -p /mycache/foo-withmount/bar
DOCKER_BUILDKIT=1 docker build .

[+] Building 11.9s (15/15) FINISHED                                                                                                                                                           
 => [internal] load build definition from Dockerfile                                                                                                                                     0.1s
 => => transferring dockerfile: 1.07kB                                                                                                                                                   0.0s
 => [internal] load .dockerignore                                                                                                                                                        0.1s
 => => transferring context: 2B                                                                                                                                                          0.0s
 => resolve image config for docker.io/docker/dockerfile:1.1.3-experimental                                                                                                              1.4s
 => docker-image://docker.io/docker/dockerfile:1.1.3-experimental@sha256:888f21826273409b5ef5ff9ceb90c64a8f8ec7760da30d1ffbe6c3e2d323a7bd                                                2.8s
 => => resolve docker.io/docker/dockerfile:1.1.3-experimental@sha256:888f21826273409b5ef5ff9ceb90c64a8f8ec7760da30d1ffbe6c3e2d323a7bd                                                    0.0s
 => => sha256:888f21826273409b5ef5ff9ceb90c64a8f8ec7760da30d1ffbe6c3e2d323a7bd 2.03kB / 2.03kB                                                                                           0.0s
 => => sha256:ee85655c57140bd20a5ebc3bb802e7410ee9ac47ca92b193ed0ab17485024fe5 527B / 527B                                                                                               0.0s
 => => sha256:80b5f664ac0c5f6b89608f7b0afd9548ac5b2d4cd631b7b723d9f2edca8676d9 897B / 897B                                                                                               0.0s
 => => sha256:73e11b97eeae87ce062a224a9ff2d7f8a919d3b50e014a3f33a79b0c219a7003 8.82MB / 8.82MB                                                                                           2.3s
 => => extracting sha256:73e11b97eeae87ce062a224a9ff2d7f8a919d3b50e014a3f33a79b0c219a7003                                                                                                0.3s
 => [internal] load metadata for docker.io/library/busybox:latest                                                                                                                        0.0s
 => [stage-0 1/9] FROM docker.io/library/busybox                                                                                                                                         0.0s
 => => resolve docker.io/library/busybox:latest                                                                                                                                          0.0s
 => [internal] settings cache mount permissions                                                                                                                                          0.1s
 => [stage-0 2/9] RUN addgroup -g 1000 foo                                                                                                                                               0.9s
 => [stage-0 3/9] RUN adduser -D -u 1000 -G foo foo                                                                                                                                      0.9s
 => [stage-0 4/9] RUN mkdir -p /mycache && chown -R "1000:1000" /mycache                                                                                                                 0.8s
 => [stage-0 5/9] RUN mkdir -p /mycache/root-nomount/bar                                                                                                                                 1.0s
 => [stage-0 6/9] RUN --mount=type=cache,target=/mycache         mkdir -p /mycache/root-withmount/bar                                                                                    0.8s
 => [stage-0 7/9] RUN mkdir -p /mycache/foo-nomount/bar                                                                                                                                  0.8s
 => [stage-0 8/9] RUN --mount=type=cache,target=/mycache,uid=1000,gid=1000         mkdir -p /mycache/foo-withmount-manually/bar                                                          0.8s
 => ERROR [stage-0 9/9] RUN --mount=type=cache,target=/mycache         mkdir -p /mycache/foo-withmount/bar                                                                               0.7s
------                                                                                                                                                                                        
 > [stage-0 9/9] RUN --mount=type=cache,target=/mycache         mkdir -p /mycache/foo-withmount/bar:
#14 0.543 mkdir: can't create directory '/mycache/foo-withmount/': Permission denied
------
failed to solve with frontend dockerfile.v0: failed to solve with frontend gateway.v0: rpc error: code = Unknown desc = failed to build LLB: executor failed running [/bin/sh -c mkdir -p /mycache/foo-withmount/bar]: runc did not terminate sucessfully

thaJeztah avatar Oct 31 '19 12:10 thaJeztah

@AkihiroSuda @tonistiigi @tiborvass ptal

thaJeztah avatar Oct 31 '19 12:10 thaJeztah

This isn't really possible as uid/gid is part of the cache. Therefore it can't be different for same mount appearing multiple times. Another issue is that USER can be username that only makes sense in the relation of /etc/passwd that probably is not on the mount. Looking that up is slow(and actually not possible as all mounts are prepared independently) and again may be different for different contexts.

tonistiigi avatar Nov 01 '19 03:11 tonistiigi

@tonistiigi I don't think a lookup is needed. I didn't mean for it to lookup the current image's USER but that BuildKit should look at the permissions of the target, so in the example above, the target (mount point) is /mycache;

  • if /mycache is an existing directory in the image, stat -c "%u:%g" /mycache, and mount the cache with uid=1000,gid=1000
  • if /mycache does not exist in the image, proceed as it does right now; create the mountpoint and mount the cache with uid/gid 0:0

(note that it should not do anything with possibly existing content in the cache; only the ownership of the target directory / mount-point)

Therefore it can't be different for same mount appearing multiple times

Isn't that an existing issue? What is the current behavior if I use the same target with different uid/gid combinations?

RUN --mount=type=cache,target=/mycache,uid=123,gid=123 \
    echo "do something"

RUN --mount=type=cache,target=/mycache,uid=456,gid=456 \
    echo "do something"

thaJeztah avatar Nov 01 '19 10:11 thaJeztah

I read it wrong but the issues remain. Both in the sense of multiple mounts colliding and mounts being prepared independently.

Isn't that an existing issue?

Yes, I believe this is the case. That's why it's better for it to be explicit so the user can point set a different id in this case. Actually, it would probably be safer to enforce this by default, so that in the example above the cache would not be shared.

tonistiigi avatar Nov 01 '19 17:11 tonistiigi

I ran into this today, it is surprising to me that the permissions for the mount don't match the target path if it already exists. It is also incredible hard to figure out that this is happening. If this behavior is intended there should at least be a warning logged if the permissions of the target path aren't root

wdhorton avatar Sep 26 '23 21:09 wdhorton

Is there a example of how to get a --mount=type=cache to work with anything other than root. As the recommendation to create containers that run as non-root. The mount option seems to be overwriting the existing permissions with root:root making the usage impossible for non root users. Any progress?

Klwilson2272 avatar Feb 13 '24 23:02 Klwilson2272

Directly specifying user ID (UID) and group ID (GID) not only simplifies user management but significantly bolsters security by ensuring precise control over access permissions, thereby mitigating the risk of unauthorized access:

FROM maven:3.9.6-eclipse-temurin-17
RUN groupadd -g 998 builder && \
useradd --system --create-home --home-dir /home/builder -u 998 -g builder builder
USER builder
WORKDIR /home/builder
ENV MAVEN_CONFIG /home/builder/.m2
COPY pom.xml .
COPY src src
RUN --mount=type=cache,uid=998,gid=998,target=/home/builder/.m2 \
touch /home/builder/.m2/LOCATE_THIS_FILE_XYZ_1 && \
mvn dependency:go-offline && \
mvn test-compile dependency:copy-dependencies -DoutputDirectory=target/dependency
CMD ls -al /home/builder/target

I hope this addresses the issue at hand. Unfortunately, specifying the UID and GID appears to be mandatory, rather than optional.

bor8 avatar Mar 19 '24 16:03 bor8