From f8207a823351055a2aaad633b428fe7c1f0585f0 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Thu, 8 Jul 2021 17:53:49 -0700 Subject: clarify safemount behavior PiperOrigin-RevId: 383750666 --- runsc/cmd/chroot.go | 2 +- runsc/cmd/gofer.go | 2 +- runsc/config/config.go | 6 ++++-- runsc/specutils/specutils.go | 16 ++++++++++------ 4 files changed, 16 insertions(+), 10 deletions(-) (limited to 'runsc') diff --git a/runsc/cmd/chroot.go b/runsc/cmd/chroot.go index 791a50135..7b11b3367 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, "/proc"); err != nil { + if err := specutils.SafeSetupAndMount(src, chrootDst, typ, flags, "/proc"); err != nil { return fmt.Errorf("error mounting %q at %q: %v", src, chrootDst, err) } return nil diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index 570df407c..f5eabce74 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -368,7 +368,7 @@ func setupMounts(conf *config.Config, mounts []specs.Mount, root, procPath strin } log.Infof("Mounting src: %q, dst: %q, flags: %#x", m.Source, dst, flags) - if err := specutils.Mount(m.Source, dst, m.Type, flags, procPath); err != nil { + if err := specutils.SafeSetupAndMount(m.Source, dst, m.Type, flags, procPath); err != nil { return fmt.Errorf("mounting %+v: %v", m, err) } diff --git a/runsc/config/config.go b/runsc/config/config.go index 3d8c7a0ab..cc4650180 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -142,7 +142,8 @@ type Config struct { // Rootless allows the sandbox to be started with a user that is not root. // Defense in depth measures are weaker in rootless mode. Specifically, the // sandbox and Gofer process run as root inside a user namespace with root - // mapped to the caller's user. + // mapped to the caller's user. When using rootless, the container root path + // should not have a symlink. Rootless bool `flag:"rootless"` // AlsoLogToStderr allows to send log messages to stderr. @@ -175,7 +176,8 @@ type Config struct { // TestOnlyAllowRunAsCurrentUserWithoutChroot should only be used in // tests. It allows runsc to start the sandbox process as the current // user, and without chrooting the sandbox process. This can be - // necessary in test environments that have limited capabilities. + // necessary in test environments that have limited capabilities. When + // disabling chroot, the container root path should not have a symlink. TestOnlyAllowRunAsCurrentUserWithoutChroot bool `flag:"TESTONLY-unsafe-nonroot"` // TestOnlyTestNameEnv should only be used in tests. It looks up for the diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 48a574373..5365b5b1b 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -434,12 +434,12 @@ 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. 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). +// SafeSetupAndMount 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 SafeSetupAndMount(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 if typ == "proc" { // Special case, as there is no source directory for proc mounts. @@ -484,6 +484,10 @@ type ErrSymlinkMount struct { // 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. +// +// SafeMount can fail when dst contains a symlink. However, it is called in the +// normal case with a destination consisting of a known root (/proc/root) and +// symlink-free path (from resolveSymlink). 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) -- cgit v1.2.3