Package: libvirt / 0.8.3-5+squeeze5

security/0019-Replace-setuid-setgid-initgroups-with-virSetUIDGID.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
From: Laine Stump <laine@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;
 }