|
 |
43fe83 |
From e6e8ccf983c15591a86805a8f9d63d7529a87639 Mon Sep 17 00:00:00 2001
|
|
|
43fe83 |
Message-Id: <e6e8ccf983c15591a86805a8f9d63d7529a87639.1379193140.git.jdenemar@redhat.com>
|
|
|
43fe83 |
From: Eric Blake <eblake@redhat.com>
|
|
|
43fe83 |
Date: Thu, 12 Sep 2013 11:37:32 -0600
|
|
|
43fe83 |
Subject: [PATCH] security: provide supplemental groups even when parsing label
|
|
|
43fe83 |
(CVE-2013-4291)
|
|
|
43fe83 |
|
|
|
43fe83 |
https://bugzilla.redhat.com/show_bug.cgi?id=1006513
|
|
|
43fe83 |
|
|
|
43fe83 |
Commit 29fe5d7 (released in 1.1.1) introduced a latent problem
|
|
|
43fe83 |
for any caller of virSecurityManagerSetProcessLabel and where
|
|
|
43fe83 |
the domain already had a uid:gid label to be parsed. Such a
|
|
|
43fe83 |
setup would collect the list of supplementary groups during
|
|
|
43fe83 |
virSecurityManagerPreFork, but then ignores that information,
|
|
|
43fe83 |
and thus fails to call setgroups() to adjust the supplementary
|
|
|
43fe83 |
groups of the process.
|
|
|
43fe83 |
|
|
|
43fe83 |
Upstream does not use virSecurityManagerSetProcessLabel for
|
|
|
43fe83 |
qemu (it uses virSecurityManagerSetChildProcessLabel instead),
|
|
|
43fe83 |
so this problem remained latent until backporting the initial
|
|
|
43fe83 |
commit into v0.10.2-maint (commit c061ff5, released in 0.10.2.7),
|
|
|
43fe83 |
where virSecurityManagerSetChildProcessLabel has not been
|
|
|
43fe83 |
backported. As a result of using a different code path in the
|
|
|
43fe83 |
backport, attempts to start a qemu domain that runs as qemu:qemu
|
|
|
43fe83 |
will end up with supplementary groups unchanged from the libvirtd
|
|
|
43fe83 |
parent process, rather than the desired supplementary groups of
|
|
|
43fe83 |
the qemu user. This can lead to failure to start a domain
|
|
|
43fe83 |
(typical Fedora setup assigns user 107 'qemu' to both group 107
|
|
|
43fe83 |
'qemu' and group 36 'kvm', so a disk image that is only readable
|
|
|
43fe83 |
under kvm group rights is locked out). Worse, it is a security
|
|
|
43fe83 |
hole (the qemu process will inherit supplemental group rights
|
|
|
43fe83 |
from the parent libvirtd process, which means it has access
|
|
|
43fe83 |
rights to files owned by group 0 even when such files should
|
|
|
43fe83 |
not normally be visible to user qemu).
|
|
|
43fe83 |
|
|
|
43fe83 |
LXC does not use the DAC security driver, so it is not vulnerable
|
|
|
43fe83 |
at this time. Still, it is better to plug the latent hole on
|
|
|
43fe83 |
the master branch first, before cherry-picking it to the only
|
|
|
43fe83 |
vulnerable branch v0.10.2-maint.
|
|
|
43fe83 |
|
|
|
43fe83 |
* src/security/security_dac.c (virSecurityDACGetIds): Always populate
|
|
|
43fe83 |
groups and ngroups, rather than only when no label is parsed.
|
|
|
43fe83 |
|
|
|
43fe83 |
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
|
43fe83 |
(cherry picked from commit 745aa55fbf3e076c4288d5ec3239f5a5d43508a6)
|
|
|
43fe83 |
---
|
|
|
43fe83 |
src/security/security_dac.c | 16 ++++++----------
|
|
|
43fe83 |
1 file changed, 6 insertions(+), 10 deletions(-)
|
|
|
43fe83 |
|
|
|
43fe83 |
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
|
|
|
43fe83 |
index 8cbb083..6876bd5 100644
|
|
|
43fe83 |
--- a/src/security/security_dac.c
|
|
|
43fe83 |
+++ b/src/security/security_dac.c
|
|
|
43fe83 |
@@ -115,13 +115,13 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
|
|
|
43fe83 |
return -1;
|
|
|
43fe83 |
}
|
|
|
43fe83 |
|
|
|
43fe83 |
- if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) {
|
|
|
43fe83 |
- if (groups)
|
|
|
43fe83 |
- *groups = NULL;
|
|
|
43fe83 |
- if (ngroups)
|
|
|
43fe83 |
- *ngroups = 0;
|
|
|
43fe83 |
+ if (groups)
|
|
|
43fe83 |
+ *groups = priv ? priv->groups : NULL;
|
|
|
43fe83 |
+ if (ngroups)
|
|
|
43fe83 |
+ *ngroups = priv ? priv->ngroups : 0;
|
|
|
43fe83 |
+
|
|
|
43fe83 |
+ if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
|
|
|
43fe83 |
return ret;
|
|
|
43fe83 |
- }
|
|
|
43fe83 |
|
|
|
43fe83 |
if (!priv) {
|
|
|
43fe83 |
virReportError(VIR_ERR_INTERNAL_ERROR,
|
|
|
43fe83 |
@@ -134,10 +134,6 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
|
|
|
43fe83 |
*uidPtr = priv->user;
|
|
|
43fe83 |
if (gidPtr)
|
|
|
43fe83 |
*gidPtr = priv->group;
|
|
|
43fe83 |
- if (groups)
|
|
|
43fe83 |
- *groups = priv->groups;
|
|
|
43fe83 |
- if (ngroups)
|
|
|
43fe83 |
- *ngroups = priv->ngroups;
|
|
|
43fe83 |
|
|
|
43fe83 |
return 0;
|
|
|
43fe83 |
}
|
|
|
43fe83 |
--
|
|
|
43fe83 |
1.8.3.2
|
|
|
43fe83 |
|