qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>
Subject: [Qemu-devel] [PULL 7/9] ui: enforce TLS when using websockets server
Date: Wed, 18 Mar 2015 14:17:44 +0100	[thread overview]
Message-ID: <1426684666-30629-8-git-send-email-kraxel@redhat.com> (raw)
In-Reply-To: <1426684666-30629-1-git-send-email-kraxel@redhat.com>

From: "Daniel P. Berrange" <berrange@redhat.com>

When TLS is required, the primary VNC server considers it to be
mandatory. ie the server admin decides whether or not TLS is used,
and the client has to comply with this decision. The websockets
server, however, treated it as optional, allowing non-TLS clients
to connect to a server which had setup TLS. Thus enabling websockets
lowers the security of the VNC server leaving the admin no way to
enforce use of TLS.

This removes the code that allows non-TLS fallback in the websockets
server, so that if TLS is requested for VNC it is now mandatory for
both the primary VNC server and the websockets VNC server.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-ws.c | 31 +++++++------------------------
 ui/vnc-ws.h |  2 +-
 ui/vnc.c    |  2 +-
 3 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 1769d52..0fcce4e 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -24,8 +24,6 @@
 #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);
@@ -53,33 +51,18 @@ static int vncws_start_tls_handshake(struct VncState *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 95c1b0a..ef229b7 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 7b66f93..9994de1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3036,7 +3036,7 @@ static void vnc_connect(VncDisplay *vd, int csock,
         vs->websocket = 1;
 #ifdef CONFIG_VNC_TLS
         if (vd->ws_tls) {
-            qemu_set_fd_handler2(vs->csock, NULL, vncws_tls_handshake_peek,
+            qemu_set_fd_handler2(vs->csock, NULL, vncws_tls_handshake_io,
                                  NULL, vs);
         } else
 #endif /* CONFIG_VNC_TLS */
-- 
1.8.3.1

  parent reply	other threads:[~2015-03-18 13:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18 13:17 [Qemu-devel] [PULL for-2.3 0/9] vnc patch queue Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 1/9] vnc: Fix QMP change not to use funky error class Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 2/9] ui: remove unused 'wiremode' variable in VncState struct Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 3/9] ui: replace printf() calls with VNC_DEBUG Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 4/9] ui: report error if user requests VNC option that is unsupported Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 5/9] ui: split setup of VNC auth scheme into separate method Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 6/9] ui: fix setup of VNC websockets auth scheme with TLS Gerd Hoffmann
2015-03-18 13:17 ` Gerd Hoffmann [this message]
2015-03-18 13:17 ` [Qemu-devel] [PULL 8/9] ui: remove separate gnutls_session for websockets server Gerd Hoffmann
2015-03-18 13:17 ` [Qemu-devel] [PULL 9/9] ui: ensure VNC websockets server checks the ACL if requested Gerd Hoffmann
2015-03-19 13:03 ` [Qemu-devel] [PULL for-2.3 0/9] vnc patch queue Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1426684666-30629-8-git-send-email-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).