[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#777649: marked as done (unblock: cgmanager/0.33-2+deb8u1)



Your message dated Wed, 18 Feb 2015 20:07:06 +0100
with message-id <54E4E2DA.7080707@thykier.net>
and subject line Re: Bug#777649: cgmanager security update for jessie
has caused the Debian Bug report #777649,
regarding unblock: cgmanager/0.33-2+deb8u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
777649: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=777649
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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

--- End Message ---
--- Begin Message ---
On 2015-02-13 05:36, Serge Hallyn wrote:
> [...]
> 
>> and then upload the target fixes into testing with
>> version 0.33-2+deb8u1.
> 
> Sorry, I'm not sure what you mean.  I don't actually have upload rights.
> Should I ask someone to sponsor such a package, or just post the debdiff
> here?  (It could be the same as the last debdiff I posted, with the version
> number changed, or I could squash the two patches as I mentioned before)
> 
>> Please remove the moreinfo tag once the above have been done.
> 
> thanks,
> -serge
> 
> 

It got uploaded and I have unblocked the t-p-u version.

Thanks,
~Niels

--- End Message ---

Reply to: