CVE-2019-16884

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+
+	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 // IsEnabled returns true if apparmor is enabled for the host.
@@ -19,7 +21,7 @@ func IsEnabled() bool {
 	return false
 }
 
-func setprocattr(attr, value string) error {
+func setProcAttr(attr, value string) error {
 	// Under AppArmor you can only change your own attr, so use /proc/self/
 	// instead of /proc/<tid>/ like libapparmor does
 	path := fmt.Sprintf("/proc/self/attr/%s", attr)
@@ -30,14 +32,18 @@ func setprocattr(attr, value string) error {
 	}
 	defer f.Close()
 
+	if err := utils.EnsureProcHandle(f); err != nil {
+		return err
+	}
+
 	_, err = fmt.Fprintf(f, "%s", value)
 	return err
 }
 
 // changeOnExec reimplements aa_change_onexec from libapparmor in Go
 func changeOnExec(name string) error {
 	value := "exec " + name
-	if err := setprocattr("exec", value); err != nil {
+	if err := setProcAttr("exec", value); err != nil {
 		return fmt.Errorf("apparmor failed to apply profile: %s", err)
 	}
 	return nil

@@ -3,33 +3,57 @@
 package utils
 
 import (
-	"io/ioutil"
+	"fmt"
 	"os"
 	"strconv"
 
 	"golang.org/x/sys/unix"
 )
 
+// EnsureProcHandle returns whether or not the given file handle is on procfs.
+func EnsureProcHandle(fh *os.File) error {
+	var buf unix.Statfs_t
+	if err := unix.Fstatfs(int(fh.Fd()), &buf); err != nil {
+		return fmt.Errorf("ensure %s is on procfs: %v", fh.Name(), err)
+	}
+	if buf.Type != unix.PROC_SUPER_MAGIC {
+		return fmt.Errorf("%s is not on procfs", fh.Name())
+	}
+	return nil
+}
+
+// CloseExecFrom applies O_CLOEXEC to all file descriptors currently open for
+// the process (except for those below the given fd value).
 func CloseExecFrom(minFd int) error {
-	fdList, err := ioutil.ReadDir("/proc/self/fd")
+	fdDir, err := os.Open("/proc/self/fd")
+	if err != nil {
+		return err
+	}
+	defer fdDir.Close()
+
+	if err := EnsureProcHandle(fdDir); err != nil {
+		return err
+	}
+
+	fdList, err := fdDir.Readdirnames(-1)
 	if err != nil {
 		return err
 	}
-	for _, fi := range fdList {
-		fd, err := strconv.Atoi(fi.Name())
+	for _, fdStr := range fdList {
+		fd, err := strconv.Atoi(fdStr)
+		// Ignore non-numeric file names.
 		if err != nil {
-			// ignore non-numeric file names
 			continue
 		}
-
+		// Ignore descriptors lower than our specified minimum.
 		if fd < minFd {
-			// ignore descriptors lower than our specified minimum
 			continue
 		}
-
-		// intentionally ignore errors from unix.CloseOnExec
+		// Intentionally ignore errors from unix.CloseOnExec -- the cases where
+		// this might fail are basically file descriptors that have already
+		// been closed (including and especially the one that was created when
+		// ioutil.ReadDir did the "opendir" syscall).
 		unix.CloseOnExec(fd)
-		// the cases where this might fail are basically file descriptors that have already been closed (including and especially the one that was created when ioutil.ReadDir did the "opendir" syscall)
 	}
 	return nil
 }

@@ -6,7 +6,7 @@ github.com/opencontainers/runtime-spec  29686dbc5559d93fb1ef402eeda3e35c38d75af4
 # Core libcontainer functionality.
 github.com/checkpoint-restore/go-criu   17b0214f6c48980c45dc47ecb0cfd6d9e02df723 # v3.11
 github.com/mrunalp/fileutils            7d4729fb36185a7c1719923406c9d40e54fb93c7
-github.com/opencontainers/selinux       3a1f366feb7aecbf7a0e71ac4cea88b31597de9e # v1.2.2
+github.com/opencontainers/selinux       5215b1806f52b1fcc2070a8826c542c9d33cd3cf # v1.3.0 (+ CVE-2019-16884)
 github.com/seccomp/libseccomp-golang    689e3c1541a84461afc49c1c87352a6cedf72e9c # v0.9.1
 github.com/sirupsen/logrus              8bdbc7bcc01dcbb8ec23dc8a28e332258d25251f # v1.4.1
 github.com/syndtr/gocapability          d98352740cb2c55f81556b63d4a1ec64c5a319c2

@@ -13,11 +13,12 @@ import (
 
 // Valid Label Options
 var validOptions = map[string]bool{
-	"disable": true,
-	"type":    true,
-	"user":    true,
-	"role":    true,
-	"level":   true,
+	"disable":  true,
+	"type":     true,
+	"filetype": true,
+	"user":     true,
+	"role":     true,
+	"level":    true,
 }
 
 var ErrIncompatibleLabel = fmt.Errorf("Bad SELinux option z and Z can not be used together")
@@ -51,13 +52,16 @@ func InitLabels(options []string) (plabel string, mlabel string, Err error) {
 				return "", mountLabel, nil
 			}
 			if i := strings.Index(opt, ":"); i == -1 {
-				return "", "", fmt.Errorf("Bad label option %q, valid options 'disable' or \n'user, role, level, type' followed by ':' and a value", opt)
+				return "", "", fmt.Errorf("Bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt)
 			}
 			con := strings.SplitN(opt, ":", 2)
 			if !validOptions[con[0]] {
-				return "", "", fmt.Errorf("Bad label option %q, valid options 'disable, user, role, level, type'", con[0])
+				return "", "", fmt.Errorf("Bad label option %q, valid options 'disable, user, role, level, type, filetype'", con[0])
 
 			}
+			if con[0] == "filetype" {
+				mcon["type"] = con[1]
+			}
 			pcon[con[0]] = con[1]
 			if con[0] == "level" || con[0] == "user" {
 				mcon[con[0]] = con[1]

@@ -18,6 +18,8 @@ import (
 	"strings"
 	"sync"
 	"syscall"
+
+	"golang.org/x/sys/unix"
 )
 
 const (
@@ -252,6 +254,12 @@ func getSELinuxPolicyRoot() string {
 	return filepath.Join(selinuxDir, readConfig(selinuxTypeTag))
 }
 
+func isProcHandle(fh *os.File) (bool, error) {
+	var buf unix.Statfs_t
+	err := unix.Fstatfs(int(fh.Fd()), &buf)
+	return buf.Type == unix.PROC_SUPER_MAGIC, err
+}
+
 func readCon(fpath string) (string, error) {
 	if fpath == "" {
 		return "", ErrEmptyPath
@@ -263,6 +271,12 @@ func readCon(fpath string) (string, error) {
 	}
 	defer in.Close()
 
+	if ok, err := isProcHandle(in); err != nil {
+		return "", err
+	} else if !ok {
+		return "", fmt.Errorf("%s not on procfs", fpath)
+	}
+
 	var retval string
 	if _, err := fmt.Fscanf(in, "%s", &retval); err != nil {
 		return "", err
@@ -345,6 +359,12 @@ func writeCon(fpath string, val string) error {
 	}
 	defer out.Close()
 
+	if ok, err := isProcHandle(out); err != nil {
+		return err
+	} else if !ok {
+		return fmt.Errorf("%s not on procfs", fpath)
+	}
+
 	if val != "" {
 		_, err = out.Write([]byte(val))
 	} else {
@@ -392,6 +412,14 @@ func SetExecLabel(label string) error {
 	return writeCon(fmt.Sprintf("/proc/self/task/%d/attr/exec", syscall.Gettid()), label)
 }
 
+/*
+SetTaskLabel sets the SELinux label for the current thread, or an error.
+This requires the dyntransition permission.
+*/
+func SetTaskLabel(label string) error {
+	return writeCon(fmt.Sprintf("/proc/self/task/%d/attr/current", syscall.Gettid()), label)
+}
+
 // SetSocketLabel takes a process label and tells the kernel to assign the
 // label to the next socket that gets created
 func SetSocketLabel(label string) error {
@@ -403,6 +431,11 @@ func SocketLabel() (string, error) {
 	return readCon(fmt.Sprintf("/proc/self/task/%d/attr/sockcreate", syscall.Gettid()))
 }
 
+// PeerLabel retrieves the label of the client on the other side of a socket
+func PeerLabel(fd uintptr) (string, error) {
+	return unix.GetsockoptString(int(fd), syscall.SOL_SOCKET, syscall.SO_PEERSEC)
+}
+
 // SetKeyLabel takes a process label and tells the kernel to assign the
 // label to the next kernel keyring that gets created
 func SetKeyLabel(label string) error {

@@ -96,6 +96,14 @@ func SetExecLabel(label string) error {
 	return nil
 }
 
+/*
+SetTaskLabel sets the SELinux label for the current thread, or an error.
+This requires the dyntransition permission.
+*/
+func SetTaskLabel(label string) error {
+        return nil
+}
+
 /*
 SetSocketLabel sets the SELinux label that the kernel will use for any programs
 that are executed by the current process thread, or an error.
@@ -109,6 +117,11 @@ func SocketLabel() (string, error) {
 	return "", nil
 }
 
+// PeerLabel retrieves the label of the client on the other side of a socket
+func PeerLabel(fd uintptr) (string, error) {
+	return "", nil
+}
+
 // SetKeyLabel takes a process label and tells the kernel to assign the
 // label to the next kernel keyring that gets created
 func SetKeyLabel(label string) error {

@@ -18,6 +18,7 @@ import (
 	"strings"
 	"sync"
 	"syscall"
+
 	"golang.org/x/sys/unix"
 )
 
@@ -253,6 +254,12 @@ func getSELinuxPolicyRoot() string {
 	return filepath.Join(selinuxDir, readConfig(selinuxTypeTag))
 }
 
+func isProcHandle(fh *os.File) (bool, error) {
+	var buf unix.Statfs_t
+	err := unix.Fstatfs(int(fh.Fd()), &buf)
+	return buf.Type == unix.PROC_SUPER_MAGIC, err
+}
+
 func readCon(fpath string) (string, error) {
 	if fpath == "" {
 		return "", ErrEmptyPath
@@ -264,6 +271,12 @@ func readCon(fpath string) (string, error) {
 	}
 	defer in.Close()
 
+	if ok, err := isProcHandle(in); err != nil {
+		return "", err
+	} else if !ok {
+		return "", fmt.Errorf("%s not on procfs", fpath)
+	}
+
 	var retval string
 	if _, err := fmt.Fscanf(in, "%s", &retval); err != nil {
 		return "", err
@@ -346,6 +359,12 @@ func writeCon(fpath string, val string) error {
 	}
 	defer out.Close()
 
+	if ok, err := isProcHandle(out); err != nil {
+		return err
+	} else if !ok {
+		return fmt.Errorf("%s not on procfs", fpath)
+	}
+
 	if val != "" {
 		_, err = out.Write([]byte(val))
 	} else {
@@ -394,7 +413,7 @@ func SetExecLabel(label string) error {
 }
 
 /*
-SetTaskLabel sets the SELinux label for the current thread, or an error. 
+SetTaskLabel sets the SELinux label for the current thread, or an error.
 This requires the dyntransition permission.
 */
 func SetTaskLabel(label string) error {

@@ -19,7 +19,7 @@ import (
 	"syscall" // only for SysProcAttr and Signal
 	"time"
 
-	"github.com/cyphar/filepath-securejoin"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/intelrdt"
@@ -1176,7 +1176,7 @@ func (c *linuxContainer) makeCriuRestoreMountpoints(m *configs.Mount) error {
 		if err != nil {
 			return err
 		}
-		if err := checkMountDestination(c.config.Rootfs, dest); err != nil {
+		if err := checkProcMount(c.config.Rootfs, dest, m.Source); err != nil {
 			return err
 		}
 		m.Destination = dest

@@ -1,5 +1,10 @@
 package mount
 
+import "errors"
+
+// ErrNotMounted is returned when a path is not mounted.
+var ErrNotMounted = errors.New("path not mounted")
+
 // GetMounts retrieves a list of mounts for the current running process.
 func GetMounts() ([]*Info, error) {
 	return parseMountTable()
@@ -21,3 +26,20 @@ func Mounted(mountpoint string) (bool, error) {
 	}
 	return false, nil
 }
+
+// FSType returns the file-system type of a mountpoint.
+//
+// ErrNotMounted is returned if the mountpoint refers to a path
+// that is not mounted.
+func FSType(mountpoint string) (string, error) {
+	entries, err := parseMountTable()
+	if err != nil {
+		return "", err
+	}
+	for _, e := range entries {
+		if e.Mountpoint == mountpoint {
+			return e.Fstype, nil
+		}
+	}
+	return "", ErrNotMounted
+}

@@ -13,7 +13,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/cyphar/filepath-securejoin"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/mrunalp/fileutils"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
@@ -197,7 +197,7 @@ func prepareBindMount(m *configs.Mount, rootfs string) error {
 	if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
 		return err
 	}
-	if err := checkMountDestination(rootfs, dest); err != nil {
+	if err := checkProcMount(rootfs, dest, m.Source); err != nil {
 		return err
 	}
 	// update the mount with the correct dest after symlinks are resolved.
@@ -414,7 +414,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string, enableCgroupns b
 		if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
 			return err
 		}
-		if err := checkMountDestination(rootfs, dest); err != nil {
+		if err := checkProcMount(rootfs, dest, m.Source); err != nil {
 			return err
 		}
 		// update the mount with the correct dest after symlinks are resolved.
@@ -461,12 +461,9 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
 	return binds, nil
 }
 
-// checkMountDestination checks to ensure that the mount destination is not over the top of /proc.
+// checkProcMount checks to ensure that the mount destination is not over the top of /proc.
 // dest is required to be an abs path and have any symlinks resolved before calling this function.
-func checkMountDestination(rootfs, dest string) error {
-	invalidDestinations := []string{
-		"/proc",
-	}
+func checkProcMount(rootfs, dest, source string) error {
 	// White list, it should be sub directories of invalid destinations
 	validDestinations := []string{
 		// These entries can be bind mounted by files emulated by fuse,
@@ -489,13 +486,23 @@ func checkMountDestination(rootfs, dest string) error {
 			return nil
 		}
 	}
-	for _, invalid := range invalidDestinations {
-		path, err := filepath.Rel(filepath.Join(rootfs, invalid), dest)
+	const procPath = "/proc"
+	path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
+	if err != nil {
+		return err
+	}
+	// check if the path is outside the rootfs
+	if path == "." || !strings.HasPrefix(path, "..") {
+		// only allow a mount on-top of proc if it's source is "procfs"
+		fstype, err := mount.FSType(source)
 		if err != nil {
+			if err == mount.ErrNotMounted {
+				return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
+			}
 			return err
 		}
-		if path != "." && !strings.HasPrefix(path, "..") {
-			return fmt.Errorf("%q cannot be mounted because it is located inside %q", dest, invalid)
+		if fstype != "proc" {
+			return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
 		}
 	}
 	return nil