[bug#34446] Runc container escape patches CVE-2019-5736

[bug#34446] Runc container escape patches CVE-2019-5736

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

These patches aim to fix CVE-2019-5736 in runc / Docker:

https://seclists.org/oss-sec/2019/q1/119

However, after applying these patches, Docker fails to build as shown
below. Runc, docker-cli, and containerd still build.

Please help :)

------
phase `setup-environment' succeeded after 0.0 seconds
starting phase `build'
# WARNING! I don't seem to be running in a Docker container.
# The result of this command might be an incorrect build, and will not be
# officially supported.
#
# Try this instead: make all
#

Removing bundles/

---> Making bundle: dynbinary (in bundles/dynbinary)
Building: bundles/dynbinary-daemon/dockerd-dev
# github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables
.gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:15: undefined: exec.Guix_doesnt_want_LookPath
.gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:45: invalid character U+005C '\'
Backtrace:
           4 (primitive-load "/gnu/store/n5jmx2wksfvcrwlpv2zafd5hany…")
In ice-9/eval.scm:
   191:35  3 (_ _)
In srfi/srfi-1.scm:
   863:16  2 (every1 #<procedure ac28a0 at /gnu/store/rkv7z31csb2xa…> …)
In /gnu/store/rkv7z31csb2xandzhnvm5kc0i78pf0ay-module-import/guix/build/gnu-build-system.scm:
   799:28  1 (_ _)
In /gnu/store/rkv7z31csb2xandzhnvm5kc0i78pf0ay-module-import/guix/build/utils.scm:
    616:6  0 (invoke _ . _)

/gnu/store/rkv7z31csb2xandzhnvm5kc0i78pf0ay-module-import/guix/build/utils.scm:616:6: In procedure invoke:
Throw to key `srfi-34' with args `(#<condition &invoke-error [program: "hack/make.sh" arguments: ("dynbinary") exit-status: 2 term-signal: #f stop-signal: #f] 491cc0>)'.
builder for `/gnu/store/ihdm0nlw118mrb8wq127864g9pgrmghk-docker-18.09.2.drv' failed with exit code 1
build of /gnu/store/ihdm0nlw118mrb8wq127864g9pgrmghk-docker-18.09.2.drv failed
View build log at '/var/log/guix/drvs/ih/dm0nlw118mrb8wq127864g9pgrmghk-docker-18.09.2.drv.bz2'.
guix build: error: build of `/gnu/store/ihdm0nlw118mrb8wq127864g9pgrmghk-docker-18.09.2.drv' failed
------

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#34446] Runc container escape patches CVE-2019-5736

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

Hi Leo,

as originally released by upstream, Docker looks up auxiliary commands in PATH,
using a Go function called "LookPath".

Our package definition patches a lot of the specific LookPath calls to
refer to inputs by absolute path.

I've booby-trapped the remaining LookPath calls so we won't accidentially
have an internal tool looked up in $PATH.

If we have not forgotten any LookPath calls, there should have been no remaining
LookPath calls and it would not have failed the build.

> .gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:15: undefined: exec.Guix_doesnt_want_LookPath
> .gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:45: invalid character U+005C '\'

Please examine line 90.  It probably has a LookPath line with a new argument we
haven't seen before.

That means we'd have to find out which Guix package has an executable named like
the argument and add a case to the existing LookPath substituter in order to
also substitute it.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#34446] Runc container escape patches CVE-2019-5736

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

> That means we'd have to find out which Guix package has an executable named like
> the argument and add a case to the existing LookPath substituter in order to
> also substitute it.

and add an input to the "docker" package if necessary.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#34446] [PATCH 1/2] gnu: runc: Update to 1.0.0-rc6 [fixes CVE-2019-5736].

[Leo Famulari]

* gnu/packages/virtualization.scm (runc): Update to 1.0.0-rc6.
[source]: Use a descriptive file-name. Add 'runc-CVE-2019-5736.patch'
* gnu/packages/patches/runc-CVE-2019-5736.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                  |   1 +
 gnu/packages/patches/runc-CVE-2019-5736.patch | 343 ++++++++++++++++++
 gnu/packages/virtualization.scm               |   6 +-
 3 files changed, 348 insertions(+), 2 deletions(-)
 create mode 100644 gnu/packages/patches/runc-CVE-2019-5736.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 3bb60d3ade..5fbd02e120 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1205,6 +1205,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/ruby-concurrent-test-arm.patch		\
   %D%/packages/patches/ruby-rack-ignore-failing-test.patch      \
   %D%/packages/patches/ruby-tzinfo-data-ignore-broken-test.patch\
+  %D%/packages/patches/runc-CVE-2019-5736.patch			\
   %D%/packages/patches/rust-1.19-mrustc.patch			\
   %D%/packages/patches/rust-1.25-accept-more-detailed-gdb-lines.patch \
   %D%/packages/patches/rust-bootstrap-stage0-test.patch		\
diff --git a/gnu/packages/patches/runc-CVE-2019-5736.patch b/gnu/packages/patches/runc-CVE-2019-5736.patch
new file mode 100644
index 0000000000..f629fcbfb4
--- /dev/null
+++ b/gnu/packages/patches/runc-CVE-2019-5736.patch
@@ -0,0 +1,343 @@
+Fix CVE-2019-5736:
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-5736
+https://seclists.org/oss-sec/2019/q1/119
+
+Patch copied from upstream source repository:
+
+https://github.com/opencontainers/runc/commit/0a8e4117e7f715d5fbeef398405813ce8e88558b
+
+From 0a8e4117e7f715d5fbeef398405813ce8e88558b Mon Sep 17 00:00:00 2001
+From: Aleksa Sarai <asarai@suse.de>
+Date: Wed, 9 Jan 2019 13:40:01 +1100
+Subject: [PATCH] nsenter: clone /proc/self/exe to avoid exposing host binary
+ to container
+
+There are quite a few circumstances where /proc/self/exe pointing to a
+pretty important container binary is a _bad_ thing, so to avoid this we
+have to make a copy (preferably doing self-clean-up and not being
+writeable).
+
+We require memfd_create(2) -- though there is an O_TMPFILE fallback --
+but we can always extend this to use a scratch MNT_DETACH overlayfs or
+tmpfs. The main downside to this approach is no page-cache sharing for
+the runc binary (which overlayfs would give us) but this is far less
+complicated.
+
+This is only done during nsenter so that it happens transparently to the
+Go code, and any libcontainer users benefit from it. This also makes
+ExtraFiles and --preserve-fds handling trivial (because we don't need to
+worry about it).
+
+Fixes: CVE-2019-5736
+Co-developed-by: Christian Brauner <christian.brauner@ubuntu.com>
+Signed-off-by: Aleksa Sarai <asarai@suse.de>
+---
+ libcontainer/nsenter/cloned_binary.c | 268 +++++++++++++++++++++++++++
+ libcontainer/nsenter/nsexec.c        |  11 ++
+ 2 files changed, 279 insertions(+)
+ create mode 100644 libcontainer/nsenter/cloned_binary.c
+
+diff --git a/libcontainer/nsenter/cloned_binary.c b/libcontainer/nsenter/cloned_binary.c
+new file mode 100644
+index 000000000..c8a42c23f
+--- /dev/null
++++ b/libcontainer/nsenter/cloned_binary.c
+@@ -0,0 +1,268 @@
++/*
++ * Copyright (C) 2019 Aleksa Sarai <cyphar@cyphar.com>
++ * Copyright (C) 2019 SUSE LLC
++ *
++ * Licensed under the Apache License, Version 2.0 (the "License");
++ * you may not use this file except in compliance with the License.
++ * You may obtain a copy of the License at
++ *
++ *     http://www.apache.org/licenses/LICENSE-2.0
++ *
++ * Unless required by applicable law or agreed to in writing, software
++ * distributed under the License is distributed on an "AS IS" BASIS,
++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++ * See the License for the specific language governing permissions and
++ * limitations under the License.
++ */
++
++#define _GNU_SOURCE
++#include <unistd.h>
++#include <stdio.h>
++#include <stdlib.h>
++#include <stdbool.h>
++#include <string.h>
++#include <limits.h>
++#include <fcntl.h>
++#include <errno.h>
++
++#include <sys/types.h>
++#include <sys/stat.h>
++#include <sys/vfs.h>
++#include <sys/mman.h>
++#include <sys/sendfile.h>
++#include <sys/syscall.h>
++
++/* Use our own wrapper for memfd_create. */
++#if !defined(SYS_memfd_create) && defined(__NR_memfd_create)
++#  define SYS_memfd_create __NR_memfd_create
++#endif
++#ifdef SYS_memfd_create
++#  define HAVE_MEMFD_CREATE
++/* memfd_create(2) flags -- copied from <linux/memfd.h>. */
++#  ifndef MFD_CLOEXEC
++#    define MFD_CLOEXEC       0x0001U
++#    define MFD_ALLOW_SEALING 0x0002U
++#  endif
++int memfd_create(const char *name, unsigned int flags)
++{
++	return syscall(SYS_memfd_create, name, flags);
++}
++#endif
++
++/* This comes directly from <linux/fcntl.h>. */
++#ifndef F_LINUX_SPECIFIC_BASE
++#  define F_LINUX_SPECIFIC_BASE 1024
++#endif
++#ifndef F_ADD_SEALS
++#  define F_ADD_SEALS (F_LINUX_SPECIFIC_BASE + 9)
++#  define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10)
++#endif
++#ifndef F_SEAL_SEAL
++#  define F_SEAL_SEAL   0x0001	/* prevent further seals from being set */
++#  define F_SEAL_SHRINK 0x0002	/* prevent file from shrinking */
++#  define F_SEAL_GROW   0x0004	/* prevent file from growing */
++#  define F_SEAL_WRITE  0x0008	/* prevent writes */
++#endif
++
++#define RUNC_SENDFILE_MAX 0x7FFFF000 /* sendfile(2) is limited to 2GB. */
++#ifdef HAVE_MEMFD_CREATE
++#  define RUNC_MEMFD_COMMENT "runc_cloned:/proc/self/exe"
++#  define RUNC_MEMFD_SEALS \
++	(F_SEAL_SEAL | F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE)
++#endif
++
++static void *must_realloc(void *ptr, size_t size)
++{
++	void *old = ptr;
++	do {
++		ptr = realloc(old, size);
++	} while(!ptr);
++	return ptr;
++}
++
++/*
++ * Verify whether we are currently in a self-cloned program (namely, is
++ * /proc/self/exe a memfd). F_GET_SEALS will only succeed for memfds (or rather
++ * for shmem files), and we want to be sure it's actually sealed.
++ */
++static int is_self_cloned(void)
++{
++	int fd, ret, is_cloned = 0;
++
++	fd = open("/proc/self/exe", O_RDONLY|O_CLOEXEC);
++	if (fd < 0)
++		return -ENOTRECOVERABLE;
++
++#ifdef HAVE_MEMFD_CREATE
++	ret = fcntl(fd, F_GET_SEALS);
++	is_cloned = (ret == RUNC_MEMFD_SEALS);
++#else
++	struct stat statbuf = {0};
++	ret = fstat(fd, &statbuf);
++	if (ret >= 0)
++		is_cloned = (statbuf.st_nlink == 0);
++#endif
++	close(fd);
++	return is_cloned;
++}
++
++/*
++ * Basic wrapper around mmap(2) that gives you the file length so you can
++ * safely treat it as an ordinary buffer. Only gives you read access.
++ */
++static char *read_file(char *path, size_t *length)
++{
++	int fd;
++	char buf[4096], *copy = NULL;
++
++	if (!length)
++		return NULL;
++
++	fd = open(path, O_RDONLY | O_CLOEXEC);
++	if (fd < 0)
++		return NULL;
++
++	*length = 0;
++	for (;;) {
++		int n;
++
++		n = read(fd, buf, sizeof(buf));
++		if (n < 0)
++			goto error;
++		if (!n)
++			break;
++
++		copy = must_realloc(copy, (*length + n) * sizeof(*copy));
++		memcpy(copy + *length, buf, n);
++		*length += n;
++	}
++	close(fd);
++	return copy;
++
++error:
++	close(fd);
++	free(copy);
++	return NULL;
++}
++
++/*
++ * A poor-man's version of "xargs -0". Basically parses a given block of
++ * NUL-delimited data, within the given length and adds a pointer to each entry
++ * to the array of pointers.
++ */
++static int parse_xargs(char *data, int data_length, char ***output)
++{
++	int num = 0;
++	char *cur = data;
++
++	if (!data || *output != NULL)
++		return -1;
++
++	while (cur < data + data_length) {
++		num++;
++		*output = must_realloc(*output, (num + 1) * sizeof(**output));
++		(*output)[num - 1] = cur;
++		cur += strlen(cur) + 1;
++	}
++	(*output)[num] = NULL;
++	return num;
++}
++
++/*
++ * "Parse" out argv and envp from /proc/self/cmdline and /proc/self/environ.
++ * This is necessary because we are running in a context where we don't have a
++ * main() that we can just get the arguments from.
++ */
++static int fetchve(char ***argv, char ***envp)
++{
++	char *cmdline = NULL, *environ = NULL;
++	size_t cmdline_size, environ_size;
++
++	cmdline = read_file("/proc/self/cmdline", &cmdline_size);
++	if (!cmdline)
++		goto error;
++	environ = read_file("/proc/self/environ", &environ_size);
++	if (!environ)
++		goto error;
++
++	if (parse_xargs(cmdline, cmdline_size, argv) <= 0)
++		goto error;
++	if (parse_xargs(environ, environ_size, envp) <= 0)
++		goto error;
++
++	return 0;
++
++error:
++	free(environ);
++	free(cmdline);
++	return -EINVAL;
++}
++
++static int clone_binary(void)
++{
++	int binfd, memfd;
++	ssize_t sent = 0;
++
++#ifdef HAVE_MEMFD_CREATE
++	memfd = memfd_create(RUNC_MEMFD_COMMENT, MFD_CLOEXEC | MFD_ALLOW_SEALING);
++#else
++	memfd = open("/tmp", O_TMPFILE | O_EXCL | O_RDWR | O_CLOEXEC, 0711);
++#endif
++	if (memfd < 0)
++		return -ENOTRECOVERABLE;
++
++	binfd = open("/proc/self/exe", O_RDONLY | O_CLOEXEC);
++	if (binfd < 0)
++		goto error;
++
++	sent = sendfile(memfd, binfd, NULL, RUNC_SENDFILE_MAX);
++	close(binfd);
++	if (sent < 0)
++		goto error;
++
++#ifdef HAVE_MEMFD_CREATE
++	int err = fcntl(memfd, F_ADD_SEALS, RUNC_MEMFD_SEALS);
++	if (err < 0)
++		goto error;
++#else
++	/* Need to re-open "memfd" as read-only to avoid execve(2) giving -EXTBUSY. */
++	int newfd;
++	char *fdpath = NULL;
++
++	if (asprintf(&fdpath, "/proc/self/fd/%d", memfd) < 0)
++		goto error;
++	newfd = open(fdpath, O_RDONLY | O_CLOEXEC);
++	free(fdpath);
++	if (newfd < 0)
++		goto error;
++
++	close(memfd);
++	memfd = newfd;
++#endif
++	return memfd;
++
++error:
++	close(memfd);
++	return -EIO;
++}
++
++int ensure_cloned_binary(void)
++{
++	int execfd;
++	char **argv = NULL, **envp = NULL;
++
++	/* Check that we're not self-cloned, and if we are then bail. */
++	int cloned = is_self_cloned();
++	if (cloned > 0 || cloned == -ENOTRECOVERABLE)
++		return cloned;
++
++	if (fetchve(&argv, &envp) < 0)
++		return -EINVAL;
++
++	execfd = clone_binary();
++	if (execfd < 0)
++		return -EIO;
++
++	fexecve(execfd, argv, envp);
++	return -ENOEXEC;
++}
+diff --git a/libcontainer/nsenter/nsexec.c b/libcontainer/nsenter/nsexec.c
+index 28269dfc0..7750af35e 100644
+--- a/libcontainer/nsenter/nsexec.c
++++ b/libcontainer/nsenter/nsexec.c
+@@ -534,6 +534,9 @@ void join_namespaces(char *nslist)
+ 	free(namespaces);
+ }
+ 
++/* Defined in cloned_binary.c. */
++extern int ensure_cloned_binary(void);
++
+ void nsexec(void)
+ {
+ 	int pipenum;
+@@ -549,6 +552,14 @@ void nsexec(void)
+ 	if (pipenum == -1)
+ 		return;
+ 
++	/*
++	 * We need to re-exec if we are not in a cloned binary. This is necessary
++	 * to ensure that containers won't be able to access the host binary
++	 * through /proc/self/exe. See CVE-2019-5736.
++	 */
++	if (ensure_cloned_binary() < 0)
++		bail("could not ensure we are a cloned binary");
++
+ 	/* Parse all of the netlink configuration. */
+ 	nl_parse(pipenum, &config);
+ 
diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index f5e4540329..8a5af2e8ea 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -847,15 +847,17 @@ monitor/GPU.")
 (define-public runc
   (package
     (name "runc")
-    (version "1.0.0-rc5")
+    (version "1.0.0-rc6")
     (source (origin
               (method url-fetch)
               (uri (string-append
                     "https://github.com/opencontainers/runc/releases/"
                     "download/v" version "/runc.tar.xz"))
+              (file-name (string-append name "-" version ".tar.xz"))
+              (patches (search-patches "runc-CVE-2019-5736.patch"))
               (sha256
                (base32
-                "081avdzwnqpk368wbaihlzsypaxpj42d7699h7jgp0fks14x4103"))))
+                "1c7832dq70slkjh8qp2civ1wxhhdd2hrx84pq7db1mmqc9fdr3cc"))))
     (build-system go-build-system)
     (arguments
      '(#:import-path "github.com/opencontainers/runc"
-- 
2.20.1

[bug#34446] [PATCH 2/2] gnu: Docker: Update to 18.09.2.

[Leo Famulari]

* gnu/packages/docker.scm (%docker-version): Update to 18.09.2.
(docker, docker-cli): Adjust accordingly.
---
 gnu/packages/docker.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
index 992eb0dcc1..5a400e6490 100644
--- a/gnu/packages/docker.scm
+++ b/gnu/packages/docker.scm
@@ -43,7 +43,7 @@
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages virtualization))
 
-(define %docker-version "18.09.0")
+(define %docker-version "18.09.2")
 
 (define-public python-docker-py
   (package
@@ -241,7 +241,7 @@ network attachments.")
       (file-name (git-file-name name version))
       (sha256
        (base32
-        "1liqbx58grqih6m8hz9y20y5waflv19pv15l3wl64skap2bsn21c"))
+        "1zfpk2n8j6gnwbrxrh6d6pj24y60dhbanpf55shrm2yxz54ka36c"))
       (patches
        (search-patches "docker-engine-test-noinstall.patch"
                        "docker-fix-tests.patch"))))
