qemu: Let empty default VNC password work as documented

CVE-2016-5008

Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".

https://bugzilla.redhat.com/show_bug.cgi?id=1180092

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit bb848feec0)

daemon/remote.c

@@ -1576,11 +1576,10 @@ remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (doms && ndomains > 0) {
+    if (doms && ndomains > 0)
         for (i = 0; i < ndomains; i++)
             virDomainFree(doms[i]);
-        VIR_FREE(doms);
+    VIR_FREE(doms);
-    }
     return rv;
 }
 
@@ -4532,11 +4531,10 @@ remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED,
         virNetMessageSaveError(rerr);
     if (dom)
         virDomainFree(dom);
-    if (snaps && nsnaps > 0) {
+    if (snaps && nsnaps > 0)
         for (i = 0; i < nsnaps; i++)
             virDomainSnapshotFree(snaps[i]);
-        VIR_FREE(snaps);
+    VIR_FREE(snaps);
-    }
     return rv;
 }
 
@@ -4603,11 +4601,10 @@ remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNU
         virDomainSnapshotFree(snapshot);
     if (dom)
         virDomainFree(dom);
-    if (snaps && nsnaps > 0) {
+    if (snaps && nsnaps > 0)
         for (i = 0; i < nsnaps; i++)
             virDomainSnapshotFree(snaps[i]);
-        VIR_FREE(snaps);
+    VIR_FREE(snaps);
-    }
     return rv;
 }
 
@@ -4662,11 +4659,10 @@ remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (pools && npools > 0) {
+    if (pools && npools > 0)
         for (i = 0; i < npools; i++)
             virStoragePoolFree(pools[i]);
-        VIR_FREE(pools);
+    VIR_FREE(pools);
-    }
     return rv;
 }
 
@@ -4725,11 +4721,10 @@ remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (vols && nvols > 0) {
+    if (vols && nvols > 0)
         for (i = 0; i < nvols; i++)
             virStorageVolFree(vols[i]);
-        VIR_FREE(vols);
+    VIR_FREE(vols);
-    }
     if (pool)
         virStoragePoolFree(pool);
     return rv;
@@ -4786,11 +4781,10 @@ remoteDispatchConnectListAllNetworks(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (nets && nnets > 0) {
+    if (nets && nnets > 0)
         for (i = 0; i < nnets; i++)
             virNetworkFree(nets[i]);
-        VIR_FREE(nets);
+    VIR_FREE(nets);
-    }
     return rv;
 }
 
@@ -4845,11 +4839,10 @@ remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (ifaces && nifaces > 0) {
+    if (ifaces && nifaces > 0)
         for (i = 0; i < nifaces; i++)
             virInterfaceFree(ifaces[i]);
-        VIR_FREE(ifaces);
+    VIR_FREE(ifaces);
-    }
     return rv;
 }
 
@@ -4904,11 +4897,10 @@ remoteDispatchConnectListAllNodeDevices(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (devices && ndevices > 0) {
+    if (devices && ndevices > 0)
         for (i = 0; i < ndevices; i++)
             virNodeDeviceFree(devices[i]);
-        VIR_FREE(devices);
+    VIR_FREE(devices);
-    }
     return rv;
 }
 
@@ -4963,11 +4955,10 @@ remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (filters && nfilters > 0) {
+    if (filters && nfilters > 0)
         for (i = 0; i < nfilters; i++)
             virNWFilterFree(filters[i]);
-        VIR_FREE(filters);
+    VIR_FREE(filters);
-    }
     return rv;
 }
 
@@ -5022,11 +5013,10 @@ remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (secrets && nsecrets > 0) {
+    if (secrets && nsecrets > 0)
         for (i = 0; i < nsecrets; i++)
             virSecretFree(secrets[i]);
-        VIR_FREE(secrets);
+    VIR_FREE(secrets);
-    }
     return rv;
 }
 
@@ -6207,11 +6197,10 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
  cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
