mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-01-11 09:17:52 +03:00
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.
This commit is contained in:
parent
d596c6dc9b
commit
f42cf7cb79
@ -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>
|
||||
@ -5499,7 +5497,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;
|
||||
|
||||
@ -5563,7 +5563,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 */
|
||||
VIR_FORCE_CLOSE(pipefd[0]);
|
||||
@ -5576,33 +5575,11 @@ parent_cleanup:
|
||||
goto child_cleanup;
|
||||
}
|
||||
|
||||
if (VIR_ALLOC_N(buf, bufsize) < 0) {
|
||||
exit_code = ENOMEM;
|
||||
virReportOOMError();
|
||||
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,
|
||||
@ -5611,6 +5588,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) {
|
||||
@ -5682,7 +5665,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;
|
||||
}
|
||||
|
@ -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);
|
||||
if (virSetUIDGID(driver->user, driver->group) < 0)
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
if (driver->user) {
|
||||
if (setreuid(driver->user, driver->user) < 0) {
|
||||
virReportSystemError(errno,
|
||||
_("cannot change to '%d' user"),
|
||||
driver->user);
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user