From 54d4857f73ee0ab3337292514fcd486d82eed82b Mon Sep 17 00:00:00 2001 From: Eduardo Vega Date: Wed, 18 Nov 2020 08:50:53 -0600 Subject: [PATCH] Add U volume flag to chown source volumes Signed-off-by: Eduardo Vega --- docs/buildah-bud.md | 11 +++++++++ docs/buildah-from.md | 11 +++++++++ docs/buildah-run.md | 13 +++++++++++ pkg/parse/parse.go | 7 +++++- run_linux.go | 54 ++++++++++++++++++++++++++++++++++++++++---- tests/from.bats | 35 ++++++++++++++++++++++++++++ tests/run.bats | 19 ++++++++++++++++ 7 files changed, 145 insertions(+), 5 deletions(-) diff --git a/docs/buildah-bud.md b/docs/buildah-bud.md index 216d6af8c51..f6b5a02fadd 100644 --- a/docs/buildah-bud.md +++ b/docs/buildah-bud.md @@ -581,6 +581,7 @@ process. container. The `OPTIONS` are a comma delimited list and can be: [[1]](#Footnote1) * [rw|ro] + * [U] * [z|Z|O] * [`[r]shared`|`[r]slave`|`[r]private`] @@ -593,10 +594,18 @@ and bind mounts that into the container. You can specify multiple **-v** options to mount one or more mounts to a container. + `Write Protected Volume Mounts` + You can add the `:ro` or `:rw` suffix to a volume to mount it read-only or read-write mode, respectively. By default, the volumes are mounted read-write. See examples. + `Chowning Volume Mounts` + +By default, Buildah does not change the owner and group of source volume directories mounted into containers. If a container is created in a new user namespace, the UID and GID in the container may correspond to another UID and GID on the host. + +The `:U` suffix tells Buildah to use the correct host UID and GID based on the UID and GID within the container, to change the owner and group of the source volume. + `Labeling Volume Mounts` Labeling systems like SELinux require that proper labels are placed on volume @@ -689,6 +698,8 @@ buildah bud --security-opt label=level:s0:c100,c200 --cgroup-parent /path/to/cgr buildah bud --volume /home/test:/myvol:ro,Z -t imageName . +buildah bud -v /home/test:/myvol:z,U -t imageName . + buildah bud -v /var/lib/dnf:/var/lib/dnf:O -t imageName . buildah bud --layers -t imageName . diff --git a/docs/buildah-from.md b/docs/buildah-from.md index 3fa6c2bd268..65f23113dab 100644 --- a/docs/buildah-from.md +++ b/docs/buildah-from.md @@ -449,6 +449,7 @@ process. container. The `OPTIONS` are a comma delimited list and can be: [[1]](#Footnote1) * [rw|ro] + * [U] * [z|Z|O] * [`[r]shared`|`[r]slave`|`[r]private`|`[r]unbindable`] @@ -461,10 +462,18 @@ and bind mounts that into the container. You can specify multiple **-v** options to mount one or more mounts to a container. + `Write Protected Volume Mounts` + You can add the `:ro` or `:rw` suffix to a volume to mount it read-only or read-write mode, respectively. By default, the volumes are mounted read-write. See examples. + `Chowning Volume Mounts` + +By default, Buildah does not change the owner and group of source volume directories mounted into containers. If a container is created in a new user namespace, the UID and GID in the container may correspond to another UID and GID on the host. + +The `:U` suffix tells Buildah to use the correct host UID and GID based on the UID and GID within the container, to change the owner and group of the source volume. + `Labeling Volume Mounts` Labeling systems like SELinux require that proper labels are placed on volume @@ -553,6 +562,8 @@ buildah from --ulimit nofile=1024:1028 --cgroup-parent /path/to/cgroup/parent my buildah from --volume /home/test:/myvol:ro,Z myregistry/myrepository/imagename:imagetag +buildah from -v /home/test:/myvol:z,U myregistry/myrepository/imagename:imagetag + buildah from -v /var/lib/yum:/var/lib/yum:O myregistry/myrepository/imagename:imagetag ## ENVIRONMENT diff --git a/docs/buildah-run.md b/docs/buildah-run.md index cc21347a3f8..ddb62c27bfe 100644 --- a/docs/buildah-run.md +++ b/docs/buildah-run.md @@ -188,6 +188,7 @@ bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Buildah container. The `OPTIONS` are a comma delimited list and can be: [[1]](#Footnote1) * [rw|ro] + * [U] * [z|Z] * [`[r]shared`|`[r]slave`|`[r]private`] @@ -200,10 +201,20 @@ and bind mounts that into the container. You can specify multiple **-v** options to mount one or more mounts to a container. + `Write Protected Volume Mounts` + You can add the `:ro` or `:rw` suffix to a volume to mount it read-only or read-write mode, respectively. By default, the volumes are mounted read-write. See examples. + `Chowning Volume Mounts` + +By default, Buildah does not change the owner and group of source volume directories mounted into containers. If a container is created in a new user namespace, the UID and GID in the container may correspond to another UID and GID on the host. + +The `:U` suffix tells Buildah to use the correct host UID and GID based on the UID and GID within the container, to change the owner and group of the source volume. + + `Labeling Volume Mounts` + Labeling systems like SELinux require that proper labels are placed on volume content mounted into a container. Without a label, the security system might prevent the processes running inside the container from using the content. By @@ -270,6 +281,8 @@ buildah run --tty=false containerID ls / buildah run --volume /path/on/host:/path/in/container:ro,z containerID sh +buildah run -v /path/on/host:/path/in/container:z,U containerID sh + buildah run --mount type=bind,src=/tmp/on:host,dst=/in:container,ro containerID sh ## SEE ALSO diff --git a/pkg/parse/parse.go b/pkg/parse/parse.go index e95d20307b2..f256e6c2a91 100644 --- a/pkg/parse/parse.go +++ b/pkg/parse/parse.go @@ -486,7 +486,7 @@ func ValidateVolumeCtrDir(ctrDir string) error { // ValidateVolumeOpts validates a volume's options func ValidateVolumeOpts(options []string) ([]string, error) { - var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid int + var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid, foundChown int finalOpts := make([]string, 0, len(options)) for _, opt := range options { switch opt { @@ -515,6 +515,11 @@ func ValidateVolumeOpts(options []string) ([]string, error) { if foundLabelChange > 1 { return nil, errors.Errorf("invalid options %q, can only specify 1 'z', 'Z', or 'O' option", strings.Join(options, ", ")) } + case "U": + foundChown++ + if foundChown > 1 { + return nil, errors.Errorf("invalid options %q, can only specify 1 'U' option", strings.Join(options, ", ")) + } case "private", "rprivate", "shared", "rshared", "slave", "rslave", "unbindable", "runbindable": foundRootPropagation++ if foundRootPropagation > 1 { diff --git a/run_linux.go b/run_linux.go index d20d39423aa..68c9a9ba25f 100644 --- a/run_linux.go +++ b/run_linux.go @@ -506,8 +506,14 @@ func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath st return err } + // Get host UID and GID of the container process. + processUID, processGID, err := util.GetHostIDs(spec.Linux.UIDMappings, spec.Linux.GIDMappings, spec.Process.User.UID, spec.Process.User.GID) + if err != nil { + return err + } + // Get the list of explicitly-specified volume mounts. - volumes, err := b.runSetupVolumeMounts(spec.Linux.MountLabel, volumeMounts, optionMounts, int(rootUID), int(rootGID)) + volumes, err := b.runSetupVolumeMounts(spec.Linux.MountLabel, volumeMounts, optionMounts, int(rootUID), int(rootGID), int(processUID), int(processGID)) if err != nil { return err } @@ -1687,7 +1693,7 @@ func (b *Builder) cleanupTempVolumes() { } } -func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, optionMounts []specs.Mount, rootUID, rootGID int) (mounts []specs.Mount, Err error) { +func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, optionMounts []specs.Mount, rootUID, rootGID, processUID, processGID int) (mounts []specs.Mount, Err error) { // Make sure the overlay directory is clean before running containerDir, err := b.store.ContainerDirectory(b.ContainerID) @@ -1699,7 +1705,7 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, } parseMount := func(mountType, host, container string, options []string) (specs.Mount, error) { - var foundrw, foundro, foundz, foundZ, foundO bool + var foundrw, foundro, foundz, foundZ, foundO, foundU bool var rootProp string for _, opt := range options { switch opt { @@ -1713,6 +1719,8 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, foundZ = true case "O": foundO = true + case "U": + foundU = true case "private", "rprivate", "slave", "rslave", "shared", "rshared": rootProp = opt } @@ -1730,13 +1738,18 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, return specs.Mount{}, err } } + if foundU { + if err := chownSourceVolume(host, processUID, processGID); err != nil { + return specs.Mount{}, err + } + } if foundO { containerDir, err := b.store.ContainerDirectory(b.ContainerID) if err != nil { return specs.Mount{}, err } - contentDir, err := overlay.TempDir(containerDir, rootUID, rootGID) + contentDir, err := overlay.TempDir(containerDir, processUID, processGID) if err != nil { return specs.Mount{}, errors.Wrapf(err, "failed to create TempDir in the %s directory", containerDir) } @@ -1789,6 +1802,39 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, return mounts, nil } +// chownSourceVolume changes the ownership of a volume source directory or file within the host. +func chownSourceVolume(path string, UID, GID int) error { + fi, err := os.Lstat(path) + if err != nil { + // Skip if path does not exist + if os.IsNotExist(err) { + logrus.Debugf("error returning file info of %q: %v", path, err) + return nil + } + return err + } + + currentUID := int(fi.Sys().(*syscall.Stat_t).Uid) + currentGID := int(fi.Sys().(*syscall.Stat_t).Gid) + + if UID != currentUID || GID != currentGID { + err := filepath.Walk(path, func(filePath string, f os.FileInfo, err error) error { + return os.Lchown(filePath, UID, GID) + }) + + if err != nil { + // Skip if path does not exist + if os.IsNotExist(err) { + logrus.Debugf("error changing the uid and gid of %q: %v", path, err) + return nil + } + return err + } + } + + return nil +} + func setupMaskedPaths(g *generate.Generator) { for _, mp := range []string{ "/proc/acpi", diff --git a/tests/from.bats b/tests/from.bats index 10973c81b9e..559771fc110 100644 --- a/tests/from.bats +++ b/tests/from.bats @@ -238,6 +238,41 @@ load helpers expect_output --substring " /myvol " } +@test "from --volume with U flag" { + skip_if_no_runtime + + # Check if we're running in an environment that can even test this. + run readlink /proc/self/ns/user + echo "readlink /proc/self/ns/user -> $output" + [ $status -eq 0 ] || skip "user namespaces not supported" + + # Generate mappings for using a user namespace. + uidbase=$((${RANDOM}+1024)) + gidbase=$((${RANDOM}+1024)) + uidsize=$((${RANDOM}+1024)) + gidsize=$((${RANDOM}+1024)) + + # Create source volume. + mkdir ${TESTDIR}/testdata + touch ${TESTDIR}/testdata/testfile1.txt + + # Create a container that uses that mapping and U volume flag. + _prefetch alpine + run_buildah from --signature-policy ${TESTSDIR}/policy.json --userns-uid-map 0:$uidbase:$uidsize --userns-gid-map 0:$gidbase:$gidsize --volume ${TESTDIR}/testdata:/mnt:z,U alpine + ctr="$output" + + # Test mounted volume has correct UID and GID ownership. + run_buildah run "$ctr" stat -c "%u:%g" /mnt/testfile1.txt + expect_output "0:0" + + # Test user can create file in the mounted volume. + run_buildah run "$ctr" touch /mnt/testfile2.txt + + # Test created file has correct UID and GID ownership. + run_buildah run "$ctr" stat -c "%u:%g" /mnt/testfile2.txt + expect_output "0:0" +} + @test "from shm-size test" { skip_if_chroot skip_if_no_runtime diff --git a/tests/run.bats b/tests/run.bats index a5bee4a504d..9912f298268 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -252,6 +252,25 @@ function configure_and_check_user() { run_buildah run -v ${TESTDIR}/was-empty/testfile:/var/different-multi-level/subdirectory/testfile $cid touch /var/different-multi-level/subdirectory/testfile } +@test "run --volume with U flag" { + skip_if_no_runtime + + # Create source volume. + mkdir ${TESTDIR}/testdata + + # Create the container. + _prefetch alpine + run_buildah from --signature-policy ${TESTSDIR}/policy.json alpine + ctr="$output" + + # Test user can create file in the mounted volume. + run_buildah run --user 888:888 --volume ${TESTDIR}/testdata:/mnt:z,U "$ctr" touch /mnt/testfile1.txt + + # Test created file has correct UID and GID ownership. + run_buildah run --user 888:888 --volume ${TESTDIR}/testdata:/mnt:z,U "$ctr" stat -c "%u:%g" /mnt/testfile1.txt + expect_output "888:888" +} + @test "run --mount" { skip_if_no_runtime