@@ -483,7 +483,7 @@ provisioning etc.")
       (file-name (git-file-name name version))
       (sha256
        (base32
-        "1ivisys20kphvbqlazc3bsg7pk0ykj9gjx5d4yg439x4n13jxwvb"))))
+        "0jzcqh1kqbfyj6ax7z67gihaqgjiz6ddz6rq6k458l68v7zn77r8"))))
     (build-system go-build-system)
     (arguments
      `(#:import-path "github.com/docker/cli"
-- 
2.20.1

[bug#34446] [PATCH 1/2] gnu: runc: Update to 1.0.0-rc6 [fixes CVE-2019-5736].

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Mon, 11 Feb 2019 19:27:35 -0500
Leo Famulari <leo@famulari.name> wrote:

>  (define-public runc
>    (package
>      (name "runc")
> -    (version "1.0.0-rc5")
> +    (version "1.0.0-rc6")
>      (source (origin
>                (method url-fetch)
>                (uri (string-append
>                      "https://github.com/opencontainers/runc/releases/"
>                      "download/v" version "/runc.tar.xz"))
> +              (file-name (string-append name "-" version ".tar.xz"))
> +              (patches (search-patches "runc-CVE-2019-5736.patch"))
>                (sha256
>                 (base32
> -                "081avdzwnqpk368wbaihlzsypaxpj42d7699h7jgp0fks14x4103"))))
> +                "1c7832dq70slkjh8qp2civ1wxhhdd2hrx84pq7db1mmqc9fdr3cc"))))
>      (build-system go-build-system)
>      (arguments
>       '(#:import-path "github.com/opencontainers/runc"

Docker still contains some vendored dependencies, among those github.com/opencontainers/runc,
in directory "vendor", and so does containerd.  It might make sense to also remove them now.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#34446: Runc container escape patches CVE-2019-5736

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

On Tue, Feb 12, 2019 at 01:10:34AM +0100, Danny Milosavljevic wrote:
> as originally released by upstream, Docker looks up auxiliary commands in PATH,
> using a Go function called "LookPath".
> 
> Our package definition patches a lot of the specific LookPath calls to
> refer to inputs by absolute path.
> 
> I've booby-trapped the remaining LookPath calls so we won't accidentially
> have an internal tool looked up in $PATH.
> 
> If we have not forgotten any LookPath calls, there should have been no remaining
> LookPath calls and it would not have failed the build.

Thanks for explaining this :)

> > .gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:15: undefined: exec.Guix_doesnt_want_LookPath
> > .gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/iptables/iptables.go:90:45: invalid character U+005C '\'
> 
> Please examine line 90.  It probably has a LookPath line with a new argument we
> haven't seen before.

Okay, they added a lookup for 'iptables-legacy' which is what Debian has
renamed iptables. I changed this to just look up 'iptables' since its
equivalent on our end and in how the Docker code uses it and pushed as
ea7cddaac782b2cdc789a354e172356ed5c183e7.

Thanks again for your help!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]