diff --git a/ChangeLog b/ChangeLog index 842bbfefb4..abb5613b6d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,17 @@ -Thu Jun 12 13:06:42 BST 2009 Daniel P. Berrange +Fri Jun 12 14:16:42 BST 2009 Daniel P. Berrange + + Remove use of getuid()==0 for privilege checks + * qemud/qemud.c, qemud/qemud.h, src/driver.h, src/libvirt.c, + src/libvirt_internal.h, src/lxc_driver.c, src/network_driver.c, + src/node_device_devkit.c, src/node_device_hal.c, + src/qemu_conf.h, src/qemu_driver.c, src/remote_internal.c, + src/storage_driver.c, src/uml_conf.h, src/uml_driver.c, + src/xen_internal.c, src/xen_unified.c: Remove all use of + getuid()/geteuid() to determine if privileged. Replace with + 'privileged' flag provided by libvirtd, or direct access + checks. + +Fri Jun 12 13:36:42 BST 2009 Daniel P. Berrange Include OS driver name (if any) in device XML * src/node_device.c: Refresh OS driver when generating XML, @@ -8,7 +21,7 @@ Thu Jun 12 13:06:42 BST 2009 Daniel P. Berrange * src/node_device_hal.c: Record sysfs path to be used for driver name fetching later. -Thu Jun 12 13:06:42 BST 2009 Daniel P. Berrange +Fri Jun 12 13:06:42 BST 2009 Daniel P. Berrange Improve error reporting for virConnectOpen URIs * src/lxc_driver.c, src/openvz_driver.c, src/qemu_driver.c, @@ -22,7 +35,7 @@ Thu Jun 12 13:06:42 BST 2009 Daniel P. Berrange * src/virterror.c: Improve error message text for VIR_ERR_NO_CONNECT code -Thu Jun 12 12:26:42 BST 2009 Daniel P. Berrange +Fri Jun 12 12:26:42 BST 2009 Daniel P. Berrange Fix re-detection of transient VMs after libvirtd restart * src/domain_conf.c, src/domain_conf.h, src/libvirt_private.syms: diff --git a/qemud/qemud.c b/qemud/qemud.c index bd2ab72dbe..a58a76747b 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -115,8 +115,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 */ @@ -515,7 +513,7 @@ static int qemudListenUnix(struct qemud_server *server, 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) { @@ -524,7 +522,7 @@ static int qemudListenUnix(struct qemud_server *server, goto cleanup; } umask(oldmask); - if (getuid() == 0) + if (server->privileged) setgid(oldgrp); if (listen(sock->fd, 30) < 0) { @@ -699,7 +697,6 @@ static int qemudInitPaths(struct qemud_server *server, char *roSockname, int maxlen) { - uid_t uid = geteuid(); char *sock_dir; char *dir_prefix = NULL; int ret = -1; @@ -709,7 +706,7 @@ static int qemudInitPaths(struct qemud_server *server, 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); @@ -719,6 +716,7 @@ static int qemudInitPaths(struct qemud_server *server, 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. */ @@ -736,7 +734,7 @@ static int qemudInitPaths(struct qemud_server *server, 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", @@ -750,10 +748,10 @@ static int qemudInitPaths(struct qemud_server *server, 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); @@ -789,6 +787,7 @@ static struct qemud_server *qemudInitialize(int sigread) { VIR_FREE(server); } + server->privileged = geteuid() == 0 ? 1 : 0; server->sigread = sigread; if (virEventInit() < 0) { @@ -851,7 +850,7 @@ static struct qemud_server *qemudInitialize(int sigread) { virEventUpdateTimeoutImpl, virEventRemoveTimeoutImpl); - virStateInitialize(); + virStateInitialize(server->privileged); return server; } @@ -922,7 +921,7 @@ static struct qemud_server *qemudNetworkInit(struct qemud_server *server) { } #ifdef HAVE_AVAHI - if (getuid() == 0 && mdns_adv) { + if (server->privileged && mdns_adv) { struct libvirtd_mdns_group *group; int port = 0; @@ -2537,9 +2536,9 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) #if HAVE_POLKIT /* Change the default back to no auth for non-root */ - if (getuid() != 0 && auth_unix_rw == REMOTE_AUTH_POLKIT) + if (!server->privileged && auth_unix_rw == REMOTE_AUTH_POLKIT) auth_unix_rw = REMOTE_AUTH_NONE; - if (getuid() != 0 && auth_unix_ro == REMOTE_AUTH_POLKIT) + if (!server->privileged && auth_unix_ro == REMOTE_AUTH_POLKIT) auth_unix_ro = REMOTE_AUTH_NONE; #endif @@ -2576,7 +2575,7 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) GET_CONF_STR (conf, filename, unix_sock_group); if (unix_sock_group) { - if (getuid() != 0) { + if (!server->privileged) { VIR_WARN0(_("Cannot set group when not running as root")); } else { int ret; @@ -2866,7 +2865,7 @@ int main(int argc, char **argv) { /* If running as root and no PID file is set, use the default */ if (pid_file == NULL && - getuid() == 0 && + geteuid() == 0 && REMOTE_PID_FILE[0] != '\0') pid_file = REMOTE_PID_FILE; @@ -2901,7 +2900,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 (geteuid() == 0) { const char *rundir = LOCAL_STATE_DIR "/run/libvirt"; if (mkdir (rundir, 0755)) { @@ -2912,6 +2911,12 @@ int main(int argc, char **argv) { } } + /* Beyond this point, nothing should rely on using + * getuid/geteuid() == 0, for privilege level checks. + * It must all use the flag 'server->privileged' + * which is also passed into all libvirt stateful + * drivers + */ if (qemudSetupPrivs() < 0) goto error2; @@ -2925,7 +2930,7 @@ int main(int argc, char **argv) { goto error2; /* Change the group ownership of /var/run/libvirt to unix_sock_gid */ - if (unix_sock_dir && geteuid() == 0) { + if (unix_sock_dir && server->privileged) { if (chown(unix_sock_dir, -1, unix_sock_gid) < 0) VIR_ERROR(_("Failed to change group ownership of %s"), unix_sock_dir); diff --git a/qemud/qemud.h b/qemud/qemud.h index e3bae46248..8880337e4a 100644 --- a/qemud/qemud.h +++ b/qemud/qemud.h @@ -172,6 +172,8 @@ struct qemud_server { virMutex lock; virCond job; + int privileged; + int nworkers; int nactiveworkers; struct qemud_worker *workers; diff --git a/src/driver.h b/src/driver.h index ef81af2b95..ca759ff035 100644 --- a/src/driver.h +++ b/src/driver.h @@ -718,7 +718,7 @@ struct _virStorageDriver { }; #ifdef WITH_LIBVIRTD -typedef int (*virDrvStateInitialize) (void); +typedef int (*virDrvStateInitialize) (int privileged); typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); typedef int (*virDrvStateActive) (void); diff --git a/src/libvirt.c b/src/libvirt.c index 262344734a..bf49018cd1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -765,7 +765,7 @@ virRegisterStateDriver(virStateDriverPtr driver) * * Return 0 if all succeed, -1 upon any failure. */ -int virStateInitialize(void) { +int virStateInitialize(int privileged) { int i, ret = 0; if (virInitialize() < 0) @@ -773,7 +773,7 @@ int virStateInitialize(void) { for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->initialize && - virStateDriverTab[i]->initialize() < 0) + virStateDriverTab[i]->initialize(privileged) < 0) ret = -1; } return ret; diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 44145d48af..8800eb904d 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -26,7 +26,7 @@ #ifdef WITH_LIBVIRTD -int virStateInitialize(void); +int virStateInitialize(int privileged); int virStateCleanup(void); int virStateReload(void); int virStateActive(void); diff --git a/src/lxc_driver.c b/src/lxc_driver.c index b286425ab8..e798c79b10 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -52,7 +52,7 @@ #define VIR_FROM_THIS VIR_FROM_LXC -static int lxcStartup(void); +static int lxcStartup(int privileged); static int lxcShutdown(void); static lxc_driver_t *lxc_driver = NULL; @@ -1146,9 +1146,8 @@ static int lxcCheckNetNsSupport(void) return 1; } -static int lxcStartup(void) +static int lxcStartup(int privileged) { - uid_t uid = getuid(); unsigned int i; char *ld; @@ -1161,7 +1160,7 @@ static int lxcStartup(void) return -1; /* Check that the user is root */ - if (0 != uid) { + if (!privileged) { return -1; } diff --git a/src/network_driver.c b/src/network_driver.c index 0c1623cbc5..c5c4c2d733 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -182,7 +182,7 @@ networkAutostartConfigs(struct network_driver *driver) { * Initialization function for the QEmu daemon */ static int -networkStartup(void) { +networkStartup(int privileged) { uid_t uid = geteuid(); char *base = NULL; int err; @@ -196,7 +196,7 @@ networkStartup(void) { } networkDriverLock(driverState); - if (!uid) { + if (privileged) { if (virAsprintf(&driverState->logDir, "%s/log/libvirt/qemu", LOCAL_STATE_DIR) == -1) goto out_of_memory; diff --git a/src/node_device_devkit.c b/src/node_device_devkit.c index 9e3fa6f191..a6c79419f0 100644 --- a/src/node_device_devkit.c +++ b/src/node_device_devkit.c @@ -284,7 +284,7 @@ static void dev_create(void *_dkdev, void *_dkclient ATTRIBUTE_UNUSED) } -static int devkitDeviceMonitorStartup(void) +static int devkitDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) { size_t caps_tbl_len = sizeof(caps_tbl) / sizeof(caps_tbl[0]); DevkitClient *devkit_client = NULL; diff --git a/src/node_device_hal.c b/src/node_device_hal.c index ea519466c5..3e3df7f4f4 100644 --- a/src/node_device_hal.c +++ b/src/node_device_hal.c @@ -685,7 +685,7 @@ static void toggle_dbus_watch(DBusWatch *watch, } -static int halDeviceMonitorStartup(void) +static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) { LibHalContext *hal_ctx = NULL; DBusConnection *dbus_conn = NULL; diff --git a/src/qemu_conf.h b/src/qemu_conf.h index ad6b0a3974..280ad25d3e 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -62,6 +62,8 @@ enum qemud_cmd_flags { struct qemud_driver { virMutex lock; + int privileged; + unsigned int qemuVersion; int nextvmid; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 4a02840eab..665bc8d68c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -127,24 +127,26 @@ static struct qemud_driver *qemu_driver = NULL; 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) - logmode |= O_TRUNC; - else + /* Only logrotate files in /var/log, so only append if running privileged */ + if (driver->privileged) logmode |= O_APPEND; + else + logmode |= O_TRUNC; + if ((fd = open(logfile, logmode, S_IRUSR | S_IWUSR)) < 0) { virReportSystemError(conn, errno, _("failed to create logfile %s"), @@ -207,9 +209,9 @@ qemudAutostartConfigs(struct qemud_driver *driver) { * to lookup the bridge associated with a virtual * network */ - virConnectPtr conn = virConnectOpen(getuid() ? - "qemu:///session" : - "qemu:///system"); + virConnectPtr conn = virConnectOpen(driver->privileged ? + "qemu:///system" : + "qemu:///session"); /* Ignoring NULL conn which is mostly harmless here */ qemuDriverLock(driver); @@ -403,8 +405,7 @@ qemudSecurityInit(struct qemud_driver *qemud_drv) * Initialization function for the QEmu daemon */ static int -qemudStartup(void) { - uid_t uid = geteuid(); +qemudStartup(int privileged) { char *base = NULL; char driverConf[PATH_MAX]; @@ -417,6 +418,7 @@ qemudStartup(void) { return -1; } qemuDriverLock(qemu_driver); + qemu_driver->privileged = privileged; /* Don't have a dom0 so start from 1 */ qemu_driver->nextvmid = 1; @@ -431,7 +433,7 @@ qemudStartup(void) { virEventAddTimeout(-1, qemuDomainEventFlush, qemu_driver, NULL)) < 0) goto error; - if (!uid) { + if (privileged) { if (virAsprintf(&qemu_driver->logDir, "%s/log/libvirt/qemu", LOCAL_STATE_DIR) == -1) goto out_of_memory; @@ -443,6 +445,7 @@ qemudStartup(void) { "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1) goto out_of_memory; } else { + uid_t uid = geteuid(); char *userdir = virGetUserDirectory(NULL, uid); if (!userdir) goto error; @@ -1370,7 +1373,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } - if ((logfile = qemudLogFD(conn, driver->logDir, vm->def->name)) < 0) + if ((logfile = qemudLogFD(conn, driver, vm->def->name)) < 0) goto cleanup; emulator = vm->def->emulator; @@ -1747,13 +1750,11 @@ qemudMonitorCommand(const virDomainObjPtr vm, static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - uid_t uid = getuid(); - if (conn->uri == NULL) { if (qemu_driver == NULL) return VIR_DRV_OPEN_DECLINED; - conn->uri = xmlParseURI(uid == 0 ? + conn->uri = xmlParseURI(qemu_driver->privileged ? "qemu:///system" : "qemu:///session"); if (!conn->uri) { @@ -1770,7 +1771,7 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn, if (conn->uri->server != NULL) return VIR_DRV_OPEN_DECLINED; - if (!uid) { + if (qemu_driver->privileged) { if (STRNEQ (conn->uri->path, "/system") && STRNEQ (conn->uri->path, "/session")) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, diff --git a/src/remote_internal.c b/src/remote_internal.c index e4fa1569c7..6c5d9f8646 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -235,7 +235,7 @@ static gnutls_session_t negotiate_gnutls_on_connection (virConnectPtr conn, stru #ifdef WITH_LIBVIRTD static int -remoteStartup(void) +remoteStartup(int privileged ATTRIBUTE_UNUSED) { /* Mark that we're inside the daemon so we can avoid * re-entering ourselves diff --git a/src/storage_driver.c b/src/storage_driver.c index 48c3bca15f..71e64a445d 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -106,8 +106,7 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { * Initialization function for the QEmu daemon */ static int -storageDriverStartup(void) { - uid_t uid = geteuid(); +storageDriverStartup(int privileged) { char *base = NULL; char driverConf[PATH_MAX]; @@ -120,10 +119,11 @@ storageDriverStartup(void) { } storageDriverLock(driverState); - if (!uid) { + if (privileged) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { + uid_t uid = geteuid(); char *userdir = virGetUserDirectory(NULL, uid); if (!userdir) diff --git a/src/uml_conf.h b/src/uml_conf.h index 7e398850a7..c319396f5b 100644 --- a/src/uml_conf.h +++ b/src/uml_conf.h @@ -42,6 +42,8 @@ struct uml_driver { virMutex lock; + int privileged; + unsigned int umlVersion; int nextvmid; diff --git a/src/uml_driver.c b/src/uml_driver.c index d3e829ad91..7949d4fcb0 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -126,9 +126,9 @@ umlAutostartConfigs(struct uml_driver *driver) { * to lookup the bridge associated with a virtual * network */ - virConnectPtr conn = virConnectOpen(getuid() ? - "uml:///session" : - "uml:///system"); + virConnectPtr conn = virConnectOpen(driver->privileged ? + "uml:///system" : + "uml:///session"); /* Ignoring NULL conn which is mostly harmless here */ for (i = 0 ; i < driver->domains.count ; i++) { @@ -302,7 +302,7 @@ cleanup: * Initialization function for the Uml daemon */ static int -umlStartup(void) { +umlStartup(int privileged) { uid_t uid = geteuid(); char *base = NULL; char driverConf[PATH_MAX]; @@ -311,6 +311,8 @@ umlStartup(void) { if (VIR_ALLOC(uml_driver) < 0) return -1; + uml_driver->privileged = privileged; + if (virMutexInit(¨_driver->lock) < 0) { VIR_FREE(uml_driver); return -1; @@ -325,7 +327,7 @@ umlStartup(void) { if (!userdir) goto error; - if (!uid) { + if (privileged) { if (virAsprintf(¨_driver->logDir, "%s/log/libvirt/uml", LOCAL_STATE_DIR) == -1) goto out_of_memory; @@ -911,13 +913,11 @@ static void umlShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED, static virDrvOpenStatus umlOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - uid_t uid = getuid(); - if (conn->uri == NULL) { if (uml_driver == NULL) return VIR_DRV_OPEN_DECLINED; - conn->uri = xmlParseURI(uid == 0 ? + conn->uri = xmlParseURI(uml_driver->privileged ? "uml:///system" : "uml:///session"); if (!conn->uri) { @@ -935,7 +935,7 @@ static virDrvOpenStatus umlOpen(virConnectPtr conn, /* Check path and tell them correct path if they made a mistake */ - if (uid == 0) { + if (uml_driver->privileged) { if (STRNEQ (conn->uri->path, "/system") && STRNEQ (conn->uri->path, "/session")) { umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, diff --git a/src/xen_internal.c b/src/xen_internal.c index 06b0cbac10..186cead8e2 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -3434,6 +3434,6 @@ xenHavePrivilege() #ifdef __sun return priv_ineffect (PRIV_XVM_CONTROL); #else - return getuid () == 0; + return access(XEN_HYPERVISOR_SOCKET, R_OK) == 0; #endif } diff --git a/src/xen_unified.c b/src/xen_unified.c index 2535f99e8e..27fc56b2f3 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -175,7 +175,7 @@ done: #ifdef WITH_LIBVIRTD static int -xenInitialize (void) +xenInitialize (int privileged ATTRIBUTE_UNUSED) { inside_daemon = 1; return 0;