[Qemu-devel] [RFC 0/7]: Add VNC connect/disconnect events

[Qemu-devel] [RFC 0/7]: Add VNC connect/disconnect events

[Luiz Capitulino]

 Hi there,

 This series contains two related changes. First a small cleanup is done in
the current 'query-vnc' command, then two new QMP asynchronous events are
introduced: VNC connect and disconnect.

 That's, everytime a VNC client connects or disconnects from QEMU, the
QMP client will get full VNC client and VNC server info.

 There's one problem though and that's why this series is a RFC.

 The connection is a two step procedure if an authentication mechism is
enabled. First the client establishes the connection then it authenticates.

 Currently, 'info vnc' and 'query-vnc' will show client information as soon
as it establishes the connection even if the client didn't autheticate yet.

 This series changes that. Now, if an authentication mechanism is enabled,
client information will only be available _after_ it has authenticated. Also,
the connect/disconnect events are only emitted after the authentication step.

 There's a way to fix this and add the old behavior back, but we'll need
one additional event (say CONNECT_AUTH) and the client will have to look
at the server info to learn that a disconnection happened before
authentication.

 Is this series ok or should the current behavior be maintained?

 Thanks.

[Qemu-devel] [PATCH 1/7] VNC: Use 'enabled' key instead of 'status'

[Luiz Capitulino]

Currently the 'status' key is a string whose value can be
"disabled" or "enabled", change it to the QMP's standard
'enabled' key, which is a bool.

Note that 'status' in being dropped and this wouldn't be
allowed if QMP were stable.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/vnc.c b/vnc.c
index c54c6e0..529abec 100644
--- a/vnc.c
+++ b/vnc.c
@@ -254,7 +254,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
     QList *clients;
 
     server = qobject_to_qdict(data);
