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)

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/domain_conf.c

@@ -6188,10 +6188,11 @@ virDomainControllerDefParseXML(xmlNodePtr node,
     cur = node->children;
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE) {
-            if (xmlStrEqual(cur->name, BAD_CAST "driver"))
+            if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
                 queues = virXMLPropString(cur, "queues");
                 cmd_per_lun = virXMLPropString(cur, "cmd_per_lun");
                 max_sectors = virXMLPropString(cur, "max_sectors");
+            }
         }
         cur = cur->next;
     }
@@ -19721,7 +19722,7 @@ virDomainListPopulate(void *payload,
     /* just count the machines */
     if (!data->domains) {
         data->ndomains++;
-        return;
+        goto cleanup;
     }
 
     if (!(dom = virGetDomain(data->conn, vm->def->name, vm->def->uuid))) {

src/libvirt.c

@@ -4369,7 +4369,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

@@ -759,7 +759,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
@@ -923,7 +923,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,
@@ -1770,8 +1771,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_driver.c

@@ -2579,7 +2579,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;
@@ -5659,7 +5659,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);
@@ -10762,8 +10762,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;
@@ -10810,8 +10812,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
@@ -13453,8 +13457,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
     }
 
     if (redefine) {
-        if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
-                                           &update_current, flags) < 0)
+        if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
+                                          &update_current, flags) < 0)
             goto cleanup;
     } else {
         /* Easiest way to clone inactive portion of vm->def is via
@@ -13880,7 +13884,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
     if (!(vm = qemuDomObjFromSnapshot(snapshot)))
         goto cleanup;
 
-    if (virDomainSnapshotGetXMLDescEnsureACL(snapshot->domain->conn, vm->def) < 0)
+    if (virDomainSnapshotGetXMLDescEnsureACL(snapshot->domain->conn, vm->def, flags) < 0)
         goto cleanup;
 
     if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
@@ -14888,23 +14892,33 @@ qemuDomainBlockPivot(virConnectPtr conn,
         }
     }
 
-    /* We previously labeled only the top-level image; but if the
-     * image includes a relative backing file, the pivot may result in
-     * qemu needing to open the entire backing chain, so we need to
-     * label the entire chain.  This action is safe even if the
-     * backing chain has already been labeled; but only necessary when
-     * we know for sure that there is a backing chain.  */
-    oldsrc = disk->src;
-    disk->src = disk->mirror;
+    /* For active commit, the mirror is part of the already labeled
+     * chain.  For blockcopy, we previously labeled only the top-level
+     * image; but if the user is reusing an external image that
+     * includes a backing file, the pivot may result in qemu needing
+     * to open the entire backing chain, so we need to label the
+     * entire chain.  This action is safe even if the backing chain
+     * has already been labeled; but only necessary when we know for
+     * sure that there is a backing chain.  */
+    if (disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+        oldsrc = disk->src;
+        disk->src = disk->mirror;
 
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
-        goto cleanup;
+        if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
+            goto cleanup;
 
-    if (disk->mirror->format && disk->mirror->format != VIR_STORAGE_FILE_RAW &&
-        (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 ||
-         qemuSetupDiskCgroup(vm, disk) < 0 ||
-         virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, disk) < 0))
-        goto cleanup;
+        if (disk->mirror->format &&
+            disk->mirror->format != VIR_STORAGE_FILE_RAW &&
+            (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm,
+                                     disk) < 0 ||
+             qemuSetupDiskCgroup(vm, disk) < 0 ||
+             virSecurityManagerSetDiskLabel(driver->securityManager, vm->def,
+                                            disk) < 0))
+            goto cleanup;
+
+        disk->src = oldsrc;
+        oldsrc = NULL;
+    }
 
     /* Attempt the pivot.  Record the attempt now, to prevent duplicate
      * attempts; but the actual disk change will be made when emitting
@@ -15103,6 +15117,9 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,
                               bandwidth, info, mode, async);
     qemuDomainObjExitMonitor(driver, vm);
+    if (info && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+        disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+        info->type = disk->mirrorJob;
     if (ret < 0) {
         if (mode == BLOCK_JOB_ABORT && disk->mirror)
             disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
@@ -16039,9 +16056,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
     }
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        int idx = virDomainDiskIndexByName(vm->def, disk, true);
-        if (idx < 0)
+        int idx = virDomainDiskIndexByName(persistentDef, disk, true);
+        if (idx < 0) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           _("disk '%s' was not found in the domain config"),
+                           disk);
             goto endjob;
+        }
         reply = persistentDef->disks[idx]->blkdeviotune;
     }
 

src/qemu/qemu_hotplug.c

@@ -3527,6 +3527,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);
 
@@ -3534,15 +3535,13 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
         ret = 0;
         goto cleanup;
     }
+    password = auth->passwd ? auth->passwd : defaultPasswd;
 
     if (auth->connected)
         connected = virDomainGraphicsAuthConnectedTypeToString(auth->connected);
 
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorSetPassword(priv->mon,
-                                 type,
-                                 auth->passwd ? auth->passwd : defaultPasswd,
-                                 connected);
+    ret = qemuMonitorSetPassword(priv->mon, type, password, connected);
 
     if (ret == -2) {
         if (type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
@@ -3550,14 +3549,15 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
                            _("Graphics password only supported for VNC"));
             ret = -1;
         } else {
-            ret = qemuMonitorSetVNCPassword(priv->mon,
-                                            auth->passwd ? auth->passwd : defaultPasswd);
+            ret = qemuMonitorSetVNCPassword(priv->mon, password);
         }
     }
     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

@@ -518,10 +518,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

@@ -3201,6 +3201,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,
 
@@ -4393,6 +4394,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,
 
@@ -4723,6 +4725,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
@@ -1006,6 +1006,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/util/virnetdev.c

@@ -1354,23 +1354,25 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
 /**
  * virNetDevLinkDump:
  *
- * @ifname: The name of the interface; only use if ifindex < 0
- * @ifindex: The interface index; may be < 0 if ifname is given
- * @nlattr: pointer to a pointer of netlink attributes that will contain
- *          the results
+ * @ifname:  The name of the interface; only use if ifindex <= 0
+ * @ifindex: The interface index; may be <= 0 if ifname is given
+ * @data:    Gets a pointer to the raw data from netlink.
+             MUST BE FREED BY CALLER!
+ * @nlattr:  Pointer to a pointer of netlink attributes that will contain
+ *           the results
  * @src_pid: pid used for nl_pid of the local end of the netlink message
  *           (0 == "use getpid()")
  * @dst_pid: pid of destination nl_pid if the kernel
  *           is not the target of the netlink message but it is to be
  *           sent to another process (0 if sending to the kernel)
  *
- * Get information about an interface given its name or index.
+ * Get information from netlink about an interface given its name or index.
  *
  * Returns 0 on success, -1 on fatal error.
  */
 int
 virNetDevLinkDump(const char *ifname, int ifindex,
-                  struct nlattr **tb,
+                  void **nlData, struct nlattr **tb,
                   uint32_t src_pid, uint32_t dst_pid)
 {
     int rc = -1;
@@ -1452,7 +1454,9 @@ virNetDevLinkDump(const char *ifname, int ifindex,
     rc = 0;
  cleanup:
     nlmsg_free(nl_msg);
-    VIR_FREE(resp);
+    if (rc < 0)
+       VIR_FREE(resp);
+    *nlData = resp;
     return rc;
 
  malformed_resp:
@@ -1648,15 +1652,18 @@ virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac,
                      int *vlanid)
 {
     int rc = -1;
+    void *nlData = NULL;
     struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
     int ifindex = -1;
 
-    rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0);
+    rc = virNetDevLinkDump(ifname, ifindex, &nlData, tb, 0, 0);
     if (rc < 0)
-        return rc;
+        goto cleanup;
 
     rc = virNetDevParseVfConfig(tb, vf, mac, vlanid);
 
+ cleanup:
+    VIR_FREE(nlData);
     return rc;
 }
 
