diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 28f2e10d2..faf88984f 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -3,6 +3,7 @@ package integration import ( "bytes" "encoding/json" + "errors" "fmt" "io/ioutil" "os" @@ -1838,3 +1839,66 @@ next_fd: t.Fatalf("found %d extra fds after container.Run", count) } } + +// Test that a container using user namespaces is able to bind mount a folder +// that does not have permissions for group/others. +func TestBindMountAndUser(t *testing.T) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { + t.Skip("userns is unsupported") + } + + if testing.Short() { + return + } + + temphost := t.TempDir() + dirhost := filepath.Join(temphost, "inaccessible", "dir") + + err := os.MkdirAll(dirhost, 0o755) + ok(t, err) + + err = ioutil.WriteFile(filepath.Join(dirhost, "foo.txt"), []byte("Hello"), 0o755) + ok(t, err) + + // Make this dir inaccessible to "group,others". + err = os.Chmod(filepath.Join(temphost, "inaccessible"), 0o700) + ok(t, err) + + config := newTemplateConfig(t, &tParam{ + userns: true, + }) + + // Set HostID to 1000 to avoid DAC_OVERRIDE bypassing the purpose of this test. + config.UidMappings[0].HostID = 1000 + config.GidMappings[0].HostID = 1000 + + // Set the owner of rootfs to the effective IDs in the host to avoid errors + // while creating the folders to perform the mounts. + err = os.Chown(config.Rootfs, 1000, 1000) + ok(t, err) + + config.Mounts = append(config.Mounts, &configs.Mount{ + Source: dirhost, + Destination: "/tmp/mnt1cont", + Device: "bind", + Flags: unix.MS_BIND | unix.MS_REC, + }) + + container, err := newContainer(t, config) + ok(t, err) + defer container.Destroy() //nolint: errcheck + + var stdout bytes.Buffer + + pconfig := libcontainer.Process{ + Cwd: "/", + Args: []string{"sh", "-c", "stat /tmp/mnt1cont/foo.txt"}, + Env: standardEnvironment, + Stdout: &stdout, + Init: true, + } + err = container.Run(&pconfig) + ok(t, err) + + waitProcess(&pconfig, t) +} diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go index 63eb9bc39..f98f3e5b5 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -105,9 +105,52 @@ func newRootfs(t *testing.T) string { if err := copyBusybox(dir); err != nil { t.Fatal(err) } + + // Make sure others can read+exec, so all tests (inside userns too) can + // read the rootfs. + if err := traversePath(dir); err != nil { + t.Fatalf("Error making newRootfs path traversable by others: %v", err) + } + return dir } +// traversePath gives read+execute permissions to others for all elements in tPath below +// os.TempDir() and errors out if elements above it don't have read+exec permissions for others. +// tPath MUST be a descendant of os.TempDir(). The path returned by testing.TempDir() usually is. +func traversePath(tPath string) error { + // Check the assumption that the argument is under os.TempDir(). + tempBase := os.TempDir() + if !strings.HasPrefix(tPath, tempBase) { + return fmt.Errorf("traversePath: %q is not a descendant of %q", tPath, tempBase) + } + + var path string + for _, p := range strings.SplitAfter(tPath, "/") { + path = path + p + stats, err := os.Stat(path) + if err != nil { + return err + } + + perm := stats.Mode().Perm() + + if perm&0o5 == 0o5 { + continue + } + + if strings.HasPrefix(tempBase, path) { + return fmt.Errorf("traversePath: directory %q MUST have read+exec permissions for others", path) + } + + if err := os.Chmod(path, perm|0o5); err != nil { + return err + } + } + + return nil +} + func remove(dir string) { _ = os.RemoveAll(dir) }