Accepting request 733753 from home:cyphar:containers:maint

Add /proc/self/fd protections to CVE-2019-16884.patch.

OBS-URL: https://build.opensuse.org/request/show/733753
OBS-URL: https://build.opensuse.org/package/show/Virtualization:containers/runc?expand=0&rev=78
This commit is contained in:
Aleksa Sarai 2019-09-27 20:18:17 +00:00 committed by Git OBS Bridge
parent c0cf07af42
commit 53bd0f1302

View File

@ -1,5 +1,5 @@
From 6ce3791ce27128f8b4ae45323effa10953fa9904 Mon Sep 17 00:00:00 2001 From 74e43887d1e124b78c6e29876cff65423b8a999a Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <asarai@suse.de> From: Michael Crosby <crosbymichael@gmail.com>
Date: Mon, 23 Sep 2019 16:45:45 -0400 Date: Mon, 23 Sep 2019 16:45:45 -0400
Subject: [PATCH] CVE-2019-16884 Subject: [PATCH] CVE-2019-16884
@ -9,21 +9,24 @@ This patch includes a squash of the following upstream patches:
As well as the following still-in-review patches: As well as the following still-in-review patches:
* ("apparmor: verify that writes to /proc/... are on procfs") * opencontainers/runc#2130:
* ("selinux: verify that writes to /proc/... are on procfs") ("*: verify that writes to /proc/... are on procfs")
* opencontainers/selinux#59:
("selinux: verify that writes to /proc/... are on procfs")
Signed-off-by: Michael Crosby <crosbymichael@gmail.com> Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de> Signed-off-by: Aleksa Sarai <asarai@suse.de>
--- ---
libcontainer/apparmor/apparmor.go | 18 ++++++- libcontainer/apparmor/apparmor.go | 12 ++++-
libcontainer/container_linux.go | 4 +- libcontainer/container_linux.go | 4 +-
libcontainer/rootfs_linux.go | 50 ++++++++++++++----- libcontainer/rootfs_linux.go | 50 ++++++++++++++-----
libcontainer/rootfs_linux_test.go | 8 +-- libcontainer/rootfs_linux_test.go | 8 +--
libcontainer/utils/utils_unix.go | 41 +++++++++++----
.../selinux/go-selinux/selinux_linux.go | 20 ++++++++ .../selinux/go-selinux/selinux_linux.go | 20 ++++++++
5 files changed, 79 insertions(+), 21 deletions(-) 6 files changed, 104 insertions(+), 31 deletions(-)
diff --git a/libcontainer/apparmor/apparmor.go b/libcontainer/apparmor/apparmor.go diff --git a/libcontainer/apparmor/apparmor.go b/libcontainer/apparmor/apparmor.go
index 7fff0627fa1b..3504b80d8643 100644 index 7fff0627fa1b..a482269141b6 100644
--- a/libcontainer/apparmor/apparmor.go --- a/libcontainer/apparmor/apparmor.go
+++ b/libcontainer/apparmor/apparmor.go +++ b/libcontainer/apparmor/apparmor.go
@@ -6,6 +6,8 @@ import ( @@ -6,6 +6,8 @@ import (
@ -31,39 +34,33 @@ index 7fff0627fa1b..3504b80d8643 100644
"io/ioutil" "io/ioutil"
"os" "os"
+ +
+ "golang.org/x/sys/unix" + "github.com/opencontainers/runc/libcontainer/utils"
) )
// IsEnabled returns true if apparmor is enabled for the host. // IsEnabled returns true if apparmor is enabled for the host.
@@ -19,7 +21,13 @@ func IsEnabled() bool { @@ -19,7 +21,7 @@ func IsEnabled() bool {
return false return false
} }
-func setprocattr(attr, value string) error { -func setprocattr(attr, value string) error {
+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 setProcAttr(attr, value string) error { +func setProcAttr(attr, value string) error {
// Under AppArmor you can only change your own attr, so use /proc/self/ // Under AppArmor you can only change your own attr, so use /proc/self/
// instead of /proc/<tid>/ like libapparmor does // instead of /proc/<tid>/ like libapparmor does
path := fmt.Sprintf("/proc/self/attr/%s", attr) path := fmt.Sprintf("/proc/self/attr/%s", attr)
@@ -30,6 +38,12 @@ func setprocattr(attr, value string) error { @@ -30,6 +32,12 @@ func setprocattr(attr, value string) error {
} }
defer f.Close() defer f.Close()
+ if ok, err := isProcHandle(f); err != nil { + if ok, err := utils.IsProcHandle(f); err != nil {
+ return err + return err
+ } else if !ok { + } else if !ok {
+ return fmt.Errorf("/proc path not on procfs: %s", path) + return fmt.Errorf("%s not on procfs", path)
+ } + }
+ +
_, err = fmt.Fprintf(f, "%s", value) _, err = fmt.Fprintf(f, "%s", value)
return err return err
} }
@@ -37,7 +51,7 @@ func setprocattr(attr, value string) error { @@ -37,7 +45,7 @@ func setprocattr(attr, value string) error {
// changeOnExec reimplements aa_change_onexec from libapparmor in Go // changeOnExec reimplements aa_change_onexec from libapparmor in Go
func changeOnExec(name string) error { func changeOnExec(name string) error {
value := "exec " + name value := "exec " + name
@ -229,8 +226,77 @@ index d755984bc0f9..1bfe7c663225 100644
if err != nil { if err != nil {
t.Fatal(err) 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 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..611df8f9b3b0 100644 index d7786c33c197..04e94176daa0 100644
--- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go
+++ b/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 ( @@ -18,6 +18,8 @@ import (
@ -262,7 +328,7 @@ index d7786c33c197..611df8f9b3b0 100644
+ if ok, err := isProcHandle(in); err != nil { + if ok, err := isProcHandle(in); err != nil {
+ return "", err + return "", err
+ } else if !ok { + } else if !ok {
+ return "", fmt.Errorf("/proc path not on procfs: %s", fpath) + return "", fmt.Errorf("%s not on procfs", fpath)
+ } + }
+ +
var retval string var retval string
@ -275,7 +341,7 @@ index d7786c33c197..611df8f9b3b0 100644
+ if ok, err := isProcHandle(out); err != nil { + if ok, err := isProcHandle(out); err != nil {
+ return err + return err
+ } else if !ok { + } else if !ok {
+ return fmt.Errorf("/proc path not on procfs: %s", fpath) + return fmt.Errorf("%s not on procfs", fpath)
+ } + }
+ +
if val != "" { if val != "" {