[Qemu-devel] [PATCH FYI 17/46] ui: convert VNC server to use QIOChannelTLS

["Daniel P. Berrange" <berrange@redhat.com>]

Switch VNC server over to using the QIOChannelTLS object for
the TLS session. This removes all remaining VNC specific code
for dealing with TLS handshakes.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc-auth-sasl.c     |  26 ++++++------
 ui/vnc-auth-vencrypt.c | 106 ++++++++++++++-----------------------------------
 ui/vnc-ws.c            |  95 +++++++++++++++++---------------------------
 ui/vnc.c               |  73 ++++------------------------------
 ui/vnc.h               |   5 +--
 5 files changed, 88 insertions(+), 217 deletions(-)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index f5bfdd1..98597a0 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -521,7 +521,7 @@ void start_auth_sasl(VncState *vs)
 {
     const char *mechlist = NULL;
     sasl_security_properties_t secprops;
-    int err;
+    int ret;
     char *localAddr, *remoteAddr;
     int mechlistlen;
 
@@ -539,7 +539,7 @@ void start_auth_sasl(VncState *vs)
         goto authabort;
     }
 
-    err = sasl_server_new("vnc",
+    ret = sasl_server_new("vnc",
                           NULL, /* FQDN - just delegates to gethostname */
                           NULL, /* User realm */
                           localAddr,
@@ -551,9 +551,9 @@ void start_auth_sasl(VncState *vs)
     g_free(remoteAddr);
     localAddr = remoteAddr = NULL;
 
-    if (err != SASL_OK) {
+    if (ret != SASL_OK) {
         VNC_DEBUG("sasl context setup failed %d (%s)",
-                  err, sasl_errstring(err, NULL, NULL));
+                  ret, sasl_errstring(ret, NULL, NULL));
         vs->sasl.conn = NULL;
         goto authabort;
     }
@@ -577,10 +577,10 @@ void start_auth_sasl(VncState *vs)
         }
         ssf = keysize * CHAR_BIT; /* tls key size is bytes, sasl wants bits */
 
-        err = sasl_setprop(vs->sasl.conn, SASL_SSF_EXTERNAL, &ssf);
-        if (err != SASL_OK) {
+        ret = sasl_setprop(vs->sasl.conn, SASL_SSF_EXTERNAL, &ssf);
+        if (ret != SASL_OK) {
             VNC_DEBUG("cannot set SASL external SSF %d (%s)\n",
-                      err, sasl_errstring(err, NULL, NULL));
+                      ret, sasl_errstring(ret, NULL, NULL));
             sasl_dispose(&vs->sasl.conn);
             vs->sasl.conn = NULL;
             goto authabort;
@@ -613,16 +613,16 @@ void start_auth_sasl(VncState *vs)
             SASL_SEC_NOANONYMOUS | SASL_SEC_NOPLAINTEXT;
     }
 
-    err = sasl_setprop(vs->sasl.conn, SASL_SEC_PROPS, &secprops);
-    if (err != SASL_OK) {
+    ret = sasl_setprop(vs->sasl.conn, SASL_SEC_PROPS, &secprops);
+    if (ret != SASL_OK) {
         VNC_DEBUG("cannot set SASL security props %d (%s)\n",
-                  err, sasl_errstring(err, NULL, NULL));
+                  ret, sasl_errstring(ret, NULL, NULL));
         sasl_dispose(&vs->sasl.conn);
         vs->sasl.conn = NULL;
         goto authabort;
     }
 
-    err = sasl_listmech(vs->sasl.conn,
+    ret = sasl_listmech(vs->sasl.conn,
                         NULL, /* Don't need to set user */
                         "", /* Prefix */
                         ",", /* Separator */
@@ -630,9 +630,9 @@ void start_auth_sasl(VncState *vs)
                         &mechlist,
                         NULL,
                         NULL);
-    if (err != SASL_OK) {
+    if (ret != SASL_OK) {
         VNC_DEBUG("cannot list SASL mechanisms %d (%s)\n",
-                  err, sasl_errdetail(vs->sasl.conn));
+                  ret, sasl_errdetail(vs->sasl.conn));
         sasl_dispose(&vs->sasl.conn);
         vs->sasl.conn = NULL;
         goto authabort;
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index 95a6823..093dd2f 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -63,71 +63,21 @@ static void start_auth_vencrypt_subauth(VncState *vs)
     }
 }
 
-static gboolean vnc_tls_handshake_io(QIOChannel *ioc,
-                                     GIOCondition condition,
-                                     void *opaque);
-
-static int vnc_start_vencrypt_handshake(VncState *vs)
+static void vnc_tls_handshake_done(Object *source,
+                                   Error *err,
+                                   gpointer user_data)
 {
-    Error *err = NULL;
-
-    if (qcrypto_tls_session_handshake(vs->tls, &err) < 0) {
-        goto error;
-    }
+    VncState *vs = user_data;
 
-    switch (qcrypto_tls_session_get_handshake_status(vs->tls)) {
-    case QCRYPTO_TLS_HANDSHAKE_COMPLETE:
-        VNC_DEBUG("Handshake done, checking credentials\n");
-        if (qcrypto_tls_session_check_credentials(vs->tls, &err) < 0) {
-            goto error;
-        }
-        VNC_DEBUG("Client verification passed, starting TLS I/O\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
+    if (err) {
+        VNC_DEBUG("Handshake failed %s\n",
+                  error_get_pretty(err));
+        vnc_client_error(vs);
+    } else {
         vs->ioc_tag = qio_channel_add_watch(
             vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
-
         start_auth_vencrypt_subauth(vs);
-        break;
-
-    case QCRYPTO_TLS_HANDSHAKE_RECVING:
-        VNC_DEBUG("Handshake interrupted (blocking read)\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
-        vs->ioc_tag = qio_channel_add_watch(
-            vs->ioc, G_IO_IN, vnc_tls_handshake_io, vs, NULL);
-        break;
-
-    case QCRYPTO_TLS_HANDSHAKE_SENDING:
-        VNC_DEBUG("Handshake interrupted (blocking write)\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
-        vs->ioc_tag = qio_channel_add_watch(
-            vs->ioc, G_IO_OUT, vnc_tls_handshake_io, vs, NULL);
-        break;
     }
-
-    return 0;
-
- error:
-    VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
-    error_free(err);
-    vnc_client_error(vs);
-    return -1;
-}
-
-static gboolean vnc_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
-                                     GIOCondition condition G_GNUC_UNUSED,
-                                     void *opaque)
-{
-    VncState *vs = (VncState *)opaque;
-
-    VNC_DEBUG("Handshake IO continue\n");
-    vnc_start_vencrypt_handshake(vs);
-    return TRUE;
 }
 
 
@@ -142,33 +92,37 @@ static int protocol_client_vencrypt_auth(VncState *vs, uint8_t *data, size_t len
         vnc_client_error(vs);
     } else {
         Error *err = NULL;
+        QIOChannelTLS *tls;
         VNC_DEBUG("Accepting auth %d, setting up TLS for handshake\n", auth);
         vnc_write_u8(vs, 1); /* Accept auth */
         vnc_flush(vs);
 
-        vs->tls = qcrypto_tls_session_new(vs->vd->tlscreds,
-                                          NULL,
-                                          vs->vd->tlsaclname,
-                                          QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
-                                          &err);
-        if (!vs->tls) {
-            VNC_DEBUG("Failed to setup TLS %s\n",
-                      error_get_pretty(err));
+        if (vs->ioc_tag) {
+            g_source_remove(vs->ioc_tag);
+            vs->ioc_tag = 0;
+        }
+
+        tls = qio_channel_tls_new_server(
+            vs->ioc,
+            vs->vd->tlscreds,
+            vs->vd->tlsaclname,
+            &err);
+        if (!tls) {
+            VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
             error_free(err);
             vnc_client_error(vs);
             return 0;
         }
 
-        qcrypto_tls_session_set_callbacks(vs->tls,
-                                          vnc_tls_push,
-                                          vnc_tls_pull,
-                                          vs);
-
         VNC_DEBUG("Start TLS VeNCrypt handshake process\n");
-        if (vnc_start_vencrypt_handshake(vs) < 0) {
-            VNC_DEBUG("Failed to start TLS handshake\n");
-            return 0;
-        }
+        object_unref(OBJECT(vs->ioc));
+        vs->ioc = QIO_CHANNEL(tls);
+        vs->tls = qio_channel_tls_get_session(tls);
+
+        qio_channel_tls_handshake(tls,
+                                  vnc_tls_handshake_done,
+                                  vs,
+                                  NULL);
     }
     return 0;
 }
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 7b2cc68..4863584 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -22,83 +22,60 @@
 #include "qemu/main-loop.h"
 #include "crypto/hash.h"
 
-static int vncws_start_tls_handshake(VncState *vs)
-{
-    Error *err = NULL;
-
-    if (qcrypto_tls_session_handshake(vs->tls, &err) < 0) {
-        goto error;
-    }
+static void vncws_handshake_read(VncState *vs);
 
-    switch (qcrypto_tls_session_get_handshake_status(vs->tls)) {
-    case QCRYPTO_TLS_HANDSHAKE_COMPLETE:
-        VNC_DEBUG("Handshake done, checking credentials\n");
-        if (qcrypto_tls_session_check_credentials(vs->tls, &err) < 0) {
-            goto error;
-        }
-        VNC_DEBUG("Client verification passed, starting TLS I/O\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
-        vs->ioc_tag = qio_channel_add_watch(
-            vs->ioc, G_IO_IN, vncws_handshake_io, vs, NULL);
-        break;
-
-    case QCRYPTO_TLS_HANDSHAKE_RECVING:
-        VNC_DEBUG("Handshake interrupted (blocking read)\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
-        vs->ioc_tag = qio_channel_add_watch(
-            vs->ioc, G_IO_IN, vncws_tls_handshake_io, vs, NULL);
-        break;
+static void vncws_tls_handshake_done(Object *source,
+                                     Error *err,
+                                     gpointer user_data)
+{
+    VncState *vs = user_data;
 
-    case QCRYPTO_TLS_HANDSHAKE_SENDING:
-        VNC_DEBUG("Handshake interrupted (blocking write)\n");
-        if (vs->ioc_tag) {
-            g_source_remove(vs->ioc_tag);
-        }
+    if (err) {
+        VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
+        vnc_client_error(vs);
+    } else {
         vs->ioc_tag = qio_channel_add_watch(
-            vs->ioc, G_IO_OUT, vncws_tls_handshake_io, vs, NULL);
-        break;
+            QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
     }
-
-    return 0;
-
- error:
-    VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
-    error_free(err);
-    vnc_client_error(vs);
-    return -1;
 }
 
+
 gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
                                 GIOCondition condition G_GNUC_UNUSED,
                                 void *opaque)
 {
-    VncState *vs = (VncState *)opaque;
+    VncState *vs = opaque;
+    QIOChannelTLS *tls;
     Error *err = NULL;
 
-    vs->tls = qcrypto_tls_session_new(vs->vd->tlscreds,
-                                      NULL,
-                                      vs->vd->tlsaclname,
-                                      QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
-                                      &err);
-    if (!vs->tls) {
-        VNC_DEBUG("Failed to setup TLS %s\n",
-                  error_get_pretty(err));
+    VNC_DEBUG("TLS Websocket connection required\n");
+    if (vs->ioc_tag) {
+        g_source_remove(vs->ioc_tag);
+        vs->ioc_tag = 0;
+    }
+
+    tls = qio_channel_tls_new_server(
+        vs->ioc,
+        vs->vd->tlscreds,
+        vs->vd->tlsaclname,
+        &err);
+    if (!tls) {
+        VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
         error_free(err);
         vnc_client_error(vs);
         return TRUE;
     }
 
-    qcrypto_tls_session_set_callbacks(vs->tls,
-                                      vnc_tls_push,
-                                      vnc_tls_pull,
-                                      vs);
-
     VNC_DEBUG("Start TLS WS handshake process\n");
-    vncws_start_tls_handshake(vs);
+    object_unref(OBJECT(vs->ioc));
+    vs->ioc = QIO_CHANNEL(tls);
+    vs->tls = qio_channel_tls_get_session(tls);
+
+    qio_channel_tls_handshake(tls,
+                              vncws_tls_handshake_done,
+                              vs,
+                              NULL);
+
     return TRUE;
 }
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 2f8c15e..3fe5fed 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1163,7 +1163,6 @@ void vnc_disconnect_finish(VncState *vs)
     vnc_tight_clear(vs);
     vnc_zrle_clear(vs);
 
-    qcrypto_tls_session_free(vs->tls);
 #ifdef CONFIG_VNC_SASL
     vnc_sasl_client_cleanup(vs);
 #endif /* CONFIG_VNC_SASL */
@@ -1225,38 +1224,6 @@ void vnc_client_error(VncState *vs)
 }
 
 
-ssize_t vnc_tls_pull(char *buf, size_t len, void *opaque)
-{
-    VncState *vs = opaque;
-    ssize_t ret = qio_channel_read(vs->ioc, buf, len, NULL);
-    if (ret < 0) {
-        if (ret == QIO_CHANNEL_ERR_BLOCK) {
-            errno = EAGAIN;
-        } else {
-            errno = EIO;
-        }
-        return -1;
-    }
-    return ret;
-}
-
-
-ssize_t vnc_tls_push(const char *buf, size_t len, void *opaque)
-{
-    VncState *vs = opaque;
-    ssize_t ret = qio_channel_write(vs->ioc, buf, len, NULL);
-    if (ret < 0) {
-        if (ret == QIO_CHANNEL_ERR_BLOCK) {
-            errno = EAGAIN;
-        } else {
-            errno = EIO;
-        }
-        return -1;
-    }
-    return ret;
-}
-
-
 /*
  * Called to write a chunk of data to the client socket. The data may
  * be the raw data, or may have already been encoded by SASL.
@@ -1276,21 +1243,8 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
 {
     Error *err = NULL;
     ssize_t ret;
-    if (vs->tls) {
-        ret = qcrypto_tls_session_write(vs->tls, (const char *)data, datalen);
-        if (ret < 0) {
-            if (errno == EAGAIN) {
-                ret = QIO_CHANNEL_ERR_BLOCK;
-            } else {
-                ret = -1;
-                error_setg_errno(&err, errno, "%s",
-                                 "Cannot write to TLS socket");
-            }
-        }
-    } else {
-        ret = qio_channel_write(
-            vs->ioc, (const char *)data, datalen, &err);
-    }
+    ret = qio_channel_write(
+        vs->ioc, (const char *)data, datalen, &err);
     VNC_DEBUG("Wrote wire %p %zd -> %ld\n", data, datalen, ret);
     return vnc_client_io_error(vs, ret, &err);
 }
@@ -1406,21 +1360,8 @@ ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
 {
     ssize_t ret;
     Error *err = NULL;
-    if (vs->tls) {
-        ret = qcrypto_tls_session_read(vs->tls, (char *)data, datalen);
-        if (ret < 0) {
-            if (errno == EAGAIN) {
-                ret = QIO_CHANNEL_ERR_BLOCK;
-            } else {
-                ret = -1;
-                error_setg_errno(&err, errno, "%s",
-                                 "Cannot read from TLS socket");
-            }
-        }
-    } else {
-        ret = qio_channel_read(
-            vs->ioc, (char *)data, datalen, &err);
-    }
+    ret = qio_channel_read(
+        vs->ioc, (char *)data, datalen, &err);
     VNC_DEBUG("Read wire %p %zd -> %ld\n", data, datalen, ret);
     return vnc_client_io_error(vs, ret, &err);
 }
@@ -2980,8 +2921,8 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
             vs->subauth = vd->subauth;
         }
     }
-    VNC_DEBUG("Client sock=%d ws=%d auth=%d subauth=%d\n",
-              csock, websocket, vs->auth, vs->subauth);
+    VNC_DEBUG("Client sock=%p ws=%d auth=%d subauth=%d\n",
+              sioc, 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) {
@@ -3710,7 +3651,7 @@ void vnc_display_open(const char *id, Error **errp)
             vs->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vs->id);
         }
         qemu_acl_init(vs->tlsaclname);
-     }
+    }
 #ifdef CONFIG_VNC_SASL
     if (acl && sasl) {
         char *aclname;
diff --git a/ui/vnc.h b/ui/vnc.h
index e9b4a00..65991c0 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -36,6 +36,7 @@
 #include "crypto/tlssession.h"
 #include "io/buffer.h"
 #include "io/channel-socket.h"
+#include "io/channel-tls.h"
 #include <zlib.h>
 #include <stdbool.h>
 
@@ -281,7 +282,7 @@ struct VncState
     int auth;
     int subauth; /* Used by VeNCrypt */
     char challenge[VNC_AUTH_CHALLENGE_SIZE];
-    QCryptoTLSSession *tls;
+    QCryptoTLSSession *tls; /* Borrowed pointer from channel, don't free */
 #ifdef CONFIG_VNC_SASL
     VncStateSASL sasl;
 #endif
@@ -511,8 +512,6 @@ gboolean vnc_client_io(QIOChannel *ioc,
 
 ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen);
 ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen);
-ssize_t vnc_tls_pull(char *buf, size_t len, void *opaque);
-ssize_t vnc_tls_push(const char *buf, size_t len, void *opaque);
 
 /* Protocol I/O functions */
 void vnc_write(VncState *vs, const void *data, size_t len);
-- 
2.4.3