[Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Gerd Hoffmann]

  Hi,

This patch series adds support for multiple vnc server instances to
qemu.  This comes handy in multiseat configurations as you can have
one vnc server for each set then.

Some cleanups along the way (use QemuOpts).  Also added support for
limiting the number of parallel vnc connections.

To be done: monitor support.  I think the best way to handle this is to
introduce a new command to query vnc server state, which returns a list
of vnc servers but otherwise works like "query-vnc".  Alternative
approach would be to add a optional 'id=' parameter to query-vnc, but
then you'll need a new list-vnc command.

Opinions on this?

set_password and expire_password commands should be easy, they can be
extended with an optional 'id=' parameter.

Gerd Hoffmann (6):
  vnc: remove vnc_display global
  vnc: remove unused DisplayState parameter, add id instead.
  vnc: switch to QemuOpts, allow multiple servers
  vnc: allow binding servers to qemu consoles
  vnc: update docs/multiseat.txt
  vnc: track & limit connections

 docs/multiseat.txt   |  18 ++-
 include/ui/console.h |  18 ++-
 qmp.c                |  15 +-
 ui/vnc.c             | 443 +++++++++++++++++++++++++++++++++++----------------
 ui/vnc.h             |   5 +
 vl.c                 |  41 ++---
 6 files changed, 359 insertions(+), 181 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] vnc: remove vnc_display global

[Gerd Hoffmann]

Replace with a vnc_displays list, so we can have multiple vnc server
instances.  Add vnc_server_find function to lookup a display by id.
With no id supplied return the first vnc server, for backward
compatibility reasons.

It is not possible (yet) to actually create mutiple vnc server
instances.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 63 +++++++++++++++++++++++++++++++++++++++++----------------------
 ui/vnc.h |  2 ++
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 092ba2e..51f18f7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -46,7 +46,8 @@ static const struct timeval VNC_REFRESH_LOSSY = { 2, 0 };
 #include "vnc_keysym.h"
 #include "d3des.h"
 
-static VncDisplay *vnc_display; /* needed for info vnc */
+static QTAILQ_HEAD(, VncDisplay) vnc_displays =
+    QTAILQ_HEAD_INITIALIZER(vnc_displays);
 
 static int vnc_cursor_define(VncState *vs);
 static void vnc_release_modifiers(VncState *vs);
@@ -226,10 +227,10 @@ static const char *vnc_auth_name(VncDisplay *vd) {
     return "unknown";
 }
 
-static VncServerInfo *vnc_server_info_get(void)
+static VncServerInfo *vnc_server_info_get(VncDisplay *vd)
 {
     VncServerInfo *info;
-    VncBasicInfo *bi = vnc_basic_info_get_from_server_addr(vnc_display->lsock);
+    VncBasicInfo *bi = vnc_basic_info_get_from_server_addr(vd->lsock);
     if (!bi) {
         return NULL;
     }
@@ -237,7 +238,7 @@ static VncServerInfo *vnc_server_info_get(void)
     info = g_malloc(sizeof(*info));
     info->base = bi;
     info->has_auth = true;
-    info->auth = g_strdup(vnc_auth_name(vnc_display));
+    info->auth = g_strdup(vnc_auth_name(vd));
     return info;
 }
 
@@ -282,7 +283,7 @@ static void vnc_qmp_event(VncState *vs, QAPIEvent event)
     }
     g_assert(vs->info->base);
 
-    si = vnc_server_info_get();
+    si = vnc_server_info_get(vs->vd);
     if (!si) {
         return;
     }
@@ -345,11 +346,27 @@ static VncClientInfo *qmp_query_vnc_client(const VncState *client)
     return info;
 }
 
