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)
(cherry picked from commit d933f68ee6)
(cherry picked from commit 139a426577)

docs/apibuild.py

@@ -1310,7 +1310,7 @@ class CParser:
         name = None
         self.comment = None
         comment = ""
-        value = "0"
+        value = "-1"
         while token != None:
             if token[0] == "sep" and token[1] == "{":
                 token = self.token()

gnulib/local/top/maint.mk.diff

@@ -0,0 +1,125 @@
+diff --git c/top/maint.mk w/top/maint.mk
+index e52deb4..d5bb427 100644
+--- c/top/maint.mk
++++ w/top/maint.mk
+@@ -2,7 +2,7 @@
+ # This Makefile fragment tries to be general-purpose enough to be
+ # used by many projects via the gnulib maintainer-makefile module.
+
+-## Copyright (C) 2001-2012 Free Software Foundation, Inc.
++## Copyright (C) 2001-2014 Free Software Foundation, Inc.
+ ##
+ ## This program is free software: you can redistribute it and/or modify
+ ## it under the terms of the GNU General Public License as published by
+@@ -155,7 +155,7 @@ export LC_ALL = C
+ ## Sanity checks.  ##
+ ## --------------- ##
+
+-_cfg_mk := $(shell test -f $(srcdir)/cfg.mk && echo '$(srcdir)/cfg.mk')
++_cfg_mk := $(wildcard $(srcdir)/cfg.mk)
+
+ # Collect the names of rules starting with 'sc_'.
+ syntax-check-rules := $(sort $(shell sed -n 's/^\(sc_[a-zA-Z0-9_-]*\):.*/\1/p' \
+@@ -591,7 +591,7 @@ sc_prohibit_root_dev_ino_without_use:
+
+ sc_prohibit_openat_without_use:
+ 	@h='openat.h' \
+-	re='\<(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat)\>' \
++	re='\<(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat|(FCHMOD|FCHOWN|STAT)AT_INLINE)\>' \
+ 	  $(_sc_header_without_use)
+
+ # Prohibit the inclusion of c-ctype.h without an actual use.
+@@ -809,7 +809,7 @@ sc_prohibit_always_true_header_tests:
+ 	  $(_sc_search_regexp)
+
+ sc_prohibit_defined_have_decl_tests:
+-	@prohibit='#[	 ]*if(n?def|.*\<defined)\>[	 (]+HAVE_DECL_'	\
++	@prohibit='(#[	 ]*ifn?def|\<defined)\>[	 (]+HAVE_DECL_'	\
+ 	halt='HAVE_DECL macros are always defined'			\
+ 	  $(_sc_search_regexp)
+
+@@ -1121,9 +1121,11 @@ fix_po_file_diag = \
+ 'you have changed the set of files with translatable diagnostics;\n\
+ apply the above patch\n'
+
+-# Verify that all source files using _() are listed in po/POTFILES.in.
++# Verify that all source files using _() (more specifically, files that
++# match $(_gl_translatable_string_re)) are listed in po/POTFILES.in.
+ po_file ?= $(srcdir)/po/POTFILES.in
+ generated_files ?= $(srcdir)/lib/*.[ch]
++_gl_translatable_string_re ?= \b(N?_|gettext *)\([^)"]*("|$$)
+ sc_po_check:
+ 	@if test -f $(po_file); then					\
+ 	  grep -E -v '^(#|$$)' $(po_file)				\
+@@ -1143,7 +1145,7 @@ sc_po_check:
+ 	    esac;							\
+ 	    files="$$files $$file";					\
+ 	  done;								\
+-	  grep -E -l '\b(N?_|gettext *)\([^)"]*("|$$)' $$files		\
++	  grep -E -l '$(_gl_translatable_string_re)' $$files		\
+ 	    | sed 's|^$(_dot_escaped_srcdir)/||' | sort -u > $@-2;	\
+ 	  diff -u -L $(po_file) -L $(po_file) $@-1 $@-2			\
+ 	    || { printf '$(ME): '$(fix_po_file_diag) 1>&2; exit 1; };	\
+@@ -1281,15 +1283,17 @@ vc-diff-check:
+ rel-files = $(DIST_ARCHIVES)
+
+ gnulib_dir ?= $(srcdir)/gnulib
+-gnulib-version = $$(cd $(gnulib_dir) && git describe)
++gnulib-version = $$(cd $(gnulib_dir)				\
++                    && { git describe || git rev-parse --short=10 HEAD; } )
+ bootstrap-tools ?= autoconf,automake,gnulib
+
++gpgv = $$(gpgv2 --version >/dev/null && echo gpgv2 || echo gpgv)
+ # If it's not already specified, derive the GPG key ID from
+ # the signed tag we've just applied to mark this release.
+ gpg_key_ID ?=								\
+   $$(cd $(srcdir)							\
+      && git cat-file tag v$(VERSION)					\
+-        | gpgv --status-fd 1 --keyring /dev/null - - 2>/dev/null	\
++        | $(gpgv) --status-fd 1 --keyring /dev/null - - 2>/dev/null	\
+         | awk '/^\[GNUPG:\] ERRSIG / {print $$3; exit}')
+
+ translation_project_ ?= coordinator@translationproject.org
+@@ -1313,7 +1317,7 @@ announcement_mail_Cc_ ?= $(announcement_mail_Cc_$(release-type))
+ announcement_mail_headers_ ?= $(announcement_mail_headers_$(release-type))
+ announcement: NEWS ChangeLog $(rel-files)
+ # Not $(AM_V_GEN) since the output of this command serves as
+-# annoucement message: it would start with " GEN announcement".
++# announcement message: it would start with " GEN announcement".
+ 	$(AM_V_at)$(srcdir)/$(_build-aux)/announce-gen			\
+ 	    --mail-headers='$(announcement_mail_headers_)'		\
+ 	    --release-type=$(release-type)				\
+@@ -1368,7 +1372,8 @@ endef
+
+ .PHONY: no-submodule-changes
+ no-submodule-changes:
+-	$(AM_V_GEN)if test -d $(srcdir)/.git; then			\
++	$(AM_V_GEN)if test -d $(srcdir)/.git				\
++		&& git --version >/dev/null 2>&1; then			\
+ 	  diff=$$(cd $(srcdir) && git submodule -q foreach		\
+ 		  git diff-index --name-only HEAD)			\
+ 	    || exit 1;							\
+@@ -1386,10 +1391,12 @@ submodule-checks ?= no-submodule-changes public-submodule-commit
+ # cannot be built from a fresh clone.
+ .PHONY: public-submodule-commit
+ public-submodule-commit:
+-	$(AM_V_GEN)if test -d $(srcdir)/.git; then			\
++	$(AM_V_GEN)if test -d $(srcdir)/.git				\
++		&& git --version >/dev/null 2>&1; then			\
+ 	  cd $(srcdir) &&						\
+-	  git submodule --quiet foreach test '$$(git rev-parse $$sha1)'	\
+-	      = '$$(git merge-base origin $$sha1)'			\
++	  git submodule --quiet foreach					\
++	      'test "$$(git rev-parse "$$sha1")"			\
++		  = "$$(git merge-base origin "$$sha1")"'		\
+ 	    || { echo '$(ME): found non-public submodule commit' >&2;	\
+ 		 exit 1; };						\
+ 	else								\
+@@ -1419,6 +1426,7 @@ alpha beta stable: $(local-check) writable-files $(submodule-checks)
+ 	$(AM_V_at)$(MAKE) -s emit_upload_commands RELEASE_TYPE=$@
+
+ release:
++	$(AM_V_GEN)$(MAKE) _version
+ 	$(AM_V_GEN)$(MAKE) $(release-type)
+
+ # Override this in cfg.mk if you follow different procedures.

src/conf/domain_conf.c

@@ -15272,7 +15272,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/libxl/libxl_driver.c

@@ -520,9 +520,22 @@ libxlVmReap(libxlDriverPrivatePtr driver,
 }
 
 /*
- * Handle previously registered event notification from libxenlight
+ * Handle previously registered event notification from libxenlight.
+ *
+ * Note: Xen 4.3 removed the const from the event handler signature.
+ * Detect which signature to use based on
+ * LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG.
  */
-static void libxlEventHandler(void *data, const libxl_event *event)
+
+#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG
+# define VIR_LIBXL_EVENT_CONST /* empty */
+#else
+# define VIR_LIBXL_EVENT_CONST const
+#endif
+
+static void
+libxlEventHandler(void *data ATTRIBUTE_UNUSED,
+                  VIR_LIBXL_EVENT_CONST libxl_event *event)
 {
     libxlDriverPrivatePtr driver = libxl_driver;
     virDomainObjPtr vm = data;

src/nwfilter/nwfilter_driver.c

@@ -170,6 +170,9 @@ nwfilterDriverStartup(int privileged)
     char *base = NULL;
     DBusConnection *sysbus = NULL;
 
+    if (!privileged)
+        return 0;
+
 #if HAVE_DBUS
     sysbus = virDBusGetSystemBus();
 #endif /* HAVE_DBUS */
@@ -185,9 +188,6 @@ nwfilterDriverStartup(int privileged)
     driverState->watchingFirewallD = (sysbus != NULL);
     driverState->privileged = privileged;
 
-    if (!privileged)
-        return 0;
-
     nwfilterDriverLock(driverState);
 
     if (virNWFilterIPAddrMapInit() < 0)

src/qemu/qemu_driver.c

@@ -8340,26 +8340,6 @@ qemuDomainBlockStats(virDomainPtr dom,
         goto cleanup;
     }
 
-    if (!virDomainObjIsActive(vm)) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       "%s", _("domain is not running"));
-        goto cleanup;
-    }
-
-    if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("invalid path: %s"), path);
-        goto cleanup;
-    }
-    disk = vm->def->disks[i];
-
-    if (!disk->info.alias) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing disk device alias name for %s"), disk->dst);
-        goto cleanup;
-    }
-
-    priv = vm->privateData;
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
         goto cleanup;
 