-    if (strcmp(qdict_get_str(server, "status"), "disabled") == 0) {
+    if (qdict_get_bool(server, "enabled") == 0) {
         monitor_printf(mon, "Server: disabled\n");
         return;
     }
@@ -282,7 +282,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * The main QDict contains the following:
  *
- * - "status": "disabled" or "enabled"
+ * - "enabled": true or false
  * - "host": server's IP address
  * - "service": server's port number
  * - "auth": authentication method (optional)
@@ -297,13 +297,13 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * Example:
  *
- * { "status": "enabled", "host": "0.0.0.0", "service": "50402", "auth": "vnc",
+ * { "enabled": true, "host": "0.0.0.0", "service": "50402", "auth": "vnc",
  *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
     if (vnc_display == NULL || vnc_display->display == NULL) {
-        *ret_data = qobject_from_jsonf("{ 'status': 'disabled' }");
+        *ret_data = qobject_from_jsonf("{ 'enabled': false }");
     } else {
         QDict *qdict;
         QList *clist;
@@ -319,7 +319,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
             }
         }
 
-        *ret_data = qobject_from_jsonf("{ 'status': 'enabled', 'clients': %p }",
+        *ret_data = qobject_from_jsonf("{ 'enabled': true, 'clients': %p }",
                                        QOBJECT(clist));
         assert(*ret_data != NULL);
 
-- 
1.6.6

[Qemu-devel] [PATCH 2/7] VNC: Make 'auth' key mandatory

[Luiz Capitulino]

There is no reason to have it as optional and the code
in the server and client gets slightly simpler if the
key is mandatory.

While there also do some cleanup on how the server info is
collected.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/vnc.c b/vnc.c
index 529abec..a1ef8fc 100644
--- a/vnc.c
+++ b/vnc.c
@@ -122,7 +122,7 @@ static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
     return 0;
 }
 
-static int vnc_qdict_local_addr(QDict *qdict, int fd)
+static int vnc_server_addr_put(QDict *qdict, int fd)
 {
     struct sockaddr_storage sa;
     socklen_t salen;
@@ -199,6 +199,16 @@ static const char *vnc_auth_name(VncDisplay *vd) {
     return "unknown";
 }
 
+static int vnc_server_info_put(QDict *qdict)
+{
+    if (vnc_server_addr_put(qdict, vnc_display->lsock) < 0) {
+        return -1;
+    }
+
+    qdict_put(qdict, "auth", qstring_from_str(vnc_auth_name(vnc_display)));
+    return 0;
+}
+
 static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 {
     QDict *qdict;
@@ -263,8 +273,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
     monitor_printf(mon, "     address: %s:%s\n",
                    qdict_get_str(server, "host"),
                    qdict_get_str(server, "service"));
-    monitor_printf(mon, "        auth: %s\n",
-        qdict_haskey(server, "auth") ? qdict_get_str(server, "auth") : "none");
+    monitor_printf(mon, "        auth: %s\n", qdict_get_str(server, "auth"));
 
     clients = qdict_get_qlist(server, "clients");
     if (qlist_empty(clients)) {
@@ -285,7 +294,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - "enabled": true or false
  * - "host": server's IP address
  * - "service": server's port number
- * - "auth": authentication method (optional)
+ * - "auth": authentication method
  * - "clients": a QList of all connected clients
  *
  * Clients are described by a QDict, with the following information:
@@ -323,14 +332,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
                                        QOBJECT(clist));
         assert(*ret_data != NULL);
 
-        qdict = qobject_to_qdict(*ret_data);
-
-        if (vnc_display->auth != VNC_AUTH_NONE) {
-            qdict_put(qdict, "auth",
-                      qstring_from_str(vnc_auth_name(vnc_display)));
-        }
-
-        if (vnc_qdict_local_addr(qdict, vnc_display->lsock) < 0) {
+        if (vnc_server_info_put(qobject_to_qdict(*ret_data)) < 0) {
             qobject_decref(*ret_data);
             *ret_data = NULL;
         }
-- 
1.6.6

[Qemu-devel] [PATCH 3/7] VNC: Rename client's 'username' key

[Luiz Capitulino]

It's the SASL username, so it's better to call it 'sasl_username'
to be consistent.

Note that this change wouldn't be allowed if QMP were stable.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/vnc.c b/vnc.c
index a1ef8fc..7b77ffc 100644
--- a/vnc.c
+++ b/vnc.c
@@ -228,7 +228,8 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 #ifdef CONFIG_VNC_SASL
     if (client->sasl.conn &&
         client->sasl.username) {
-        qdict_put(qdict, "username", qstring_from_str(client->sasl.username));
+        qdict_put(qdict, "sasl_username",
+                  qstring_from_str(client->sasl.username));
     }
 #endif
 
@@ -253,8 +254,8 @@ static void info_vnc_iter(QObject *obj, void *opaque)
 #endif
 #ifdef CONFIG_VNC_SASL
     monitor_printf(mon, "    username: %s\n",
-        qdict_haskey(client, "username") ?
-        qdict_get_str(client, "username") : "none");
+        qdict_haskey(client, "sasl_username") ?
+        qdict_get_str(client, "sasl_username") : "none");
 #endif
 }
 
@@ -302,7 +303,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - "host": client's IP address
  * - "service": client's port number
  * - "x509_dname": TLS dname (optional)
- * - "username": SASL username (optional)
+ * - "sasl_username": SASL username (optional)
  *
  * Example:
  *
-- 
1.6.6

[Qemu-devel] [PATCH 4/7] VNC: Add 'family' key

[Luiz Capitulino]

It contains the socket adress family name, like "ipv4" or
"ipv6".

This is useful for clients so that they can interpret the
'host' key reliably.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/vnc.c b/vnc.c
index 7b77ffc..a6e54f3 100644
--- a/vnc.c
+++ b/vnc.c
@@ -100,6 +100,26 @@ char *vnc_socket_remote_addr(const char *format, int fd) {
     return addr_to_string(format, &sa, salen);
 }
 
+static QString *get_sock_family(const struct sockaddr_storage *sa)
+{
+    const char *name;
+
+    switch (sa->ss_family)
+    {
+        case AF_INET:
+            name = "ipv4";
+            break;
+        case AF_INET6:
+            name = "ipv6";
+            break;
+        default:
+            name = "unknown";
+            break;
+    }
+
+    return qstring_from_str(name);
+}
+
 static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
                           socklen_t salen)
 {
@@ -118,6 +138,7 @@ static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
 
     qdict_put(qdict, "host", qstring_from_str(host));
     qdict_put(qdict, "service", qstring_from_str(serv));
+    qdict_put(qdict, "family", get_sock_family(sa));
 
     return 0;
 }
@@ -294,6 +315,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * - "enabled": true or false
  * - "host": server's IP address
+ * - "family": address family ("ipv4" or "ipv6")
  * - "service": server's port number
  * - "auth": authentication method
  * - "clients": a QList of all connected clients
@@ -308,7 +330,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * Example:
  *
  * { "enabled": true, "host": "0.0.0.0", "service": "50402", "auth": "vnc",
- *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
+ *   "family": "ipv4", "clients": [ { "host": "127.0.0.1", "service": "50401"}]}
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
-- 
1.6.6

[Qemu-devel] [PATCH 5/7] VNC: Cache client info at connection time

[Luiz Capitulino]

When a disconnection happens the client's socket on QEMU
side may become invalid, this way it won't be possible
to query it to get client information, which is going to
be needed by the future VNC disconnect QMP event.

To always have this information available we query the
socket at connection time and cache the client info in
struct VncState.

Note that the caching occurs in protocol_client_init(),
meaning that client information is only available _after_
the client has successfully authenticated if an
authentication method is being used.

This is also true for 'query-vnc' or 'info vnc'. If any
kind of authentication is enabled and the client is
connected but didn't authenticate yet, its info won't
be available.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   19 ++++++++++++-------
 vnc.h |    2 ++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/vnc.c b/vnc.c
index a6e54f3..f340d08 100644
--- a/vnc.c
+++ b/vnc.c
@@ -230,14 +230,14 @@ static int vnc_server_info_put(QDict *qdict)
     return 0;
 }
 
-static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
+static void vnc_client_cache_info(VncState *client)
 {
     QDict *qdict;
 
     qdict = qdict_new();
     if (vnc_qdict_remote_addr(qdict, client->csock) < 0) {
         QDECREF(qdict);
-        return NULL;
+        return;
     }
 
 #ifdef CONFIG_VNC_TLS
@@ -254,7 +254,7 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
     }
 #endif
 
-    return qdict;
+    client->info = QOBJECT(qdict);
 }
 
 static void info_vnc_iter(QObject *obj, void *opaque)
