Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE
docker-runc.11352
CVE-2019-16884.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2019-16884.patch of Package docker-runc.11352
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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