@@ -8369,6 +8349,21 @@ qemuDomainBlockStats(virDomainPtr dom,
         goto endjob;
     }
 
+    if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("invalid path: %s"), path);
+        goto endjob;
+    }
+    disk = vm->def->disks[i];
+
+    if (!disk->info.alias) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("missing disk device alias name for %s"), disk->dst);
+        goto endjob;
+    }
+
+    priv = vm->privateData;
+
     qemuDomainObjEnterMonitor(driver, vm);
     ret = qemuMonitorGetBlockStatsInfo(priv->mon,
                                        disk->info.alias,
@@ -9164,6 +9159,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
     struct stat sb;
     int i;
     int format;
+    char *alias = NULL;
 
     virCheckFlags(0, -1);
 
@@ -9286,13 +9282,18 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
         virDomainObjIsActive(vm)) {
         qemuDomainObjPrivatePtr priv = vm->privateData;
 
+        if (!(alias = strdup(disk->info.alias))) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
         if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
             goto cleanup;
 
         if (virDomainObjIsActive(vm)) {
             qemuDomainObjEnterMonitor(driver, vm);
             ret = qemuMonitorGetBlockExtent(priv->mon,
-                                            disk->info.alias,
+                                            alias,
                                             &info->allocation);
             qemuDomainObjExitMonitor(driver, vm);
         } else {
@@ -9306,6 +9307,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
     }
 
 cleanup:
+    VIR_FREE(alias);
     virStorageFileFreeMetadata(meta);
     VIR_FORCE_CLOSE(fd);
     if (vm)
@@ -12617,11 +12619,6 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
         goto cleanup;
     }
 
