[libvirt] [PATCH 0/3 v2] qemu: Allow serving VNC over a unix domain socket
This series enables use of qemu's -vnc unix:/some/socket/path functionality. A qemu.conf option is provided to make this the default for VNC devices without an explicit listen or socket value. Serving VNC over a unix socket prevents unprivileged local users from accessing a guest's console. The downside is that no clients currently support it (though virt-manager support is ready), and certain common usage scenarios cannot handle the tighter permissions (like a regular user connecting to qemu:///system with policykit). v2: schema: Make listen vs. socket a <choice> Add qemu.conf option for auto allocating a socket Cole Robinson (3): qemu: Set domain def transient at beginning of startup process qemu: Allow serving VNC over a unix domain socket qemu: Add conf option to auto setup VNC unix sockets docs/formatdomain.html.in | 6 ++- docs/schemas/domain.rng | 47 ++++++++++------ src/conf/domain_conf.c | 41 +++++++++----- src/conf/domain_conf.h | 4 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu.conf | 8 +++ src/qemu/qemu_command.c | 60 ++++++++++++++------ src/qemu/qemu_conf.c | 4 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 14 +++-- src/test/test_driver.c | 2 +- src/uml/uml_driver.c | 2 +- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-graphics-vnc-socket.args | 1 + .../qemuxml2argv-graphics-vnc-socket.xml | 30 ++++++++++ tests/qemuxml2argvtest.c | 1 + 16 files changed, 164 insertions(+), 60 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml -- 1.7.3.2
This will allow us to record transient runtime state in vm->def, like default VNC parameters. Accomplish this by adding an extra 'live' parameter to SetDefTransient, with similar semantics to the 'live' flag for AssignDef. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 11 ++++++++--- src/conf/domain_conf.h | 3 ++- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 13 ++++++++----- src/test/test_driver.c | 2 +- src/uml/uml_driver.c | 2 +- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e5b89a2..5b1c907 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1002,17 +1002,22 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps, * * @param caps pointer to capabilities info * @param domain domain object pointer + * @param live if true, run this operation even for an inactive domain. + * this allows freely updated domain->def with runtime defaults before + * starting the VM, which will be discarded on VM shutdown. Any cleanup + * paths need to be sure to handle newDef if the domain is never started. * @return 0 on success, -1 on failure */ int virDomainObjSetDefTransient(virCapsPtr caps, - virDomainObjPtr domain) + virDomainObjPtr domain, + bool live) { int ret = -1; char *xml = NULL; virDomainDefPtr newDef = NULL; - if (!virDomainObjIsActive(domain)) + if (!virDomainObjIsActive(domain) && !live) return 0; if (!domain->persistent) @@ -1047,7 +1052,7 @@ virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainObjPtr domain) { - if (virDomainObjSetDefTransient(caps, domain) < 0) + if (virDomainObjSetDefTransient(caps, domain, false) < 0) return NULL; if (domain->newDef) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 81409f8..3da26e9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1125,7 +1125,8 @@ void virDomainObjAssignDef(virDomainObjPtr domain, const virDomainDefPtr def, bool live); int virDomainObjSetDefTransient(virCapsPtr caps, - virDomainObjPtr domain); + virDomainObjPtr domain, + bool live); virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainObjPtr domain); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index eb58086..5eaccf8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1555,7 +1555,7 @@ static int lxcVmStart(virConnectPtr conn, if (virDomainSaveConfig(driver->stateDir, vm->def) < 0) goto cleanup; - if (virDomainObjSetDefTransient(driver->caps, vm) < 0) + if (virDomainObjSetDefTransient(driver->caps, vm, false) < 0) goto cleanup; rc = 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3745cce..a56b2f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2631,6 +2631,14 @@ static int qemudStartVMDaemon(virConnectPtr conn, return -1; } + /* Do this upfront, so any part of the startup process can add + * runtime state to vm->def that won't be persisted. This let's us + * report implicit runtime defaults in the XML, like vnc listen/socket + */ + DEBUG0("Setting current domain def as transient"); + if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) + goto cleanup; + /* Must be run before security labelling */ DEBUG0("Preparing host devices"); if (qemuPrepareHostDevices(driver, vm->def) < 0) @@ -2924,11 +2932,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto cleanup; - /* Do this last, since it depends on domain being active */ - DEBUG0("Setting running domain def as transient"); - if (virDomainObjSetDefTransient(driver->caps, vm) < 0) - goto cleanup; - virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ddff160..6550832 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -487,7 +487,7 @@ testDomainStartState(virConnectPtr conn, dom->state = VIR_DOMAIN_RUNNING; dom->def->id = privconn->nextDomID++; - if (virDomainObjSetDefTransient(privconn->caps, dom) < 0) { + if (virDomainObjSetDefTransient(privconn->caps, dom, false) < 0) { goto cleanup; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 92b5153..dbceb40 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -891,7 +891,7 @@ static int umlStartVMDaemon(virConnectPtr conn, if (ret < 0) goto cleanup; - ret = virDomainObjSetDefTransient(driver->caps, vm); + ret = virDomainObjSetDefTransient(driver->caps, vm, false); cleanup: virCommandFree(cmd); -- 1.7.3.2
On Wed, Jan 12, 2011 at 12:32:42PM -0500, Cole Robinson wrote:
This will allow us to record transient runtime state in vm->def, like default VNC parameters. Accomplish this by adding an extra 'live' parameter to SetDefTransient, with similar semantics to the 'live' flag for AssignDef.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 11 ++++++++--- src/conf/domain_conf.h | 3 ++- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 13 ++++++++----- src/test/test_driver.c | 2 +- src/uml/uml_driver.c | 2 +- 6 files changed, 21 insertions(+), 12 deletions(-)
ACK Daniel
QEMU supports serving VNC over a unix domain socket rather than traditional TCP host/port. This is specified with: <graphics type='vnc' socket='/foo/bar/baz'/> This provides better security access control than VNC listening on 127.0.0.1, but will cause issues with tools that rely on the lax security (virt-manager in fedora runs as regular user by default, and wouldn't be able to access a socket owned by 'qemu' or 'root'). Also not currently supported by any clients, though I have patches for virt-manager, and virt-viewer should be simple to update. v2: schema: Make listen vs. socket a <choice> Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/formatdomain.html.in | 6 ++- docs/schemas/domain.rng | 47 +++++++++++------- src/conf/domain_conf.c | 30 +++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 52 +++++++++++++------- src/qemu/qemu_driver.c | 1 + tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-graphics-vnc-socket.args | 1 + .../qemuxml2argv-graphics-vnc-socket.xml | 30 +++++++++++ tests/qemuxml2argvtest.c | 1 + 10 files changed, 122 insertions(+), 48 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e9fcea1..fbcff0b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1153,7 +1153,11 @@ qemu-kvm -net nic,model=? /dev/null in clear text. The <code>keymap</code> attribute specifies the keymap to use. It is possible to set a limit on the validity of the password be giving an timestamp <code>passwdValidTo='2010-04-09T15:51:00'</code> - assumed to be in UTC. NB, this may not be supported by all hypervisors. + assumed to be in UTC. NB, this may not be supported by all hypervisors.<br> + <br> + Rather than using listen/port, QEMU supports a <code>socket</code> + attribute for listening on a unix domain socket path. + <span class="since">Since 0.8.8</span> </dd> <dt><code>"spice"</code></dt> <dd> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 9c1e9bb..1907e49 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1104,24 +1104,35 @@ <attribute name="type"> <value>vnc</value> </attribute> - <optional> - <attribute name="port"> - <ref name="PortNumber"/> - </attribute> - </optional> - <optional> - <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> - </optional> - <optional> - <attribute name="listen"> - <ref name="addrIP"/> - </attribute> - </optional> + <choice> + <group> + <optional> + <attribute name="port"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="autoport"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="listen"> + <ref name="addrIP"/> + </attribute> + </optional> + </group> + <group> + <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + </group> + </choice> <optional> <attribute name="passwd"> <text/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5b1c907..4621ae2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -477,6 +477,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: VIR_FREE(def->data.vnc.listenAddr); + VIR_FREE(def->data.vnc.socket); VIR_FREE(def->data.vnc.keymap); virDomainGraphicsAuthDefClear(&def->data.vnc.auth); break; @@ -3365,6 +3366,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags) { } def->data.vnc.listenAddr = virXMLPropString(node, "listen"); + def->data.vnc.socket = virXMLPropString(node, "socket"); def->data.vnc.keymap = virXMLPropString(node, "keymap"); if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth) < 0) @@ -6868,19 +6870,25 @@ virDomainGraphicsDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (def->data.vnc.port && - (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_XML_INACTIVE))) - virBufferVSprintf(buf, " port='%d'", - def->data.vnc.port); - else if (def->data.vnc.autoport) - virBufferAddLit(buf, " port='-1'"); + if (def->data.vnc.socket) { + if (def->data.vnc.socket) + virBufferVSprintf(buf, " socket='%s'", + def->data.vnc.socket); + } else { + if (def->data.vnc.port && + (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_XML_INACTIVE))) + virBufferVSprintf(buf, " port='%d'", + def->data.vnc.port); + else if (def->data.vnc.autoport) + virBufferAddLit(buf, " port='-1'"); - virBufferVSprintf(buf, " autoport='%s'", - def->data.vnc.autoport ? "yes" : "no"); + virBufferVSprintf(buf, " autoport='%s'", + def->data.vnc.autoport ? "yes" : "no"); - if (def->data.vnc.listenAddr) - virBufferVSprintf(buf, " listen='%s'", - def->data.vnc.listenAddr); + if (def->data.vnc.listenAddr) + virBufferVSprintf(buf, " listen='%s'", + def->data.vnc.listenAddr); + } if (def->data.vnc.keymap) virBufferEscapeString(buf, " keymap='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3da26e9..8c637b9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -574,6 +574,7 @@ struct _virDomainGraphicsDef { unsigned int autoport :1; char *listenAddr; char *keymap; + char *socket; virDomainGraphicsAuthDef auth; } vnc; struct { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a0075a4..8e86f43 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3512,7 +3512,11 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBuffer opt = VIR_BUFFER_INITIALIZER; - if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) { + if (def->graphics[0]->data.vnc.socket) { + virBufferVSprintf(&opt, "unix:%s", + def->graphics[0]->data.vnc.socket); + + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) { if (def->graphics[0]->data.vnc.listenAddr) virBufferAdd(&opt, def->graphics[0]->data.vnc.listenAddr, -1); else if (driver->vncListen) @@ -3521,6 +3525,12 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferVSprintf(&opt, ":%d", def->graphics[0]->data.vnc.port - 5900); + } else { + virBufferVSprintf(&opt, "%d", + def->graphics[0]->data.vnc.port - 5900); + } + + if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) { if (def->graphics[0]->data.vnc.auth.passwd || driver->vncPassword) virBufferAddLit(&opt, ",password"); @@ -3545,9 +3555,6 @@ qemuBuildCommandLine(virConnectPtr conn, /* TODO: Support ACLs later */ } - } else { - virBufferVSprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); } virCommandAddArg(cmd, "-vnc"); @@ -5175,24 +5182,33 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; vnc->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC; - tmp = strchr(val, ':'); - if (tmp) { - char *opts; - if (virStrToLong_i(tmp+1, &opts, 10, &vnc->data.vnc.port) < 0) { - VIR_FREE(vnc); - qemuReportError(VIR_ERR_INTERNAL_ERROR, \ - _("cannot parse VNC port '%s'"), tmp+1); - goto error; - } - vnc->data.vnc.listenAddr = strndup(val, tmp-val); - if (!vnc->data.vnc.listenAddr) { + if (STRPREFIX(val, "unix:")) { + vnc->data.vnc.socket = strdup(val + 5); + if (!vnc->data.vnc.socket) { VIR_FREE(vnc); goto no_memory; } - vnc->data.vnc.port += 5900; - vnc->data.vnc.autoport = 0; } else { - vnc->data.vnc.autoport = 1; + tmp = strchr(val, ':'); + if (tmp) { + char *opts; + if (virStrToLong_i(tmp+1, &opts, 10, + &vnc->data.vnc.port) < 0) { + VIR_FREE(vnc); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse VNC port '%s'"), tmp+1); + goto error; + } + vnc->data.vnc.listenAddr = strndup(val, tmp-val); + if (!vnc->data.vnc.listenAddr) { + VIR_FREE(vnc); + goto no_memory; + } + vnc->data.vnc.port += 5900; + vnc->data.vnc.autoport = 0; + } else { + vnc->data.vnc.autoport = 1; + } } if (VIR_REALLOC_N(def->graphics, def->ngraphics+1) < 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a56b2f4..8a9ed42 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2672,6 +2672,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (vm->def->ngraphics == 1) { if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !vm->def->graphics[0]->data.vnc.socket && vm->def->graphics[0]->data.vnc.autoport) { int port = qemudNextFreePort(driver, QEMU_VNC_PORT_MIN); if (port < 0) { diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 8338af3..7499ba0 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -178,6 +178,7 @@ mymain(int argc, char **argv) DO_TEST("disk-drive-network-sheepdog"); DO_TEST("disk-usb"); DO_TEST("graphics-vnc"); + DO_TEST("graphics-vnc-socket"); driver.vncSASL = 1; driver.vncSASLdir = strdup("/root/.sasl2"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args new file mode 100644 index 0000000..055c562 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu -S -M pc -m 214 -smp 1 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -vnc unix:/tmp/foo.socket diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml new file mode 100644 index 0000000..d6ad72b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' socket='/tmp/foo.socket'/> + <video> + <model type='cirrus' vram='9216' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6760f67..8a1d5f7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -316,6 +316,7 @@ mymain(int argc, char **argv) DO_TEST("disk-scsi-device-auto", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); DO_TEST("graphics-vnc", 0, false); + DO_TEST("graphics-vnc-socket", 0, false); driver.vncSASL = 1; driver.vncSASLdir = strdup("/root/.sasl2"); -- 1.7.3.2
On Wed, Jan 12, 2011 at 12:32:43PM -0500, Cole Robinson wrote:
QEMU supports serving VNC over a unix domain socket rather than traditional TCP host/port. This is specified with:
<graphics type='vnc' socket='/foo/bar/baz'/>
This provides better security access control than VNC listening on 127.0.0.1, but will cause issues with tools that rely on the lax security (virt-manager in fedora runs as regular user by default, and wouldn't be able to access a socket owned by 'qemu' or 'root').
Also not currently supported by any clients, though I have patches for virt-manager, and virt-viewer should be simple to update.
v2: schema: Make listen vs. socket a <choice>
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/formatdomain.html.in | 6 ++- docs/schemas/domain.rng | 47 +++++++++++------- src/conf/domain_conf.c | 30 +++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 52 +++++++++++++------- src/qemu/qemu_driver.c | 1 + tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-graphics-vnc-socket.args | 1 + .../qemuxml2argv-graphics-vnc-socket.xml | 30 +++++++++++ tests/qemuxml2argvtest.c | 1 + 10 files changed, 122 insertions(+), 48 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml
ACK My only thought would be whether 'socket' is the best name. Perhaps 'sockpath' or 'path' would be better, but its not a big deal ? Daniel
If vnc_auto_unix_socket is enabled, any VNC devices without a hardcoded listen or socket value will be setup to serve over a unix socket in /var/lib/libvirt/qemu/$vmname.vnc. We store the generated socket path in the transient VM definition at CLI build time. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu.conf | 8 ++++++++ src/qemu/qemu_command.c | 10 +++++++++- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 4 files changed, 22 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ba41f80..ae6136f 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -11,6 +11,14 @@ # # vnc_listen = "0.0.0.0" +# Enable this option to have VNC served over an automatically created +# unix socket. This prevents unprivileged access from users on the +# host machine, though most VNC clients do not support it. +# +# This will only be enabled for VNC configurations that do not have +# a hardcoded 'listen' or 'socket' value. +# +# vnc_auto_unix_socket = 1 # Enable use of TLS encryption on the VNC server. This requires # a VNC client which supports the VeNCrypt protocol extension. diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8e86f43..5015935 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3512,7 +3512,15 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBuffer opt = VIR_BUFFER_INITIALIZER; - if (def->graphics[0]->data.vnc.socket) { + if (def->graphics[0]->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!def->graphics[0]->data.vnc.socket && + virAsprintf(&def->graphics[0]->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + virBufferVSprintf(&opt, "unix:%s", def->graphics[0]->data.vnc.socket); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e1502dc..9f9e99e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -138,6 +138,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, return -1; \ } + p = virConfGetValue (conf, "vnc_auto_unix_socket"); + CHECK_TYPE ("vnc_auto_unix_socket", VIR_CONF_LONG); + if (p) driver->vncAutoUnixSocket = p->l; + p = virConfGetValue (conf, "vnc_tls"); CHECK_TYPE ("vnc_tls", VIR_CONF_LONG); if (p) driver->vncTLS = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..af1be2e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; + unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; unsigned int vncSASL : 1; -- 1.7.3.2
On Wed, Jan 12, 2011 at 12:32:44PM -0500, Cole Robinson wrote:
If vnc_auto_unix_socket is enabled, any VNC devices without a hardcoded listen or socket value will be setup to serve over a unix socket in /var/lib/libvirt/qemu/$vmname.vnc.
We store the generated socket path in the transient VM definition at CLI build time.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu.conf | 8 ++++++++ src/qemu/qemu_command.c | 10 +++++++++- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 4 files changed, 22 insertions(+), 1 deletions(-)
Also needs to change the 2 augeas data files in the qemu directory.
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ba41f80..ae6136f 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -11,6 +11,14 @@ # # vnc_listen = "0.0.0.0"
+# Enable this option to have VNC served over an automatically created +# unix socket. This prevents unprivileged access from users on the +# host machine, though most VNC clients do not support it. +# +# This will only be enabled for VNC configurations that do not have +# a hardcoded 'listen' or 'socket' value. +# +# vnc_auto_unix_socket = 1
We likely need to indicate in here which of 'vnc_auto_unix_socket' and 'vnc_listen' take priority if both are enabled, since they are mutually exclusive. It looks like vnc_listen is totally ignored, if auto_unix_socket is set.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8e86f43..5015935 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3512,7 +3512,15 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBuffer opt = VIR_BUFFER_INITIALIZER;
- if (def->graphics[0]->data.vnc.socket) { + if (def->graphics[0]->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!def->graphics[0]->data.vnc.socket && + virAsprintf(&def->graphics[0]->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + virBufferVSprintf(&opt, "unix:%s", def->graphics[0]->data.vnc.socket);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e1502dc..9f9e99e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -138,6 +138,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, return -1; \ }
+ p = virConfGetValue (conf, "vnc_auto_unix_socket"); + CHECK_TYPE ("vnc_auto_unix_socket", VIR_CONF_LONG); + if (p) driver->vncAutoUnixSocket = p->l; + p = virConfGetValue (conf, "vnc_tls"); CHECK_TYPE ("vnc_tls", VIR_CONF_LONG); if (p) driver->vncTLS = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..af1be2e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; + unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; unsigned int vncSASL : 1;
Regards, Daniel
On 01/13/2011 08:21 AM, Daniel P. Berrange wrote:
On Wed, Jan 12, 2011 at 12:32:44PM -0500, Cole Robinson wrote:
If vnc_auto_unix_socket is enabled, any VNC devices without a hardcoded listen or socket value will be setup to serve over a unix socket in /var/lib/libvirt/qemu/$vmname.vnc.
We store the generated socket path in the transient VM definition at CLI build time.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu.conf | 8 ++++++++ src/qemu/qemu_command.c | 10 +++++++++- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 4 files changed, 22 insertions(+), 1 deletions(-)
Also needs to change the 2 augeas data files in the qemu directory.
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ba41f80..ae6136f 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -11,6 +11,14 @@ # # vnc_listen = "0.0.0.0"
+# Enable this option to have VNC served over an automatically created +# unix socket. This prevents unprivileged access from users on the +# host machine, though most VNC clients do not support it. +# +# This will only be enabled for VNC configurations that do not have +# a hardcoded 'listen' or 'socket' value. +# +# vnc_auto_unix_socket = 1
We likely need to indicate in here which of 'vnc_auto_unix_socket' and 'vnc_listen' take priority if both are enabled, since they are mutually exclusive. It looks like vnc_listen is totally ignored, if auto_unix_socket is set.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8e86f43..5015935 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3512,7 +3512,15 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBuffer opt = VIR_BUFFER_INITIALIZER;
- if (def->graphics[0]->data.vnc.socket) { + if (def->graphics[0]->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!def->graphics[0]->data.vnc.socket && + virAsprintf(&def->graphics[0]->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + virBufferVSprintf(&opt, "unix:%s", def->graphics[0]->data.vnc.socket);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e1502dc..9f9e99e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -138,6 +138,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, return -1; \ }
+ p = virConfGetValue (conf, "vnc_auto_unix_socket"); + CHECK_TYPE ("vnc_auto_unix_socket", VIR_CONF_LONG); + if (p) driver->vncAutoUnixSocket = p->l; + p = virConfGetValue (conf, "vnc_tls"); CHECK_TYPE ("vnc_tls", VIR_CONF_LONG); if (p) driver->vncTLS = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..af1be2e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; + unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; unsigned int vncSASL : 1;
Here's the diff: diff --git a/daemon/test_libvirtd.aug b/daemon/test_libvirtd.aug index 5f8b644..31fa643 100644 --- a/daemon/test_libvirtd.aug +++ b/daemon/test_libvirtd.aug @@ -271,6 +271,9 @@ log_filters=\"a\" # Auditing: audit_level = 2 + +# VNC socket +vnc_auto_unix_socket = 1 " test Libvirtd.lns get conf = @@ -549,3 +552,6 @@ audit_level = 2 { "#empty" } { "#comment" = "Auditing:" } { "audit_level" = "2" } + { "#empty" } + { "#comment" = "VNC socket:" } + { "vnc_auto_unix_socket" = "1" } diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ae6136f..66310d4 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -16,7 +16,8 @@ # host machine, though most VNC clients do not support it. # # This will only be enabled for VNC configurations that do not have -# a hardcoded 'listen' or 'socket' value. +# a hardcoded 'listen' or 'socket' value. This setting takes preference +# over vnc_listen. # # vnc_auto_unix_socket = 1 Anyone have a preference over 'socket' for the XML attribute, or should I just push? Thanks, Cole
On 01/14/2011 09:26 AM, Cole Robinson wrote:
On 01/13/2011 08:21 AM, Daniel P. Berrange wrote:
On Wed, Jan 12, 2011 at 12:32:44PM -0500, Cole Robinson wrote:
If vnc_auto_unix_socket is enabled, any VNC devices without a hardcoded listen or socket value will be setup to serve over a unix socket in /var/lib/libvirt/qemu/$vmname.vnc.
We store the generated socket path in the transient VM definition at CLI build time.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu.conf | 8 ++++++++ src/qemu/qemu_command.c | 10 +++++++++- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 4 files changed, 22 insertions(+), 1 deletions(-)
Also needs to change the 2 augeas data files in the qemu directory.
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ba41f80..ae6136f 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -11,6 +11,14 @@ # # vnc_listen = "0.0.0.0"
+# Enable this option to have VNC served over an automatically created +# unix socket. This prevents unprivileged access from users on the +# host machine, though most VNC clients do not support it. +# +# This will only be enabled for VNC configurations that do not have +# a hardcoded 'listen' or 'socket' value. +# +# vnc_auto_unix_socket = 1
We likely need to indicate in here which of 'vnc_auto_unix_socket' and 'vnc_listen' take priority if both are enabled, since they are mutually exclusive. It looks like vnc_listen is totally ignored, if auto_unix_socket is set.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8e86f43..5015935 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3512,7 +3512,15 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBuffer opt = VIR_BUFFER_INITIALIZER;
- if (def->graphics[0]->data.vnc.socket) { + if (def->graphics[0]->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!def->graphics[0]->data.vnc.socket && + virAsprintf(&def->graphics[0]->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + virBufferVSprintf(&opt, "unix:%s", def->graphics[0]->data.vnc.socket);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e1502dc..9f9e99e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -138,6 +138,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, return -1; \ }
+ p = virConfGetValue (conf, "vnc_auto_unix_socket"); + CHECK_TYPE ("vnc_auto_unix_socket", VIR_CONF_LONG); + if (p) driver->vncAutoUnixSocket = p->l; + p = virConfGetValue (conf, "vnc_tls"); CHECK_TYPE ("vnc_tls", VIR_CONF_LONG); if (p) driver->vncTLS = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..af1be2e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; + unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; unsigned int vncSASL : 1;
Here's the diff:
diff --git a/daemon/test_libvirtd.aug b/daemon/test_libvirtd.aug index 5f8b644..31fa643 100644 --- a/daemon/test_libvirtd.aug +++ b/daemon/test_libvirtd.aug @@ -271,6 +271,9 @@ log_filters=\"a\"
# Auditing: audit_level = 2 + +# VNC socket +vnc_auto_unix_socket = 1 "
test Libvirtd.lns get conf = @@ -549,3 +552,6 @@ audit_level = 2 { "#empty" } { "#comment" = "Auditing:" } { "audit_level" = "2" } + { "#empty" } + { "#comment" = "VNC socket:" } + { "vnc_auto_unix_socket" = "1" } diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ae6136f..66310d4 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -16,7 +16,8 @@ # host machine, though most VNC clients do not support it. # # This will only be enabled for VNC configurations that do not have -# a hardcoded 'listen' or 'socket' value. +# a hardcoded 'listen' or 'socket' value. This setting takes preference +# over vnc_listen. # # vnc_auto_unix_socket = 1
Anyone have a preference over 'socket' for the XML attribute, or should I just push?
I've pushed this series now (though I forgot to squash in the above diff, so it was pushed as a separate commit. Sorry :( ) - Cole
On 01/21/2011 02:23 PM, Cole Robinson wrote:
Here's the diff:
diff --git a/daemon/test_libvirtd.aug b/daemon/test_libvirtd.aug index 5f8b644..31fa643 100644 --- a/daemon/test_libvirtd.aug +++ b/daemon/test_libvirtd.aug @@ -271,6 +271,9 @@ log_filters=\"a\"
# Auditing: audit_level = 2 + +# VNC socket +vnc_auto_unix_socket = 1 "
Problem. This diff is to the libvirtd.conf file,
+++ b/src/qemu/qemu.conf @@ -16,7 +16,8 @@ # host machine, though most VNC clients do not support it. # # This will only be enabled for VNC configurations that do not have -# a hardcoded 'listen' or 'socket' value. +# a hardcoded 'listen' or 'socket' value. This setting takes preference +# over vnc_listen. # # vnc_auto_unix_socket = 1
but the option belongs to the qemu.conf file. I noticed this because a 'make check' run was noisy; but it still succeeded, so something's wrong with our make rule:
make[3]: Entering directory `/home/remote/eblake/libvirt/daemon' test -x '/usr/bin/augparse' \ && '/usr/bin/augparse' -I . ./test_libvirtd.aug || : ./test_libvirtd.aug:279.3-557.40:exception thrown in test ./test_libvirtd.aug:279.8-.29:exception: Iterated lens matched less than it should Lens: ./libvirtd.aug:81.13-.43: Error encountered at 275:0 (8070 characters into string) <dit_level = 2\n\n# VNC socket\n|=|vnc_auto_unix_socket = 1\n>
Tree generated so far: /#comment[1] = "Master libvirt daemon configuration file" ... /#comment[188] = "VNC socket"
Syntax error in lens definition Failed to load ./test_libvirtd.aug make[3]: Leaving directory `/home/remote/eblake/libvirt/daemon'
I believe we need a followup patch which moves the augeas portions into the correct src/qemu/{test_,}libvirtd_qemu.aug. Meanwhile, it would be nice to have 'make check' fail on errors like this. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake