From b460dc39b7c9113db60cf81bb9edf77d523c5be0 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 10 Feb 2023 15:54:05 +0100 Subject: [PATCH] tests/integration: Add tests for idmap mounts Co-authored-by: Francis Laniel Signed-off-by: Rodrigo Campos --- .gitignore | 1 + Makefile | 7 +- contrib/cmd/fs-idmap/fs-idmap.go | 50 ++++++++++ tests/integration/helpers.bash | 40 ++++++++ tests/integration/idmap.bats | 160 +++++++++++++++++++++++++++++++ 5 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 contrib/cmd/fs-idmap/fs-idmap.go create mode 100644 tests/integration/idmap.bats diff --git a/.gitignore b/.gitignore index 76aefa1e233..4df0d6abfde 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ vendor/pkg contrib/cmd/recvtty/recvtty contrib/cmd/sd-helper/sd-helper contrib/cmd/seccompagent/seccompagent +contrib/cmd/fs-idmap/fs-idmap man/man8 release Vagrantfile diff --git a/Makefile b/Makefile index 00c8d71ab68..c72dd2f414d 100644 --- a/Makefile +++ b/Makefile @@ -60,9 +60,9 @@ endif runc: $(GO_BUILD) -o runc . -all: runc recvtty sd-helper seccompagent +all: runc recvtty sd-helper seccompagent fs-idmap -recvtty sd-helper seccompagent: +recvtty sd-helper seccompagent fs-idmap: $(GO_BUILD) -o contrib/cmd/$@/$@ ./contrib/cmd/$@ static: @@ -151,6 +151,7 @@ clean: rm -f contrib/cmd/recvtty/recvtty rm -f contrib/cmd/sd-helper/sd-helper rm -f contrib/cmd/seccompagent/seccompagent + rm -f contrib/cmd/fs-idmap/fs-idmap rm -rf release rm -rf man/man8 @@ -191,7 +192,7 @@ verify-dependencies: vendor validate-keyring: script/keyring_validate.sh -.PHONY: runc all recvtty sd-helper seccompagent static releaseall release \ +.PHONY: runc all recvtty sd-helper seccompagent fs-idmap static releaseall release \ localrelease dbuild lint man runcimage \ test localtest unittest localunittest integration localintegration \ rootlessintegration localrootlessintegration shell install install-bash \ diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go new file mode 100644 index 00000000000..b48114dfe03 --- /dev/null +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -0,0 +1,50 @@ +package main + +import ( + "fmt" + "log" + "os" + "os/exec" + "syscall" + + "golang.org/x/sys/unix" +) + +func main() { + if len(os.Args) < 2 { + log.Fatalf("usage: %s path_to_mount_set_attr", os.Args[0]) + } + + src := os.Args[1] + treeFD, err := unix.OpenTree(-1, src, uint(unix.OPEN_TREE_CLONE|unix.OPEN_TREE_CLOEXEC|unix.AT_EMPTY_PATH|unix.AT_RECURSIVE)) + if err != nil { + log.Fatalf("error calling open_tree %q: %v", src, err) + } + defer unix.Close(treeFD) + + cmd := exec.Command("/usr/bin/sleep", "5") + cmd.SysProcAttr = &syscall.SysProcAttr{ + Cloneflags: syscall.CLONE_NEWUSER, + UidMappings: []syscall.SysProcIDMap{{ContainerID: 0, HostID: 65536, Size: 65536}}, + GidMappings: []syscall.SysProcIDMap{{ContainerID: 0, HostID: 65536, Size: 65536}}, + } + if err := cmd.Start(); err != nil { + log.Fatalf("failed to run the helper binary: %v", err) + } + + path := fmt.Sprintf("/proc/%d/ns/user", cmd.Process.Pid) + var userNsFile *os.File + if userNsFile, err = os.Open(path); err != nil { + log.Fatalf("unable to get user ns file descriptor: %v", err) + return + } + defer userNsFile.Close() + + attr := unix.MountAttr{ + Attr_set: unix.MOUNT_ATTR_IDMAP, + Userns_fd: uint64(userNsFile.Fd()), + } + if err := unix.MountSetattr(treeFD, "", unix.AT_EMPTY_PATH|unix.AT_RECURSIVE, &attr); err != nil { + log.Fatalf("error calling mount_setattr: %v", err) + } +} diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index 4998390e2ee..d4494bea045 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -16,6 +16,7 @@ unset IMAGES RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty" SD_HELPER="${INTEGRATION_ROOT}/../../contrib/cmd/sd-helper/sd-helper" SECCOMP_AGENT="${INTEGRATION_ROOT}/../../contrib/cmd/seccompagent/seccompagent" +FS_IDMAP="${INTEGRATION_ROOT}/../../contrib/cmd/fs-idmap/fs-idmap" # Some variables may not always be set. Set those to empty value, # if unset, to avoid "unbound variable" error. @@ -622,3 +623,42 @@ function requires_kernel() { skip "requires kernel $1" fi } + +function requires_idmap_fs() { + local fs + fs=$1 + + # We need to "|| true" it to avoid CI failure as this binary may return with + # something different than 0. + stderr=$($FS_IDMAP "$fs" 2>&1 >/dev/null || true) + + case $stderr in + *invalid\ argument) + skip "$fs underlying file system does not support ID map mounts" + ;; + *operation\ not\ permitted) + if uname -r | grep -q el9; then + # centos kernel 5.14.0-200 does not permit using ID map mounts due to a + # specific patch added to their sources: + # https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/131 + # + # There doesn't seem to be any technical reason behind + # it, none was provided in numerous examples, like: + # https://lore.kernel.org/lkml/20210213130042.828076-1-christian.brauner@ubuntu.com/T/#m3a9df31aa183e8797c70bc193040adfd601399ad + # https://lore.kernel.org/lkml/20210213130042.828076-1-christian.brauner@ubuntu.com/T/#m59cdad9630d5a279aeecd0c1f117115144bc15eb + # https://lore.kernel.org/lkml/m1r1ifzf8x.fsf@fess.ebiederm.org + # https://lore.kernel.org/lkml/20210510125147.tkgeurcindldiwxg@wittgenstein + # + # So, sadly we just need to skip this on centos. + # + # TODO Nonetheless, there are ongoing works to revert the patch + # deactivating ID map mounts: + # https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/2179/diffs?commit_id=06f4fe946394cb94d2cf274aa7f3091d8f8469dc + # Once this patch is merge, we should be able to remove the below skip + # if the revert is backported or if CI centos kernel is upgraded. + skip "sadly, centos kernel 5.14 does not permit using ID map mounts" + fi + ;; + esac + # If we have another error, the integration test will fail and report it. +} diff --git a/tests/integration/idmap.bats b/tests/integration/idmap.bats new file mode 100644 index 00000000000..375191bf705 --- /dev/null +++ b/tests/integration/idmap.bats @@ -0,0 +1,160 @@ +#!/usr/bin/env bats + +load helpers + +function setup() { + requires root + requires_kernel 5.12 + requires_idmap_fs /tmp + + setup_debian + + # Prepare source folders for bind mount + mkdir -p source-{1,2}/ + touch source-{1,2}/foo.txt + + # Use other owner for source-2 + chown 1:1 source-2/foo.txt + + mkdir -p rootfs/{proc,sys,tmp} + mkdir -p rootfs/tmp/mount-{1,2} + mkdir -p rootfs/mnt/bind-mount-{1,2} + + update_config ' .linux.namespaces += [{"type": "user"}] + | .linux.uidMappings += [{"hostID": 100000, "containerID": 0, "size": 65536}] + | .linux.gidMappings += [{"hostID": 100000, "containerID": 0, "size": 65536}] + | .process.args += ["-c", "stat -c =%u=%g= /tmp/mount-1/foo.txt"] + | .mounts += [ + { + "source": "source-1/", + "destination": "/tmp/mount-1", + "options": ["bind"], + "uidMappings": [ { + "containerID": 0, + "hostID": 100000, + "size": 65536 + } + ], + "gidMappings": [ { + "containerID": 0, + "hostID": 100000, + "size": 65536 + } + ] + } + ] ' +} + +function teardown() { + teardown_bundle +} + +@test "simple idmap mount" { + runc run test_debian + [ "$status" -eq 0 ] + [[ "$output" == *"=0=0="* ]] +} + +@test "write to an idmap mount" { + update_config ' .process.args = ["sh", "-c", "touch /tmp/mount-1/bar && stat -c =%u=%g= /tmp/mount-1/bar"]' + runc run test_debian + [ "$status" -eq 0 ] + [[ "$output" == *"=0=0="* ]] +} + +@test "idmap mount with propagation flag" { + update_config ' .process.args = ["sh", "-c", "findmnt -o PROPAGATION /tmp/mount-1"]' + # Add the shared option to the idmap mount + update_config ' .mounts |= map((select(.source == "source-1/") | .options += ["shared"]) // .)' + + runc run test_debian + [ "$status" -eq 0 ] + [[ "$output" == *"shared"* ]] +} + +@test "idmap mount with bind mount" { + update_config ' .mounts += [ + { + "source": "source-2/", + "destination": "/tmp/mount-2", + "options": ["bind"] + } + ] ' + + runc run test_debian + [ "$status" -eq 0 ] + [[ "$output" == *"=0=0="* ]] +} + +@test "two idmap mounts with two bind mounts" { + update_config ' .process.args = ["sh", "-c", "stat -c =%u=%g= /tmp/mount-1/foo.txt /tmp/mount-2/foo.txt"] + | .mounts += [ + { + "source": "source-1/", + "destination": "/mnt/bind-mount-1", + "options": ["bind"] + }, + { + "source": "source-2/", + "destination": "/mnt/bind-mount-2", + "options": ["bind"] + }, + { + "source": "source-2/", + "destination": "/tmp/mount-2", + "options": ["bind"], + "uidMappings": [ { + "containerID": 0, + "hostID": 100000, + "size": 65536 + } + ], + "gidMappings": [ { + "containerID": 0, + "hostID": 100000, + "size": 65536 + } + ] + } + ] ' + + runc run test_debian + [ "$status" -eq 0 ] + [[ "$output" == *"=0=0="* ]] + # source-2/ is owned by 1:1, so we expect this with the idmap mount too. + [[ "$output" == *"=1=1="* ]] +} + +@test "idmap mount without gidMappings fails" { + update_config ' .mounts |= map((select(.source == "source-1/") | del(.gidMappings) ) // .)' + + runc run test_debian + [ "$status" -eq 1 ] + [[ "${output}" == *"invalid mount"* ]] +} + +@test "idmap mount without uidMappings fails" { + update_config ' .mounts |= map((select(.source == "source-1/") | del(.uidMappings) ) // .)' + + runc run test_debian + [ "$status" -eq 1 ] + [[ "${output}" == *"invalid mount"* ]] +} + +@test "idmap mount without bind fails" { + update_config ' .mounts |= map((select(.source == "source-1/") | .options = [""]) // .)' + + runc run test_debian + [ "$status" -eq 1 ] + [[ "${output}" == *"invalid mount"* ]] +} + +@test "idmap mount with different mapping than userns fails" { + # Let's modify the containerID to be 1, instead of 0 as it is in the + # userns config. + update_config ' .mounts |= map((select(.source == "source-1/") | .uidMappings[0]["containerID"] = 1 ) // .)' + + runc run test_debian + [ "$status" -eq 1 ] + [[ "${output}" == *"invalid mount"* ]] +}