runsc: validate mount targets

PiperOrigin-RevId: 382845950

4 files changed, 233 insertions, 4 deletions

diff --git a/runsc/specutils/safemount_test/BUILD b/runsc/specutils/safemount_test/BUILD
new file mode 100644
index 000000000..c39c40492
--- /dev/null
+++ b/runsc/specutils/safemount_test/BUILD
@@ -0,0 +1,23 @@
+load("//tools:defs.bzl", "go_binary", "go_test")
+
+package(licenses = ["notice"])
+
+go_test(
+    name = "safemount_test",
+    size = "small",
+    srcs = ["safemount_test.go"],
+    data = [":safemount_runner"],
+    deps = [
+        "//pkg/test/testutil",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
+)
+
+go_binary(
+    name = "safemount_runner",
+    srcs = ["safemount_runner.go"],
+    deps = [
+        "//runsc/specutils",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
+)
diff --git a/runsc/specutils/safemount_test/safemount_runner.go b/runsc/specutils/safemount_test/safemount_runner.go
new file mode 100644
index 000000000..b23193033
--- /dev/null
+++ b/runsc/specutils/safemount_test/safemount_runner.go
@@ -0,0 +1,117 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// safemount_runner is used to test the SafeMount function. Because use of
+// unix.Mount requires privilege, tests must launch this process with
+// CLONE_NEWNS and CLONE_NEWUSER.
+package main
+
+import (
+	"errors"
+	"fmt"
+	"log"
+	"os"
+	"path/filepath"
+
+	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/runsc/specutils"
+)
+
+func main() {
+	// The test temporary directory is the first argument.
+	tempdir := os.Args[1]
+
+	tcs := []struct {
+		name     string
+		testfunc func() error
+	}{{
+		name: "unix.Mount to folder succeeds",
+		testfunc: func() error {
+			dir2Path := filepath.Join(tempdir, "subdir2")
+			if err := unix.Mount(filepath.Join(tempdir, "subdir"), dir2Path, "bind", unix.MS_BIND, ""); err != nil {
+				return fmt.Errorf("mount: %v", err)
+			}
+			return unix.Unmount(dir2Path, unix.MNT_DETACH)
+		},
+	}, {
+		// unix.Mount doesn't care whether the target is a symlink.
+		name: "unix.Mount to symlink succeeds",
+		testfunc: func() error {
+			symlinkPath := filepath.Join(tempdir, "symlink")
+			if err := unix.Mount(filepath.Join(tempdir, "subdir"), symlinkPath, "bind", unix.MS_BIND, ""); err != nil {
+				return fmt.Errorf("mount: %v", err)
+			}
+			return unix.Unmount(symlinkPath, unix.MNT_DETACH)
+		},
+	}, {
+		name: "SafeMount to folder succeeds",
+		testfunc: func() error {
+			dir2Path := filepath.Join(tempdir, "subdir2")
+			if err := specutils.SafeMount(filepath.Join(tempdir, "subdir"), dir2Path, "bind", unix.MS_BIND, "", "/proc"); err != nil {
+				return fmt.Errorf("SafeMount: %v", err)
+			}
+			return unix.Unmount(dir2Path, unix.MNT_DETACH)
+		},
+	}, {
+		name: "SafeMount to symlink fails",
+		testfunc: func() error {
+			err := specutils.SafeMount(filepath.Join(tempdir, "subdir"), filepath.Join(tempdir, "symlink"), "bind", unix.MS_BIND, "", "/proc")
+			if err == nil {
+				return fmt.Errorf("SafeMount didn't fail, but should have")
+			}
+			var symErr *specutils.ErrSymlinkMount
+			if !errors.As(err, &symErr) {
+				return fmt.Errorf("expected SafeMount to fail with ErrSymlinkMount, but got: %v", err)
+			}
+			return nil
+		},
+	}}
+
+	for _, tc := range tcs {
+		if err := runTest(tempdir, tc.testfunc); err != nil {
+			log.Fatalf("failed test %q: %v", tc.name, err)
+		}
+	}
+}
+
+// runTest runs testfunc with the following directory structure:
+// tempdir/
+//   subdir/
+//   subdir2/
+//   symlink --> ./subdir2
+func runTest(tempdir string, testfunc func() error) error {
+	// Create tempdir/subdir/.
+	dirPath := filepath.Join(tempdir, "subdir")
+	if err := os.Mkdir(dirPath, 0777); err != nil {
+		return fmt.Errorf("os.Mkdir(%s, 0777)", dirPath)
+	}
+	defer os.Remove(dirPath)
+
+	// Create tempdir/subdir2/.
+	dir2Path := filepath.Join(tempdir, "subdir2")
+	if err := os.Mkdir(dir2Path, 0777); err != nil {
+		return fmt.Errorf("os.Mkdir(%s, 0777)", dir2Path)
+	}
+	defer os.Remove(dir2Path)
+
+	// Create tempdir/symlink, which points to ./subdir2.
+	symlinkPath := filepath.Join(tempdir, "symlink")
+	if err := os.Symlink("./subdir2", symlinkPath); err != nil {
+		return fmt.Errorf("failed to create symlink %s: %v", symlinkPath, err)
+	}
+	defer os.Remove(symlinkPath)
+
+	// Run the actual test.
+	return testfunc()
+}
diff --git a/runsc/specutils/safemount_test/safemount_test.go b/runsc/specutils/safemount_test/safemount_test.go
new file mode 100644
index 000000000..8820978c4
--- /dev/null
+++ b/runsc/specutils/safemount_test/safemount_test.go
@@ -0,0 +1,53 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package safemount_test
+
+import (
+	"os"
+	"os/exec"
+	"syscall"
+	"testing"
+
+	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/test/testutil"
+)
+
+func TestSafeMount(t *testing.T) {
+	// We run the actual tests in another process, as we need CAP_SYS_ADMIN to
+	// call mount(2). The new process runs in its own user and mount namespaces.
+	runner, err := testutil.FindFile("runsc/specutils/safemount_test/safemount_runner")
+	if err != nil {
+		t.Fatalf("failed to find test runner binary: %v", err)
+	}
+	cmd := exec.Command(runner, t.TempDir())
+	cmd.SysProcAttr = &unix.SysProcAttr{
+		Cloneflags: unix.CLONE_NEWNS | unix.CLONE_NEWUSER,
+		UidMappings: []syscall.SysProcIDMap{
+			{ContainerID: 0, HostID: os.Getuid(), Size: 1},
+		},
+		GidMappings: []syscall.SysProcIDMap{
+			{ContainerID: 0, HostID: os.Getgid(), Size: 1},
+		},
+		GidMappingsEnableSetgroups: false,
+		Credential: &syscall.Credential{
+			Uid: 0,
+			Gid: 0,
+		},
+	}
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("failed running %s with error: %v\ntest output:\n%s", cmd, err, output)
+	}
+}
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index c228d6299..48a574373 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -217,7 +217,7 @@ func ReadMounts(f *os.File) ([]specs.Mount, error) {
 	}
 	var mounts []specs.Mount
 	if err := json.Unmarshal(bytes, &mounts); err != nil {
-		return nil, fmt.Errorf("error unmarshaling mounts: %v\n %s", err, string(bytes))
+		return nil, fmt.Errorf("error unmarshaling mounts: %v\nJSON bytes:\n%s", err, string(bytes))
 	}
 	return mounts, nil
 }
