From 8542322dfe80b92c94291fd7427a9ca307017403 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= <mauricio@kinvolk.io>
Date: Tue, 15 Dec 2020 10:39:27 -0500
Subject: [PATCH] libcontainer: Add unit tests with userns and mounts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <mauricio@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
---
 libcontainer/integration/exec_test.go  | 64 ++++++++++++++++++++++++++
 libcontainer/integration/utils_test.go | 43 +++++++++++++++++
 2 files changed, 107 insertions(+)

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