-    device = qemuDiskPathToAlias(vm, path, &idx);
-    if (!device)
-        goto cleanup;
-    disk = vm->def->disks[idx];
-
     if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
 
@@ -12631,6 +12628,11 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
         goto endjob;
     }
 
+    device = qemuDiskPathToAlias(vm, path, &idx);
+    if (!device)
+        goto endjob;
+    disk = vm->def->disks[idx];
+
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     /* XXX - libvirt should really be tracking the backing file chain
      * itself, and validating that base is on the chain, rather than
@@ -13025,12 +13027,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
         goto cleanup;
     }
 
-    device = qemuDiskPathToAlias(vm, disk, NULL);
-
-    if (!device) {
-        goto cleanup;
-    }
-
     if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
 
@@ -13038,6 +13034,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
                                         &persistentDef) < 0)
         goto endjob;
 
+    device = qemuDiskPathToAlias(vm, disk, NULL);
+    if (!device) {
+        goto endjob;
+    }
+
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         priv = vm->privateData;
         qemuDomainObjEnterMonitorWithDriver(driver, vm);
@@ -13048,9 +13049,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

@@ -2358,19 +2358,19 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver,
     time_t now = time(NULL);
     char expire_time [64];
     const char *connected = NULL;
+    const char *password;
     int ret;
 
     if (!auth->passwd && !driver->vncPassword)
         return 0;
 
+    password = auth->passwd ? auth->passwd : defaultPasswd;
+
     if (auth->connected)
         connected = virDomainGraphicsAuthConnectedTypeToString(auth->connected);
 
     qemuDomainObjEnterMonitorWithDriver(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) {
@@ -2378,14 +2378,15 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *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 cleanup;
 
-    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

@@ -4945,7 +4945,7 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn,
     /* If that was the last callback for this eventID, we need to disable
      * events on the server */
     if (count == 0) {
-        args.eventID = callbackID;
+        args.eventID = eventID;
 
         if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER_ANY,
                   (xdrproc_t) xdr_remote_domain_events_deregister_any_args, (char *) &args,

src/rpc/virnetclient.c

@@ -799,7 +799,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
         if (ret < 0 && (errno == EAGAIN || errno == EINTR))
             goto repoll;
 
-        ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL));
+        ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
     }
 
     ret = virNetTLSContextCheckCertificate(tls, client->tls);
