[PATCH] libxl: Query VNC listening port through QMP

[PATCH] libxl: Query VNC listening port through QMP

[Anthony PERARD]

Currently `xl vncviewer $dom` does not work because the VNC port is not
registered in xenstore when using qemu-upstream. This patch attempted to fix
this.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
 tools/libxl/libxl_qmp.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f5a3edc..72ff4a4 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -154,6 +154,55 @@ static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
     return ret;
 }
 
+static int qmp_write_domain_console_item(libxl__gc *gc, int domid,
+                                         const char *item, const char *value)
+{
+    char *path;
+
+    path = libxl__xs_get_dompath(gc, domid);
+    path = libxl__sprintf(gc, "%s/console/%s", path, item);
+
+    return libxl__xs_write(gc, XBT_NULL, path, "%s", value);
+}
+
+static int qmp_register_vnc_callback(libxl__qmp_handler *qmp,
+                                     const libxl__json_object *o,
+                                     void *unused)
+{
+    GC_INIT(qmp->ctx);
+    const libxl__json_object *obj;
+    const char *listen, *port;
+    int rc = -1;
+
+    if (!libxl__json_object_is_map(o)) {
+        goto out;
+    }
+
+    if (libxl__json_map_get("enabled", o, JSON_FALSE)) {
+        rc = 0;
+        goto out;
+    }
+
+    obj = libxl__json_map_get("host", o, JSON_STRING);
+    listen = libxl__json_object_get_string(obj);
+    obj = libxl__json_map_get("service", o, JSON_STRING);
+    port = libxl__json_object_get_string(obj);
+
+    if (!listen || !port) {
+        LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
+                   "Failed to retreive VNC connect information.");
+        goto out;
+    }
+
+    rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-listen", listen);
+    if (!rc)
+        rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-port", port);
+
+out:
+    GC_FREE;
+    return rc;
+}
+
 static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
                                      const libxl__json_object *o, void *unused)
 {
@@ -688,6 +737,13 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
                                 NULL, qmp->timeout);
 }
 
+static int qmp_query_vnc(libxl__qmp_handler *qmp)
+{
+    return qmp_synchronous_send(qmp, "query-vnc", NULL,
+                                qmp_register_vnc_callback,
+                                NULL, qmp->timeout);
+}
+
 static int pci_add_callback(libxl__qmp_handler *qmp,
                             const libxl__json_object *response, void *opaque)
 {
@@ -917,6 +973,9 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
     if (!ret && vnc && vnc->passwd) {
         ret = qmp_change(gc, qmp, "vnc", "password", vnc->passwd);
     }
+    if (!ret) {
+        ret = qmp_query_vnc(qmp);
+    }
     libxl__qmp_close(qmp);
     return ret;
 }
-- 
Anthony PERARD

Re: [PATCH] libxl: Query VNC listening port through QMP

[Ian Campbell]

On Mon, 2012-04-16 at 19:15 +0100, Anthony PERARD wrote:
> Currently `xl vncviewer $dom` does not work because the VNC port is not
> registered in xenstore when using qemu-upstream. This patch attempted to fix
> this.

libxl_vncviewer_exec also potentially reads vnc-pass, although frankly
having such a thing in xenstore strikes me as something of a
misfeature...