@@ -434,8 +434,10 @@ func DebugLogFile(logPattern, command, test string) (*os.File, error) {
 	return os.OpenFile(logPattern, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
 }
 
-// Mount creates the mount point and calls Mount with the given flags.
-func Mount(src, dst, typ string, flags uint32) error {
+// Mount creates the mount point and calls Mount with the given flags. procPath
+// is the path to procfs. If it is "", procfs is assumed to be mounted at
+// /proc.
+func Mount(src, dst, typ string, flags uint32, procPath string) error {
 	// Create the mount point inside. The type must be the same as the
 	// source (file or directory).
 	var isDir bool
@@ -468,12 +470,46 @@ func Mount(src, dst, typ string, flags uint32) error {
 	}
 
 	// Do the mount.
-	if err := unix.Mount(src, dst, typ, uintptr(flags), ""); err != nil {
+	if err := SafeMount(src, dst, typ, uintptr(flags), "", procPath); err != nil {
 		return fmt.Errorf("mount(%q, %q, %d) failed: %v", src, dst, flags, err)
 	}
 	return nil
 }
 
+// ErrSymlinkMount is returned by SafeMount when the mount destination is found
+// to be a symlink.
+type ErrSymlinkMount struct {
+	error
+}
+
+// SafeMount is like unix.Mount, but will fail if dst is a symlink. procPath is
+// the path to procfs. If it is "", procfs is assumed to be mounted at /proc.
+func SafeMount(src, dst, fstype string, flags uintptr, data, procPath string) error {
+	// Open the destination.
+	fd, err := unix.Open(dst, unix.O_PATH|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return fmt.Errorf("failed to safely mount: Open(%s, _, _): %w", dst, err)
+	}
+	defer unix.Close(fd)
+
+	// Use /proc/self/fd/ to verify that we opened the intended destination. This
+	// guards against dst being a symlink, in which case we could accidentally
+	// mount over the symlink's target.
+	if procPath == "" {
+		procPath = "/proc"
+	}
+	safePath := filepath.Join(procPath, "self/fd", strconv.Itoa(fd))
+	target, err := os.Readlink(safePath)
+	if err != nil {
+		return fmt.Errorf("failed to safely mount: Readlink(%s): %w", safePath, err)
+	}
+	if dst != target {
+		return &ErrSymlinkMount{fmt.Errorf("failed to safely mount: expected to open %s, but found %s", dst, target)}
+	}
+
+	return unix.Mount(src, safePath, fstype, flags, data)
+}
+
 // ContainsStr returns true if 'str' is inside 'strs'.
 func ContainsStr(strs []string, str string) bool {
 	for _, s := range strs {