@@ -337,16 +337,17 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
     if (vnc_display == NULL || vnc_display->display == NULL) {
         *ret_data = qobject_from_jsonf("{ 'enabled': false }");
     } else {
-        QDict *qdict;
         QList *clist;
 
         clist = qlist_new();
         if (vnc_display->clients) {
             VncState *client = vnc_display->clients;
             while (client) {
-                qdict = do_info_vnc_client(mon, client);
-                if (qdict)
-                    qlist_append(clist, qdict);
+                if (client->info) {
+                    /* incref so that it's not freed by upper layers */
+                    qobject_incref(client->info);
+                    qlist_append_obj(clist, client->info);
+                }
                 client = client->next;
             }
         }
@@ -1099,6 +1100,8 @@ static void vnc_disconnect_finish(VncState *vs)
     if (!vs->vd->clients)
         dcl->idle = 1;
 
+    qobject_decref(vs->info);
+
     vnc_remove_timer(vs->vd);
     qemu_free(vs);
 }
@@ -2053,6 +2056,8 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     char buf[1024];
     int size;
 
+    vnc_client_cache_info(vs);
+
     vnc_write_u16(vs, ds_get_width(vs->ds));
     vnc_write_u16(vs, ds_get_height(vs->ds));
 
diff --git a/vnc.h b/vnc.h
index fcc6824..1210824 100644
--- a/vnc.h
+++ b/vnc.h
@@ -144,6 +144,8 @@ struct VncState
     VncStateSASL sasl;
 #endif
 
+    QObject *info;
+
     Buffer output;
     Buffer input;
     /* current output mode information */
-- 
1.6.6

[Qemu-devel] [PATCH 6/7] QMP: Introduce QMP disconnect event

[Luiz Capitulino]

It's emitted everytime a VNC client disconnects from QEMU,
client's information such as port and IP address is provided.

Note that if any kind of authentication is enabled, this
event is only emitted if the disconnection happens after
the client has been authenticated.

For example:

{ "event": "VNC_DISCONNECT",
    "timestamp": { "seconds": 1262976601, "microseconds": 975795 },
    "data": {
        "server": { "auth": "sasl", "family": "ipv4",
                    "service": "5901", "host": "0.0.0.0" },
        "client": { "family": "ipv4", "service": "58425",
                    "host": "127.0.0.1", "sasl_username": "foo" } } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 +++
 monitor.h |    1 +
 vnc.c     |   28 ++++++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 7536c1e..78fd33a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -357,6 +357,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_STOP:
             event_name = "STOP";
             break;
+        case QEVENT_VNC_DISCONNECT:
+            event_name = "VNC_DISCONNECT";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 6ed117a..e836dd6 100644
--- a/monitor.h
+++ b/monitor.h
@@ -20,6 +20,7 @@ typedef enum MonitorEvent {
     QEVENT_RESET,
     QEVENT_POWERDOWN,
     QEVENT_STOP,
+    QEVENT_VNC_DISCONNECT,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vnc.c b/vnc.c
index f340d08..f2f0ef3 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1068,6 +1068,29 @@ static void vnc_disconnect_start(VncState *vs)
     vs->csock = -1;
 }
 
+static void vnc_qmp_event(VncState *vs, MonitorEvent event)
+{
+    QDict *server;
+    QObject *data;
+
+    if (!vs->info) {
+        return;
+    }
+
+    server = qdict_new();
+    if (vnc_server_info_put(server) < 0) {
+        QDECREF(server);
+        return;
+    }
+
+    data = qobject_from_jsonf("{ 'client': %p, 'server': %p }",
+                              vs->info, QOBJECT(server));
+
+    monitor_protocol_event(event, data);
+
+    qobject_decref(data); /* will also free 'vs->info' */
+}
+
 static void vnc_disconnect_finish(VncState *vs)
 {
     if (vs->input.buffer) {
@@ -1078,6 +1101,9 @@ static void vnc_disconnect_finish(VncState *vs)
         qemu_free(vs->output.buffer);
         vs->output.buffer = NULL;
     }
+
+    vnc_qmp_event(vs, QEVENT_VNC_DISCONNECT);
+
 #ifdef CONFIG_VNC_TLS
     vnc_tls_client_cleanup(vs);
 #endif /* CONFIG_VNC_TLS */
@@ -1100,8 +1126,6 @@ static void vnc_disconnect_finish(VncState *vs)
     if (!vs->vd->clients)
         dcl->idle = 1;
 
-    qobject_decref(vs->info);
-
     vnc_remove_timer(vs->vd);
     qemu_free(vs);
 }
-- 
1.6.6

[Qemu-devel] [PATCH 7/7] QMP: Introduce QMP connect event

[Luiz Capitulino]

It's emitted everytime a VNC client connects to QEMU,
client's information such as port and IP address is provided.

Note that if any kind of authentication is enabled, this
event is only emitted when the client successfully
authenticates.

For example:

{ "event": "VNC_CONNECT",
    "timestamp": { "seconds": 1262976601, "microseconds": 975795 },
    "data": {
        "server": { "auth": "sasl", "family": "ipv4",
                    "service": "5901", "host": "0.0.0.0" },
        "client": { "family": "ipv4", "service": "58425",
                    "host": "127.0.0.1", "sasl_username": "foo" } } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 +++
 monitor.h |    1 +
 vnc.c     |    7 ++++++-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 78fd33a..632a620 100644
--- a/monitor.c
+++ b/monitor.c
@@ -357,6 +357,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_STOP:
             event_name = "STOP";
             break;
+        case QEVENT_VNC_CONNECT:
+            event_name = "VNC_CONNECT";
+            break;
         case QEVENT_VNC_DISCONNECT:
             event_name = "VNC_DISCONNECT";
             break;
diff --git a/monitor.h b/monitor.h
index e836dd6..50067f6 100644
--- a/monitor.h
+++ b/monitor.h
@@ -20,6 +20,7 @@ typedef enum MonitorEvent {
     QEVENT_RESET,
     QEVENT_POWERDOWN,
     QEVENT_STOP,
+    QEVENT_VNC_CONNECT,
     QEVENT_VNC_DISCONNECT,
     QEVENT_MAX,
 } MonitorEvent;
diff --git a/vnc.c b/vnc.c
index f2f0ef3..aaba75d 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1088,7 +1088,11 @@ static void vnc_qmp_event(VncState *vs, MonitorEvent event)
 
     monitor_protocol_event(event, data);
 
-    qobject_decref(data); /* will also free 'vs->info' */
+    if (event == QEVENT_VNC_CONNECT) {
+        qobject_incref(vs->info);
+    }
+
+    qobject_decref(data); /* will also free 'vs->info' on disconnect */
 }
 
 static void vnc_disconnect_finish(VncState *vs)
@@ -2081,6 +2085,7 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     int size;
 
     vnc_client_cache_info(vs);
+    vnc_qmp_event(vs, QEVENT_VNC_CONNECT);
 
     vnc_write_u16(vs, ds_get_width(vs->ds));
     vnc_write_u16(vs, ds_get_height(vs->ds));
-- 
1.6.6

[Qemu-devel] Re: [RFC 0/7]: Add VNC connect/disconnect events

[Daniel P. Berrange]

On Fri, Jan 08, 2010 at 07:47:09PM -0200, Luiz Capitulino wrote:
>  Hi there,
> 
>  This series contains two related changes. First a small cleanup is done in
> the current 'query-vnc' command, then two new QMP asynchronous events are
> introduced: VNC connect and disconnect.
> 
>  That's, everytime a VNC client connects or disconnects from QEMU, the
> QMP client will get full VNC client and VNC server info.
> 
>  There's one problem though and that's why this series is a RFC.
> 
>  The connection is a two step procedure if an authentication mechism is
> enabled. First the client establishes the connection then it authenticates.
> 
>  Currently, 'info vnc' and 'query-vnc' will show client information as soon
> as it establishes the connection even if the client didn't autheticate yet.
> 
>  This series changes that. Now, if an authentication mechanism is enabled,
> client information will only be available _after_ it has authenticated. Also,
> the connect/disconnect events are only emitted after the authentication step.
> 
>  There's a way to fix this and add the old behavior back, but we'll need
> one additional event (say CONNECT_AUTH) and the client will have to look
> at the server info to learn that a disconnection happened before
> authentication.

Hmm, I had not considered the timing problems when I discussed this with
you previously. I think it is important to separate the connect event
from the authentication event, because a mgmt application using QEMU
may want to see clients connecting even if they abort/delay before 
authenticating,

So perhaps we should declare that the lifecycle is

  -  CONNECT       (provide IP / port details)
  -  AUTHENTICATED (provide IP / port details + authenticated ID details
                    eg x509 dname, or SASL usernsmae)
  -  DISCONNECT    (provide IP / port details)


Obviously AUTHENTICATED may be optional if the client goes away
immedaitely before trying auth. The AUTHENTICATED event probably
also ought to allow for an indication of success vs failure so 
the app can see failed login attempts

Daniel
-- 
|: 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 :|

[Qemu-devel] Re: [RFC 0/7]: Add VNC connect/disconnect events

[Luiz Capitulino]

On Mon, 11 Jan 2010 13:55:19 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> So perhaps we should declare that the lifecycle is
> 
>   -  CONNECT       (provide IP / port details)
>   -  AUTHENTICATED (provide IP / port details + authenticated ID details
>                     eg x509 dname, or SASL usernsmae)
>   -  DISCONNECT    (provide IP / port details)
> 
> 
> Obviously AUTHENTICATED may be optional if the client goes away
> immedaitely before trying auth. The AUTHENTICATED event probably
> also ought to allow for an indication of success vs failure so 
> the app can see failed login attempts

 I'm having an issue with the reporting of failure.

 Turns out we can have a few error conditions on login and they are
auth mechanism dependent. Also, as I'm not familiar with the code,
it's not always easy to get the ID information on failures.

 So, what is simple to do is to have an event called VNC_AUTHENTICATION,
it will have a 'authenticated' key which can be true or false. If it's true
authentication has been successful and ID information is available,
otherwise authentication has failed and only IP/port info is available.

 Of course that CONNECT and DISCONNECT events are also provided.

Re: [Qemu-devel] Re: [RFC 0/7]: Add VNC connect/disconnect events

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]