+static VncDisplay *vnc_display_find(const char *id)
+{
+    VncDisplay *vd;
+
+    if (id == NULL) {
+        return QTAILQ_FIRST(&vnc_displays);
+    }
+    QTAILQ_FOREACH(vd, &vnc_displays, next) {
+        if (strcmp(id, vd->id) == 0) {
+            return vd;
+        }
+    }
+    return NULL;
+}
+
 VncInfo *qmp_query_vnc(Error **errp)
 {
     VncInfo *info = g_malloc0(sizeof(*info));
+    VncDisplay *vd = vnc_display_find(NULL);
 
-    if (vnc_display == NULL || vnc_display->display == NULL) {
+    if (vd == NULL || vd->display == NULL) {
         info->enabled = false;
     } else {
         VncClientInfoList *cur_item = NULL;
@@ -364,7 +381,7 @@ VncInfo *qmp_query_vnc(Error **errp)
         /* for compatibility with the original command */
         info->has_clients = true;
 
-        QTAILQ_FOREACH(client, &vnc_display->clients, next) {
+        QTAILQ_FOREACH(client, &vd->clients, next) {
             VncClientInfoList *cinfo = g_malloc0(sizeof(*info));
             cinfo->value = qmp_query_vnc_client(client);
 
@@ -377,11 +394,11 @@ VncInfo *qmp_query_vnc(Error **errp)
             }
         }
 
-        if (vnc_display->lsock == -1) {
+        if (vd->lsock == -1) {
             return info;
         }
 
-        if (getsockname(vnc_display->lsock, (struct sockaddr *)&sa,
+        if (getsockname(vd->lsock, (struct sockaddr *)&sa,
                         &salen) == -1) {
             error_set(errp, QERR_UNDEFINED_ERROR);
             goto out_error;
@@ -405,7 +422,7 @@ VncInfo *qmp_query_vnc(Error **errp)
         info->family = inet_netfamily(sa.ss_family);
 
         info->has_auth = true;
-        info->auth = g_strdup(vnc_auth_name(vnc_display));
+        info->auth = g_strdup(vnc_auth_name(vd));
     }
 
     return info;
@@ -853,7 +870,7 @@ static int vnc_cursor_define(VncState *vs)
 static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
                                   QEMUCursor *c)
 {
-    VncDisplay *vd = vnc_display;
+    VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
     VncState *vs;
 
     cursor_put(vd->cursor);
@@ -2808,6 +2825,7 @@ static void vnc_connect(VncDisplay *vd, int csock,
     int i;
 
     vs->csock = csock;
+    vs->vd = vd;
 
     if (skipauth) {
 	vs->auth = VNC_AUTH_NONE;
@@ -2852,8 +2870,6 @@ static void vnc_connect(VncDisplay *vd, int csock,
     vnc_qmp_event(vs, QAPI_EVENT_VNC_CONNECTED);
     vnc_set_share_mode(vs, VNC_SHARE_MODE_CONNECTING);
 
-    vs->vd = vd;
-
 #ifdef CONFIG_VNC_WS
     if (!vs->websocket)
 #endif
@@ -2945,7 +2961,7 @@ void vnc_display_init(DisplayState *ds)
 {
     VncDisplay *vs = g_malloc0(sizeof(*vs));
 
-    vnc_display = vs;
+    QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
 
     vs->lsock = -1;
 #ifdef CONFIG_VNC_WS
@@ -2975,7 +2991,7 @@ void vnc_display_init(DisplayState *ds)
 
 static void vnc_display_close(DisplayState *ds)
 {
-    VncDisplay *vs = vnc_display;
+    VncDisplay *vs = vnc_display_find(NULL);
 
     if (!vs)
         return;
@@ -3004,7 +3020,7 @@ static void vnc_display_close(DisplayState *ds)
 
 int vnc_display_password(DisplayState *ds, const char *password)
 {
-    VncDisplay *vs = vnc_display;
+    VncDisplay *vs = vnc_display_find(NULL);
 
     if (!vs) {
         return -EINVAL;
@@ -3023,7 +3039,7 @@ int vnc_display_password(DisplayState *ds, const char *password)
 
 int vnc_display_pw_expire(DisplayState *ds, time_t expires)
 {
-    VncDisplay *vs = vnc_display;
+    VncDisplay *vs = vnc_display_find(NULL);
 
     if (!vs) {
         return -EINVAL;
@@ -3035,14 +3051,14 @@ int vnc_display_pw_expire(DisplayState *ds, time_t expires)
 
 char *vnc_display_local_addr(DisplayState *ds)
 {
-    VncDisplay *vs = vnc_display;
-    
+    VncDisplay *vs = vnc_display_find(NULL);
+
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
 void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
 {
-    VncDisplay *vs = vnc_display;
+    VncDisplay *vs = vnc_display_find(NULL);
     const char *options;
     int password = 0;
     int reverse = 0;
@@ -3058,7 +3074,7 @@ void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
 #endif
     int lock_key_sync = 1;
 
-    if (!vnc_display) {
+    if (!vs) {
         error_setg(errp, "VNC display not active");
         return;
     }
@@ -3357,7 +3373,10 @@ fail:
 
 void vnc_display_add_client(DisplayState *ds, int csock, bool skipauth)
 {
-    VncDisplay *vs = vnc_display;
+    VncDisplay *vs = vnc_display_find(NULL);
 
+    if (!vs) {
+        return;
+    }
     vnc_connect(vs, csock, skipauth, false);
 }
diff --git a/ui/vnc.h b/ui/vnc.h
index 334de9d..6fe8278 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -171,6 +171,8 @@ struct VncDisplay
     struct VncSurface guest;   /* guest visible surface (aka ds->surface) */
     pixman_image_t *server;    /* vnc server surface */
 
+    const char *id;
+    QTAILQ_ENTRY(VncDisplay) next;
     char *display;
     char *password;
     time_t expires;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/6] vnc: remove unused DisplayState parameter, add id instead.

[Gerd Hoffmann]

DisplayState isn't used anywhere, drop it.  Add the vnc server ID as
parameter instead, so it is possible to specify the server instance.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h | 16 ++++++++--------
 ui/vnc.c             | 29 ++++++++++++++---------------
 vl.c                 |  7 ++++---
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 22ef8ca..5ff2e27 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -327,19 +327,19 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
 void cocoa_display_init(DisplayState *ds, int full_screen);
 
 /* vnc.c */
-void vnc_display_init(DisplayState *ds);
-void vnc_display_open(DisplayState *ds, const char *display, Error **errp);
-void vnc_display_add_client(DisplayState *ds, int csock, bool skipauth);
-char *vnc_display_local_addr(DisplayState *ds);
+void vnc_display_init(const char *id);
+void vnc_display_open(const char *id, const char *display, Error **errp);
+void vnc_display_add_client(const char *id, int csock, bool skipauth);
+char *vnc_display_local_addr(const char *id);
 #ifdef CONFIG_VNC
-int vnc_display_password(DisplayState *ds, const char *password);
-int vnc_display_pw_expire(DisplayState *ds, time_t expires);
+int vnc_display_password(const char *id, const char *password);
+int vnc_display_pw_expire(const char *id, time_t expires);
 #else
-static inline int vnc_display_password(DisplayState *ds, const char *password)
+static inline int vnc_display_password(const char *id, const char *password)
 {
     return -ENODEV;
 }
-static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires)
+static inline int vnc_display_pw_expire(const char *id, time_t expires)
 {
     return -ENODEV;
 };
diff --git a/ui/vnc.c b/ui/vnc.c
index 51f18f7..e4bd6f1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2957,10 +2957,11 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define = vnc_dpy_cursor_define,
 };
 
-void vnc_display_init(DisplayState *ds)
+void vnc_display_init(const char *id)
 {
     VncDisplay *vs = g_malloc0(sizeof(*vs));
 
+    vs->id = strdup(id);
     QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
 
     vs->lsock = -1;
@@ -2989,10 +2990,8 @@ void vnc_display_init(DisplayState *ds)
 }
 
 
-static void vnc_display_close(DisplayState *ds)
+static void vnc_display_close(VncDisplay *vs)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
-
     if (!vs)
         return;
     g_free(vs->display);
@@ -3018,9 +3017,9 @@ static void vnc_display_close(DisplayState *ds)
 #endif
 }
 
-int vnc_display_password(DisplayState *ds, const char *password)
+int vnc_display_password(const char *id, const char *password)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
+    VncDisplay *vs = vnc_display_find(id);
 
     if (!vs) {
         return -EINVAL;
@@ -3037,9 +3036,9 @@ int vnc_display_password(DisplayState *ds, const char *password)
     return 0;
 }
 
-int vnc_display_pw_expire(DisplayState *ds, time_t expires)
+int vnc_display_pw_expire(const char *id, time_t expires)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
+    VncDisplay *vs = vnc_display_find(id);
 
     if (!vs) {
         return -EINVAL;
@@ -3049,16 +3048,16 @@ int vnc_display_pw_expire(DisplayState *ds, time_t expires)
     return 0;
 }
 
-char *vnc_display_local_addr(DisplayState *ds)
+char *vnc_display_local_addr(const char *id)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
+    VncDisplay *vs = vnc_display_find(id);
 
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
-void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
+void vnc_display_open(const char *id, const char *display, Error **errp)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
+    VncDisplay *vs = vnc_display_find(id);
     const char *options;
     int password = 0;
     int reverse = 0;
@@ -3078,7 +3077,7 @@ void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
         error_setg(errp, "VNC display not active");
         return;
     }
-    vnc_display_close(ds);
+    vnc_display_close(vs);
     if (strcmp(display, "none") == 0)
         return;
 
@@ -3371,9 +3370,9 @@ fail:
 #endif /* CONFIG_VNC_WS */
 }
 
-void vnc_display_add_client(DisplayState *ds, int csock, bool skipauth)
+void vnc_display_add_client(const char *id, int csock, bool skipauth)
 {
-    VncDisplay *vs = vnc_display_find(NULL);
+    VncDisplay *vs = vnc_display_find(id);
 
     if (!vs) {
         return;
diff --git a/vl.c b/vl.c
index 964d634..6813b1e 100644
--- a/vl.c
+++ b/vl.c
@@ -4433,8 +4433,9 @@ int main(int argc, char **argv, char **envp)
     /* init remote displays */
     if (vnc_display) {
         Error *local_err = NULL;
-        vnc_display_init(ds);
-        vnc_display_open(ds, vnc_display, &local_err);
+        const char *id = "default";
+        vnc_display_init(id);
+        vnc_display_open(id, vnc_display, &local_err);
         if (local_err != NULL) {
             error_report("Failed to start VNC server on `%s': %s",
                          vnc_display, error_get_pretty(local_err));
@@ -4443,7 +4444,7 @@ int main(int argc, char **argv, char **envp)
         }
 
         if (show_vnc_port) {
-            printf("VNC server running on `%s'\n", vnc_display_local_addr(ds));
+            printf("VNC server running on `%s'\n", vnc_display_local_addr(id));
         }
     }
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/6] vnc: switch to QemuOpts, allow multiple servers

[Gerd Hoffmann]

This patch switches vnc over to QemuOpts, and it (more or less
as side effect) allows multiple vnc server instances.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h |   4 +-
 qmp.c                |  15 ++-
 ui/vnc.c             | 271 ++++++++++++++++++++++++++++++++-------------------
 vl.c                 |  42 +++-----
 4 files changed, 200 insertions(+), 132 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 5ff2e27..887ed91 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -328,12 +328,14 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
 
 /* vnc.c */
 void vnc_display_init(const char *id);
-void vnc_display_open(const char *id, const char *display, Error **errp);
+void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 char *vnc_display_local_addr(const char *id);
 #ifdef CONFIG_VNC
 int vnc_display_password(const char *id, const char *password);
 int vnc_display_pw_expire(const char *id, time_t expires);
+QemuOpts *vnc_parse_func(const char *str);
+int vnc_init_func(QemuOpts *opts, void *opaque);
 #else
 static inline int vnc_display_password(const char *id, const char *password)
 {
diff --git a/qmp.c b/qmp.c
index c6767c4..e37a05d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -368,7 +368,20 @@ void qmp_change_vnc_password(const char *password, Error **errp)
 
 static void qmp_change_vnc_listen(const char *target, Error **errp)
 {
-    vnc_display_open(NULL, target, errp);
+    QemuOptsList *olist = qemu_find_opts("vnc");
+    QemuOpts *opts;
+
+    if (strstr(target, "id=")) {
+        error_setg(errp, "id not supported");
+        return;
+    }
+
+    opts = qemu_opts_find(olist, "default");
+    if (opts) {
+        qemu_opts_del(opts);
+    }
+    opts = vnc_parse_func(target);
+    vnc_init_func(opts, NULL);
 }
 
 static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
diff --git a/ui/vnc.c b/ui/vnc.c
index e4bd6f1..ff6be71 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -31,6 +31,7 @@
 #include "qemu/sockets.h"
 #include "qemu/timer.h"
 #include "qemu/acl.h"
+#include "qemu/config-file.h"
 #include "qapi/qmp/types.h"
 #include "qmp-commands.h"
 #include "qemu/osdep.h"
@@ -2959,7 +2960,12 @@ static const DisplayChangeListenerOps dcl_ops = {
 
 void vnc_display_init(const char *id)
 {
-    VncDisplay *vs = g_malloc0(sizeof(*vs));
+    VncDisplay *vs;
+
+    if (vnc_display_find(id) != NULL) {
+        return;
+    }
+    vs = g_malloc0(sizeof(*vs));
 
     vs->id = strdup(id);
     QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
@@ -3055,14 +3061,65 @@ char *vnc_display_local_addr(const char *id)
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
-void vnc_display_open(const char *id, const char *display, Error **errp)
+static QemuOptsList qemu_vnc_opts = {
+    .name = "vnc",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_vnc_opts.head),
+    .implied_opt_name = "vnc",
+    .desc = {
+        {
+            .name = "vnc",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "websocket",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "x509",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "share",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "password",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "reverse",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "lock-key-sync",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "sasl",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "tls",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "x509verify",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "acl",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "lossy",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "non-adaptive",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
+void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vs = vnc_display_find(id);
-    const char *options;
+    QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
+    const char *display, *websocket, *share;
     int password = 0;
     int reverse = 0;
 #ifdef CONFIG_VNC_TLS
     int tls = 0, x509 = 0;
+    const char *path;
 #endif
 #ifdef CONFIG_VNC_SASL
     int sasl = 0;
@@ -3078,115 +3135,86 @@ void vnc_display_open(const char *id, const char *display, Error **errp)
         return;
     }
     vnc_display_close(vs);
-    if (strcmp(display, "none") == 0)
-        return;
 
+    if (!opts) {
+        return;
+    }
+    display = qemu_opt_get(opts, "vnc");
+    if (!display || strcmp(display, "none") == 0) {
+        return;
+    }
     vs->display = g_strdup(display);
-    vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
-
-    options = display;
-    while ((options = strchr(options, ','))) {
-        options++;
-        if (strncmp(options, "password", 8) == 0) {
-            if (fips_get_state()) {
-                error_setg(errp,
-                           "VNC password auth disabled due to FIPS mode, "
-                           "consider using the VeNCrypt or SASL authentication "
-                           "methods as an alternative");
-                goto fail;
-            }
-            password = 1; /* Require password auth */
-        } else if (strncmp(options, "reverse", 7) == 0) {
-            reverse = 1;
-        } else if (strncmp(options, "no-lock-key-sync", 16) == 0) {
-            lock_key_sync = 0;
+
+    password = qemu_opt_get_bool(opts, "password", false);
+    if (password && fips_get_state()) {
+        error_setg(errp,
+                   "VNC password auth disabled due to FIPS mode, "
+                   "consider using the VeNCrypt or SASL authentication "
+                   "methods as an alternative");
+        goto fail;
+    }
+
+    reverse = qemu_opt_get_bool(opts, "reverse", false);
+    lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
 #ifdef CONFIG_VNC_SASL
-        } else if (strncmp(options, "sasl", 4) == 0) {
-            sasl = 1; /* Require SASL auth */
+    sasl = qemu_opt_get_bool(opts, "sasl", false);
 #endif
-#ifdef CONFIG_VNC_WS
-        } else if (strncmp(options, "websocket", 9) == 0) {
-            char *start, *end;
-            vs->websocket = 1;
-
-            /* Check for 'websocket=<port>' */
-            start = strchr(options, '=');
-            end = strchr(options, ',');
-            if (start && (!end || (start < end))) {
-                int len = end ? end-(start+1) : strlen(start+1);
-                if (len < 6) {
-                    /* extract the host specification from display */
-                    char  *host = NULL, *port = NULL, *host_end = NULL;
-                    port = g_strndup(start + 1, len);
-
-                    /* ipv6 hosts have colons */
-                    end = strchr(display, ',');
-                    host_end = g_strrstr_len(display, end - display, ":");
-
-                    if (host_end) {
-                        host = g_strndup(display, host_end - display + 1);
-                    } else {
-                        host = g_strndup(":", 1);
-                    }
-                    vs->ws_display = g_strconcat(host, port, NULL);
-                    g_free(host);
-                    g_free(port);
-                }
-            }
-#endif /* CONFIG_VNC_WS */
 #ifdef CONFIG_VNC_TLS
-        } else if (strncmp(options, "tls", 3) == 0) {
-            tls = 1; /* Require TLS */
-        } else if (strncmp(options, "x509", 4) == 0) {
-            char *start, *end;
-            x509 = 1; /* Require x509 certificates */
-            if (strncmp(options, "x509verify", 10) == 0)
-                vs->tls.x509verify = 1; /* ...and verify client certs */
-
-            /* Now check for 'x509=/some/path' postfix
-             * and use that to setup x509 certificate/key paths */
-            start = strchr(options, '=');
-            end = strchr(options, ',');
-            if (start && (!end || (start < end))) {
-                int len = end ? end-(start+1) : strlen(start+1);
-                char *path = g_strndup(start + 1, len);
-
-                VNC_DEBUG("Trying certificate path '%s'\n", path);
-                if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
-                    error_setg(errp, "Failed to find x509 certificates/keys in %s", path);
-                    g_free(path);
-                    goto fail;
-                }
-                g_free(path);
-            } else {
-                error_setg(errp, "No certificate path provided");
-                goto fail;
-            }
+    tls  = qemu_opt_get_bool(opts, "tls", false);
+    path = qemu_opt_get(opts, "x509");
+    if (path) {
+        x509 = 1;
+        vs->tls.x509verify = qemu_opt_get_bool(opts, "x509verify", false);
+        if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
+            error_setg(errp, "Failed to find x509 certificates/keys in %s",
+                       path);
+            goto fail;
+        }
+    }
 #endif
 #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
-        } else if (strncmp(options, "acl", 3) == 0) {
-            acl = 1;
-#endif
-        } else if (strncmp(options, "lossy", 5) == 0) {
-#ifdef CONFIG_VNC_JPEG
-            vs->lossy = true;
+    acl = qemu_opt_get_bool(opts, "acl", false);
 #endif
-        } else if (strncmp(options, "non-adaptive", 12) == 0) {
-            vs->non_adaptive = true;
-        } else if (strncmp(options, "share=", 6) == 0) {
-            if (strncmp(options+6, "ignore", 6) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_IGNORE;
-            } else if (strncmp(options+6, "allow-exclusive", 15) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
-            } else if (strncmp(options+6, "force-shared", 12) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
-            } else {
-                error_setg(errp, "unknown vnc share= option");
-                goto fail;
-            }
+
+    share = qemu_opt_get(opts, "share");
+    if (share) {
+        if (strcmp(share, "ignore") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_IGNORE;
+        } else if (strcmp(share, "allow-exclusive") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+        } else if (strcmp(share, "force-shared") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
+        } else {
+            error_setg(errp, "unknown vnc share= option");
+            goto fail;
+        }
+    } else {
+        vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+    }
+
+ #ifdef CONFIG_VNC_WS
+    websocket = qemu_opt_get(opts, "websocket");
+    if (websocket) {
+        /* extract the host specification from display */
+        char  *host = NULL, *host_end = NULL;
+        vs->websocket = 1;
+
+        /* ipv6 hosts have colons */
+        host_end = strrchr(display, ':');
+        if (host_end) {
+            host = g_strndup(display, host_end - display + 1);
+        } else {
+            host = g_strdup(":");
         }
+        vs->ws_display = g_strconcat(host, websocket, NULL);
+        g_free(host);
     }
+#endif /* CONFIG_VNC_WS */
 
+#ifdef CONFIG_VNC_JPEG
+    vs->lossy = qemu_opt_get_bool(opts, "lossy", false);
+#endif
+    vs->non_adaptive = qemu_opt_get_bool(opts, "non-adaptive", false);
     /* adaptive updates are only used with tight encoding and
      * if lossy updates are enabled so we can disable all the
      * calculations otherwise */
@@ -3379,3 +3407,44 @@ void vnc_display_add_client(const char *id, int csock, bool skipauth)
     }
     vnc_connect(vs, csock, skipauth, false);
 }
+
+QemuOpts *vnc_parse_func(const char *str)
+{
+    return qemu_opts_parse(qemu_find_opts("vnc"), str, 1);
+}
+
+int vnc_init_func(QemuOpts *opts, void *opaque)
+{
+    Error *local_err = NULL;
+    QemuOptsList *olist = qemu_find_opts("vnc");
+    char *id = (char *)qemu_opts_id(opts);
+
+    if (!id) {
+        /* auto-assign id if not present */
+        int i = 2;
+        id = g_strdup("default");
+        while (qemu_opts_find(olist, id)) {
+            g_free(id);
+            id = g_strdup_printf("vnc%d", i++);
+        }
+        qemu_opts_set_id(opts, id);
+    }
+    fprintf(stderr, "%s: id \"%s\"\n", __func__, id);
+
+    vnc_display_init(id);
+    vnc_display_open(id, &local_err);
+    if (local_err != NULL) {
+        error_report("Failed to start VNC server on `%s': %s",
+                     qemu_opt_get(opts, "display"),
+                     error_get_pretty(local_err));
+        error_free(local_err);
+        exit(1);
+    }
+    return 0;
+}
+
+static void vnc_register_config(void)
+{
+    qemu_add_opts(&qemu_vnc_opts);
+}
+machine_init(vnc_register_config);
diff --git a/vl.c b/vl.c
index 6813b1e..e13a434 100644
--- a/vl.c
+++ b/vl.c
@@ -159,9 +159,6 @@ int smp_cpus = 1;
 int max_cpus = 0;
 int smp_cores = 1;
 int smp_threads = 1;
-#ifdef CONFIG_VNC
-const char *vnc_display;
-#endif
 int acpi_enabled = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
@@ -2239,16 +2236,12 @@ static DisplayType select_display(const char *p)
 #endif
     } else if (strstart(p, "vnc", &opts)) {
 #ifdef CONFIG_VNC
-        display_remote++;
-
-        if (*opts) {
-            const char *nextopt;
-
-            if (strstart(opts, "=", &nextopt)) {
-                vnc_display = nextopt;
+        if (*opts == '=') {
+            display_remote++;
+            if (vnc_parse_func(opts+1) == NULL) {
+                exit(1);
             }
-        }
-        if (!vnc_display) {
+        } else {
             fprintf(stderr, "VNC requires a display argument vnc=<display>\n");
             exit(1);
         }
@@ -3680,7 +3673,9 @@ int main(int argc, char **argv, char **envp)
 	    case QEMU_OPTION_vnc:
 #ifdef CONFIG_VNC
                 display_remote++;
-                vnc_display = optarg;
+                if (vnc_parse_func(optarg) == NULL) {
+                    exit(1);
+                }
 #else
                 fprintf(stderr, "VNC support is disabled\n");
                 exit(1);
@@ -4128,7 +4123,7 @@ int main(int argc, char **argv, char **envp)
 #elif defined(CONFIG_SDL) || defined(CONFIG_COCOA)
         display_type = DT_SDL;
 #elif defined(CONFIG_VNC)
-        vnc_display = "localhost:0,to=99";
+        vnc_parse_func("localhost:0,to=99,id=default");
         show_vnc_port = 1;
 #else
         display_type = DT_NONE;
@@ -4431,21 +4426,10 @@ int main(int argc, char **argv, char **envp)
 
 #ifdef CONFIG_VNC
     /* init remote displays */
-    if (vnc_display) {
-        Error *local_err = NULL;
-        const char *id = "default";
-        vnc_display_init(id);
-        vnc_display_open(id, vnc_display, &local_err);
-        if (local_err != NULL) {
-            error_report("Failed to start VNC server on `%s': %s",
-                         vnc_display, error_get_pretty(local_err));
-            error_free(local_err);
-            exit(1);
-        }
-
-        if (show_vnc_port) {
-            printf("VNC server running on `%s'\n", vnc_display_local_addr(id));
-        }
+    qemu_opts_foreach(qemu_find_opts("vnc"), vnc_init_func, NULL, 0);
+    if (show_vnc_port) {
+        printf("VNC server running on `%s'\n",
+               vnc_display_local_addr("default"));
     }
 #endif
 #ifdef CONFIG_SPICE
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] vnc: allow binding servers to qemu consoles

[Gerd Hoffmann]

This patch adds a display= parameter to the vnc options.  This allows to
bind a vnc server instance to a specific display, allowing to create a
multiseat setup with a vnc server for each seat.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index ff6be71..a03a7e1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -27,6 +27,7 @@
 #include "vnc.h"
 #include "vnc-jobs.h"
 #include "trace.h"
+#include "hw/qdev.h"
 #include "sysemu/sysemu.h"
 #include "qemu/sockets.h"
 #include "qemu/timer.h"
@@ -1665,7 +1666,8 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
             vs->modifiers_state[keycode] = 0;
         break;
     case 0x02 ... 0x0a: /* '1' to '9' keys */
-        if (down && vs->modifiers_state[0x1d] && vs->modifiers_state[0x38]) {
+        if (vs->vd->dcl.con == NULL &&
+            down && vs->modifiers_state[0x1d] && vs->modifiers_state[0x38]) {
             /* Reset the modifiers sent to the current console */
             reset_keys(vs);
             console_select(keycode - 0x02);
@@ -2063,8 +2065,8 @@ static void set_pixel_format(VncState *vs,
 
     set_pixel_conversion(vs);
 
-    graphic_hw_invalidate(NULL);
-    graphic_hw_update(NULL);
+    graphic_hw_invalidate(vs->vd->dcl.con);
+    graphic_hw_update(vs->vd->dcl.con);
 }
 
 static void pixel_format_message (VncState *vs) {
@@ -2791,7 +2793,7 @@ static void vnc_refresh(DisplayChangeListener *dcl)
         return;
     }
 
-    graphic_hw_update(NULL);
+    graphic_hw_update(vd->dcl.con);
 
     if (vnc_trylock_display(vd)) {
         update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
@@ -2897,7 +2899,7 @@ void vnc_init_state(VncState *vs)
 
     QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
 
-    graphic_hw_update(NULL);
+    graphic_hw_update(vd->dcl.con);
 
     vnc_write(vs, "RFB 003.008\n", 12);
     vnc_flush(vs);
@@ -2920,7 +2922,7 @@ static void vnc_listen_read(void *opaque, bool websocket)
     int csock;
 
     /* Catch-up */
-    graphic_hw_update(NULL);
+    graphic_hw_update(vs->dcl.con);
 #ifdef CONFIG_VNC_WS
     if (websocket) {
         csock = qemu_accept(vs->lwebsock, (struct sockaddr *)&addr, &addrlen);
@@ -3079,6 +3081,12 @@ static QemuOptsList qemu_vnc_opts = {
             .name = "share",
             .type = QEMU_OPT_STRING,
         },{
+            .name = "display",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "head",
+            .type = QEMU_OPT_NUMBER,
+        },{
             .name = "password",
             .type = QEMU_OPT_BOOL,
         },{
@@ -3114,7 +3122,8 @@ void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vs = vnc_display_find(id);
     QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
-    const char *display, *websocket, *share;
+    const char *display, *websocket, *share, *device_id;
+    QemuConsole *con;
     int password = 0;
     int reverse = 0;
 #ifdef CONFIG_VNC_TLS
@@ -3325,6 +3334,33 @@ void vnc_display_open(const char *id, Error **errp)
 #endif
     vs->lock_key_sync = lock_key_sync;
 
+    device_id = qemu_opt_get(opts, "display");
+    if (device_id) {
+        DeviceState *dev;
+        int head = qemu_opt_get_number(opts, "head", 0);
+
+        dev = qdev_find_recursive(sysbus_get_default(), device_id);
+        if (dev == NULL) {
+            error_set(errp, QERR_DEVICE_NOT_FOUND, device_id);
+            goto fail;
+        }
+
+        con = qemu_console_lookup_by_device(dev, head);
+        if (con == NULL) {
+            error_setg(errp, "Device %s is not bound to a QemuConsole",
+                       device_id);
+            goto fail;
+        }
+    } else {
+        con = NULL;
+    }
+
+    if (con != vs->dcl.con) {
+        unregister_displaychangelistener(&vs->dcl);
+        vs->dcl.con = con;
+        register_displaychangelistener(&vs->dcl);
+    }
+
     if (reverse) {
         /* connect to viewer */
         int csock;
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/6] vnc: update docs/multiseat.txt

[Gerd Hoffmann]

vnc joins the party ;)
Also some s/head/seat/ to clarify.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/multiseat.txt | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/docs/multiseat.txt b/docs/multiseat.txt
index 67151e0..b963665 100644
--- a/docs/multiseat.txt
+++ b/docs/multiseat.txt
@@ -7,7 +7,7 @@ host side
 
 First you must compile qemu with a user interface supporting
 multihead/multiseat and input event routing.  Right now this
-list includes sdl2 and gtk (both 2+3):
+list includes sdl2, gtk (both 2+3) and vnc:
 
   ./configure --enable-sdl --with-sdlabi=2.0
 
@@ -16,16 +16,16 @@ or
   ./configure --enable-gtk
 
 
-Next put together the qemu command line:
+Next put together the qemu command line (sdk/gtk):
 
 qemu	-enable-kvm -usb $memory $disk $whatever \
 	-display [ sdl | gtk ] \
 	-vga std \
 	-device usb-tablet
 
-That is it for the first head, which will use the standard vga, the
+That is it for the first seat, which will use the standard vga, the
 standard ps/2 keyboard (implicitly there) and the usb-tablet.  Now the
-additional switches for the second head:
+additional switches for the second seat:
 
 	-device pci-bridge,addr=12.0,chassis_nr=2,id=head.2 \
 	-device secondary-vga,bus=head.2,addr=02.0,id=video.2 \
@@ -47,6 +47,16 @@ in a separate tab.  You can either simply switch tabs to switch heads,
 or use the "View / Detach tab" menu item to move one of the displays
 to its own window so you can see both display devices side-by-side.
 
+For vnc some additional configuration on the command line is needed.
+We'll create two vnc server instances, and bind the second one to the
+second seat, simliar to input devices:
+
+	-display vnc=:1,id=primary \
+	-display vnc=:2,id=secondary,display=video.2
+
+Connecting to vnc display :1 gives you access to the first seat, and
+likewise connecting to vnc display :2 shows the second seat.
+
 Note on spice: Spice handles multihead just fine.  But it can't do
 multiseat.  For tablet events the event source is sent to the spice
 agent.  But qemu can't figure it, so it can't do input routing.
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

Also track the number of connections in "connecting" and "shared" state
(additionally to "exclusive" state).  Apply a configurable limit to
these connections.

The logic to apply the limit to connections in "shared" state is pretty
simple:  When the limit is reached no new connections are allowed.

The logic to apply the limit to connections in "connecting" state (this
is the state you are in *before* successfull authentication) is
slightly different:  A new connect kicks out the oldest client which is
still in "connecting" state.  This avoids a easy DoS by unauthenticated
users by simply opening connections until the limit is reached.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
 ui/vnc.h |  3 +++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index a03a7e1..cc4ac5c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -68,12 +68,34 @@ static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
             vs->csock, mn[vs->share_mode], mn[mode]);
 #endif
 
-    if (vs->share_mode == VNC_SHARE_MODE_EXCLUSIVE) {
+    switch (vs->share_mode) {
+    case VNC_SHARE_MODE_CONNECTING:
+        vs->vd->num_connecting--;
+        break;
+    case VNC_SHARE_MODE_SHARED:
+        vs->vd->num_shared--;
+        break;
+    case VNC_SHARE_MODE_EXCLUSIVE:
         vs->vd->num_exclusive--;
+        break;
+    default:
+        break;
     }
+
     vs->share_mode = mode;
-    if (vs->share_mode == VNC_SHARE_MODE_EXCLUSIVE) {
+
+    switch (vs->share_mode) {
+    case VNC_SHARE_MODE_CONNECTING:
+        vs->vd->num_connecting++;
+        break;
+    case VNC_SHARE_MODE_SHARED:
+        vs->vd->num_shared++;
+        break;
+    case VNC_SHARE_MODE_EXCLUSIVE:
         vs->vd->num_exclusive++;
+        break;
+    default:
+        break;
     }
 }
 
@@ -2327,6 +2349,11 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     }
     vnc_set_share_mode(vs, mode);
 
+    if (vs->vd->num_shared > vs->vd->connections_limit) {
+        vnc_disconnect_start(vs);
+        return 0;
+    }
+
     vs->client_width = pixman_image_get_width(vs->vd->server);
     vs->client_height = pixman_image_get_height(vs->vd->server);
     vnc_write_u16(vs, vs->client_width);
@@ -2879,6 +2906,15 @@ static void vnc_connect(VncDisplay *vd, int csock,
     {
         vnc_init_state(vs);
     }
+
+    if (vd->num_connecting > vd->connections_limit) {
+        QTAILQ_FOREACH(vs, &vd->clients, next) {
+            if (vs->share_mode == VNC_SHARE_MODE_CONNECTING) {
+                vnc_disconnect_start(vs);
+                return;
+            }
+        }
+    }
 }
 
 void vnc_init_state(VncState *vs)
@@ -2897,7 +2933,7 @@ void vnc_init_state(VncState *vs)
     qemu_mutex_init(&vs->output_mutex);
     vs->bh = qemu_bh_new(vnc_jobs_bh, vs);
 
-    QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
+    QTAILQ_INSERT_TAIL(&vd->clients, vs, next);
 
     graphic_hw_update(vd->dcl.con);
 
@@ -3087,6 +3123,9 @@ static QemuOptsList qemu_vnc_opts = {
             .name = "head",
             .type = QEMU_OPT_NUMBER,
         },{
+            .name = "connections",
+            .type = QEMU_OPT_NUMBER,
+        },{
             .name = "password",
             .type = QEMU_OPT_BOOL,
         },{
@@ -3200,6 +3239,7 @@ void vnc_display_open(const char *id, Error **errp)
     } else {
         vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
     }
+    vs->connections_limit = qemu_opt_get_number(opts, "connections", 32);
 
  #ifdef CONFIG_VNC_WS
     websocket = qemu_opt_get(opts, "websocket");
diff --git a/ui/vnc.h b/ui/vnc.h
index 6fe8278..5e2b1a5 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -150,7 +150,10 @@ typedef enum VncSharePolicy {
 struct VncDisplay
 {
     QTAILQ_HEAD(, VncState) clients;
+    int num_connecting;
+    int num_shared;
     int num_exclusive;
+    int connections_limit;
     VncSharePolicy share_policy;
     int lsock;
 #ifdef CONFIG_VNC_WS
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Daniel P. Berrange]

On Wed, Oct 15, 2014 at 02:19:45PM +0200, Gerd Hoffmann wrote:
> Also track the number of connections in "connecting" and "shared" state
> (additionally to "exclusive" state).  Apply a configurable limit to
> these connections.
> 
> The logic to apply the limit to connections in "shared" state is pretty
> simple:  When the limit is reached no new connections are allowed.
> 
> The logic to apply the limit to connections in "connecting" state (this
> is the state you are in *before* successfull authentication) is
> slightly different:  A new connect kicks out the oldest client which is
> still in "connecting" state.  This avoids a easy DoS by unauthenticated
> users by simply opening connections until the limit is reached.

I'd suggest that rather than kicking off the oldest client QEMU
should simply stop calling accept() when it reaches the limit
of active unauthenticated client connections.

By allowing the connection to succeeed & then kicking off another
client QEMU's still burning CPU to do memory allocation & free'ing
for each client.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

  Hi,

> > The logic to apply the limit to connections in "connecting" state (this
> > is the state you are in *before* successfull authentication) is
> > slightly different:  A new connect kicks out the oldest client which is
> > still in "connecting" state.  This avoids a easy DoS by unauthenticated
> > users by simply opening connections until the limit is reached.
> 
> I'd suggest that rather than kicking off the oldest client QEMU
> should simply stop calling accept() when it reaches the limit
> of active unauthenticated client connections.

Looks like I need to be a bit more verbose.  The DoS I try to prevent is
that anybody can open $limit connections to the vnc server, let them sit
around idle, thereby blocking further connects.

Whenever you stop calling accept or drop the new connection doesn't make
much of a difference.

I try to prevent that by dropping the *oldest* connection, so you have a
chance to connect even if a unprivileged attacker tries to use up all
connection slots.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Daniel P. Berrange]

On Wed, Oct 15, 2014 at 04:19:29PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > The logic to apply the limit to connections in "connecting" state (this
> > > is the state you are in *before* successfull authentication) is
> > > slightly different:  A new connect kicks out the oldest client which is
> > > still in "connecting" state.  This avoids a easy DoS by unauthenticated
> > > users by simply opening connections until the limit is reached.
> > 
> > I'd suggest that rather than kicking off the oldest client QEMU
> > should simply stop calling accept() when it reaches the limit
> > of active unauthenticated client connections.
> 
> Looks like I need to be a bit more verbose.  The DoS I try to prevent is
> that anybody can open $limit connections to the vnc server, let them sit
> around idle, thereby blocking further connects.

Ah, so the DoS I'm thinking of is that you can open connections
to VNC until the max file descriptor limit is exhausted and thus
break anything in QEMU that needs to open a new FD (eg migration).
The key to fixing that particular DoS is simply having a small
finite limit so we cannot exhaust file descriptors. Stopping
calling accept() works in that scenario.

> Whenever you stop calling accept or drop the new connection doesn't make
> much of a difference.
> 
> I try to prevent that by dropping the *oldest* connection, so you have a
> chance to connect even if a unprivileged attacker tries to use up all
> connection slots.

Lets say the limit is 5. The bad guy has 5 open idle connections.
The good guy opens a new one and pushes off one of the bad guy's
connections. Fine so far. The bad guy though can simply open 5 more
connections and he'll push the good guy's connection off again. This
fix would only address the scenario you describe if the good guy can
connect & complete auth before the bad guy opens more connections.
With TLS and/or SASL there's a non-negligible time window for auth
to take place.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

  Hi,

> > I try to prevent that by dropping the *oldest* connection, so you have a
> > chance to connect even if a unprivileged attacker tries to use up all
> > connection slots.
> 
> Lets say the limit is 5. The bad guy has 5 open idle connections.
> The good guy opens a new one and pushes off one of the bad guy's
> connections. Fine so far. The bad guy though can simply open 5 more
> connections and he'll push the good guy's connection off again.

Correct.  It can't fully prevent the attack, but makes it harder to pull
off.  Just having $limit idle connects isn't enough any more, the bad
guy has to constantly bomb qemu with vnc connect requests, hoping this
kicks out the good guy before it managed to authenticate.  The chances
for the good guy are a bit better and it is also more likely that the
attack sets off alarms in network monitoring.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gonglei]

On 2014/10/16 18:46, Gerd Hoffmann wrote:

>   Hi,
> 
>>> I try to prevent that by dropping the *oldest* connection, so you have a
>>> chance to connect even if a unprivileged attacker tries to use up all
>>> connection slots.
>>
>> Lets say the limit is 5. The bad guy has 5 open idle connections.
>> The good guy opens a new one and pushes off one of the bad guy's
>> connections. Fine so far. The bad guy though can simply open 5 more
>> connections and he'll push the good guy's connection off again.
> 
> Correct.  It can't fully prevent the attack, but makes it harder to pull
> off.  Just having $limit idle connects isn't enough any more, the bad
> guy has to constantly bomb qemu with vnc connect requests, hoping this
> kicks out the good guy before it managed to authenticate.  The chances
> for the good guy are a bit better and it is also more likely that the
> attack sets off alarms in network monitoring.
> 

Hi,

Happy that I don't miss this patch series and conversation.
I have a approach to prevent the brute force attack (which
had been tested in my team). Firstly, we must set password for
vnc server for security. Secondly, the DoS may bomb qemu
with vnc connect requests, trying to decrypt password at present.

If we set the max trying times, and then
There are some concepts:
 - INTERVAL_TIME: a time window that user can connnet vnc server
 - REJECT_TIME: the time of reject any connection
 - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
   if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
   before REJECT_TIME attached. The old connected client will not be influenced.

Do you think it make sense if I post this approach in upstream ? Thanks.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Daniel P. Berrange]

On Fri, Oct 17, 2014 at 02:34:07PM +0800, Gonglei wrote:
> On 2014/10/16 18:46, Gerd Hoffmann wrote:
> 
> >   Hi,
> > 
> >>> I try to prevent that by dropping the *oldest* connection, so you have a
> >>> chance to connect even if a unprivileged attacker tries to use up all
> >>> connection slots.
> >>
> >> Lets say the limit is 5. The bad guy has 5 open idle connections.
> >> The good guy opens a new one and pushes off one of the bad guy's
> >> connections. Fine so far. The bad guy though can simply open 5 more
> >> connections and he'll push the good guy's connection off again.
> > 
> > Correct.  It can't fully prevent the attack, but makes it harder to pull
> > off.  Just having $limit idle connects isn't enough any more, the bad
> > guy has to constantly bomb qemu with vnc connect requests, hoping this
> > kicks out the good guy before it managed to authenticate.  The chances
> > for the good guy are a bit better and it is also more likely that the
> > attack sets off alarms in network monitoring.
> > 
> 
> Hi,
> 
> Happy that I don't miss this patch series and conversation.
> I have a approach to prevent the brute force attack (which
> had been tested in my team). Firstly, we must set password for
> vnc server for security. Secondly, the DoS may bomb qemu
> with vnc connect requests, trying to decrypt password at present.

Note that VNC passwords offer no meaningful level of security.

If you want security for VNC you must *always* use the TLS extension
or the SASL extension, or both. These offer proven cryptographically
strong authentication protocols.

> If we set the max trying times, and then
> There are some concepts:
>  - INTERVAL_TIME: a time window that user can connnet vnc server
>  - REJECT_TIME: the time of reject any connection
>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
>    before REJECT_TIME attached. The old connected client will not be influenced.

How are you defining "user" in this description. Do you mean "Source IP address"
here ? Or any client connection ? Or something else ?

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gonglei]

On 2014/10/17 14:38, Daniel P. Berrange wrote:

> On Fri, Oct 17, 2014 at 02:34:07PM +0800, Gonglei wrote:
>> On 2014/10/16 18:46, Gerd Hoffmann wrote:
>>
>>>   Hi,
>>>
>>>>> I try to prevent that by dropping the *oldest* connection, so you have a
>>>>> chance to connect even if a unprivileged attacker tries to use up all
>>>>> connection slots.
>>>>
>>>> Lets say the limit is 5. The bad guy has 5 open idle connections.
>>>> The good guy opens a new one and pushes off one of the bad guy's
>>>> connections. Fine so far. The bad guy though can simply open 5 more
>>>> connections and he'll push the good guy's connection off again.
>>>
>>> Correct.  It can't fully prevent the attack, but makes it harder to pull
>>> off.  Just having $limit idle connects isn't enough any more, the bad
>>> guy has to constantly bomb qemu with vnc connect requests, hoping this
>>> kicks out the good guy before it managed to authenticate.  The chances
>>> for the good guy are a bit better and it is also more likely that the
>>> attack sets off alarms in network monitoring.
>>>
>>
>> Hi,
>>
>> Happy that I don't miss this patch series and conversation.
>> I have a approach to prevent the brute force attack (which
>> had been tested in my team). Firstly, we must set password for
>> vnc server for security. Secondly, the DoS may bomb qemu
>> with vnc connect requests, trying to decrypt password at present.
> 
> Note that VNC passwords offer no meaningful level of security.
> 

Encryption algorithms?

> If you want security for VNC you must *always* use the TLS extension
> or the SASL extension, or both. These offer proven cryptographically
> strong authentication protocols.
> 
>> If we set the max trying times, and then
>> There are some concepts:
>>  - INTERVAL_TIME: a time window that user can connect vnc server
>>  - REJECT_TIME: the time of reject any connection
>>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
>>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
>>    before REJECT_TIME attached. The old connected client will not be influenced.
> 
> How are you defining "user" in this description. Do you mean "Source IP address"
> here ? Or any client connection ? Or something else ?
> 

I mean "any client connection". :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

  Hi,

> If we set the max trying times, and then
> There are some concepts:
>  - INTERVAL_TIME: a time window that user can connnet vnc server
>  - REJECT_TIME: the time of reject any connection
>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
>    before REJECT_TIME attached. The old connected client will not be influenced.

i.e. effectively rate-limit login attempts.  Makes sense to have an
option for that, although I'm not sure it is worth the trouble doing
something beyond a simple "one attempt per second allowed" (i.e. stop
polling the listening socket for a second after each accept).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gonglei]

On 2014/10/20 15:02, Gerd Hoffmann wrote:

> 
>   Hi,
> 
>> If we set the max trying times, and then
>> There are some concepts:
>>  - INTERVAL_TIME: a time window that user can connnet vnc server
>>  - REJECT_TIME: the time of reject any connection
>>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
>>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
>>    before REJECT_TIME attached. The old connected client will not be influenced.
> 
> i.e. effectively rate-limit login attempts.  Makes sense to have an
> option for that, although I'm not sure it is worth the trouble doing
> something beyond a simple "one attempt per second allowed" (i.e. stop
> polling the listening socket for a second after each accept).
> 

Hi,

"one attempt per second allowed" is just reduce the frequency for attack,
but I don't think the effect is very well. It can limit the login attempts for
attack problem from a flood of attack to "one attempt per second" (not
the same magnitude with my approach for security). However,
For it is not effective for bad guys, whose operation time is greater than one
second per login attempt usually.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

On Di, 2014-10-21 at 14:06 +0800, Gonglei wrote:
> On 2014/10/20 15:02, Gerd Hoffmann wrote:
> 
> > 
> >   Hi,
> > 
> >> If we set the max trying times, and then
> >> There are some concepts:
> >>  - INTERVAL_TIME: a time window that user can connnet vnc server
> >>  - REJECT_TIME: the time of reject any connection
> >>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
> >>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
> >>    before REJECT_TIME attached. The old connected client will not be influenced.
> > 
> > i.e. effectively rate-limit login attempts.  Makes sense to have an
> > option for that, although I'm not sure it is worth the trouble doing
> > something beyond a simple "one attempt per second allowed" (i.e. stop
> > polling the listening socket for a second after each accept).
> > 
> 
> Hi,
> 
> "one attempt per second allowed" is just reduce the frequency for attack,

Yes.  Which is common practice, to slow down dictionary attacks.

> but I don't think the effect is very well. It can limit the login attempts for
> attack problem from a flood of attack to "one attempt per second" (not
> the same magnitude with my approach for security).

Problem with rejecting is any login attempts for REJECT_TIME is that you
also lock out the good guys.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gonglei]

On 2014/10/21 16:57, Gerd Hoffmann wrote:

> On Di, 2014-10-21 at 14:06 +0800, Gonglei wrote:
>> On 2014/10/20 15:02, Gerd Hoffmann wrote:
>>
>>>
>>>   Hi,
>>>
>>>> If we set the max trying times, and then
>>>> There are some concepts:
>>>>  - INTERVAL_TIME: a time window that user can connnet vnc server
>>>>  - REJECT_TIME: the time of reject any connection
>>>>  - MAX_TRY_TIMES: the times that user can connect vnc server in INTERVAL_TIME,
>>>>    if attach the MAX_TRY_TIMES, the server will lock, any user can not connect again
>>>>    before REJECT_TIME attached. The old connected client will not be influenced.
>>>
>>> i.e. effectively rate-limit login attempts.  Makes sense to have an
>>> option for that, although I'm not sure it is worth the trouble doing
>>> something beyond a simple "one attempt per second allowed" (i.e. stop
>>> polling the listening socket for a second after each accept).
>>>
>>
>> Hi,
>>
>> "one attempt per second allowed" is just reduce the frequency for attack,
> 
> Yes.  Which is common practice, to slow down dictionary attacks.
> 
>> but I don't think the effect is very well. It can limit the login attempts for
>> attack problem from a flood of attack to "one attempt per second" (not
>> the same magnitude with my approach for security).
> 
> Problem with rejecting is any login attempts for REJECT_TIME is that you
> also lock out the good guys.
> 

Yes. But I think it is not a big problem, when the REJECT_TIME is over,
the good guys can connect vnc successfully immediately.
Or maybe we just lock those guys with "the same Source IP address" ?

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gerd Hoffmann]

  Hi,

> Yes. But I think it is not a big problem, when the REJECT_TIME is over,
> the good guys can connect vnc successfully immediately.
> Or maybe we just lock those guys with "the same Source IP address" ?

Better.  Question is whenever we really want implement those schemes
within qemu or leave that to the firewall to handle (connlimit comes to
mind, see "man iptables-extensions").

Doing it in qemu IMO only makes sense when using information the
firewall doesn't have.  With sasl enabled we can slow down login
attempts *per user* for example.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Gonglei]

On 2014/10/21 17:35, Gerd Hoffmann wrote:

>   Hi,
> 
>> Yes. But I think it is not a big problem, when the REJECT_TIME is over,
>> the good guys can connect vnc successfully immediately.
>> Or maybe we just lock those guys with "the same Source IP address" ?
> 
> Better.  Question is whenever we really want implement those schemes
> within qemu or leave that to the firewall to handle (connlimit comes to
> mind, see "man iptables-extensions").
> 

Got it.

> Doing it in qemu IMO only makes sense when using information the
> firewall doesn't have.  With sasl enabled we can slow down login
> attempts *per user* for example.
> 

OK. Thanks for your opinion. :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections

[Eric Blake]

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

On 10/15/2014 06:19 AM, Gerd Hoffmann wrote:
> Also track the number of connections in "connecting" and "shared" state
> (additionally to "exclusive" state).  Apply a configurable limit to

s/additionally to/in addition to the/

> these connections.
> 
> The logic to apply the limit to connections in "shared" state is pretty
> simple:  When the limit is reached no new connections are allowed.
> 
> The logic to apply the limit to connections in "connecting" state (this
> is the state you are in *before* successfull authentication) is

s/successfull/successful/

> slightly different:  A new connect kicks out the oldest client which is
> still in "connecting" state.  This avoids a easy DoS by unauthenticated
> users by simply opening connections until the limit is reached.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
>  ui/vnc.h |  3 +++
>  2 files changed, 46 insertions(+), 3 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Daniel P. Berrange]

On Wed, Oct 15, 2014 at 02:19:39PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> This patch series adds support for multiple vnc server instances to
> qemu.  This comes handy in multiseat configurations as you can have
> one vnc server for each set then.
> 
> Some cleanups along the way (use QemuOpts).  Also added support for
> limiting the number of parallel vnc connections.
> 
> To be done: monitor support.  I think the best way to handle this is to
> introduce a new command to query vnc server state, which returns a list
> of vnc servers but otherwise works like "query-vnc".  Alternative
> approach would be to add a optional 'id=' parameter to query-vnc, but
> then you'll need a new list-vnc command.
> 
> Opinions on this?

I don't think it makes a whole lot of difference really. Either way
mgmt apps like libvirt will need updating to deal with the new monitor
commands.

> set_password and expire_password commands should be easy, they can be
> extended with an optional 'id=' parameter.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Gerd Hoffmann]

  Hi,

> > To be done: monitor support.  I think the best way to handle this is to
> > introduce a new command to query vnc server state, which returns a list
> > of vnc servers but otherwise works like "query-vnc".  Alternative
> > approach would be to add a optional 'id=' parameter to query-vnc, but
> > then you'll need a new list-vnc command.
> > 
> > Opinions on this?
> 
> I don't think it makes a whole lot of difference really. Either way
> mgmt apps like libvirt will need updating to deal with the new monitor
> commands.

One thing that comes do mind is that if we do a completely new command
we can easily fix any shortcomings of the existing query-vnc command.

The listening address could be changed to be a *list* of addresses,
allowing to notify libvirt about multiple listening sockets.  One ipv4,
one ipv6 being a use case.

If someone has more issues / suggestions I'd be happy to hear about
them.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Daniel P. Berrange]

On Wed, Oct 15, 2014 at 04:29:47PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > To be done: monitor support.  I think the best way to handle this is to
> > > introduce a new command to query vnc server state, which returns a list
> > > of vnc servers but otherwise works like "query-vnc".  Alternative
> > > approach would be to add a optional 'id=' parameter to query-vnc, but
> > > then you'll need a new list-vnc command.
> > > 
> > > Opinions on this?
> > 
> > I don't think it makes a whole lot of difference really. Either way
> > mgmt apps like libvirt will need updating to deal with the new monitor
> > commands.
> 
> One thing that comes do mind is that if we do a completely new command
> we can easily fix any shortcomings of the existing query-vnc command.
> 
> The listening address could be changed to be a *list* of addresses,
> allowing to notify libvirt about multiple listening sockets.  One ipv4,
> one ipv6 being a use case.

That sounds like a good reason. I can even imagine in the future there
could be multiple IPv4 and multiple IPv6 addresses per VNC server.
eg if the host has many NICs present and we only want to listen on a
subset of them we might want to pass multiple IPv4 addresses to QEMU
on the CLI.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Daniel P. Berrange]

On Wed, Oct 15, 2014 at 02:19:39PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> This patch series adds support for multiple vnc server instances to
> qemu.  This comes handy in multiseat configurations as you can have
> one vnc server for each set then.
> 
> Some cleanups along the way (use QemuOpts).  Also added support for
> limiting the number of parallel vnc connections.
> 
> To be done: monitor support.  I think the best way to handle this is to
> introduce a new command to query vnc server state, which returns a list
> of vnc servers but otherwise works like "query-vnc".  Alternative
> approach would be to add a optional 'id=' parameter to query-vnc, but
> then you'll need a new list-vnc command.
> 
> Opinions on this?
> 
> set_password and expire_password commands should be easy, they can be
> extended with an optional 'id=' parameter.

In any serious deployment password auth won't be something that's used
due to its horrific insecurity. For TLS/SASL authentication protocols
we currently have a single acces control list defined

   qemu_acl_init("vnc.x509dname")   (validates x509 certificate)
   qemu_acl_init("vnc.username")    (validates SASL user name)

I think we are going to need to make one ACL list per seat. eg

   qemu_acl_init("vnc.x509dname")   (validates x509 certificate on seat 0)
   qemu_acl_init("vnc.username")    (validates SASL user name on seat 0)

   qemu_acl_init("vnc.x509dname.1")   (validates x509 certificate on seat 1)
   qemu_acl_init("vnc.username.1")    (validates SASL user name on seat 1)

   qemu_acl_init("vnc.x509dname.2")   (validates x509 certificate on seat 2)
   qemu_acl_init("vnc.username.2")    (validates SASL user name on seat 2)

Note, not changing the first ACL names for compat reasons.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Gerd Hoffmann]

  Hi,

> I think we are going to need to make one ACL list per seat. eg
> 
>    qemu_acl_init("vnc.x509dname")   (validates x509 certificate on seat 0)
>    qemu_acl_init("vnc.username")    (validates SASL user name on seat 0)
> 
>    qemu_acl_init("vnc.x509dname.1")   (validates x509 certificate on seat 1)
>    qemu_acl_init("vnc.username.1")    (validates SASL user name on seat 1)

Thanks.  I'll go look into this for v2.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances.

[Eric Blake]

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

On 10/15/2014 06:19 AM, Gerd Hoffmann wrote:
>   Hi,
> 
> This patch series adds support for multiple vnc server instances to
> qemu.  This comes handy in multiseat configurations as you can have
> one vnc server for each set then.
> 
> Some cleanups along the way (use QemuOpts).  Also added support for
> limiting the number of parallel vnc connections.
> 
> To be done: monitor support.  I think the best way to handle this is to
> introduce a new command to query vnc server state, which returns a list
> of vnc servers but otherwise works like "query-vnc".  Alternative
> approach would be to add a optional 'id=' parameter to query-vnc, but
> then you'll need a new list-vnc command.
> 
> Opinions on this?

Bummer - query-vnc returns a single struct VncInfo instead of an array.
Maybe the thing to do is add a new command:

{ 'command': 'query-vncs', 'returns': [ 'VncInfo' ] }

which returns an array where the first element matches the older
'query-vnc', but by being an array it lets you return all vnc
information in one call.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]