@@ -1799,6 +1806,7 @@ virNetDevRestoreNetConfig(const char *linkdev, int vf, const char *stateDir)
 int
 virNetDevLinkDump(const char *ifname ATTRIBUTE_UNUSED,
                   int ifindex ATTRIBUTE_UNUSED,
+                  void **nlData ATTRIBUTE_UNUSED,
                   struct nlattr **tb ATTRIBUTE_UNUSED,
                   uint32_t src_pid ATTRIBUTE_UNUSED,
                   uint32_t dst_pid ATTRIBUTE_UNUSED)

src/util/virnetdev.h

@@ -130,7 +130,7 @@ int virNetDevGetVirtualFunctions(const char *pfname,
     ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
 
 int virNetDevLinkDump(const char *ifname, int ifindex,
-                      struct nlattr **tb,
+                      void **nlData, struct nlattr **tb,
                       uint32_t src_pid, uint32_t dst_pid)
     ATTRIBUTE_RETURN_CHECK;
 

src/util/virnetdevvportprofile.c

@@ -787,7 +787,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int
                                   int *parent_ifindex, char *parent_ifname,
                                   unsigned int *nth)
 {
-    int rc;
+    int rc = -1;
+    void *nlData = NULL;
     struct nlattr *tb[IFLA_MAX + 1] = { NULL, };
     bool end = false;
     size_t i = 0;
@@ -798,7 +799,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int
         return -1;
 
     while (!end && i <= nthParent) {
-        rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0);
+        VIR_FREE(nlData);
+        rc = virNetDevLinkDump(ifname, ifindex, &nlData, tb, 0, 0);
         if (rc < 0)
             break;
 
@@ -807,7 +809,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int
                            IFNAMSIZ)) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("buffer for root interface name is too small"));