Otherwise: 
Acked-by: Ian Campbell <ian.campbell@citrix.com>

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> ---
>  tools/libxl/libxl_qmp.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 59 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index f5a3edc..72ff4a4 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -154,6 +154,55 @@ static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
>      return ret;
>  }
>  
> +static int qmp_write_domain_console_item(libxl__gc *gc, int domid,
> +                                         const char *item, const char *value)
> +{
> +    char *path;
> +
> +    path = libxl__xs_get_dompath(gc, domid);
> +    path = libxl__sprintf(gc, "%s/console/%s", path, item);
> +
> +    return libxl__xs_write(gc, XBT_NULL, path, "%s", value);
> +}
> +
> +static int qmp_register_vnc_callback(libxl__qmp_handler *qmp,
> +                                     const libxl__json_object *o,
> +                                     void *unused)
> +{
> +    GC_INIT(qmp->ctx);
> +    const libxl__json_object *obj;
> +    const char *listen, *port;
> +    int rc = -1;
> +
> +    if (!libxl__json_object_is_map(o)) {
> +        goto out;
> +    }
> +
> +    if (libxl__json_map_get("enabled", o, JSON_FALSE)) {
> +        rc = 0;
> +        goto out;
> +    }
> +
> +    obj = libxl__json_map_get("host", o, JSON_STRING);
> +    listen = libxl__json_object_get_string(obj);
> +    obj = libxl__json_map_get("service", o, JSON_STRING);
> +    port = libxl__json_object_get_string(obj);
> +
> +    if (!listen || !port) {
> +        LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> +                   "Failed to retreive VNC connect information.");
> +        goto out;
> +    }
> +
> +    rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-listen", listen);
> +    if (!rc)
> +        rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-port", port);
> +
> +out:
> +    GC_FREE;
> +    return rc;
> +}
> +
>  static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
>                                       const libxl__json_object *o, void *unused)
>  {
> @@ -688,6 +737,13 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
>                                  NULL, qmp->timeout);
>  }
>  
> +static int qmp_query_vnc(libxl__qmp_handler *qmp)
> +{
> +    return qmp_synchronous_send(qmp, "query-vnc", NULL,
> +                                qmp_register_vnc_callback,
> +                                NULL, qmp->timeout);
> +}
> +
>  static int pci_add_callback(libxl__qmp_handler *qmp,
>                              const libxl__json_object *response, void *opaque)
>  {
> @@ -917,6 +973,9 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
>      if (!ret && vnc && vnc->passwd) {
>          ret = qmp_change(gc, qmp, "vnc", "password", vnc->passwd);
>      }
> +    if (!ret) {
> +        ret = qmp_query_vnc(qmp);
> +    }
>      libxl__qmp_close(qmp);
>      return ret;
>  }

Re: [PATCH] libxl: Query VNC listening port through QMP

[Anthony PERARD]

On 17/04/12 16:30, Ian Campbell wrote:
> On Mon, 2012-04-16 at 19:15 +0100, Anthony PERARD wrote:
>> Currently `xl vncviewer $dom` does not work because the VNC port is not
>> registered in xenstore when using qemu-upstream. This patch attempted to fix
>> this.
>
> libxl_vncviewer_exec also potentially reads vnc-pass, although frankly
> having such a thing in xenstore strikes me as something of a
> misfeature...

Well, I thought of that, but when I tried `xl vncviewer` with a password 
set, the result was that vncviewer asked me a password. That why I 
haven't put more effort in querrying the vnc password from QEMU.

> Otherwise:
> Acked-by: Ian Campbell<ian.campbell@citrix.com>
>
>> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com>



-- 
Anthony PERARD

Re: [PATCH] libxl: Query VNC listening port through QMP

[Ian Campbell]

On Tue, 2012-04-17 at 16:45 +0100, Anthony PERARD wrote:
> On 17/04/12 16:30, Ian Campbell wrote:
> > On Mon, 2012-04-16 at 19:15 +0100, Anthony PERARD wrote:
> >> Currently `xl vncviewer $dom` does not work because the VNC port is not
> >> registered in xenstore when using qemu-upstream. This patch attempted to fix
> >> this.
> >
> > libxl_vncviewer_exec also potentially reads vnc-pass, although frankly
> > having such a thing in xenstore strikes me as something of a
> > misfeature...
> 
> Well, I thought of that, but when I tried `xl vncviewer` with a password 
> set, the result was that vncviewer asked me a password. That why I 
> haven't put more effort in querrying the vnc password from QEMU.

Same here even with qemu-xen-traditional.

I'm tempted to suggest that we remove this support -- having plain text
passwords in xenstore (thankfully with perms set somewhat sanely) just
doesn't seem like a Good Thing to me...

> 
> > Otherwise:
> > Acked-by: Ian Campbell<ian.campbell@citrix.com>
> >
> >> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com>
> 
> 
> 

