libcontainer: Add unit tests with userns and mounts

Add a unit test to check that bind mounts that have a part of its
path non accessible by others still work when using user namespaces.

To do this, we also modify newRoot() to return rootfs directories that
can be traverse by others, so the rootfs created works for all test
(either running in a userns or not).

Signed-off-by: Mauricio Vásquez <[email protected]>
Signed-off-by: Rodrigo Campos <[email protected]>
Co-authored-by: Rodrigo Campos <[email protected]>

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)
+}

libcontainer/integration/utils_test.go

@@ -105,9 +105,61 @@ 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 the
+// path below "tPath/../../" and errors if elements in the path above it don't
+// already have read+exec permissions.
+// tPath MUST be a path created with t.TempDir().
+func traversePath(tPath string) error {
+	// We need to change permissions below tPath/../../.
+	// t.TempDir() returns <base_dir>/a/b, where dir a is created with
+	// permissions only to the owner. So, to make sure others can traverse
+	// the path, we need to start changing from a inclusive.
+	// We don't change anything above tPath/../../, as that wasn't created
+	// by t.TempDir(), it might be a system dir like /tmp. We don't change them.
+	tempBaseSlice := strings.Split(tPath, "/")
+	tempBase := strings.Join(tempBaseSlice[:len(tempBaseSlice)-2], "/")
+
+	var path string
+	for _, p := range strings.SplitAfter(tPath, "/") {
+		path = path + p
+		stats, err := os.Stat(path)
+		if err != nil {
+			return err
+		}
+
+		if strings.HasPrefix(tempBase, path) {
+			// We can't safely change permissions if it is not below tempBase.
+			if stats.Mode()&0o5 == 0 {
+				return fmt.Errorf("directory %q MUST have read+exec permissions for others", path)
+			}
+
+			// Don't do anything when path is not below tempBase.
+			continue
+		}
+
+		if stats.Mode()&0o5 != 0 {
+			continue
+		}
+
+		perm := stats.Mode() | 0o5
+		if err := os.Chmod(path, perm); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func remove(dir string) {
 	_ = os.RemoveAll(dir)
 }