Bug#777649: cgmanager security update for jessie
Package: release.debian.org
Usertags: jessie-pu
A security issue was found in cgmanager, allowing root-owned privileged
containers to fully administer cgroups on the host. Two other issues
were found which allow cgmanager to be crashed by unprivileged users.
These have all been fixed in sid. The debdiff below, against the current
jessie package, fixes them for jessie.
debdiff:
diff -Nru cgmanager-0.33/debian/changelog cgmanager-0.33/debian/changelog
--- cgmanager-0.33/debian/changelog 2014-10-13 18:35:43.000000000 -0500
+++ cgmanager-0.33/debian/changelog 2015-01-26 09:15:49.000000000 -0600
@@ -1,3 +1,16 @@
+cgmanager (0.33-3) testing; urgency=medium
+
+ * SECURITY UPDATE: Cross-cgroup resource control bypass.
+ - debian/patches/0003-make-sure-to-check-cgroup-hierarchy.patch, modify
+ cgmanager.c to verify that requests are allowed under the caller's
+ cgroup.
+ - CVE-2014-1425
+ * 0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch and
+ 0005-prevent-some-cgmanager-asserts.patch: prevent cgmanager
+ crashing on unhandled asserts or dbus error (LP: #1407787)
+
+ -- Serge Hallyn <serge.hallyn@ubuntu.com> Mon, 26 Jan 2015 09:12:02 -0600
+
cgmanager (0.33-2) unstable; urgency=medium
* Cherrypick two upstream patches to ensure that 'movepid all' continues
diff -Nru cgmanager-0.33/debian/patches/0003-make-sure-to-check-cgroup-hierarchy.patch cgmanager-0.33/debian/patches/0003-make-sure-to-check-cgroup-hierarchy.patch
--- cgmanager-0.33/debian/patches/0003-make-sure-to-check-cgroup-hierarchy.patch 1969-12-31 18:00:00.000000000 -0600
+++ cgmanager-0.33/debian/patches/0003-make-sure-to-check-cgroup-hierarchy.patch 2015-01-26 09:15:58.000000000 -0600
@@ -0,0 +1,201 @@
+From 6267916d4ea939794e0583cd8b08bd0b9594a6e2 Mon Sep 17 00:00:00 2001
+From: Serge Hallyn <serge.hallyn@ubuntu.com>
+Date: Wed, 26 Nov 2014 01:00:10 -0600
+Subject: [PATCH 1/1] make sure to check cgroup hierarchy
+
+Some cases weren't doing that, although at least those were still
+checking for proper ownership.
+
+Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
+---
+ cgmanager.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
+ 1 file changed, 80 insertions(+), 5 deletions(-)
+
+Index: cgmanager-0.33/cgmanager.c
+===================================================================
+--- cgmanager-0.33.orig/cgmanager.c
++++ cgmanager-0.33/cgmanager.c
+@@ -558,13 +558,20 @@ next:
+ int get_value_main(void *parent, const char *controller, const char *cgroup,
+ const char *key, struct ucred p, struct ucred r, char **value)
+ {
+- char path[MAXPATHLEN];
++ char pcgpath[MAXPATHLEN], path[MAXPATHLEN];
+
+ if (!sane_cgroup(cgroup)) {
+ nih_error("%s: unsafe cgroup", __func__);
+ return -1;
+ }
+
++ // Get p's current cgroup in pcgpath
++ if (!compute_pid_cgroup(p.pid, controller, "", pcgpath, NULL)) {
++ nih_error("%s: Could not determine the proxy's cgroup for %s",
++ __func__, controller);
++ return -1;
++ }
++
+ if (!compute_pid_cgroup(r.pid, controller, cgroup, path, NULL)) {
+ nih_error("%s: Could not determine the requested cgroup (%s:%s)",
+ __func__, controller, cgroup);
+@@ -577,6 +584,14 @@ int get_value_main(void *parent, const c
+ return -1;
+ }
+
++ // Make sure target cgroup is under proxy's
++ int plen = strlen(pcgpath);
++ if (strncmp(pcgpath, path, plen) != 0) {
++ nih_error("%s: target cgroup is not below r (%d)'s", __func__,
++ r.pid);
++ return -1;
++ }
++
+ /* append the filename */
+ if (strlen(path) + strlen(key) + 2 > MAXPATHLEN) {
+ nih_error("%s: filename too long for cgroup %s key %s", __func__, path, key);
+@@ -608,19 +623,34 @@ int set_value_main(const char *controlle
+ struct ucred r)
+
+ {
+- char path[MAXPATHLEN];
++ char pcgpath[MAXPATHLEN], path[MAXPATHLEN];
+
+ if (!sane_cgroup(cgroup)) {
+ nih_error("%s: unsafe cgroup", __func__);
+ return -1;
+ }
+
++ // Get p's current cgroup in pcgpath
++ if (!compute_pid_cgroup(p.pid, controller, "", pcgpath, NULL)) {
++ nih_error("%s: Could not determine the proxy's cgroup for %s",
++ __func__, controller);
++ return -1;
++ }
++
+ if (!compute_pid_cgroup(r.pid, controller, cgroup, path, NULL)) {
+ nih_error("%s: Could not determine the requested cgroup (%s:%s)",
+ __func__, controller, cgroup);
+ return -1;
+ }
+
++ // Make sure target cgroup is under proxy's
++ int plen = strlen(pcgpath);
++ if (strncmp(pcgpath, path, plen) != 0) {
++ nih_error("%s: target cgroup is not below r (%d)'s", __func__,
++ r.pid);
++ return -1;
++ }
++
+ /* Check access rights to the cgroup directory */
+ if (!may_access(r.pid, r.uid, r.gid, path, O_RDONLY)) {
+ nih_error("%s: Pid %d may not access %s\n", __func__, r.pid, path);
+@@ -823,7 +853,7 @@ next:
+ int get_tasks_main(void *parent, const char *controller, const char *cgroup,
+ struct ucred p, struct ucred r, int32_t **pids)
+ {
+- char path[MAXPATHLEN];
++ char pcgpath[MAXPATHLEN], path[MAXPATHLEN];
+ const char *key = "tasks";
+ int alloced_pids = 0, nrpids = 0;
+
+@@ -832,12 +862,27 @@ int get_tasks_main(void *parent, const c
+ return -1;
+ }
+
++ // Get p's current cgroup in pcgpath
++ if (!compute_pid_cgroup(p.pid, controller, "", pcgpath, NULL)) {
++ nih_error("%s: Could not determine the proxy's cgroup for %s",
++ __func__, controller);
++ return -1;
++ }
++
+ if (!compute_pid_cgroup(r.pid, controller, cgroup, path, NULL)) {
+ nih_error("%s: Could not determine the requested cgroup (%s:%s)",
+ __func__, controller, cgroup);
+ return -1;
+ }
+
++ // Make sure target cgroup is under proxy's
++ int plen = strlen(pcgpath);
++ if (strncmp(pcgpath, path, plen) != 0) {
++ nih_error("%s: target cgroup is not below r (%d)'s", __func__,
++ r.pid);
++ return -1;
++ }
++
+ /* Check access rights to the cgroup directory */
+ if (!may_access(r.pid, r.uid, r.gid, path, O_RDONLY)) {
+ nih_error("%s: Pid %d may not access %s\n", __func__, r.pid, path);
+@@ -906,7 +951,7 @@ int collect_tasks(void *parent, const ch
+ struct ucred p, struct ucred r, int32_t **pids,
+ int *alloced_pids, int *nrpids)
+ {
+- char path[MAXPATHLEN];
++ char pcgpath[MAXPATHLEN], path[MAXPATHLEN];
+ nih_local char *rpath = NULL;
+
+ if (!sane_cgroup(cgroup)) {
+@@ -914,12 +959,27 @@ int collect_tasks(void *parent, const ch
+ return -1;
+ }
+
++ // Get p's current cgroup in pcgpath
++ if (!compute_pid_cgroup(p.pid, controller, "", pcgpath, NULL)) {
++ nih_error("%s: Could not determine the proxy's cgroup for %s",
++ __func__, controller);
++ return -1;
++ }
++
+ if (!compute_pid_cgroup(r.pid, controller, cgroup, path, NULL)) {
+ nih_error("%s: Could not determine the requested cgroup (%s:%s)",
+ __func__, controller, cgroup);
+ return -2;
+ }
+
++ // Make sure target cgroup is under proxy's
++ int plen = strlen(pcgpath);
++ if (strncmp(pcgpath, path, plen) != 0) {
++ nih_error("%s: target cgroup is not below r (%d)'s", __func__,
++ r.pid);
++ return -1;
++ }
++
+ /* Check access rights to the cgroup directory */
+ if (!may_access(r.pid, r.uid, r.gid, path, O_RDONLY)) {
+ nih_error("%s: Pid %d may not access %s\n", __func__, r.pid, path);
+@@ -982,7 +1042,7 @@ next:
+ int list_children_main(void *parent, const char *controller, const char *cgroup,
+ struct ucred p, struct ucred r, char ***output)
+ {
+- char path[MAXPATHLEN];
++ char pcgpath[MAXPATHLEN], path[MAXPATHLEN];
+
+ *output = NULL;
+ if (!sane_cgroup(cgroup)) {
+@@ -990,12 +1050,27 @@ int list_children_main(void *parent, con
+ return -1;
+ }
+
++ // Get p's current cgroup in pcgpath
++ if (!compute_pid_cgroup(p.pid, controller, "", pcgpath, NULL)) {
++ nih_error("%s: Could not determine the proxy's cgroup for %s",
++ __func__, controller);
++ return -1;
++ }
++
+ if (!compute_pid_cgroup(r.pid, controller, cgroup, path, NULL)) {
+ nih_error("%s: Could not determine the requested cgroup (%s:%s)",
+ __func__, controller, cgroup);
+ return -1;
+ }
+
++ // Make sure target cgroup is under proxy's
++ int plen = strlen(pcgpath);
++ if (strncmp(pcgpath, path, plen) != 0) {
++ nih_error("%s: target cgroup is not below r (%d)'s", __func__,
++ r.pid);
++ return -1;
++ }
++
+ /* Check access rights to the cgroup directory */
+ if (!may_access(r.pid, r.uid, r.gid, path, O_RDONLY)) {
+ nih_error("%s: Pid %d may not access %s\n", __func__, r.pid, path);
diff -Nru cgmanager-0.33/debian/patches/0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch cgmanager-0.33/debian/patches/0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch
--- cgmanager-0.33/debian/patches/0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch 1969-12-31 18:00:00.000000000 -0600
+++ cgmanager-0.33/debian/patches/0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch 2015-01-26 09:16:00.000000000 -0600
@@ -0,0 +1,26 @@
+From a7fddc8078e7a1fc1d8e33516c90d1a37b8bfc8c Mon Sep 17 00:00:00 2001
+From: Serge Hallyn <serge.hallyn@ubuntu.com>
+Date: Mon, 15 Dec 2014 18:34:27 -0600
+Subject: [PATCH 1/4] chown: stop cgmanager crash on chown of bad file
+
+Wrong error function was being used.
+
+Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
+---
+ cgmanager.c | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+Index: cgmanager-0.33/cgmanager.c
+===================================================================
+--- cgmanager-0.33.orig/cgmanager.c
++++ cgmanager-0.33/cgmanager.c
+@@ -525,8 +525,7 @@ int chmod_main(const char *controller, c
+ }
+
+ if (file && ( strchr(file, '/') || strchr(file, '\\')) ) {
+- nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
+- "invalid file");
++ nih_error("%s: invalid file", __func__);
+ return -1;
+ }
+
diff -Nru cgmanager-0.33/debian/patches/0005-prevent-some-cgmanager-asserts.patch cgmanager-0.33/debian/patches/0005-prevent-some-cgmanager-asserts.patch
--- cgmanager-0.33/debian/patches/0005-prevent-some-cgmanager-asserts.patch 1969-12-31 18:00:00.000000000 -0600
+++ cgmanager-0.33/debian/patches/0005-prevent-some-cgmanager-asserts.patch 2015-01-26 09:16:02.000000000 -0600
@@ -0,0 +1,44 @@
+From c56249afbccfb66f4896c1fc3380e7c1459c1afc Mon Sep 17 00:00:00 2001
+From: Serge Hallyn <serge.hallyn@ubuntu.com>
+Date: Mon, 29 Dec 2014 01:19:56 -0600
+Subject: [PATCH 4/4] prevent some cgmanager asserts
+
+Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
+---
+ cgmanager.c | 9 ++++++---
+ 1 file changed, 6 insertions(+), 3 deletions(-)
+
+Index: cgmanager-0.33/cgmanager.c
+===================================================================
+--- cgmanager-0.33.orig/cgmanager.c
++++ cgmanager-0.33/cgmanager.c
+@@ -899,7 +899,8 @@ int get_tasks_main(void *parent, const c
+
+ *pids = NULL;
+ if (file_read_pids(parent, path, pids, &alloced_pids, &nrpids) < 0) {
+- nih_free(*pids);
++ if (*pids)
++ nih_free(*pids);
+ return -1;
+ }
+ return nrpids;
+@@ -1007,7 +1008,8 @@ int get_tasks_recursive_main(void *paren
+ if (strcmp(controller, "all") != 0 && !strchr(controller, ',')) {
+ if (collect_tasks(parent, controller, cgroup, p, r, pids,
+ &alloced_pids, &nrpids) < 0) {
+- nih_free(*pids);
++ if (*pids)
++ nih_free(*pids);
+ return -1;
+ }
+ return nrpids;
+@@ -1028,7 +1030,8 @@ int get_tasks_recursive_main(void *paren
+ if (ret == -2) // permission denied - ignore
+ goto next;
+ if (ret != 0) {
+- nih_free(*pids);
++ if (*pids)
++ nih_free(*pids);
+ return -1;
+ }
+ next:
diff -Nru cgmanager-0.33/debian/patches/series cgmanager-0.33/debian/patches/series
--- cgmanager-0.33/debian/patches/series 2014-10-13 18:32:58.000000000 -0500
+++ cgmanager-0.33/debian/patches/series 2015-01-26 09:11:59.000000000 -0600
@@ -1,2 +1,5 @@
0001-do_move_pid_main-don-t-break-out-of-while-loop-on-er.patch
0002-check-enabled-column-when-parsing-proc-cgroups.patch
+0003-make-sure-to-check-cgroup-hierarchy.patch
+0004-chown-stop-cgmanager-crash-on-chown-of-bad-file.patch
+0005-prevent-some-cgmanager-asserts.patch
Reply to: