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)

src/libvirt_private.syms

@@ -1439,6 +1439,7 @@ virFileSanitizePath;
 virFileSkipRoot;
 virFileStripSuffix;
 virFileTouch;
+virFileUnlink;
 virFileUnlock;
 virFileUpdatePerm;
 virFileWaitForDevices;

src/lxc/lxc_container.c

@@ -850,7 +850,7 @@ typedef struct {
 
 static const virLXCBasicMountInfo lxcBasicMounts[] = {
     { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false, false },
-    { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false, false, false },
+    { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },
     { "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", NULL, MS_BIND, false, false, true },
     { "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", NULL, MS_BIND, false, false, true },
     { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },
@@ -1030,7 +1030,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

@@ -597,7 +597,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/lxc/lxc_process.c

@@ -750,7 +750,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
                                 int *files,
                                 size_t nfiles,
                                 int handshakefd,
-                                int logfd,
+                                int * const logfd,
                                 const char *pidfile)
 {
     size_t i;
@@ -820,8 +820,8 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
     virCommandPassFD(cmd, handshakefd, 0);
     virCommandDaemonize(cmd);
     virCommandSetPidFile(cmd, pidfile);
-    virCommandSetOutputFD(cmd, &logfd);
+    virCommandSetOutputFD(cmd, logfd);
-    virCommandSetErrorFD(cmd, &logfd);
+    virCommandSetErrorFD(cmd, logfd);
     /* So we can pause before exec'ing the controller to
      * write the live domain status XML with the PID */
     virCommandRequireHandshake(cmd);
@@ -1208,7 +1208,7 @@ int virLXCProcessStart(virConnectPtr conn,
                                                 ttyFDs, nttyFDs,
                                                 files, nfiles,
                                                 handshakefds[1],
-                                                logfd,
+                                                &logfd,
                                                 pidfile)))
         goto cleanup;
 

src/qemu/qemu_driver.c

@@ -2694,7 +2694,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;

src/qemu/qemu_hotplug.c

@@ -4013,6 +4013,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);
 
@@ -4020,16 +4021,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) {
@@ -4037,14 +4036,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

@@ -546,10 +546,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/storage/storage_backend.c

@@ -479,6 +479,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
     int fd = -1;
     int operation_flags;
     bool reflink_copy = false;
+    mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
                   VIR_STORAGE_VOL_CREATE_REFLINK,
@@ -511,11 +512,12 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
     if (pool->def->type == VIR_STORAGE_POOL_NETFS)
         operation_flags |= VIR_FILE_OPEN_FORK;
 
+    if (vol->target.perms->mode != (mode_t) -1)
+        open_mode = vol->target.perms->mode;
+
     if ((fd = virFileOpenAs(vol->target.path,
                             O_RDWR | O_CREAT | O_EXCL,
-                            (vol->target.perms->mode ?
+                            open_mode,
-                             VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
-                             vol->target.perms->mode),
                             vol->target.perms->uid,
                             vol->target.perms->gid,
                             operation_flags)) < 0) {

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
@@ -770,7 +770,8 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
     char *parent = NULL;
     char *p = NULL;
     mode_t mode;
-    bool needs_create_as_uid, dir_create_flags;
+    bool needs_create_as_uid;
+    unsigned int dir_create_flags;
 
     virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
                   VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
@@ -1039,6 +1040,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,
@@ -1185,7 +1194,8 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     switch ((virStorageVolType) vol->type) {
     case VIR_STORAGE_VOL_FILE:
-        if (unlink(vol->target.path) < 0) {
+        if (virFileUnlink(vol->target.path, vol->target.perms->uid,
+                          vol->target.perms->gid) < 0) {
             /* Silently ignore failures where the vol has already gone away */
             if (errno != ENOENT) {
                 virReportSystemError(errno,

src/storage/storage_driver.c

@@ -1872,8 +1872,12 @@ storageVolCreateXML(virStoragePoolPtr obj,
     }
 
     if (backend->refreshVol &&
-        backend->refreshVol(obj->conn, pool, voldef) < 0)
+        backend->refreshVol(obj->conn, pool, voldef) < 0) {
+        storageVolDeleteInternal(volobj, backend, pool, voldef,
+                                 0, false);
+        voldef = NULL;
         goto cleanup;
+    }
 
     /* Update pool metadata ignoring the disk backend since
      * it updates the pool values.

src/util/virfile.c

@@ -2280,6 +2280,112 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
     return ret;
 }
 
+
+/* virFileUnlink:
+ * @path: file to unlink
+ * @uid: uid that was used to create the file (not required)
+ * @gid: gid that was used to create the file (not required)
+ *
+ * If a file/volume was created in an NFS root-squash environment,
+ * then we must 'unlink' the file in the same environment. Unlike
+ * the virFileOpenAs[Forked] and virDirCreate[NoFork], this code
+ * takes no extra flags and does not bother with EACCES failures
+ * from the child.
+ */
+int
+virFileUnlink(const char *path,
+              uid_t uid,
+              gid_t gid)
+{
+    pid_t pid;
+    int waitret;
+    int status, ret = 0;
+    gid_t *groups;
+    int ngroups;
+
+    /* If not running as root or if a non explicit uid/gid was being used for
+     * the file/volume, then use unlink directly
+     */
+    if ((geteuid() != 0) ||
+        ((uid == (uid_t) -1) && (gid == (gid_t) -1)))
+        return unlink(path);
+
+    /* Otherwise, we have to deal with the NFS root-squash craziness
+     * to run under the uid/gid that created the volume in order to
+     * perform the unlink of the volume.
+     */
+    if (uid == (uid_t) -1)
+        uid = geteuid();
+    if (gid == (gid_t) -1)
+        gid = getegid();
+
+    ngroups = virGetGroupList(uid, gid, &groups);
+    if (ngroups < 0)
+        return -errno;
+
+    pid = virFork();
+
+    if (pid < 0) {
+        ret = -errno;
+        VIR_FREE(groups);
+        return ret;
+    }
+
+    if (pid) { /* parent */
+        /* wait for child to complete, and retrieve its exit code */
+        VIR_FREE(groups);
+
+        while ((waitret = waitpid(pid, &status, 0)) == -1 && errno == EINTR);
+        if (waitret == -1) {
+            ret = -errno;
+            virReportSystemError(errno,
+                                 _("failed to wait for child unlinking '%s'"),
+                                 path);
+            goto parenterror;
+        }
+
+        /*
+         * If waitpid succeeded, but if the child exited abnormally or
+         * reported non-zero status, report failure
+         */
+        if (!WIFEXITED(status) || (WEXITSTATUS(status)) != 0) {
+            char *msg = virProcessTranslateStatus(status);
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("child failed to unlink '%s': %s"),
+                           path, msg);
+            VIR_FREE(msg);
+            if (WIFEXITED(status))
+                ret = -WEXITSTATUS(status);
+            else
+                ret = -EACCES;
+        }
+
+ parenterror:
+        return ret;
+    }
+
+    /* child */
+
+    /* set desired uid/gid, then attempt to unlink the file */
+    if (virSetUIDGID(uid, gid, groups, ngroups) < 0) {
+        ret = errno;
+        goto childerror;
+    }
+
+    if (unlink(path) < 0) {
+        ret = errno;
+        goto childerror;
+    }
+
+ childerror:
+    if ((ret & 0xff) != ret) {
+        VIR_WARN("unable to pass desired return value %d", ret);
+        ret = 0xff;
+    }
+    _exit(ret);
+}
+
+
 /* return -errno on failure, or 0 on success */
 static int
 virDirCreateNoFork(const char *path,

src/util/virfile.h

@@ -219,6 +219,7 @@ int virFileOpenAs(const char *path, int openflags, mode_t mode,
                   uid_t uid, gid_t gid,
                   unsigned int flags)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+int virFileUnlink(const char *path, uid_t uid, gid_t gid);
 
 enum {
     VIR_DIR_CREATE_NONE        = 0,