@@ -823,7 +823,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
     if (ret < 0 && (errno == EAGAIN || errno == EINTR))
         goto repoll2;
 
-    ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL));
+    ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
 
     len = virNetTLSSessionRead(client->tls, buf, 1);
     if (len < 0 && errno != ENOMSG) {

src/rpc/virnetserverclient.c

@@ -1209,9 +1209,22 @@ cleanup:
 int
 virNetServerClientStartKeepAlive(virNetServerClientPtr client)
 {
-    int ret;
+    int ret = -1;
+
     virNetServerClientLock(client);
+
+    /* The connection might have been closed before we got here and thus the
+     * keepalive object could have been removed too.
+     */
+    if (!client->keepalive) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("connection not open"));
+        goto cleanup;
+    }
+
     ret = virKeepAliveStart(client->keepalive, 0, 0);
+
+cleanup:
     virNetServerClientUnlock(client);
     return ret;
 }

src/util/xml.c

@@ -739,11 +739,11 @@ virXMLParseHelper(int domcode,
 
     if (filename) {
         xml = xmlCtxtReadFile(pctxt, filename, NULL,
-                              XML_PARSE_NOENT | XML_PARSE_NONET |
+                              XML_PARSE_NONET |
                               XML_PARSE_NOWARNING);
     } else {
         xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL,
-                             XML_PARSE_NOENT | XML_PARSE_NONET |
+                             XML_PARSE_NONET |
                              XML_PARSE_NOWARNING);
     }
     if (!xml)

tools/virsh.pod

@@ -2951,6 +2951,8 @@ of C<virsh>
 
 Turn on verbose debugging of virsh commands. Valid levels are
 
+=over 4
+
 =item * VIRSH_DEBUG=0
 
 DEBUG - Messages at ALL levels get logged
@@ -2971,6 +2973,8 @@ WARNING - Logs messages at levels WARNING and ERROR
 
 ERROR - Messages at only ERROR level gets logged.
 
+=back
+
 =item VIRSH_LOG_FILE=C<LOGFILE>
 
 The file to log virsh debug messages.