Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-20150318-1' into staging

vnc: fix websockets & QMP.

# gpg: Signature made Wed Mar 18 13:12:35 2015 GMT using RSA key ID D3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>"
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>"
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>"

* remotes/kraxel/tags/pull-vnc-20150318-1:
  ui: ensure VNC websockets server checks the ACL if requested
  ui: remove separate gnutls_session for websockets server
  ui: enforce TLS when using websockets server
  ui: fix setup of VNC websockets auth scheme with TLS
  ui: split setup of VNC auth scheme into separate method
  ui: report error if user requests VNC option that is unsupported
  ui: replace printf() calls with VNC_DEBUG
  ui: remove unused 'wiremode' variable in VncState struct
  vnc: Fix QMP change not to use funky error class

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

7 files changed, 223 insertions, 203 deletions

diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index bc7032e695..a420ccbd1d 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -93,7 +93,6 @@ static int vnc_start_vencrypt_handshake(struct VncState *vs) {
     }
 
     VNC_DEBUG("Handshake done, switching to TLS data mode\n");
-    vs->tls.wiremode = VNC_WIREMODE_TLS;
     qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, vnc_client_write, vs);
 
     start_auth_vencrypt_subauth(vs);
diff --git a/ui/vnc-tls.c b/ui/vnc-tls.c
index 0f59f9b28e..eddd39b08e 100644
--- a/ui/vnc-tls.c
+++ b/ui/vnc-tls.c
@@ -334,82 +334,77 @@ static int vnc_set_gnutls_priority(gnutls_session_t s, int x509)
 
 int vnc_tls_client_setup(struct VncState *vs,
                          int needX509Creds) {
-    VncStateTLS *tls;
-
     VNC_DEBUG("Do TLS setup\n");
-#ifdef CONFIG_VNC_WS
-    if (vs->websocket) {
-        tls = &vs->ws_tls;
-    } else
-#endif /* CONFIG_VNC_WS */
-    {
-        tls = &vs->tls;
-    }
     if (vnc_tls_initialize() < 0) {
         VNC_DEBUG("Failed to init TLS\n");
         vnc_client_error(vs);
         return -1;
     }
-    if (tls->session == NULL) {
-        if (gnutls_init(&tls->session, GNUTLS_SERVER) < 0) {
+    if (vs->tls.session == NULL) {
+        if (gnutls_init(&vs->tls.session, GNUTLS_SERVER) < 0) {
             vnc_client_error(vs);
             return -1;
         }
 
-        if (gnutls_set_default_priority(tls->session) < 0) {
-            gnutls_deinit(tls->session);
-            tls->session = NULL;
+        if (gnutls_set_default_priority(vs->tls.session) < 0) {
+            gnutls_deinit(vs->tls.session);
+            vs->tls.session = NULL;
             vnc_client_error(vs);
             return -1;
         }
 
-        if (vnc_set_gnutls_priority(tls->session, needX509Creds) < 0) {
-            gnutls_deinit(tls->session);
-            tls->session = NULL;
+        if (vnc_set_gnutls_priority(vs->tls.session, needX509Creds) < 0) {
+            gnutls_deinit(vs->tls.session);
+            vs->tls.session = NULL;
             vnc_client_error(vs);
             return -1;
         }
 
         if (needX509Creds) {
-            gnutls_certificate_server_credentials x509_cred = vnc_tls_initialize_x509_cred(vs->vd);
+            gnutls_certificate_server_credentials x509_cred =
+                vnc_tls_initialize_x509_cred(vs->vd);
             if (!x509_cred) {
-                gnutls_deinit(tls->session);
-                tls->session = NULL;
+                gnutls_deinit(vs->tls.session);
+                vs->tls.session = NULL;
                 vnc_client_error(vs);
                 return -1;
             }
-            if (gnutls_credentials_set(tls->session, GNUTLS_CRD_CERTIFICATE, x509_cred) < 0) {
-                gnutls_deinit(tls->session);
-                tls->session = NULL;
+            if (gnutls_credentials_set(vs->tls.session,
+                                       GNUTLS_CRD_CERTIFICATE, x509_cred) < 0) {
+                gnutls_deinit(vs->tls.session);
+                vs->tls.session = NULL;
                 gnutls_certificate_free_credentials(x509_cred);
                 vnc_client_error(vs);
                 return -1;
             }
             if (vs->vd->tls.x509verify) {
                 VNC_DEBUG("Requesting a client certificate\n");
-                gnutls_certificate_server_set_request (tls->session, GNUTLS_CERT_REQUEST);
+                gnutls_certificate_server_set_request(vs->tls.session,
+                                                      GNUTLS_CERT_REQUEST);
             }
 
         } else {
-            gnutls_anon_server_credentials_t anon_cred = vnc_tls_initialize_anon_cred();
+            gnutls_anon_server_credentials_t anon_cred =
+                vnc_tls_initialize_anon_cred();
             if (!anon_cred) {
-                gnutls_deinit(tls->session);
-                tls->session = NULL;
+                gnutls_deinit(vs->tls.session);
+                vs->tls.session = NULL;
                 vnc_client_error(vs);
                 return -1;
             }
-            if (gnutls_credentials_set(tls->session, GNUTLS_CRD_ANON, anon_cred) < 0) {
-                gnutls_deinit(tls->session);
-                tls->session = NULL;
+            if (gnutls_credentials_set(vs->tls.session,
+                                       GNUTLS_CRD_ANON, anon_cred) < 0) {
+                gnutls_deinit(vs->tls.session);
+                vs->tls.session = NULL;
                 gnutls_anon_free_server_credentials(anon_cred);
                 vnc_client_error(vs);
                 return -1;
             }
         }
 
-        gnutls_transport_set_ptr(tls->session, (gnutls_transport_ptr_t)vs);
-        gnutls_transport_set_push_function(tls->session, vnc_tls_push);
-        gnutls_transport_set_pull_function(tls->session, vnc_tls_pull);
+        gnutls_transport_set_ptr(vs->tls.session, (gnutls_transport_ptr_t)vs);
+        gnutls_transport_set_push_function(vs->tls.session, vnc_tls_push);
+        gnutls_transport_set_pull_function(vs->tls.session, vnc_tls_pull);
     }
     return 0;
 }
@@ -421,16 +416,7 @@ void vnc_tls_client_cleanup(struct VncState *vs)
         gnutls_deinit(vs->tls.session);
         vs->tls.session = NULL;
     }
-    vs->tls.wiremode = VNC_WIREMODE_CLEAR;
     g_free(vs->tls.dname);
-#ifdef CONFIG_VNC_WS
-    if (vs->ws_tls.session) {
-        gnutls_deinit(vs->ws_tls.session);
-        vs->ws_tls.session = NULL;
-    }
-    vs->ws_tls.wiremode = VNC_WIREMODE_CLEAR;
-    g_free(vs->ws_tls.dname);
-#endif /* CONFIG_VNC_WS */
 }
 
 
diff --git a/ui/vnc-tls.h b/ui/vnc-tls.h
index 36a2227fec..f9829c7824 100644
--- a/ui/vnc-tls.h
+++ b/ui/vnc-tls.h
@@ -33,11 +33,6 @@
 
 #include "qemu/acl.h"
 
-enum {
-    VNC_WIREMODE_CLEAR,
-    VNC_WIREMODE_TLS,
-};
-
 typedef struct VncDisplayTLS VncDisplayTLS;
 typedef struct VncStateTLS VncStateTLS;
 
@@ -55,8 +50,6 @@ struct VncDisplayTLS {
 
 /* Per client state */
 struct VncStateTLS {
-    /* Whether data is being TLS encrypted yet */
-    int wiremode;
     gnutls_session_t session;
 
     /* Client's Distinguished Name from the x509 cert */
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index d75950d7b1..85dbb7e6ae 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -24,16 +24,14 @@
 #ifdef CONFIG_VNC_TLS
 #include "qemu/sockets.h"
 
-static void vncws_tls_handshake_io(void *opaque);
-
 static int vncws_start_tls_handshake(struct VncState *vs)
 {
-    int ret = gnutls_handshake(vs->ws_tls.session);
+    int ret = gnutls_handshake(vs->tls.session);
 
     if (ret < 0) {
         if (!gnutls_error_is_fatal(ret)) {
             VNC_DEBUG("Handshake interrupted (blocking)\n");
-            if (!gnutls_record_get_direction(vs->ws_tls.session)) {
+            if (!gnutls_record_get_direction(vs->tls.session)) {
                 qemu_set_fd_handler(vs->csock, vncws_tls_handshake_io,
                                     NULL, vs);
             } else {
@@ -47,40 +45,34 @@ static int vncws_start_tls_handshake(struct VncState *vs)
         return -1;
     }
 
+    if (vs->vd->tls.x509verify) {
+        if (vnc_tls_validate_certificate(vs) < 0) {
+            VNC_DEBUG("Client verification failed\n");
+            vnc_client_error(vs);
+            return -1;
+        } else {
+            VNC_DEBUG("Client verification passed\n");
+        }
+    }
+
     VNC_DEBUG("Handshake done, switching to TLS data mode\n");
-    vs->ws_tls.wiremode = VNC_WIREMODE_TLS;
     qemu_set_fd_handler2(vs->csock, NULL, vncws_handshake_read, NULL, vs);
 
     return 0;
 }
 
-static void vncws_tls_handshake_io(void *opaque)
+void vncws_tls_handshake_io(void *opaque)
 {
     struct VncState *vs = (struct VncState *)opaque;
 
-    VNC_DEBUG("Handshake IO continue\n");
-    vncws_start_tls_handshake(vs);
-}
-
-void vncws_tls_handshake_peek(void *opaque)
-{
-    VncState *vs = opaque;
-    long ret;
-
-    if (!vs->ws_tls.session) {
-        char peek[4];
-        ret = qemu_recv(vs->csock, peek, sizeof(peek), MSG_PEEK);
-        if (ret && (strncmp(peek, "\x16", 1) == 0
-                    || strncmp(peek, "\x80", 1) == 0)) {
-            VNC_DEBUG("TLS Websocket connection recognized");
-            vnc_tls_client_setup(vs, 1);
-            vncws_start_tls_handshake(vs);
-        } else {
-            vncws_handshake_read(vs);
+    if (!vs->tls.session) {
+        VNC_DEBUG("TLS Websocket setup\n");
+        if (vnc_tls_client_setup(vs, vs->vd->tls.x509cert != NULL) < 0) {
+            return;
         }
-    } else {
-        qemu_set_fd_handler2(vs->csock, NULL, vncws_handshake_read, NULL, vs);
     }
+    VNC_DEBUG("Handshake IO continue\n");
+    vncws_start_tls_handshake(vs);
 }
 #endif /* CONFIG_VNC_TLS */
 
diff --git a/ui/vnc-ws.h b/ui/vnc-ws.h
index 95c1b0aeae..ef229b7c0c 100644
--- a/ui/vnc-ws.h
+++ b/ui/vnc-ws.h
@@ -75,7 +75,7 @@ enum {
 };
 
 #ifdef CONFIG_VNC_TLS
-void vncws_tls_handshake_peek(void *opaque);
+void vncws_tls_handshake_io(void *opaque);
 #endif /* CONFIG_VNC_TLS */
 void vncws_handshake_read(void *opaque);
 long vnc_client_write_ws(VncState *vs);
diff --git a/ui/vnc.c b/ui/vnc.c
index 6f9b718814..cffb5b74b3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1343,15 +1343,8 @@ long vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
     if (vs->tls.session) {
         ret = vnc_client_write_tls(&vs->tls.session, data, datalen);
     } else {
-#ifdef CONFIG_VNC_WS
-        if (vs->ws_tls.session) {
-            ret = vnc_client_write_tls(&vs->ws_tls.session, data, datalen);
-        } else
-#endif /* CONFIG_VNC_WS */
 #endif /* CONFIG_VNC_TLS */
-        {
-            ret = send(vs->csock, (const void *)data, datalen, 0);
-        }
+        ret = send(vs->csock, (const void *)data, datalen, 0);
 #ifdef CONFIG_VNC_TLS
     }
 #endif /* CONFIG_VNC_TLS */
@@ -1491,15 +1484,8 @@ long vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
     if (vs->tls.session) {
         ret = vnc_client_read_tls(&vs->tls.session, data, datalen);
     } else {
-#ifdef CONFIG_VNC_WS
-        if (vs->ws_tls.session) {
-            ret = vnc_client_read_tls(&vs->ws_tls.session, data, datalen);
-        } else
-#endif /* CONFIG_VNC_WS */
 #endif /* CONFIG_VNC_TLS */
-        {
-            ret = qemu_recv(vs->csock, data, datalen, 0);
-        }
+        ret = qemu_recv(vs->csock, data, datalen, 0);
 #ifdef CONFIG_VNC_TLS
     }
 #endif /* CONFIG_VNC_TLS */
@@ -2400,34 +2386,34 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
                 case 4: vs->as.fmt = AUD_FMT_U32; break;
                 case 5: vs->as.fmt = AUD_FMT_S32; break;
                 default:
-                    printf("Invalid audio format %d\n", read_u8(data, 4));
+                    VNC_DEBUG("Invalid audio format %d\n", read_u8(data, 4));
                     vnc_client_error(vs);
                     break;
                 }
                 vs->as.nchannels = read_u8(data, 5);
                 if (vs->as.nchannels != 1 && vs->as.nchannels != 2) {
-                    printf("Invalid audio channel coount %d\n",
-                           read_u8(data, 5));
+                    VNC_DEBUG("Invalid audio channel coount %d\n",
+                              read_u8(data, 5));
                     vnc_client_error(vs);
                     break;
                 }
                 vs->as.freq = read_u32(data, 6);
                 break;
             default:
-                printf ("Invalid audio message %d\n", read_u8(data, 4));
+                VNC_DEBUG("Invalid audio message %d\n", read_u8(data, 4));
                 vnc_client_error(vs);
                 break;
             }
             break;
 
         default:
-            printf("Msg: %d\n", read_u16(data, 0));
+            VNC_DEBUG("Msg: %d\n", read_u16(data, 0));
             vnc_client_error(vs);
             break;
         }
         break;
     default:
-        printf("Msg: %d\n", data[0]);
+        VNC_DEBUG("Msg: %d\n", data[0]);
         vnc_client_error(vs);
         break;
     }
@@ -3010,15 +2996,18 @@ static void vnc_connect(VncDisplay *vd, int csock,
 
     if (skipauth) {
 	vs->auth = VNC_AUTH_NONE;
-#ifdef CONFIG_VNC_TLS
 	vs->subauth = VNC_AUTH_INVALID;
-#endif
     } else {
-	vs->auth = vd->auth;
-#ifdef CONFIG_VNC_TLS
-	vs->subauth = vd->subauth;
-#endif
+        if (websocket) {
+            vs->auth = vd->ws_auth;
+            vs->subauth = VNC_AUTH_INVALID;
+        } else {
+            vs->auth = vd->auth;
+            vs->subauth = vd->subauth;
+        }
     }
+    VNC_DEBUG("Client sock=%d ws=%d auth=%d subauth=%d\n",
+              csock, websocket, vs->auth, vs->subauth);
 
     vs->lossy_rect = g_malloc0(VNC_STAT_ROWS * sizeof (*vs->lossy_rect));
     for (i = 0; i < VNC_STAT_ROWS; ++i) {
@@ -3032,8 +3021,8 @@ static void vnc_connect(VncDisplay *vd, int csock,
     if (websocket) {
         vs->websocket = 1;
 #ifdef CONFIG_VNC_TLS
-        if (vd->tls.x509cert) {
-            qemu_set_fd_handler2(vs->csock, NULL, vncws_tls_handshake_peek,
+        if (vd->ws_tls) {
+            qemu_set_fd_handler2(vs->csock, NULL, vncws_tls_handshake_io,
                                  NULL, vs);
         } else
 #endif /* CONFIG_VNC_TLS */
@@ -3206,8 +3195,8 @@ static void vnc_display_close(VncDisplay *vs)
     }
 #endif /* CONFIG_VNC_WS */
     vs->auth = VNC_AUTH_INVALID;
-#ifdef CONFIG_VNC_TLS
     vs->subauth = VNC_AUTH_INVALID;
+#ifdef CONFIG_VNC_TLS
     vs->tls.x509verify = 0;
 #endif
 }
@@ -3318,6 +3307,134 @@ static QemuOptsList qemu_vnc_opts = {
     },
 };
 
+
+static void
+vnc_display_setup_auth(VncDisplay *vs,
+                       bool password,
+                       bool sasl,
+                       bool tls,
+                       bool x509,
+                       bool websocket)
+{
+    /*
+     * We have a choice of 3 authentication options
+     *
+     *   1. none
+     *   2. vnc
+     *   3. sasl
+     *
+     * The channel can be run in 2 modes
+     *
+     *   1. clear
+     *   2. tls
+     *
+     * And TLS can use 2 types of credentials
+     *
+     *   1. anon
+     *   2. x509
+     *
+     * We thus have 9 possible logical combinations
+     *
+     *   1. clear + none
+     *   2. clear + vnc
+     *   3. clear + sasl
+     *   4. tls + anon + none
+     *   5. tls + anon + vnc
+     *   6. tls + anon + sasl
+     *   7. tls + x509 + none
+     *   8. tls + x509 + vnc
+     *   9. tls + x509 + sasl
+     *
+     * These need to be mapped into the VNC auth schemes
+     * in an appropriate manner. In regular VNC, all the
+     * TLS options get mapped into VNC_AUTH_VENCRYPT
+     * sub-auth types.
+     *
+     * In websockets, the https:// protocol already provides
+     * TLS support, so there is no need to make use of the
+     * VeNCrypt extension. Furthermore, websockets browser
+     * clients could not use VeNCrypt even if they wanted to,
+     * as they cannot control when the TLS handshake takes
+     * place. Thus there is no option but to rely on https://,
+     * meaning combinations 4->6 and 7->9 will be mapped to
+     * VNC auth schemes in the same way as combos 1->3.
+     *
+     * Regardless of fact that we have a different mapping to
+     * VNC auth mechs for plain VNC vs websockets VNC, the end
+     * result has the same security characteristics.
+     */
+    if (password) {
+        if (tls) {
+            vs->auth = VNC_AUTH_VENCRYPT;
+            if (websocket) {
+                vs->ws_tls = true;
+            }
+            if (x509) {
+                VNC_DEBUG("Initializing VNC server with x509 password auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_X509VNC;
+            } else {
+                VNC_DEBUG("Initializing VNC server with TLS password auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_TLSVNC;
+            }
+        } else {
+            VNC_DEBUG("Initializing VNC server with password auth\n");
+            vs->auth = VNC_AUTH_VNC;
+            vs->subauth = VNC_AUTH_INVALID;
+        }
+        if (websocket) {
+            vs->ws_auth = VNC_AUTH_VNC;
+        } else {
+            vs->ws_auth = VNC_AUTH_INVALID;
+        }
+    } else if (sasl) {
+        if (tls) {
+            vs->auth = VNC_AUTH_VENCRYPT;
+            if (websocket) {
+                vs->ws_tls = true;
+            }
+            if (x509) {
+                VNC_DEBUG("Initializing VNC server with x509 SASL auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_X509SASL;
+            } else {
+                VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
+            }
+        } else {
+            VNC_DEBUG("Initializing VNC server with SASL auth\n");
+            vs->auth = VNC_AUTH_SASL;
+            vs->subauth = VNC_AUTH_INVALID;
+        }
+        if (websocket) {
+            vs->ws_auth = VNC_AUTH_SASL;
+        } else {
+            vs->ws_auth = VNC_AUTH_INVALID;
+        }
+    } else {
+        if (tls) {
+            vs->auth = VNC_AUTH_VENCRYPT;
+            if (websocket) {
+                vs->ws_tls = true;
+            }
+            if (x509) {
+                VNC_DEBUG("Initializing VNC server with x509 no auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_X509NONE;
+            } else {
+                VNC_DEBUG("Initializing VNC server with TLS no auth\n");
+                vs->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
+            }
+        } else {
+            VNC_DEBUG("Initializing VNC server with no auth\n");
+            vs->auth = VNC_AUTH_NONE;
+            vs->subauth = VNC_AUTH_INVALID;
+        }
+        if (websocket) {
+            vs->ws_auth = VNC_AUTH_NONE;
+        } else {
+            vs->ws_auth = VNC_AUTH_INVALID;
+        }
+    }
+}
+
 void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vs = vnc_display_find(id);
@@ -3332,15 +3449,13 @@ void vnc_display_open(const char *id, Error **errp)
     char *h;
     bool has_ipv4 = false;
     bool has_ipv6 = false;
-#ifdef CONFIG_VNC_WS
     const char *websocket;
-#endif
-#ifdef CONFIG_VNC_TLS
     bool tls = false, x509 = false;
+#ifdef CONFIG_VNC_TLS
     const char *path;
 #endif
-#ifdef CONFIG_VNC_SASL
     bool sasl = false;
+#ifdef CONFIG_VNC_SASL
     int saslErr;
 #endif
 #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
@@ -3404,11 +3519,15 @@ void vnc_display_open(const char *id, Error **errp)
 
     reverse = qemu_opt_get_bool(opts, "reverse", false);
     lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
-#ifdef CONFIG_VNC_SASL
     sasl = qemu_opt_get_bool(opts, "sasl", false);
-#endif
-#ifdef CONFIG_VNC_TLS
+#ifndef CONFIG_VNC_SASL
+    if (sasl) {
+        error_setg(errp, "VNC SASL auth requires cyrus-sasl support");
+        goto fail;
+    }
+#endif /* CONFIG_VNC_SASL */
     tls  = qemu_opt_get_bool(opts, "tls", false);
+#ifdef CONFIG_VNC_TLS
     path = qemu_opt_get(opts, "x509");
     if (!path) {
         path = qemu_opt_get(opts, "x509verify");
@@ -3424,7 +3543,12 @@ void vnc_display_open(const char *id, Error **errp)
             goto fail;
         }
     }
-#endif
+#else /* ! CONFIG_VNC_TLS */
+    if (tls) {
+        error_setg(errp, "VNC TLS auth requires gnutls support");
+        goto fail;
+    }
+#endif /* ! CONFIG_VNC_TLS */
 #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
     acl = qemu_opt_get_bool(opts, "acl", false);
 #endif
@@ -3446,14 +3570,16 @@ void vnc_display_open(const char *id, Error **errp)
     }
     vs->connections_limit = qemu_opt_get_number(opts, "connections", 32);
 
- #ifdef CONFIG_VNC_WS
     websocket = qemu_opt_get(opts, "websocket");
     if (websocket) {
+#ifdef CONFIG_VNC_WS
         vs->ws_enabled = true;
         qemu_opt_set(wsopts, "port", websocket, &error_abort);
-
+#else /* ! CONFIG_VNC_WS */
+        error_setg(errp, "Websockets protocol requires gnutls support");
+        goto fail;
+#endif /* ! CONFIG_VNC_WS */
     }
-#endif /* CONFIG_VNC_WS */
 
 #ifdef CONFIG_VNC_JPEG
     vs->lossy = qemu_opt_get_bool(opts, "lossy", false);
@@ -3501,82 +3627,7 @@ void vnc_display_open(const char *id, Error **errp)
     }
 #endif
 
-    /*
-     * Combinations we support here:
-     *
-     *  - no-auth                (clear text, no auth)
-     *  - password               (clear text, weak auth)
-     *  - sasl                   (encrypt, good auth *IF* using Kerberos via GSSAPI)
-     *  - tls                    (encrypt, weak anonymous creds, no auth)
-     *  - tls + password         (encrypt, weak anonymous creds, weak auth)
-     *  - tls + sasl             (encrypt, weak anonymous creds, good auth)
-     *  - tls + x509             (encrypt, good x509 creds, no auth)
-     *  - tls + x509 + password  (encrypt, good x509 creds, weak auth)
-     *  - tls + x509 + sasl      (encrypt, good x509 creds, good auth)
-     *
-     * NB1. TLS is a stackable auth scheme.
-     * NB2. the x509 schemes have option to validate a client cert dname
-     */
-    if (password) {
-#ifdef CONFIG_VNC_TLS
-        if (tls) {
-            vs->auth = VNC_AUTH_VENCRYPT;
-            if (x509) {
-                VNC_DEBUG("Initializing VNC server with x509 password auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_X509VNC;
-            } else {
-                VNC_DEBUG("Initializing VNC server with TLS password auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_TLSVNC;
-            }
-        } else {
-#endif /* CONFIG_VNC_TLS */
-            VNC_DEBUG("Initializing VNC server with password auth\n");
-            vs->auth = VNC_AUTH_VNC;
-#ifdef CONFIG_VNC_TLS
-            vs->subauth = VNC_AUTH_INVALID;
-        }
-#endif /* CONFIG_VNC_TLS */
-#ifdef CONFIG_VNC_SASL
-    } else if (sasl) {
-#ifdef CONFIG_VNC_TLS
-        if (tls) {
-            vs->auth = VNC_AUTH_VENCRYPT;
-            if (x509) {
-                VNC_DEBUG("Initializing VNC server with x509 SASL auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_X509SASL;
-            } else {
-                VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
-            }
-        } else {
-#endif /* CONFIG_VNC_TLS */
-            VNC_DEBUG("Initializing VNC server with SASL auth\n");
-            vs->auth = VNC_AUTH_SASL;
-#ifdef CONFIG_VNC_TLS
-            vs->subauth = VNC_AUTH_INVALID;
-        }
-#endif /* CONFIG_VNC_TLS */
-#endif /* CONFIG_VNC_SASL */
-    } else {
-#ifdef CONFIG_VNC_TLS
-        if (tls) {
-            vs->auth = VNC_AUTH_VENCRYPT;
-            if (x509) {
-                VNC_DEBUG("Initializing VNC server with x509 no auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_X509NONE;
-            } else {
-                VNC_DEBUG("Initializing VNC server with TLS no auth\n");
-                vs->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
-            }
-        } else {
-#endif
-            VNC_DEBUG("Initializing VNC server with no auth\n");
-            vs->auth = VNC_AUTH_NONE;
-#ifdef CONFIG_VNC_TLS
-            vs->subauth = VNC_AUTH_INVALID;
-        }
-#endif
-    }
+    vnc_display_setup_auth(vs, password, sasl, tls, x509, websocket);
 
 #ifdef CONFIG_VNC_SASL
     if ((saslErr = sasl_server_init(NULL, "qemu")) != SASL_OK) {
@@ -3594,7 +3645,7 @@ void vnc_display_open(const char *id, Error **errp)
 
         dev = qdev_find_recursive(sysbus_get_default(), device_id);
         if (dev == NULL) {
-            error_set(errp, QERR_DEVICE_NOT_FOUND, device_id);
+            error_setg(errp, "Device '%s' not found", device_id);
             goto fail;
         }
 
diff --git a/ui/vnc.h b/ui/vnc.h
index 66a02986c7..e19ac396f2 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -180,10 +180,12 @@ struct VncDisplay
     char *password;
     time_t expires;
     int auth;
+    int subauth; /* Used by VeNCrypt */
+    int ws_auth; /* Used by websockets */
+    bool ws_tls; /* Used by websockets */
     bool lossy;
     bool non_adaptive;
 #ifdef CONFIG_VNC_TLS
-    int subauth; /* Used by VeNCrypt */
     VncDisplayTLS tls;
 #endif
 #ifdef CONFIG_VNC_SASL
@@ -284,18 +286,15 @@ struct VncState
     int minor;
 
     int auth;
+    int subauth; /* Used by VeNCrypt */
     char challenge[VNC_AUTH_CHALLENGE_SIZE];
 #ifdef CONFIG_VNC_TLS
-    int subauth; /* Used by VeNCrypt */
     VncStateTLS tls;
 #endif
 #ifdef CONFIG_VNC_SASL
     VncStateSASL sasl;
 #endif
 #ifdef CONFIG_VNC_WS
-#ifdef CONFIG_VNC_TLS
-    VncStateTLS ws_tls;
-#endif /* CONFIG_VNC_TLS */
     bool encode_ws;
     bool websocket;
 #endif /* CONFIG_VNC_WS */