[libvirt] PATCH: Remove all getuid==0 checks from code
This patch is preparing the way for future work on allowing the libvirtd
daemon to run as a less-privileged user ID. The idea is that we will
switch from 'root' to 'libvirtd', but use Linux capabilties to keep the
handful of higher privileges we need for our work. Thus any code which
does a check of 'getuid() == 0' is guarenteed to break [1].
The way this patch approaches this problem, is to change the driver
initialization function virStateInitialize() to have it be passed in a
'int privileged' flag from the libvirtd daemon. Each driver is updated
to record this flag, and use it for checks where needed. The only real
exception is the Xen driver, where we simply check access(2) against
the file we need to open.
[1] Technically this is already broken on Solaris, since they already
change user id
qemud/qemud.c| 39 ++-
qemud/qemud.h|2 ++
src/driver.h |2 +-
src/libvirt.c|4 ++--
src/libvirt_internal.h |2 +-
src/lxc_driver.c | 25 +++--
src/network_driver.c |4 ++--
src/node_device_devkit.c |2 +-
src/node_device_hal.c|2 +-
src/qemu_conf.h |2 ++
src/qemu_driver.c| 37 +++--
src/remote_internal.c|2 +-
src/storage_driver.c |6 +++---
src/uml_conf.h |2 ++
src/uml_driver.c | 18 +-
src/xen_internal.c |2 +-
src/xen_unified.c|2 +-
17 files changed, 73 insertions(+), 80 deletions(-)
Daniel
diff -r 5e3b5d1f91c2 qemud/qemud.c
--- a/qemud/qemud.c Thu May 21 16:21:20 2009 +0100
+++ b/qemud/qemud.c Thu May 21 16:27:16 2009 +0100
@@ -112,8 +112,6 @@ static int unix_sock_ro_mask = 0666;
#else
-#define SYSTEM_UID 0
-
static gid_t unix_sock_gid = 0; /* Only root by default */
static int unix_sock_rw_mask = 0700; /* Allow user only */
static int unix_sock_ro_mask = 0777; /* Allow world */
@@ -512,7 +510,7 @@ static int qemudListenUnix(struct qemud_
oldgrp = getgid();
oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask);
-if (getuid() == 0)
+if (server-privileged)
setgid(unix_sock_gid);
if (bind(sock-fd, (struct sockaddr *)addr, sizeof(addr)) 0) {
@@ -521,7 +519,7 @@ static int qemudListenUnix(struct qemud_
goto cleanup;
}
umask(oldmask);
-if (getuid() == 0)
+if (server-privileged)
setgid(oldgrp);
if (listen(sock-fd, 30) 0) {
@@ -696,7 +694,6 @@ static int qemudInitPaths(struct qemud_s
char *roSockname,
int maxlen)
{
-uid_t uid = geteuid();
char *sock_dir;
char *dir_prefix = NULL;
int ret = -1;
@@ -706,7 +703,7 @@ static int qemudInitPaths(struct qemud_s
sock_dir = unix_sock_dir;
else {
sock_dir = sockname;
-if (uid == SYSTEM_UID) {
+if (server-privileged) {
dir_prefix = strdup (LOCAL_STATE_DIR);
if (dir_prefix == NULL) {
virReportOOMError(NULL);
@@ -716,6 +713,7 @@ static int qemudInitPaths(struct qemud_s
dir_prefix) = maxlen)
goto snprintf_error;
} else {
+uid_t uid = geteuid();
dir_prefix = virGetUserDirectory(NULL, uid);
if (dir_prefix == NULL) {
/* Do not diagnose here; virGetUserDirectory does that. */
@@ -733,7 +731,7 @@ static int qemudInitPaths(struct qemud_s
goto cleanup;
}
-if (uid == SYSTEM_UID) {
+if (server-privileged) {
if (snprintf (sockname, maxlen, %s/libvirt-sock,
sock_dir_prefix) = maxlen
|| (snprintf (roSockname, maxlen, %s/libvirt-sock-ro,
@@ -747,10 +745,10 @@ static int qemudInitPaths(struct qemud_s
goto snprintf_error;
}
-if (uid == SYSTEM_UID)
- server-logDir = strdup (LOCAL_STATE_DIR /log/libvirt);
+if (server-privileged)
+server-logDir = strdup (LOCAL_STATE_DIR /log/libvirt);
else
- virAsprintf(server-logDir, %s/.libvirt/log, dir_prefix);
+virAsprintf(server-logDir, %s/.libvirt/log, dir_prefix);
if (server-logDir == NULL)
virReportOOMError(NULL);
@@ -786,6 +784,7 @@ static struct qemud_server *qemudInitial
VIR_FREE(server);
}
+server-privileged = getuid() == 0 ? 1 : 0;
server-sigread = sigread;
if (virEventInit() 0) {
@@ -844,7 +843,7 @@ static struct qemud_server *qemudInitial
virEventUpdateTimeoutImpl,
virEventRemoveTimeoutImpl);
-virStateInitialize();
+virStateInitialize(server-privileged);
return server;
}
@@ -915,7 +914,7 @@ static struct qemud_server *qemudNetwork
}
#ifdef HAVE_AVAHI
-if (getuid() == 0 mdns_adv) {
+if (server-privileged mdns_adv) {
Re: [libvirt] PATCH: Remove all getuid==0 checks from code
Quoting Daniel P. Berrange (berra...@redhat.com):
This patch is preparing the way for future work on allowing the libvirtd
daemon to run as a less-privileged user ID. The idea is that we will
switch from 'root' to 'libvirtd', but use Linux capabilties to keep the
handful of higher privileges we need for our work. Thus any code which
does a check of 'getuid() == 0' is guarenteed to break [1].
The way this patch approaches this problem, is to change the driver
initialization function virStateInitialize() to have it be passed in a
'int privileged' flag from the libvirtd daemon. Each driver is updated
to record this flag, and use it for checks where needed. The only real
exception is the Xen driver, where we simply check access(2) against
the file we need to open.
Hi Daniel,
just a few questions:
...
diff -r 5e3b5d1f91c2 qemud/qemud.c
...
@@ -2871,7 +2870,7 @@ int main(int argc, char **argv) {
sigaction(SIGPIPE, sig_action, NULL);
/* Ensure the rundir exists (on tmpfs on some systems) */
-if (geteuid () == 0) {
+if (getuid() == 0) {
Why this change?
...
diff -r 5e3b5d1f91c2 src/qemu_driver.c
--- a/src/qemu_driver.c Thu May 21 16:21:20 2009 +0100
+++ b/src/qemu_driver.c Thu May 21 16:27:16 2009 +0100
@@ -130,24 +130,26 @@ static struct qemud_driver *qemu_driver
static int
-qemudLogFD(virConnectPtr conn, const char* logDir, const char* name)
+qemudLogFD(virConnectPtr conn, struct qemud_driver *driver, const char* name)
{
char logfile[PATH_MAX];
mode_t logmode;
-uid_t uid = geteuid();
int ret, fd = -1;
-if ((ret = snprintf(logfile, sizeof(logfile), %s/%s.log, logDir, name))
+if ((ret = snprintf(logfile, sizeof(logfile), %s/%s.log,
+driver-logDir, name))
0 || ret = sizeof(logfile)) {
virReportOOMError(conn);
return -1;
}
logmode = O_CREAT | O_WRONLY;
-if (uid != 0)
+/* Only logrotate files in /var/log, so only append if running
privileged */
+if (driver-privileged)
+logmode |= O_APPEND;
+else
logmode |= O_TRUNC;
-else
-logmode |= O_APPEND;
Hmm, so if I run as unpriv user my logfiles will always be truncated?
thanks,
-serge
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Remove all getuid==0 checks from code
On Tue, Jun 02, 2009 at 08:29:47AM -0500, Serge E. Hallyn wrote:
Quoting Daniel P. Berrange (berra...@redhat.com):
This patch is preparing the way for future work on allowing the libvirtd
daemon to run as a less-privileged user ID. The idea is that we will
switch from 'root' to 'libvirtd', but use Linux capabilties to keep the
handful of higher privileges we need for our work. Thus any code which
does a check of 'getuid() == 0' is guarenteed to break [1].
The way this patch approaches this problem, is to change the driver
initialization function virStateInitialize() to have it be passed in a
'int privileged' flag from the libvirtd daemon. Each driver is updated
to record this flag, and use it for checks where needed. The only real
exception is the Xen driver, where we simply check access(2) against
the file we need to open.
Hi Daniel,
just a few questions:
...
diff -r 5e3b5d1f91c2 qemud/qemud.c
...
@@ -2871,7 +2870,7 @@ int main(int argc, char **argv) {
sigaction(SIGPIPE, sig_action, NULL);
/* Ensure the rundir exists (on tmpfs on some systems) */
-if (geteuid () == 0) {
+if (getuid() == 0) {
Why this change?
I removed that line originally. And then put it back wrong. Will
fix that.
logmode = O_CREAT | O_WRONLY;
-if (uid != 0)
+/* Only logrotate files in /var/log, so only append if running
privileged */
+if (driver-privileged)
+logmode |= O_APPEND;
+else
logmode |= O_TRUNC;
-else
-logmode |= O_APPEND;
Hmm, so if I run as unpriv user my logfiles will always be truncated?
Yeah, when running as privileged, logs are in /var/log where a logrotate
script takes care of them. With non-privileged, we truncate because we
don't want them to grow without bound forever. Arguably we could make
this a config file option for the daemon...
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