-                return -1;
+                rc = -1;
+                goto cleanup;
             }
             *parent_ifindex = ifindex;
         }
@@ -823,6 +826,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int
 
     *nth = i - 1;
 
+ cleanup:
+    VIR_FREE(nlData);
     return rc;
 }
 
@@ -844,6 +849,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
     int rc;
     int src_pid = 0;
     uint32_t dst_pid = 0;
+    void *nlData = NULL;
     struct nlattr *tb[IFLA_MAX + 1] = { NULL, };
     int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC;
     uint16_t status = 0;
@@ -876,7 +882,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
     }
 
     while (--repeats >= 0) {
-        rc = virNetDevLinkDump(NULL, ifindex, tb, src_pid, dst_pid);
+        VIR_FREE(nlData);
+        rc = virNetDevLinkDump(NULL, ifindex, &nlData, tb, src_pid, dst_pid);
         if (rc < 0)
             goto cleanup;
 
@@ -908,7 +915,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
     }
 
  cleanup:
-
+    VIR_FREE(nlData);
     return rc;
 }
 

src/util/virstoragefile.c

@@ -817,6 +817,7 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
             goto cleanup;
     }
 
+    VIR_FREE(meta->backingStoreRaw);
     if (fileTypeInfo[meta->format].getBackingStore != NULL) {
         int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw,
                                                          backingFormat,

src/util/viruri.c

@@ -258,6 +258,13 @@ virURIFormat(virURIPtr uri)
         xmluri.server = tmpserver;
     }
 
+    /*
+     * This helps libxml2 deal with the difference
+     * between uri:/absolute/path and uri:///absolute/path.
+     */
+    if (!xmluri.server && !xmluri.port)
+        xmluri.port = -1;
+
     ret = (char *)xmlSaveUri(&xmluri);
     if (!ret) {
         virReportOOMError();

src/xen/xend_internal.c

@@ -3338,9 +3338,10 @@ virDomainXMLDevID(virConnectPtr conn,
     xenUnifiedPrivatePtr priv = conn->privateData;
     char *xref;
     char *tmp;
-    const char *driver = virDomainDiskGetDriver(dev->data.disk);
 
     if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+        const char *driver = virDomainDiskGetDriver(dev->data.disk);
+
         if (STREQ_NULLABLE(driver, "tap") || STREQ_NULLABLE(driver, "tap2"))
             strcpy(class, driver);
         else