On 01/12/2010 03:28 PM, Luiz Capitulino wrote:
> On Mon, 11 Jan 2010 13:55:19 +0000
> "Daniel P. Berrange"<berrange@redhat.com>  wrote:
>
>    
>> So perhaps we should declare that the lifecycle is
>>
>>    -  CONNECT       (provide IP / port details)
>>    -  AUTHENTICATED (provide IP / port details + authenticated ID details
>>                      eg x509 dname, or SASL usernsmae)
>>    -  DISCONNECT    (provide IP / port details)
>>
>>
>> Obviously AUTHENTICATED may be optional if the client goes away
>> immedaitely before trying auth. The AUTHENTICATED event probably
>> also ought to allow for an indication of success vs failure so
>> the app can see failed login attempts
>>      
>   I'm having an issue with the reporting of failure.
>
>   Turns out we can have a few error conditions on login and they are
> auth mechanism dependent. Also, as I'm not familiar with the code,
> it's not always easy to get the ID information on failures.
>
>   So, what is simple to do is to have an event called VNC_AUTHENTICATION,
> it will have a 'authenticated' key which can be true or false. If it's true
> authentication has been successful and ID information is available,
> otherwise authentication has failed and only IP/port info is available.
>
>   Of course that CONNECT and DISCONNECT events are also provided.
>    

