[meta-virtualization] [PATCH] runc: fix CVE-2019-16884
ChenQi
Qi.Chen at windriver.com
Tue Nov 5 18:46:12 PST 2019
On 11/06/2019 10:27 AM, Bruce Ashfield wrote:
> On Wed, Oct 30, 2019 at 4:27 AM Chen Qi <Qi.Chen at windriver.com> wrote:
>> Signed-off-by: Chen Qi <Qi.Chen at windriver.com>
>> ---
>> ...nly-allow-proc-mount-if-it-is-procfs.patch | 201 ++++++++++++++++++
>> recipes-containers/runc/runc-docker_git.bb | 1 +
>> 2 files changed, 202 insertions(+)
>> create mode 100644 recipes-containers/runc/files/0001-Only-allow-proc-mount-if-it-is-procfs.patch
>>
>> diff --git a/recipes-containers/runc/files/0001-Only-allow-proc-mount-if-it-is-procfs.patch b/recipes-containers/runc/files/0001-Only-allow-proc-mount-if-it-is-procfs.patch
>> new file mode 100644
>> index 0000000..5aca99e
>> --- /dev/null
>> +++ b/recipes-containers/runc/files/0001-Only-allow-proc-mount-if-it-is-procfs.patch
>> @@ -0,0 +1,201 @@
>> +From d75b05441772417a0828465a9483f16287937724 Mon Sep 17 00:00:00 2001
>> +From: Michael Crosby <crosbymichael at gmail.com>
>> +Date: Mon, 23 Sep 2019 16:45:45 -0400
>> +Subject: [PATCH] Only allow proc mount if it is procfs
>> +
>> +Fixes #2128
>> +
>> +This allows proc to be bind mounted for host and rootless namespace usecases but
>> +it removes the ability to mount over the top of proc with a directory.
>> +
>> +```bash
>> +> sudo docker run --rm apparmor
>> +docker: Error response from daemon: OCI runtime create failed:
>> +container_linux.go:346: starting container process caused "process_linux.go:449:
>> +container init caused \"rootfs_linux.go:58: mounting
>> +\\\"/var/lib/docker/volumes/aae28ea068c33d60e64d1a75916cf3ec2dc3634f97571854c9ed30c8401460c1/_data\\\"
>> +to rootfs
>> +\\\"/var/lib/docker/overlay2/a6be5ae911bf19f8eecb23a295dec85be9a8ee8da66e9fb55b47c841d1e381b7/merged\\\"
>> +at \\\"/proc\\\" caused
>> +\\\"\\\\\\\"/var/lib/docker/overlay2/a6be5ae911bf19f8eecb23a295dec85be9a8ee8da66e9fb55b47c841d1e381b7/merged/proc\\\\\\\"
>> +cannot be mounted because it is not of type proc\\\"\"": unknown.
>> +
>> +> sudo docker run --rm -v /proc:/proc apparmor
>> +
>> +docker-default (enforce) root 18989 0.9 0.0 1288 4 ?
>> +Ss 16:47 0:00 sleep 20
>> +```
>> +
>> +Signed-off-by: Michael Crosby <crosbymichael at gmail.com>
>> +
>> +Upstream-Status: Backport [https://github.com/opencontainers/runc/pull/2129/commits/331692baa7afdf6c186f8667cb0e6362ea0802b3]
>> +
>> +CVE: CVE-2019-16884
>> +
>> +Signed-off-by: Chen Qi <Qi.Chen at windriver.com>
>> +---
>> + libcontainer/container_linux.go | 4 +--
>> + libcontainer/rootfs_linux.go | 50 +++++++++++++++++++++++--------
>> + libcontainer/rootfs_linux_test.go | 8 ++---
>> + 3 files changed, 43 insertions(+), 19 deletions(-)
>> +
>> +diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
>> +index 7e58e5e0..d51e35df 100644
>> +--- a/src/import/libcontainer/container_linux.go
>> ++++ b/src/import/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 f13b226e..5650b0ac 100644
>> +--- a/src/import/libcontainer/rootfs_linux.go
>> ++++ b/src/import/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 d755984b..1bfe7c66 100644
>> +--- a/src/import/libcontainer/rootfs_linux_test.go
>> ++++ b/src/import/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)
>> + }
>> +--
>> +2.17.1
>> +
>> diff --git a/recipes-containers/runc/runc-docker_git.bb b/recipes-containers/runc/runc-docker_git.bb
>> index c9f460b..8d810d0 100644
>> --- a/recipes-containers/runc/runc-docker_git.bb
>> +++ b/recipes-containers/runc/runc-docker_git.bb
>> @@ -7,6 +7,7 @@ SRC_URI = "git://github.com/opencontainers/runc;nobranch=1;name=runc-docker \
>> file://0001-runc-Add-console-socket-dev-null.patch \
>> file://0001-Makefile-respect-GOBUILDFLAGS-for-runc-and-remove-re.patch \
>> file://0001-runc-docker-SIGUSR1-daemonize.patch \
>> + file://0001-Only-allow-proc-mount-if-it-is-procfs.patch \
>> "
> Sorry for the slow reply, I was traveling and didn't have a lot of time.
>
> Is this only on the runc-docker side ? or is the runc-opencontainers
> also vulnerable ?
>
> Bruce
Both.
I'll send out V2. Thanks for reminding.
Best Regards,
Chen Qi
>> RUNC_VERSION = "1.0.0-rc8"
>> --
>> 2.17.1
>>
>> --
>> _______________________________________________
>> meta-virtualization mailing list
>> meta-virtualization at yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/meta-virtualization
>
>
More information about the meta-virtualization
mailing list