File CVE-2019-16884.patch of Package docker-runc.12716

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2019-16884.patch of Package docker-runc.12716

From 74e43887d1e124b78c6e29876cff65423b8a999a Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <asarai@suse.de>
Date: Mon, 23 Sep 2019 16:45:45 -0400
Subject: [PATCH] CVE-2019-16884

This patch includes a squash of the following upstream patches:

* 331692baa7af ("Only allow proc mount if it is procfs")

As well as the following still-in-review patches:

* opencontainers/runc#2130:
   ("*: verify that writes to /proc/... are on procfs")
 * opencontainers/selinux#59:
   ("selinux: verify that writes to /proc/... are on procfs")

SUSE-Bugs: bsc#1152308
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
---
 libcontainer/apparmor/apparmor.go             | 12 ++++-
 libcontainer/container_linux.go               |  4 +-
 libcontainer/rootfs_linux.go                  | 50 ++++++++++++++-----
 libcontainer/rootfs_linux_test.go             |  8 +--
 libcontainer/utils/utils_unix.go              | 41 +++++++++++----
 .../selinux/go-selinux/selinux_linux.go       | 20 ++++++++
 6 files changed, 104 insertions(+), 31 deletions(-)

diff --git a/libcontainer/apparmor/apparmor.go b/libcontainer/apparmor/apparmor.go
index 7fff0627fa1b..a482269141b6 100644
--- a/libcontainer/apparmor/apparmor.go
+++ b/libcontainer/apparmor/apparmor.go
@@ -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,6 +32,12 @@ func setprocattr(attr, value string) error {
 	}
 	defer f.Close()
 
+	if ok, err := utils.IsProcHandle(f); err != nil {
+		return err
+	} else if !ok {
+		return fmt.Errorf("%s not on procfs", path)
+	}
+
 	_, err = fmt.Fprintf(f, "%s", value)
 	return err
 }
@@ -37,7 +45,7 @@ func setprocattr(attr, value string) error {
 // 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
diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index 7e58e5e00824..d51e35dffb93 100644
--- a/libcontainer/container_linux.go
+++ b/libcontainer/container_linux.go
@@ -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"
@@ -1160,7 +1160,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, ""); err != nil {
 			return err
 		}
 		m.Destination = dest
diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go
index f13b226e444e..5650b0acbca8 100644
--- a/libcontainer/rootfs_linux.go
+++ b/libcontainer/rootfs_linux.go
@@ -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.
@@ -388,7 +388,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.
@@ -435,12 +435,12 @@ 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",
-	}
+//
+// if source is nil, don't stat the filesystem.  This is used for restore of a checkpoint.
+func checkProcMount(rootfs, dest, source string) error {
+	const procPath = "/proc"
 	// White list, it should be sub directories of invalid destinations
 	validDestinations := []string{
 		// These entries can be bind mounted by files emulated by fuse,
@@ -463,16 +463,40 @@ func checkMountDestination(rootfs, dest string) error {
 			return nil
 		}
 	}
-	for _, invalid := range invalidDestinations {
-		path, err := filepath.Rel(filepath.Join(rootfs, invalid), dest)
+	path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
+	if err != nil {
+		return err
+	}
+	// pass if the mount path is located outside of /proc
+	if strings.HasPrefix(path, "..") {
+		return nil
+	}
+	if path == "." {
+		// an empty source is pasted on restore
+		if source == "" {
+			return nil
+		}
+		// only allow a mount on-top of proc if it's source is "proc"
+		isproc, err := isProc(source)
 		if err != nil {
 			return err
 		}
-		if path != "." && !strings.HasPrefix(path, "..") {
-			return fmt.Errorf("%q cannot be mounted because it is located inside %q", dest, invalid)
+		// pass if the mount is happening on top of /proc and the source of
+		// the mount is a proc filesystem
+		if isproc {
+			return nil
 		}
+		return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
 	}
-	return nil
+	return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
+}
+
+func isProc(path string) (bool, error) {
+	var s unix.Statfs_t
+	if err := unix.Statfs(path, &s); err != nil {
+		return false, err
+	}
+	return s.Type == unix.PROC_SUPER_MAGIC, nil
 }
 
 func setupDevSymlinks(rootfs string) error {
diff --git a/libcontainer/rootfs_linux_test.go b/libcontainer/rootfs_linux_test.go
index d755984bc0f9..1bfe7c663225 100644
--- a/libcontainer/rootfs_linux_test.go
+++ b/libcontainer/rootfs_linux_test.go
@@ -10,7 +10,7 @@ import (
 
 func TestCheckMountDestOnProc(t *testing.T) {
 	dest := "/rootfs/proc/sys"
-	err := checkMountDestination("/rootfs", dest)
+	err := checkProcMount("/rootfs", dest, "")
 	if err == nil {
 		t.Fatal("destination inside proc should return an error")
 	}
@@ -18,7 +18,7 @@ func TestCheckMountDestOnProc(t *testing.T) {
 
 func TestCheckMountDestOnProcChroot(t *testing.T) {
 	dest := "/rootfs/proc/"
-	err := checkMountDestination("/rootfs", dest)
+	err := checkProcMount("/rootfs", dest, "/proc")
 	if err != nil {
 		t.Fatal("destination inside proc when using chroot should not return an error")
 	}
@@ -26,7 +26,7 @@ func TestCheckMountDestOnProcChroot(t *testing.T) {
 
 func TestCheckMountDestInSys(t *testing.T) {
 	dest := "/rootfs//sys/fs/cgroup"
-	err := checkMountDestination("/rootfs", dest)
+	err := checkProcMount("/rootfs", dest, "")
 	if err != nil {
 		t.Fatal("destination inside /sys should not return an error")
 	}
@@ -34,7 +34,7 @@ func TestCheckMountDestInSys(t *testing.T) {
 
 func TestCheckMountDestFalsePositive(t *testing.T) {
 	dest := "/rootfs/sysfiles/fs/cgroup"
-	err := checkMountDestination("/rootfs", dest)
+	err := checkProcMount("/rootfs", dest, "")
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go
index c96088988a6d..cac37c449c6a 100644
--- a/libcontainer/utils/utils_unix.go
+++ b/libcontainer/utils/utils_unix.go
@@ -3,33 +3,54 @@
 package utils
 
 import (
-	"io/ioutil"
+	"fmt"
 	"os"
 	"strconv"
 
 	"golang.org/x/sys/unix"
 )
 
+// IsProcHandle returns whether or not the given file handle is on procfs.
+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
+}
+
+// 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
 	}
-	for _, fi := range fdList {
-		fd, err := strconv.Atoi(fi.Name())
+	defer fdDir.Close()
+
+	if ok, err := IsProcHandle(fdDir); err != nil {
+		return err
+	} else if !ok {
+		return fmt.Errorf("/proc/self/fd not on procfs")
+	}
+
+	fdList, err := fdDir.Readdirnames(-1)
+	if err != nil {
+		return err
+	}
+	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
 }
diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go
index d7786c33c197..04e94176daa0 100644
--- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go
+++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go
@@ -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 {
-- 
2.23.0

Places

File CVE-2019-16884.patch of Package docker-runc.12716

Places