It might be worthwhile looking at the events that gtk-vnc supports.

|	VNC_CONNECTED,    <-  client has connected
	VNC_INITIALIZED,<- initialized is completed
	VNC_DISCONNECTED,<- client has disconnected
	VNC_AUTH_FAILURE,  <- authorization has failed
	VNC_AUTH_UNSUPPORTED,<- authorization has failed (could not negotiate an auth type)

Initialized can provide you all of the credential information.  I think it's stronger than AUTHENTICATED because authentication alone does not imply that a session is active.  Initialized tells a listener that at the moment this is received, the VNC session is active.  If I'm a management tool, that's the thing I'm likely interested in.

Regards,

Anthony Liguori

|




[-- Attachment #2: Type: text/html, Size: 2650 bytes --]

Re: [Qemu-devel] Re: [RFC 0/7]: Add VNC connect/disconnect events

[Daniel P. Berrange]

On Tue, Jan 12, 2010 at 04:28:46PM -0600, Anthony Liguori wrote:
> On 01/12/2010 03:28 PM, Luiz Capitulino wrote:
> >On Mon, 11 Jan 2010 13:55:19 +0000
> >"Daniel P. Berrange"<berrange@redhat.com>  wrote:
> >
> >   
> >>So perhaps we should declare that the lifecycle is
> >>
> >>   -  CONNECT       (provide IP / port details)
> >>   -  AUTHENTICATED (provide IP / port details + authenticated ID details
> >>                     eg x509 dname, or SASL usernsmae)
> >>   -  DISCONNECT    (provide IP / port details)
> >>
> >>
> >>Obviously AUTHENTICATED may be optional if the client goes away
> >>immedaitely before trying auth. The AUTHENTICATED event probably
> >>also ought to allow for an indication of success vs failure so
> >>the app can see failed login attempts
> >>     
> >  I'm having an issue with the reporting of failure.
> >
> >  Turns out we can have a few error conditions on login and they are
> >auth mechanism dependent. Also, as I'm not familiar with the code,
> >it's not always easy to get the ID information on failures.
> >
> >  So, what is simple to do is to have an event called VNC_AUTHENTICATION,
> >it will have a 'authenticated' key which can be true or false. If it's true
> >authentication has been successful and ID information is available,
> >otherwise authentication has failed and only IP/port info is available.
> >
> >  Of course that CONNECT and DISCONNECT events are also provided.
> >   
> 
> It might be worthwhile looking at the events that gtk-vnc supports.
> 
> |	VNC_CONNECTED,    <-  client has connected
> 	VNC_INITIALIZED,<- initialized is completed
> 	VNC_DISCONNECTED,<- client has disconnected
> 	VNC_AUTH_FAILURE,  <- authorization has failed
> 	VNC_AUTH_UNSUPPORTED,<- authorization has failed (could not 
> 	negotiate an auth type)
> 
> Initialized can provide you all of the credential information.  I think 
> it's stronger than AUTHENTICATED because authentication alone does not 
> imply that a session is active.  Initialized tells a listener that at the 
> moment this is received, the VNC session is active.  If I'm a management 
> tool, that's the thing I'm likely interested in.

That is a good point, and if we have the three events at time of 'connect',
'initialized' and 'disconnected', then if a mgmt app sees a 'disconnect' but
no 'initialized' event, it knows the authentication was unsuccessful so do
not need an explicit flag for that.

Daniel
-- 
|: 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 :|