Remove use of getuid()==0 for privilege checks

ChangeLog

@@ -1,4 +1,17 @@
-Thu Jun 12 13:06:42 BST 2009 Daniel P. Berrange <berrange@redhat.com>
+Fri Jun 12 14:16:42 BST 2009 Daniel P. Berrange <berrange@redhat.com>
+
+	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 <berrange@redhat.com>
 
 	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 <berrange@redhat.com>
 	* 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 <berrange@redhat.com>
+Fri Jun 12 13:06:42 BST 2009 Daniel P. Berrange <berrange@redhat.com>
 
 	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 <berrange@redhat.com>
 	* src/virterror.c: Improve error message text for
 	VIR_ERR_NO_CONNECT code
 
-Thu Jun 12 12:26:42 BST 2009 Daniel P. Berrange <berrange@redhat.com>
+Fri Jun 12 12:26:42 BST 2009 Daniel P. Berrange <berrange@redhat.com>
 
 	Fix re-detection of transient VMs after libvirtd restart
 	* src/domain_conf.c, src/domain_conf.h, src/libvirt_private.syms:

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)
+    if (server->privileged)
-      server->logDir = strdup (LOCAL_STATE_DIR "/log/libvirt");
+        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);

qemud/qemud.h

@@ -172,6 +172,8 @@ struct qemud_server {
     virMutex lock;
     virCond job;
 
+    int privileged;
+
     int nworkers;
     int nactiveworkers;
     struct qemud_worker *workers;

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);

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;

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);

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;
     }
 

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;

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;

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;

src/qemu_conf.h

@@ -62,6 +62,8 @@ enum qemud_cmd_flags {
 struct qemud_driver {
     virMutex lock;
 
+    int privileged;
+
     unsigned int qemuVersion;
     int nextvmid;
 

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)
+    /* Only logrotate files in /var/log, so only append if running privileged */
-        logmode |= O_TRUNC;
+    if (driver->privileged)
-    else
         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() ?
+    virConnectPtr conn = virConnectOpen(driver->privileged ?
-                                        "qemu:///session" :
+                                        "qemu:///system" :
-                                        "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) {
+qemudStartup(int privileged) {
-    uid_t uid = geteuid();
     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,

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

src/storage_driver.c

@@ -106,8 +106,7 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
  * Initialization function for the QEmu daemon
  */
 static int
-storageDriverStartup(void) {
+storageDriverStartup(int privileged) {
-    uid_t uid = geteuid();
     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)

src/uml_conf.h

@@ -42,6 +42,8 @@
 struct uml_driver {
     virMutex lock;
 
+    int privileged;
+
     unsigned int umlVersion;
     int nextvmid;
 

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() ?
+    virConnectPtr conn = virConnectOpen(driver->privileged ?
-                                        "uml:///session" :
+                                        "uml:///system" :
-                                        "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(&uml_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(&uml_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,

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
 }

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;