summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--runsc/cmd/chroot.go8
-rw-r--r--runsc/cmd/gofer.go26
-rw-r--r--runsc/specutils/safemount_test/BUILD23
-rw-r--r--runsc/specutils/safemount_test/safemount_runner.go117
-rw-r--r--runsc/specutils/safemount_test/safemount_test.go53
-rw-r--r--runsc/specutils/specutils.go44
6 files changed, 252 insertions, 19 deletions
diff --git a/runsc/cmd/chroot.go b/runsc/cmd/chroot.go
index e988247da..791a50135 100644
--- a/runsc/cmd/chroot.go
+++ b/runsc/cmd/chroot.go
@@ -30,7 +30,7 @@ func mountInChroot(chroot, src, dst, typ string, flags uint32) error {
chrootDst := filepath.Join(chroot, dst)
log.Infof("Mounting %q at %q", src, chrootDst)
- if err := specutils.Mount(src, chrootDst, typ, flags); err != nil {
+ if err := specutils.Mount(src, chrootDst, typ, flags, "/proc"); err != nil {
return fmt.Errorf("error mounting %q at %q: %v", src, chrootDst, err)
}
return nil
@@ -70,11 +70,11 @@ func setUpChroot(pidns bool) error {
// Convert all shared mounts into slave to be sure that nothing will be
// propagated outside of our namespace.
- if err := unix.Mount("", "/", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil {
+ if err := specutils.SafeMount("", "/", "", unix.MS_SLAVE|unix.MS_REC, "", "/proc"); err != nil {
return fmt.Errorf("error converting mounts: %v", err)
}
- if err := unix.Mount("runsc-root", chroot, "tmpfs", unix.MS_NOSUID|unix.MS_NODEV|unix.MS_NOEXEC, ""); err != nil {
+ if err := specutils.SafeMount("runsc-root", chroot, "tmpfs", unix.MS_NOSUID|unix.MS_NODEV|unix.MS_NOEXEC, "", "/proc"); err != nil {
return fmt.Errorf("error mounting tmpfs in choot: %v", err)
}
@@ -89,7 +89,7 @@ func setUpChroot(pidns bool) error {
}
}
- if err := unix.Mount("", chroot, "", unix.MS_REMOUNT|unix.MS_RDONLY|unix.MS_BIND, ""); err != nil {
+ if err := specutils.SafeMount("", chroot, "", unix.MS_REMOUNT|unix.MS_RDONLY|unix.MS_BIND, "", "/proc"); err != nil {
return fmt.Errorf("error remounting chroot in read-only: %v", err)
}
diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go
index 80da9c9a2..570df407c 100644
--- a/runsc/cmd/gofer.go
+++ b/runsc/cmd/gofer.go
@@ -265,7 +265,8 @@ func isReadonlyMount(opts []string) bool {
func setupRootFS(spec *specs.Spec, conf *config.Config) error {
// Convert all shared mounts into slaves to be sure that nothing will be
// propagated outside of our namespace.
- if err := unix.Mount("", "/", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil {
+ procPath := "/proc"
+ if err := specutils.SafeMount("", "/", "", unix.MS_SLAVE|unix.MS_REC, "", procPath); err != nil {
Fatalf("error converting mounts: %v", err)
}
@@ -278,21 +279,24 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
// We need a directory to construct a new root and we know that
// runsc can't start without /proc, so we can use it for this.
flags := uintptr(unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC)
- if err := unix.Mount("runsc-root", "/proc", "tmpfs", flags, ""); err != nil {
+ if err := specutils.SafeMount("runsc-root", "/proc", "tmpfs", flags, "", procPath); err != nil {
Fatalf("error mounting tmpfs: %v", err)
}
// Prepare tree structure for pivot_root(2).
os.Mkdir("/proc/proc", 0755)
os.Mkdir("/proc/root", 0755)
+ // This cannot use SafeMount because there's no available procfs. But we
+ // know that /proc is an empty tmpfs mount, so this is safe.
if err := unix.Mount("runsc-proc", "/proc/proc", "proc", flags|unix.MS_RDONLY, ""); err != nil {
Fatalf("error mounting proc: %v", err)
}
root = "/proc/root"
+ procPath = "/proc/proc"
}
// Mount root path followed by submounts.
- if err := unix.Mount(spec.Root.Path, root, "bind", unix.MS_BIND|unix.MS_REC, ""); err != nil {
+ if err := specutils.SafeMount(spec.Root.Path, root, "bind", unix.MS_BIND|unix.MS_REC, "", procPath); err != nil {
return fmt.Errorf("mounting root on root (%q) err: %v", root, err)
}
@@ -300,12 +304,12 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
if spec.Linux != nil && spec.Linux.RootfsPropagation != "" {
flags = specutils.PropOptionsToFlags([]string{spec.Linux.RootfsPropagation})
}
- if err := unix.Mount("", root, "", uintptr(flags), ""); err != nil {
+ if err := specutils.SafeMount("", root, "", uintptr(flags), "", procPath); err != nil {
return fmt.Errorf("mounting root (%q) with flags: %#x, err: %v", root, flags, err)
}
// Replace the current spec, with the clean spec with symlinks resolved.
- if err := setupMounts(conf, spec.Mounts, root); err != nil {
+ if err := setupMounts(conf, spec.Mounts, root, procPath); err != nil {
Fatalf("error setting up FS: %v", err)
}
@@ -327,7 +331,7 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
// to make it read-only for extra safety.
log.Infof("Remounting root as readonly: %q", root)
flags := uintptr(unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY | unix.MS_REC)
- if err := unix.Mount(root, root, "bind", flags, ""); err != nil {
+ if err := specutils.SafeMount(root, root, "bind", flags, "", procPath); err != nil {
return fmt.Errorf("remounting root as read-only with source: %q, target: %q, flags: %#x, err: %v", root, root, flags, err)
}
}
@@ -343,10 +347,10 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
return nil
}
-// setupMounts binds mount all mounts specified in the spec in their correct
+// setupMounts bind mounts all mounts specified in the spec in their correct
// location inside root. It will resolve relative paths and symlinks. It also
// creates directories as needed.
-func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
+func setupMounts(conf *config.Config, mounts []specs.Mount, root, procPath string) error {
for _, m := range mounts {
if !specutils.Is9PMount(m, conf.VFS2) {
continue
@@ -364,14 +368,14 @@ func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
}
log.Infof("Mounting src: %q, dst: %q, flags: %#x", m.Source, dst, flags)
- if err := specutils.Mount(m.Source, dst, m.Type, flags); err != nil {
- return fmt.Errorf("mounting %v: %v", m, err)
+ if err := specutils.Mount(m.Source, dst, m.Type, flags, procPath); err != nil {
+ return fmt.Errorf("mounting %+v: %v", m, err)
}
// Set propagation options that cannot be set together with other options.
flags = specutils.PropOptionsToFlags(m.Options)
if flags != 0 {
- if err := unix.Mount("", dst, "", uintptr(flags), ""); err != nil {
+ if err := specutils.SafeMount("", dst, "", uintptr(flags), "", procPath); err != nil {
return fmt.Errorf("mount dst: %q, flags: %#x, err: %v", dst, flags, err)
}
}
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 {
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-02 18:19:28 +0000