-    if (leases && nleases > 0) {
+    if (leases && nleases > 0)
         for (i = 0; i < nleases; i++)
             virNetworkDHCPLeaseFree(leases[i]);
-        VIR_FREE(leases);
+    VIR_FREE(leases);
-    }
     virNetworkFree(net);
     return rv;
 }

m4/virt-udev.m4

@@ -24,6 +24,13 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[
   if test "$with_udev" = "yes" && test "$with_pciaccess" != "yes" ; then
     AC_MSG_ERROR([You must install the pciaccess module to build with udev])
   fi
+
+  if test "$with_udev" = "yes" ; then
+     PKG_CHECK_EXISTS([libudev >= 218], [with_udev_logging=no], [with_udev_logging=yes])
+     if test "$with_udev_logging" = "yes" ; then
+        AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging can be used])
+     fi
+  fi
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_UDEV],[

src/conf/cpu_conf.c

@@ -158,6 +158,7 @@ virCPUDefCopy(const virCPUDef *cpu)
 
         for (i = 0; i < cpu->ncells; i++) {
             copy->cells[i].mem = cpu->cells[i].mem;
+            copy->cells[i].memAccess = cpu->cells[i].memAccess;
 
             copy->cells[i].cpumask = virBitmapNewCopy(cpu->cells[i].cpumask);
 

src/libvirt-domain.c

@@ -2607,7 +2607,8 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     virCheckDomainReturn(domain, NULL);
     conn = domain->conn;
 
-    if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) {
+    if ((conn->flags & VIR_CONNECT_RO) &&
+        (flags & (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) {
         virReportError(VIR_ERR_OPERATION_DENIED, "%s",
                        _("virDomainGetXMLDesc with secure flag"));
         goto error;

src/lxc/lxc_container.c

@@ -760,7 +760,7 @@ typedef struct {
 
 static const virLXCBasicMountInfo lxcBasicMounts[] = {
     { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false },
-    { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false, false },
+    { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },
     { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false },
     { "securityfs", "/sys/kernel/security", "securityfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
 #if WITH_SELINUX
@@ -926,7 +926,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled,
 
         if (bindOverReadonly &&
             mount(mnt_src, mnt->dst, NULL,
-                  MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
+                  MS_BIND|MS_REMOUNT|mnt_mflags|MS_RDONLY, NULL) < 0) {
             virReportSystemError(errno,
                                  _("Failed to re-mount %s on %s flags=%x"),
                                  mnt_src, mnt->dst,

src/lxc/lxc_driver.c

@@ -584,7 +584,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
 
     if (!virDomainObjIsActive(vm)) {
         info->cpuTime = 0;
-        info->memory = 0;
+        info->memory = vm->def->mem.cur_balloon;
     } else {
         if (virCgroupGetCpuacctUsage(priv->cgroup, &(info->cpuTime)) < 0) {
             virReportError(VIR_ERR_OPERATION_FAILED,

src/node_device/node_device_udev.c

@@ -347,7 +347,7 @@ static int udevGenerateDeviceName(struct udev_device *device,
     return ret;
 }
 
-
+#if HAVE_UDEV_LOGGING
 typedef void (*udevLogFunctionPtr)(struct udev *udev,
                                    int priority,
                                    const char *file,
@@ -380,6 +380,7 @@ udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
 
     VIR_FREE(format);
 }
+#endif
 
 
 static int udevTranslatePCIIds(unsigned int vendor,
@@ -1785,8 +1786,10 @@ static int nodeStateInitialize(bool privileged,
      * its return value.
      */
     udev = udev_new();
+#if HAVE_UDEV_LOGGING
     /* cast to get rid of missing-format-attribute warning */
     udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction);
+#endif
 
     priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
     if (priv->udev_monitor == NULL) {

src/qemu/qemu_cgroup.c

@@ -611,6 +611,7 @@ static int
 qemuSetupCpusetMems(virDomainObjPtr vm,
                     virBitmapPtr nodemask)
 {
+    virCgroupPtr cgroup_temp = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *mem_mask = NULL;
     int ret = -1;
@@ -623,13 +624,16 @@ qemuSetupCpusetMems(virDomainObjPtr vm,
                                             &mem_mask, -1) < 0)
         goto cleanup;
 
-    if (mem_mask &&
+    if (mem_mask)
-        virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0)
+        if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0 ||
-        goto cleanup;
+            virCgroupSetCpusetMems(cgroup_temp, mem_mask) < 0 ||
+            virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0)
+            goto cleanup;
 
     ret = 0;
  cleanup:
     VIR_FREE(mem_mask);
+    virCgroupFree(&cgroup_temp);
     return ret;
 }
 

src/qemu/qemu_driver.c

@@ -2669,7 +2669,7 @@ static int qemuDomainGetInfo(virDomainPtr dom,
             info->memory = vm->def->mem.cur_balloon;
         }
     } else {
-        info->memory = 0;
+        info->memory = vm->def->mem.cur_balloon;
     }
 
     info->nrVirtCpu = vm->def->vcpus;
@@ -6007,7 +6007,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
     if (fd < 0)
         goto cleanup;
 
-    if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
+    if (virDomainSaveImageGetXMLDescEnsureACL(conn, def, flags) < 0)
         goto cleanup;
 
     ret = qemuDomainDefFormatXML(driver, def, flags);
@@ -11350,8 +11350,10 @@ qemuDomainMigratePerform(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
-    if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0)
+    if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0) {
+        virObjectUnlock(vm);
         goto cleanup;
+    }
 
     if (flags & VIR_MIGRATE_PEER2PEER) {
         dconnuri = uri;
@@ -11398,8 +11400,10 @@ qemuDomainMigrateFinish2(virConnectPtr dconn,
         goto cleanup;
     }
 
-    if (virDomainMigrateFinish2EnsureACL(dconn, vm->def) < 0)
+    if (virDomainMigrateFinish2EnsureACL(dconn, vm->def) < 0) {
+        virObjectUnlock(vm);
         goto cleanup;
+    }
 
     /* Do not use cookies in v2 protocol, since the cookie
      * length was not sufficiently large, causing failures
@@ -14377,7 +14381,7 @@ qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
     if (!(vm = qemuDomObjFromSnapshot(snapshot)))
         return NULL;
 
-    if (virDomainSnapshotGetXMLDescEnsureACL(snapshot->domain->conn, vm->def) < 0)
+    if (virDomainSnapshotGetXMLDescEnsureACL(snapshot->domain->conn, vm->def, flags) < 0)
         goto cleanup;
 
     if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
@@ -18514,20 +18518,23 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
         privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
 
     for (i = 0; i < ndoms; i++) {
-        domflags = privflags;
         virDomainStatsRecordPtr tmp = NULL;
+        domflags = 0;
 
         if (!(dom = qemuDomObjFromDomain(doms[i])))
             continue;
 
         if (doms != domlist &&
-            !virConnectGetAllDomainStatsCheckACL(conn, dom->def))
+            !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) {
+            virObjectUnlock(dom);
+            dom = NULL;
             continue;
+        }
 
-        if (HAVE_JOB(domflags) &&
+        if (HAVE_JOB(privflags) &&
-            qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
+            qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0)
-            /* As it was never requested. Gather as much as possible anyway. */
+            domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
-            domflags &= ~QEMU_DOMAIN_STATS_HAVE_JOB;
+        /* else: without a job it's still possible to gather some data */
 
         if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
             goto endjob;
@@ -18535,9 +18542,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
         if (tmp)
             tmpstats[nstats++] = tmp;
 
-        if (HAVE_JOB(domflags) && !qemuDomainObjEndJob(driver, dom)) {
+        if (HAVE_JOB(domflags)) {
-            dom = NULL;
+            domflags = 0;
-            continue;
+            if (!qemuDomainObjEndJob(driver, dom)) {
+                dom = NULL;
+                continue;
+            }
         }
 
         virObjectUnlock(dom);

src/qemu/qemu_hotplug.c

@@ -3589,6 +3589,7 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
     time_t now = time(NULL);
     char expire_time [64];
     const char *connected = NULL;
+    const char *password;
     int ret = -1;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
@@ -3596,16 +3597,14 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
         ret = 0;
         goto cleanup;
     }
+    password = auth->passwd ? auth->passwd : defaultPasswd;
 
     if (auth->connected)
         connected = virDomainGraphicsAuthConnectedTypeToString(auth->connected);
 
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         goto cleanup;
-    ret = qemuMonitorSetPassword(priv->mon,
+    ret = qemuMonitorSetPassword(priv->mon, type, password, connected);
-                                 type,
-                                 auth->passwd ? auth->passwd : defaultPasswd,
-                                 connected);
 
     if (ret == -2) {
         if (type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
@@ -3613,14 +3612,15 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
                            _("Graphics password only supported for VNC"));
             ret = -1;
         } else {
-            ret = qemuMonitorSetVNCPassword(priv->mon,
+            ret = qemuMonitorSetVNCPassword(priv->mon, password);
-                                            auth->passwd ? auth->passwd : defaultPasswd);
         }
     }
     if (ret != 0)
         goto end_job;
 
-    if (auth->expires) {
+    if (password[0] == '\0') {
+        snprintf(expire_time, sizeof(expire_time), "now");
+    } else if (auth->expires) {
         time_t lifetime = auth->validTo - now;
         if (lifetime <= 0)
             snprintf(expire_time, sizeof(expire_time), "now");

src/remote/remote_driver.c

@@ -531,10 +531,6 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED,
         cbdata->freeCallback = NULL;
     }
     virObjectUnlock(cbdata);
-
-    /* free the connection reference that comes along with the callback
-     * registration */
-    virObjectUnref(cbdata->conn);
 }
 
 /* helper macro to ease extraction of arguments from the URI */

src/remote/remote_protocol.x

@@ -3255,6 +3255,7 @@ enum remote_procedure {
      * @generate: both
      * @acl: domain:read
      * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE
+     * @acl: domain:read_secure:VIR_DOMAIN_XML_MIGRATABLE
      */
     REMOTE_PROC_DOMAIN_GET_XML_DESC = 14,
 
@@ -4447,6 +4448,7 @@ enum remote_procedure {
      * @generate: both
      * @priority: high
      * @acl: domain:read
+     * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE
      */
     REMOTE_PROC_DOMAIN_SNAPSHOT_GET_XML_DESC = 186,
 
@@ -4777,6 +4779,7 @@ enum remote_procedure {
      * @generate: both
      * @priority: high
      * @acl: domain:read
+     * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE
      */
     REMOTE_PROC_DOMAIN_SAVE_IMAGE_GET_XML_DESC = 235,
 

src/storage/storage_backend_fs.c

@@ -1,7 +1,7 @@
 /*
  * storage_backend_fs.c: storage backend for FS and directory handling
  *
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -1005,6 +1005,14 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     vol->type = VIR_STORAGE_VOL_FILE;
 
+    /* Volumes within a directory pools are not recursive; do not
+     * allow escape to ../ or a subdir */
+    if (strchr(vol->name, '/')) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("volume name '%s' cannot contain '/'"), vol->name);
+        return -1;
+    }
+
     VIR_FREE(vol->target.path);
     if (virAsprintf(&vol->target.path, "%s/%s",
                     pool->def->target.path,

src/storage/storage_driver.c

@@ -2088,8 +2088,9 @@ storageVolUpload(virStorageVolPtr obj,
             goto cleanup;
     }
 
-    ret = backend->uploadVol(obj->conn, pool, vol, stream,
+    if ((ret = backend->uploadVol(obj->conn, pool, vol, stream,
-                             offset, length, flags);
+                                  offset, length, flags)) < 0)
+        goto cleanup;
 
     /* Add cleanup callback - call after uploadVol since the stream
      * is then fully set up