Re: [PATCH] libxl: Query VNC listening port through QMP

[Anthony PERARD]

On 18/04/12 12:05, Ian Campbell wrote:
> On Tue, 2012-04-17 at 16:45 +0100, Anthony PERARD wrote:
>> On 17/04/12 16:30, Ian Campbell wrote:
>>> On Mon, 2012-04-16 at 19:15 +0100, Anthony PERARD wrote:
>>>> Currently `xl vncviewer $dom` does not work because the VNC port is not
>>>> registered in xenstore when using qemu-upstream. This patch attempted to fix
>>>> this.
>>>
>>> libxl_vncviewer_exec also potentially reads vnc-pass, although frankly
>>> having such a thing in xenstore strikes me as something of a
>>> misfeature...
>>
>> Well, I thought of that, but when I tried `xl vncviewer` with a password
>> set, the result was that vncviewer asked me a password. That why I
>> haven't put more effort in querrying the vnc password from QEMU.
>
> Same here even with qemu-xen-traditional.

There is actually an option to xl vncviewer: --autopass, so with this, 
no need to enter the vnc pass manually. And it's works fine with the 
traditionnal.

> I'm tempted to suggest that we remove this support -- having plain text
> passwords in xenstore (thankfully with perms set somewhat sanely) just
> doesn't seem like a Good Thing to me...


-- 
Anthony PERARD

Re: [PATCH] libxl: Query VNC listening port through QMP

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxl: Query VNC listening port through QMP"):
> I'm tempted to suggest that we remove this support -- having plain text
> passwords in xenstore (thankfully with perms set somewhat sanely) just
> doesn't seem like a Good Thing to me...

It isn't a good thing.  But currently we have the following three
options:

(a) allow access to anyone who can reach the vnc server's TCP port;

(b) make noninteractive invocation of vnc clients (including
    screenshot utilities, and automatic invocation of the client
    by xl) impossible;

(c) put a plaintext password in the config file (or the xl/xm
    command line) and copy it to xenstore.

I don't think we should abolish (c) until we have another way of
avoiding the problems of (a) and (b).

Ian.

Re: [PATCH] libxl: Query VNC listening port through QMP

[Ian Campbell]

On Tue, 2012-04-24 at 14:41 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxl: Query VNC listening port through QMP"):
> > I'm tempted to suggest that we remove this support -- having plain text
> > passwords in xenstore (thankfully with perms set somewhat sanely) just
> > doesn't seem like a Good Thing to me...
> 
> It isn't a good thing.  But currently we have the following three
> options:
> 
> (a) allow access to anyone who can reach the vnc server's TCP port;
> 
> (b) make noninteractive invocation of vnc clients (including
>     screenshot utilities, and automatic invocation of the client
>     by xl) impossible;
> 
> (c) put a plaintext password in the config file (or the xl/xm
>     command line) and copy it to xenstore.
> 
> I don't think we should abolish (c) until we have another way of
> avoiding the problems of (a) and (b).

Fair enough.

I should revisit my vnc TLS patches (with client cert support) for 4.3.

Ian.

Re: [PATCH] libxl: Query VNC listening port through QMP [and 1 more messages]

[Ian Jackson]

Anthony PERARD writes ("[Xen-devel] [PATCH] libxl: Query VNC listening port through QMP"):
> Currently `xl vncviewer $dom` does not work because the VNC port is not
> registered in xenstore when using qemu-upstream. This patch attempted to fix
> this.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>


Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxl: Query VNC listening port through QMP"):
> On Mon, 2012-04-16 at 19:15 +0100, Anthony PERARD wrote:
> > Currently `xl vncviewer $dom` does not work because the VNC port is not
> > registered in xenstore when using qemu-upstream. This patch attempted to fix
> > this.
> 
> libxl_vncviewer_exec also potentially reads vnc-pass, although frankly
> having such a thing in xenstore strikes me as something of a
> misfeature...
> 
> Otherwise: 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think this is an acceptable lack-of-feature for now, certainly.

Ian.