[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Support VNC password for QEMU guests



On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
> This patch adds support for using the monitor interface to set the VNC
> password
> 
>   (qemu) change vnc password
>   Password: ********
> 
> A minor tricky thing is that we can't just send the command and password
> all in one go, we must wait for the 'Password' prompt before sending the
> password.
> 
> When doing this I noticed that virsh dumpxml has no way to request a
> secure XML dump (required to see the password element), nor did the
> virsh edit command set the SECURE or INACTIVE flags when changing
> the XML.
> 
>  qemu_conf.c   |   45 ++++++++++++-----------
>  qemu_driver.c |  112 ++++++++++++++++++++++++++++++++++++++++++++--------------
>  virsh.c       |   30 ++++++++++-----
>  3 files changed, 131 insertions(+), 56 deletions(-)

This update also makes it possible to set a global default password
for all QEMU vms in /etc/libvirt/qemu.conf. This mirrors a similar
option XenD has. A per-VM specified password always takes priority

 qemud/Makefile.am            |    2 
 qemud/libvirtd_qemu.aug      |    1 
 qemud/test_libvirtd.aug      |   18 ++++++
 qemud/test_libvirtd_qemu.aug |   33 +++++++-----
 src/qemu.conf                |   11 ++++
 src/qemu_conf.c              |   58 ++++++++++++++-------
 src/qemu_conf.h              |    1 
 src/qemu_driver.c            |  116 +++++++++++++++++++++++++++++++++----------
 src/uml_driver.c             |    4 +
 src/virsh.c                  |   30 +++++++----
 10 files changed, 204 insertions(+), 70 deletions(-)

Daniel

diff --git a/qemud/Makefile.am b/qemud/Makefile.am
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -246,6 +246,8 @@ libvirtd.init: libvirtd.init.in
 check-local:
 	test -x '$(AUGPARSE)' \
 	  && '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd.aug || :
+	test -x '$(AUGPARSE)' \
+	  && '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd_qemu.aug || :
 
 else
 
diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug
--- a/qemud/libvirtd_qemu.aug
+++ b/qemud/libvirtd_qemu.aug
@@ -26,6 +26,7 @@ module Libvirtd_qemu =
                  | bool_entry "vnc_tls"
                  | str_entry "vnc_tls_x509_cert_dir"
                  | bool_entry "vnc_tls_x509_verify"
+                 | str_entry "vnc_password"
 
    (* Each enty in the config is one of the following three ... *)
    let entry = vnc_entry
diff --git a/qemud/test_libvirtd.aug b/qemud/test_libvirtd.aug
--- a/qemud/test_libvirtd.aug
+++ b/qemud/test_libvirtd.aug
@@ -259,6 +259,15 @@ max_requests = 20
 # this should be a small fraction of the global max_requests
 # and max_workers parameter
 max_client_requests = 5
+
+# Logging level:
+log_level = 4
+
+# Logging outputs:
+log_outputs=\"4:stderr\"
+
+# Logging filters:
+log_filters=\"a\"
 "
 
    test Libvirtd.lns get conf =
@@ -525,3 +534,12 @@ max_client_requests = 5
         { "#comment" = "this should be a small fraction of the global max_requests" }
         { "#comment" = "and max_workers parameter" }
         { "max_client_requests" = "5" }
+	{ "#empty" }
+        { "#comment" = "Logging level:" }
+        { "log_level" = "4" }
+	{ "#empty" }
+        { "#comment" = "Logging outputs:" }
+        { "log_outputs" = "4:stderr" }
+	{ "#empty" }
+        { "#comment" = "Logging filters:" }
+        { "log_filters" = "a" }
diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug
--- a/qemud/test_libvirtd_qemu.aug
+++ b/qemud/test_libvirtd_qemu.aug
@@ -50,14 +50,16 @@ vnc_tls_x509_cert_dir = \"/etc/pki/libvi
 #
 vnc_tls_x509_verify = 1
 
-# Logging level:
-log_level = 4
 
-# Logging outputs:
-log_outputs="4:stderr"
-
-# Logging filters:
-log_filters=""
+# The default VNC password. Only 8 letters are significant for
+# VNC passwords. This parameter is only used if the per-domain
+# XML config does not already provide a password. To allow
+# access without passwords, leave this commented out. An empty
+# string will still enable passwords, but be rejected by QEMU
+# effectively preventing any use of VNC. Obviously change this
+# example here before you set this
+#
+vnc_password = \"XYZ12345\"
 "
 
    test Libvirtd_qemu.lns get conf =
@@ -110,9 +112,14 @@ log_filters=""
 { "#comment" = "certificate signed by the CA in /etc/pki/libvirt-vnc/ca-cert.pem" }
 { "#comment" = "" }
 { "vnc_tls_x509_verify" = "1" }
-{ "#comment" = "Logging level:" }
-{ "log_level" = "4" }
-{ "#comment" = "Logging outputs:" }
-{ "log_outputs" = "4:stderr" }
-{ "#comment" = "Logging filters" }
-{ "log_filters" = "" }
+{ "#empty" }
+{ "#empty" }
+{ "#comment" = "The default VNC password. Only 8 letters are significant for" }
+{ "#comment" = "VNC passwords. This parameter is only used if the per-domain" }
+{ "#comment" = "XML config does not already provide a password. To allow" }
+{ "#comment" = "access without passwords, leave this commented out. An empty" }
+{ "#comment" = "string will still enable passwords, but be rejected by QEMU" }
+{ "#comment" = "effectively preventing any use of VNC. Obviously change this" }
+{ "#comment" = "example here before you set this" }
+{ "#comment" = "" }
+{ "vnc_password" = "XYZ12345" }
diff --git a/src/qemu.conf b/src/qemu.conf
--- a/src/qemu.conf
+++ b/src/qemu.conf
@@ -47,3 +47,14 @@
 # certificate signed by the CA in /etc/pki/libvirt-vnc/ca-cert.pem
 #
 # vnc_tls_x509_verify = 1
+
+
+# The default VNC password. Only 8 letters are significant for
+# VNC passwords. This parameter is only used if the per-domain
+# XML config does not already provide a password. To allow
+# access without passwords, leave this commented out. An empty
+# string will still enable passwords, but be rejected by QEMU
+# effectively preventing any use of VNC. Obviously change this
+# example here before you set this
+#
+# vnc_password = "XYZ12345"
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -129,6 +129,18 @@ int qemudLoadDriverConfig(struct qemud_d
         }
     }
 
