Hi,
attached is a debdiff that fixes the issue. I'd be great if somebody
could check out the packages at:
http://honk.sigxcpu.org/projects/libvirt/snapshots/
I didn't squash the upstream commits so we can easily pull in more stuff
iff needed.
Cheers,
-- Guido
diff --git a/debian/changelog b/debian/changelog
index 719b74e..867d751 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+libvirt (0.8.3-5+squeeze5) stable-security; urgency=low
+
+ * [465cafd] Invoke initgroups when starting kvm
+ so we don't fail to open /dev/kvm. (Closes: #703208)
+
+ -- Guido Günther <a...@sigxcpu.org> Sun, 17 Mar 2013 14:39:03 +0100
+
libvirt (0.8.3-5+squeeze4) stable-security; urgency=low
* [9d7846f] CVE-2013-1766: Use libvirt-qemu as group to run qemu/kvm
diff --git a/debian/patches/0020-Rerun-autoconf.patch b/debian/patches/0020-Rerun-autoconf.patch
new file mode 100644
index 0000000..e74f0a8
--- /dev/null
+++ b/debian/patches/0020-Rerun-autoconf.patch
@@ -0,0 +1,35 @@
+From: =?UTF-8?q?Guido=20G=C3=BCnther?= <a...@sigxcpu.org>
+Date: Sun, 17 Mar 2013 14:19:08 +0100
+Subject: Rerun autoconf
+
+---
+ config.h.in | 3 +++
+ configure | 1 +
+ 2 files changed, 4 insertions(+)
+
+diff --git a/config.h.in b/config.h.in
+index 187fdbc..d839f22 100644
+--- a/config.h.in
++++ b/config.h.in
+@@ -452,6 +452,9 @@
+ /* Define to 1 if you have the `inet_pton' function. */
+ #undef HAVE_INET_PTON
+
++/* Define to 1 if you have the `initgroups' function. */
++#undef HAVE_INITGROUPS
++
+ /* Define if you have the 'intmax_t' type in <stdint.h> or <inttypes.h>. */
+ #undef HAVE_INTMAX_T
+
+diff --git a/configure b/configure
+index 37adb0a..f24ad5d 100755
+--- a/configure
++++ b/configure
+@@ -3009,6 +3009,7 @@ gl_func_list="$gl_func_list regexec"
+ gl_func_list="$gl_func_list sched_getaffinity"
+ gl_func_list="$gl_func_list getuid"
+ gl_func_list="$gl_func_list getgid"
++gl_func_list="$gl_func_list initgroups"
+ gl_func_list="$gl_func_list posix_fallocate"
+ gl_func_list="$gl_func_list mmap"
+ gl_func_list="$gl_func_list strerror_r"
diff --git a/debian/patches/security/0017-New-virSetUIDGID-utility-function.patch b/debian/patches/security/0017-New-virSetUIDGID-utility-function.patch
new file mode 100644
index 0000000..80352d6
--- /dev/null
+++ b/debian/patches/security/0017-New-virSetUIDGID-utility-function.patch
@@ -0,0 +1,123 @@
+From: Laine Stump <la...@laine.org>
+Date: Sun, 17 Mar 2013 13:32:59 +0100
+Subject: New virSetUIDGID() utility function
+
+virSetUIDGID() sets both the real and effective group and user of the
+process, and additionally calls initgroups() to assure that the
+process joins all the auxiliary groups that the given uid is a member
+of.
+
+---
+ configure.ac | 2 +-
+ src/util/util.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ src/util/util.h | 2 ++
+ 3 files changed, 67 insertions(+), 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index cd48c1b..a075c58 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -93,7 +93,7 @@ AC_MSG_RESULT([$have_cpuid])
+
+
+ dnl Availability of various common functions (non-fatal if missing).
+-AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \
++AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid initgroups \
+ posix_fallocate mmap])
+
+ dnl Availability of various not common threadsafe functions
+diff --git a/src/util/util.c b/src/util/util.c
+index 8f2a17e..08e6137 100644
+--- a/src/util/util.c
++++ b/src/util/util.c
+@@ -2748,6 +2748,61 @@ int virGetGroupID(const char *name,
+ return 0;
+ }
+
++
++/* Set the real and effective uid and gid to the given values, and call
++ * initgroups so that the process has all the assumed group membership of
++ * that uid. return 0 on success, -1 on failure.
++ */
++int
++virSetUIDGID(uid_t uid, gid_t gid)
++{
++ if (gid > 0) {
++ if (setregid(gid, gid) < 0) {
++ virReportSystemError(errno,
++ _("cannot change to '%d' group"), gid);
++ return -1;
++ }
++ }
++
++ if (uid > 0) {
++# ifdef HAVE_INITGROUPS
++ struct passwd pwd, *pwd_result;
++ char *buf = NULL;
++ size_t bufsize;
++
++ bufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
++ if (bufsize == -1)
++ bufsize = 16384;
++
++ if (VIR_ALLOC_N(buf, bufsize) < 0) {
++ virReportOOMError();
++ return -1;
++ }
++ getpwuid_r(uid, &pwd, buf, bufsize, &pwd_result);
++ if (!pwd_result) {
++ virReportSystemError(errno,
++ _("cannot getpwuid_r(%d)"), uid);
++ VIR_FREE(buf);
++ return -1;
++ }
++ if (initgroups(pwd.pw_name, pwd.pw_gid) < 0) {
++ virReportSystemError(errno,
++ _("cannot initgroups(\"%s\", %d)"),
++ pwd.pw_name, pwd.pw_gid);
++ VIR_FREE(buf);
++ return -1;
++ }
++ VIR_FREE(buf);
++# endif
++ if (setreuid(uid, uid) < 0) {
++ virReportSystemError(errno,
++ _("cannot change to uid to '%d'"), uid);
++ return -1;
++ }
++ }
++ return 0;
++}
++
+ #else /* HAVE_GETPWUID_R */
+
+ char *
+@@ -2786,6 +2841,15 @@ int virGetGroupID(const char *name ATTRIBUTE_UNUSED,
+
+ return 0;
+ }
++
++int
++virSetUIDGID(uid_t uid ATTRIBUTE_UNUSED,
++ gid_t gid ATTRIBUTE_UNUSED)
++{
++ virUtilError(VIR_ERR_INTERNAL_ERROR,
++ "%s", _("virSetUIDGID is not available"));
++ return -1;
++}
+ #endif /* HAVE_GETPWUID_R */
+
+
+diff --git a/src/util/util.h b/src/util/util.h
+index 476eac4..f746cce 100644
+--- a/src/util/util.h
++++ b/src/util/util.h
+@@ -92,6 +92,8 @@ int virPipeReadUntilEOF(int outfd, int errfd,
+ char **outbuf, char **errbuf);
+ int virFork(pid_t *pid);
+
++int virSetUIDGID(uid_t uid, gid_t gid);
++
+ int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
+
+ int virFileReadAll(const char *path, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
diff --git a/debian/patches/security/0018-Run-initgroups-in-qemudOpenAsUID.patch b/debian/patches/security/0018-Run-initgroups-in-qemudOpenAsUID.patch
new file mode 100644
index 0000000..87b8c0d
--- /dev/null
+++ b/debian/patches/security/0018-Run-initgroups-in-qemudOpenAsUID.patch
@@ -0,0 +1,74 @@
+From: Dan Kenigsberg <dan...@redhat.com>
+Date: Tue, 19 Oct 2010 15:22:57 +0200
+Subject: Run initgroups() in qemudOpenAsUID()
+
+qemudOpenAsUID is intended to open a file with the credentials of a
+specified uid. Current implementation fails if the file is accessible to
+one of uid's groups but not owned by uid.
+
+This patch replaces the supplementary group list that the child process
+inherited from libvirtd with the default group list of uid.
+
+---
+ src/qemu/qemu_driver.c | 27 ++++++++++++++++++++++-----
+ 1 file changed, 22 insertions(+), 5 deletions(-)
+
+diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
+index 6097648..5629885 100644
+--- a/src/qemu/qemu_driver.c
++++ b/src/qemu/qemu_driver.c
+@@ -41,6 +41,7 @@
+ #include <signal.h>
+ #include <paths.h>
+ #include <pwd.h>
++#include <grp.h>
+ #include <stdio.h>
+ #include <sys/wait.h>
+ #include <sys/ioctl.h>
+@@ -6308,6 +6309,7 @@ parent_cleanup:
+ char *buf = NULL;
+ size_t bufsize = 1024 * 1024;
+ int bytesread;
++ struct passwd pwd, *pwd_result;
+
+ /* child doesn't need the read side of the pipe */
+ close(pipefd[0]);
+@@ -6320,6 +6322,26 @@ parent_cleanup:
+ goto child_cleanup;
+ }
+
++ if (VIR_ALLOC_N(buf, bufsize) < 0) {
++ exit_code = ENOMEM;
++ virReportOOMError();
++ goto child_cleanup;
++ }
++
++ exit_code = getpwuid_r(uid, &pwd, buf, bufsize, &pwd_result);
++ if (pwd_result == NULL) {
++ virReportSystemError(errno,
++ _("cannot getpwuid_r(%d) to read '%s'"),
++ uid, path);
++ goto child_cleanup;
++ }
++ if (initgroups(pwd.pw_name, pwd.pw_gid) != 0) {
++ exit_code = errno;
++ virReportSystemError(errno,
++ _("cannot initgroups(\"%s\", %d) to read '%s'"),
++ pwd.pw_name, pwd.pw_gid, path);
++ goto child_cleanup;
++ }
+ if (setuid(uid) != 0) {
+ exit_code = errno;
+ virReportSystemError(errno,
+@@ -6334,11 +6356,6 @@ parent_cleanup:
+ path, uid);
+ goto child_cleanup;
+ }
+- if (VIR_ALLOC_N(buf, bufsize) < 0) {
+- exit_code = ENOMEM;
+- virReportOOMError();
+- goto child_cleanup;
+- }
+
+ /* read from fd and write to pipefd[1] until EOF */
+ do {
diff --git a/debian/patches/security/0019-Replace-setuid-setgid-initgroups-with-virSetUIDGID.patch b/debian/patches/security/0019-Replace-setuid-setgid-initgroups-with-virSetUIDGID.patch
new file mode 100644
index 0000000..46e1785
--- /dev/null
+++ b/debian/patches/security/0019-Replace-setuid-setgid-initgroups-with-virSetUIDGID.patch
@@ -0,0 +1,148 @@
+From: Laine Stump <la...@laine.org>
+Date: Sun, 17 Mar 2013 13:29:13 +0100
+Subject: Replace setuid/setgid/initgroups with virSetUIDGID()
+
+This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406
+
+If qemu is run as a different uid, it has been unable to access mode
+0660 files that are owned by a different user, but with a group that
+the qemu is a member of (aside from the one group listed in the passwd
+file), because initgroups() is not being called prior to the
+exec. initgroups will change the group membership of the process (and
+its children) to match the new uid.
+
+To make this happen, the setregid()/setreuid() code in
+qemuSecurityDACSetProcessLabel has been replaced with a call to
+virSetUIDGID(), which does both of those, plus calls initgroups.
+
+Similar, but not identical, code in qemudOpenAsUID() has been replaced
+with virSetUIDGID(). This not only consolidates the functionality to a
+single location, but also potentially fixes some as-yet unreported
+bugs.
+
+---
+ src/qemu/qemu_driver.c | 44 ++++++++++++++----------------------------
+ src/qemu/qemu_security_dac.c | 18 ++---------------
+ 2 files changed, 16 insertions(+), 46 deletions(-)
+
+diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
+index 5629885..4843884 100644
+--- a/src/qemu/qemu_driver.c
++++ b/src/qemu/qemu_driver.c
+@@ -40,8 +40,6 @@
+ #include <fcntl.h>
+ #include <signal.h>
+ #include <paths.h>
+-#include <pwd.h>
+-#include <grp.h>
+ #include <stdio.h>
+ #include <sys/wait.h>
+ #include <sys/ioctl.h>
+@@ -6242,7 +6240,9 @@ cleanup:
+ after it's finished reading (to avoid a zombie, if nothing
+ else). */
+
+-static int qemudOpenAsUID(const char *path, uid_t uid, pid_t *child_pid) {
++static int
++qemudOpenAsUID(const char *path, uid_t uid, gid_t gid, pid_t *child_pid)
++{
+ int pipefd[2];
+ int fd = -1;
+
+@@ -6309,7 +6309,6 @@ parent_cleanup:
+ char *buf = NULL;
+ size_t bufsize = 1024 * 1024;
+ int bytesread;
+- struct passwd pwd, *pwd_result;
+
+ /* child doesn't need the read side of the pipe */
+ close(pipefd[0]);
+@@ -6322,33 +6321,11 @@ parent_cleanup:
+ goto child_cleanup;
+ }
+
+- if (VIR_ALLOC_N(buf, bufsize) < 0) {
+- exit_code = ENOMEM;
+- virReportOOMError();
+- goto child_cleanup;
++ if (virSetUIDGID(uid, gid) < 0) {
++ exit_code = errno;
++ goto child_cleanup;
+ }
+
+- exit_code = getpwuid_r(uid, &pwd, buf, bufsize, &pwd_result);
+- if (pwd_result == NULL) {
+- virReportSystemError(errno,
+- _("cannot getpwuid_r(%d) to read '%s'"),
+- uid, path);
+- goto child_cleanup;
+- }
+- if (initgroups(pwd.pw_name, pwd.pw_gid) != 0) {
+- exit_code = errno;
+- virReportSystemError(errno,
+- _("cannot initgroups(\"%s\", %d) to read '%s'"),
+- pwd.pw_name, pwd.pw_gid, path);
+- goto child_cleanup;
+- }
+- if (setuid(uid) != 0) {
+- exit_code = errno;
+- virReportSystemError(errno,
+- _("cannot setuid(%d) to read '%s'"),
+- uid, path);
+- goto child_cleanup;
+- }
+ if ((fd = open(path, O_RDONLY)) < 0) {
+ exit_code = errno;
+ virReportSystemError(errno,
+@@ -6357,6 +6334,12 @@ parent_cleanup:
+ goto child_cleanup;
+ }
+
++ if (VIR_ALLOC_N(buf, bufsize) < 0) {
++ exit_code = ENOMEM;
++ virReportOOMError();
++ goto child_cleanup;
++ }
++
+ /* read from fd and write to pipefd[1] until EOF */
+ do {
+ if ((bytesread = saferead(fd, buf, bufsize)) < 0) {
+@@ -6428,7 +6411,8 @@ qemudDomainSaveImageOpen(struct qemud_driver *driver,
+ that might have better luck. Create a pipe, then fork a
+ child process to run as the qemu user, which will hopefully
+ have the necessary authority to read the file. */
+- if ((fd = qemudOpenAsUID(path, driver->user, &read_pid)) < 0) {
++ if ((fd = qemudOpenAsUID(path,
++ driver->user, driver->group, &read_pid)) < 0) {
+ /* error already reported */
+ goto error;
+ }
+diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c
+index 55dc0c6..b2841a1 100644
+--- a/src/qemu/qemu_security_dac.c
++++ b/src/qemu/qemu_security_dac.c
+@@ -549,22 +549,8 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
+ if (!driver->privileged)
+ return 0;
+
+- if (driver->group) {
+- if (setregid(driver->group, driver->group) < 0) {
+- virReportSystemError(errno,
+- _("cannot change to '%d' group"),
+- driver->group);
+- return -1;
+- }
+- }
+- if (driver->user) {
+- if (setreuid(driver->user, driver->user) < 0) {
+- virReportSystemError(errno,
+- _("cannot change to '%d' user"),
+- driver->user);
+- return -1;
+- }
+- }
++ if (virSetUIDGID(driver->user, driver->group) < 0)
++ return -1;
+
+ return 0;
+ }
diff --git a/debian/patches/series b/debian/patches/series
index 1c3ce85..d9d07e2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -14,3 +14,7 @@ security/0013-Add-missing-checks-for-read-only-connections.patch
security/0014-Make-error-reporting-in-libvirtd-thread-safe.patch
security/0015-Fix-integer-overflow-in-VirDomainGetVcpus.patch
0016-Add-missing-return-on-error-path.patch
+security/0017-New-virSetUIDGID-utility-function.patch
+security/0018-Run-initgroups-in-qemudOpenAsUID.patch
+security/0019-Replace-setuid-setgid-initgroups-with-virSetUIDGID.patch
+0020-Rerun-autoconf.patch