+    p = virConfGetValue (conf, "vnc_password");
+    CHECK_TYPE ("vnc_password", VIR_CONF_STRING);
+    if (p && p->str) {
+        VIR_FREE(driver->vncPassword);
+        if (!(driver->vncPassword = strdup(p->str))) {
+            qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY,
+                             "%s", _("failed to allocate vnc_listen"));
+            virConfFree(conf);
+            return -1;
+        }
+    }
+
     virConfFree (conf);
     return 0;
 }
@@ -1138,37 +1150,43 @@ int qemudBuildCommandLine(virConnectPtr 
 
     if (vm->def->graphics &&
         vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-        char vncdisplay[PATH_MAX];
-        int ret;
+        virBuffer opt = VIR_BUFFER_INITIALIZER;
+        char *optstr;
 
         if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) {
-            char options[PATH_MAX] = "";
+            if (vm->def->graphics->data.vnc.listenAddr)
+                virBufferAdd(&opt, vm->def->graphics->data.vnc.listenAddr, -1);
+            else if (driver->vncListen)
+                virBufferAdd(&opt, driver->vncListen, -1);
+
+            virBufferVSprintf(&opt, ":%d",
+                              vm->def->graphics->data.vnc.port - 5900);
+
+            if (vm->def->graphics->data.vnc.passwd ||
+                driver->vncPassword)
+                virBufferAddLit(&opt, ",password");
+
             if (driver->vncTLS) {
-                strcat(options, ",tls");
+                virBufferAddLit(&opt, ",tls");
                 if (driver->vncTLSx509verify) {
-                    strcat(options, ",x509verify=");
+                    virBufferVSprintf(&opt, ",x509verify=%s",
+                                      driver->vncTLSx509certdir);
                 } else {
-                    strcat(options, ",x509=");
+                    virBufferVSprintf(&opt, ",x509=%s",
+                                      driver->vncTLSx509certdir);
                 }
-                strncat(options, driver->vncTLSx509certdir,
-                        sizeof(options) - (strlen(driver->vncTLSx509certdir)-1));
-                options[sizeof(options)-1] = '\0';
             }
-            ret = snprintf(vncdisplay, sizeof(vncdisplay), "%s:%d%s",
-                           (vm->def->graphics->data.vnc.listenAddr ?
-                            vm->def->graphics->data.vnc.listenAddr :
-                            (driver->vncListen ? driver->vncListen : "")),
-                           vm->def->graphics->data.vnc.port - 5900,
-                           options);
         } else {
-            ret = snprintf(vncdisplay, sizeof(vncdisplay), "%d",
-                           vm->def->graphics->data.vnc.port - 5900);
+            virBufferVSprintf(&opt, "%d",
+                              vm->def->graphics->data.vnc.port - 5900);
         }
-        if (ret < 0 || ret >= (int)sizeof(vncdisplay))
-            goto error;
+        if (virBufferError(&opt))
+            goto no_memory;
+
+        optstr = virBufferContentAndReset(&opt);
 
         ADD_ARG_LIT("-vnc");
-        ADD_ARG_LIT(vncdisplay);
+        ADD_ARG(optstr);
         if (vm->def->graphics->data.vnc.keymap) {
             ADD_ARG_LIT("-k");
             ADD_ARG_LIT(vm->def->graphics->data.vnc.keymap);
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -68,6 +68,7 @@ struct qemud_driver {
     unsigned int vncTLSx509verify : 1;
     char *vncTLSx509certdir;
     char *vncListen;
+    char *vncPassword;
 
     virCapsPtr caps;
 
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -74,6 +74,10 @@
 /* For storing short-lived temporary files. */
 #define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt"
 
+#define QEMU_CMD_PROMPT "\n(qemu) "
+#define QEMU_PASSWD_PROMPT "Password: "
+
+
 static int qemudShutdown(void);
 
 #define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -138,9 +142,14 @@ static void qemudShutdownVMDaemon(virCon
 
 static int qemudDomainGetMaxVcpus(virDomainPtr dom);
 
-static int qemudMonitorCommand (const virDomainObjPtr vm,
-                                const char *cmd,
-                                char **reply);
+static int qemudMonitorCommand(const virDomainObjPtr vm,
+                               const char *cmd,
+                               char **reply);
+static int qemudMonitorCommandExtra(const virDomainObjPtr vm,
+                                    const char *cmd,
+                                    const char *extra,
+                                    const char *extraPrompt,
+                                    char **reply);
 
 static struct qemud_driver *qemu_driver = NULL;
 
@@ -583,6 +592,7 @@ qemudShutdown(void) {
     VIR_FREE(qemu_driver->stateDir);
     VIR_FREE(qemu_driver->vncTLSx509certdir);
     VIR_FREE(qemu_driver->vncListen);
+    VIR_FREE(qemu_driver->vncPassword);
 
     /* Free domain callback list */
     virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks);
@@ -1012,6 +1022,39 @@ qemudInitCpus(virConnectPtr conn,
 }
 
 
+static int
+qemudInitPasswords(virConnectPtr conn,
+                   struct qemud_driver *driver,
+                   virDomainObjPtr vm) {
+    char *info = NULL;
+
+    /*
+     * NB: Might have more passwords to set in the future. eg a qcow
+     * disk decryption password, but there's no monitor command
+     * for that yet...
+     */
+
+    if (vm->def->graphics &&
+        vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+        vm->def->graphics->data.vnc.passwd) {
+
+        if (qemudMonitorCommandExtra(vm, "change vnc password",
+                                     vm->def->graphics->data.vnc.passwd ?
+                                     vm->def->graphics->data.vnc.passwd :
+                                     driver->vncPassword,
+                                     QEMU_PASSWD_PROMPT,
+                                     &info) < 0) {
+            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("setting VNC password failed"));
+            return -1;
+        }
+        VIR_FREE(info);
+    }
+
+    return 0;
+}
+
+
 static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
     int i;
 
@@ -1204,7 +1247,8 @@ static int qemudStartVMDaemon(virConnect
     if (ret == 0) {
         if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
             (qemudDetectVcpuPIDs(conn, vm) < 0) ||
-            (qemudInitCpus(conn, vm, migrateFrom) < 0)) {
+            (qemudInitCpus(conn, vm, migrateFrom) < 0) ||
+            (qemudInitPasswords(conn, driver, vm) < 0)) {
             qemudShutdownVMDaemon(conn, driver, vm);
             return -1;
         }
@@ -1314,12 +1358,15 @@ cleanup:
 }
 
 static int
-qemudMonitorCommand (const virDomainObjPtr vm,
-                     const char *cmd,
-                     char **reply) {
+qemudMonitorCommandExtra(const virDomainObjPtr vm,
+                         const char *cmd,
+                         const char *extra,
+                         const char *extraPrompt,
+                         char **reply) {
     int size = 0;
     char *buf = NULL;
     size_t cmdlen = strlen(cmd);
+    size_t extralen = extra ? strlen(extra) : 0;
 
     if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen)
         return -1;
@@ -1355,25 +1402,34 @@ qemudMonitorCommand (const virDomainObjP
         }
 
         /* Look for QEMU prompt to indicate completion */
-        if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
-            char *commptr = NULL, *nlptr = NULL;
-
-            /* Preserve the newline */
-            tmp[1] = '\0';
-
-            /* The monitor doesn't dump clean output after we have written to
-             * it. Every character we write dumps a bunch of useless stuff,
-             * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
-             * Try to throw away everything before the first full command
-             * occurence, and inbetween the command and the newline starting
-             * the response
-             */
-            if ((commptr = strstr(buf, cmd)))
-                memmove(buf, commptr, strlen(commptr)+1);
-            if ((nlptr = strchr(buf, '\n')))
-                memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
-
-            break;
+        if (buf) {
+            if (extra) {
+                if (strstr(buf, extraPrompt) != NULL) {
+                    if (safewrite(vm->monitor, extra, extralen) != extralen)
+                        return -1;
+                    if (safewrite(vm->monitor, "\r", 1) != 1)
+                        return -1;
+                    extra = NULL;
+                }
+            } else if ((tmp = strstr(buf, QEMU_CMD_PROMPT)) != NULL) {
+                char *commptr = NULL, *nlptr = NULL;
+                /* Preserve the newline */
+                tmp[1] = '\0';
+
+                /* The monitor doesn't dump clean output after we have written to
+                 * it. Every character we write dumps a bunch of useless stuff,
+                 * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
+                 * Try to throw away everything before the first full command
+                 * occurence, and inbetween the command and the newline starting
+                 * the response
+                 */
+                if ((commptr = strstr(buf, cmd)))
+                    memmove(buf, commptr, strlen(commptr)+1);
+                if ((nlptr = strchr(buf, '\n')))
+                    memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
+
+                break;
+            }
         }
     pollagain:
         /* Need to wait for more data */
@@ -1403,6 +1459,14 @@ qemudMonitorCommand (const virDomainObjP
     return -1;
 }
 
+static int
+qemudMonitorCommand(const virDomainObjPtr vm,
+                    const char *cmd,
+                    char **reply) {
+    return qemudMonitorCommandExtra(vm, cmd, NULL, NULL, reply);
+}
+
+
 /**
  * qemudProbe:
  *
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -324,6 +324,7 @@ umlStartup(void) {
 
     /* Don't have a dom0 so start from 1 */
     uml_driver->nextvmid = 1;
+    uml_driver->inotifyWatch = -1;
 
     userdir = virGetUserDirectory(NULL, uid);
     if (!userdir)
@@ -484,7 +485,8 @@ umlShutdown(void) {
         return -1;
 
     umlDriverLock(uml_driver);
-    virEventRemoveHandle(uml_driver->inotifyWatch);
+    if (uml_driver->inotifyWatch != -1)
+        virEventRemoveHandle(uml_driver->inotifyWatch);
     close(uml_driver->inotifyFD);
     virCapabilitiesFree(uml_driver->caps);
 
diff --git a/src/virsh.c b/src/virsh.c
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2079,6 +2079,8 @@ static const vshCmdInfo info_dumpxml[] =
 
 static const vshCmdOptDef opts_dumpxml[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
+    {"inactive", VSH_OT_BOOL, 0, gettext_noop("show inactive defined XML")},
+    {"secure", VSH_OT_BOOL, 0, gettext_noop("include security sensitive data")},
     {NULL, 0, 0, NULL}
 };
 
@@ -2088,14 +2090,22 @@ cmdDumpXML(vshControl *ctl, const vshCmd
     virDomainPtr dom;
     int ret = TRUE;
     char *dump;
-
-    if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
-        return FALSE;
-
-    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
-        return FALSE;
-
-    dump = virDomainGetXMLDesc(dom, 0);
+    int flags = 0;
+    int inactive = vshCommandOptBool(cmd, "inactive");
+    int secure = vshCommandOptBool(cmd, "secure");
+
+    if (inactive)
+        flags |= VIR_DOMAIN_XML_INACTIVE;
+    if(secure)
+        flags |= VIR_DOMAIN_XML_SECURE;
+
+    if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+        return FALSE;
+
+    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+        return FALSE;
+
+    dump = virDomainGetXMLDesc(dom, flags);
     if (dump != NULL) {
         printf("%s", dump);
         free(dump);
@@ -5374,7 +5384,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
         goto cleanup;
 
     /* Get the XML configuration of the domain. */
-    doc = virDomainGetXMLDesc (dom, 0);
+    doc = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE);
     if (!doc)
         goto cleanup;
 
@@ -5404,7 +5414,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
      * it was being edited?  This also catches problems such as us
      * losing a connection or the domain going away.
      */
-    doc_reread = virDomainGetXMLDesc (dom, 0);
+    doc_reread = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE);
     if (!doc_reread)
         goto cleanup;
 

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]