From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47132)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dify.ltd@gmail.com>) id 1bAD4S-0000vE-Iv
	for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:13:13 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dify.ltd@gmail.com>) id 1bAD4N-0007R5-44
	for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:13:11 -0400
Received: from mail-oi0-x242.google.com ([2607:f8b0:4003:c06::242]:33486)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dify.ltd@gmail.com>) id 1bAD4M-0007R1-Su
	for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:13:07 -0400
Received: by mail-oi0-x242.google.com with SMTP id x204so3022778oia.0
	for <qemu-devel@nongnu.org>; Tue, 07 Jun 2016 02:13:06 -0700 (PDT)
MIME-Version: 1.0
From: Attila-Mihaly Balazs <dify.ltd@gmail.com>
Date: Tue, 7 Jun 2016 12:13:06 +0300
Message-ID: <CAOqyLXj2KizZN=PAT2hukWoTGD+vc=5TrAJFH05q4zWyZdXjdg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: [Qemu-devel] [PATCH] Make password based authentication the default
 for VNC
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org
Cc: kraxel@redhat.com

To improve the security of the embedded VNC server make password
based authentication the default when no authentication mechanism
is specified.

If you really want to use "no authentication", a new authentication
option called "insecure" is introduced which needs to be explicitly
specified.

Note: because password authentication is not availble in FIPS mode,
you must explicitly set an authentication method when using VNC with
QEMU compiled in FIPS mode or it won't start.

Signed-off-by: Attila-Mihaly Balazs
---
 qemu-doc.texi   | 10 ++++++----
 qemu-options.hx |  7 ++++++-
 ui/vnc.c        | 51 ++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index f37fd31..a8795b6 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1493,12 +1493,14 @@ considerations depending on the deployment scenarios.
 @node vnc_sec_none
 @subsection Without passwords

-The simplest VNC server setup does not include any form of authentication.
-For this setup it is recommended to restrict it to listen on a UNIX domain
-socket only. For example
+Unless otherwise specified, the VNC server starts in password authentication
+mode (see the next section for details). It is possible to disable
+authentication entirely when needed however it is strongly recommended to use
+other methods of access control to ensure that only trusted persons can access
+it. For example you could restrict it to listen on a UNIX domain socket only:

 @example
-qemu-system-i386 [...OPTIONS...] -vnc unix:/home/joebloggs/.qemu-myvm-vnc
+qemu-system-i386 [...OPTIONS...] -vnc unix:/home/jb/.qemu-myvm-vnc,insecure
 @end example

 This ensures that only users on local box with read/write access to that
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f33361..25a17cb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1284,7 +1284,8 @@ requires encrypted client connections.

 @item password

-Require that password based authentication is used for client connections.
+Require that password based authentication is used for client connections. This
+is the default if no authentiation mechanism is specified.

 The password must be set separately using the @code{set_password} command in
 the @ref{pcsys_monitor}. The syntax to change your password is:
@@ -1301,6 +1302,10 @@ date and time).
 You can also use keywords "now" or "never" for the expiration time to
 allow <protocol> password to expire immediately or never expire.

+@item insecure
+
+Run the VNC server without any authentication.
+
 @item tls-creds=@var{ID}

 Provides the ID of a set of TLS credentials to use to secure the
diff --git a/ui/vnc.c b/ui/vnc.c
index c862fdc..c58db4b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3275,6 +3275,9 @@ static QemuOptsList qemu_vnc_opts = {
             .name = "password",
             .type = QEMU_OPT_BOOL,
         },{
+            .name = "insecure",
+            .type = QEMU_OPT_BOOL,
+        },{
             .name = "reverse",
             .type = QEMU_OPT_BOOL,
         },{
@@ -3312,6 +3315,7 @@ static QemuOptsList qemu_vnc_opts = {
 static int
 vnc_display_setup_auth(VncDisplay *vs,
                        bool password,
+                       bool insecure,
                        bool sasl,
                        bool websocket,
                        Error **errp)
@@ -3454,6 +3458,16 @@ vnc_display_setup_auth(VncDisplay *vs,
             vs->ws_auth = VNC_AUTH_INVALID;
         }
     }
+
+    if (vs->auth == VNC_AUTH_NONE && !insecure) {
+        /*
+         * If authentication is not specified use password authentication.
+         */
+        VNC_DEBUG("Initializing VNC server with password auth\n");
+        vs->auth = VNC_AUTH_VNC;
+        vs->subauth = VNC_AUTH_INVALID;
+    }
+
     return 0;
 }

@@ -3511,6 +3525,7 @@ void vnc_display_open(const char *id, Error **errp)
     const char *share, *device_id;
     QemuConsole *con;
     bool password = false;
+    bool insecure = false;
     bool reverse = false;
     const char *vnc;
     char *h;
@@ -3622,22 +3637,7 @@ void vnc_display_open(const char *id, Error **errp)
     }

     password = qemu_opt_get_bool(opts, "password", false);
-    if (password) {
-        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;
-        }
-        if (!qcrypto_cipher_supports(
-                QCRYPTO_CIPHER_ALG_DES_RFB)) {
-            error_setg(errp,
-                       "Cipher backend does not support DES RFB algorithm");
-            goto fail;
-        }
-    }
-
+    insecure = qemu_opt_get_bool(opts, "insecure", false);
     reverse = qemu_opt_get_bool(opts, "reverse", false);
     lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
     key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);
@@ -3760,10 +3760,27 @@ void vnc_display_open(const char *id, Error **errp)
     }
 #endif

-    if (vnc_display_setup_auth(vs, password, sasl, vs->ws_enabled, errp) < 0) {
+    if (vnc_display_setup_auth(
+            vs, password, insecure, sasl, vs->ws_enabled, errp) < 0) {
         goto fail;
     }

+    if (vs->auth == VNC_AUTH_VNC) {
+        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;
+        }
+        if (!qcrypto_cipher_supports(
+                QCRYPTO_CIPHER_ALG_DES_RFB)) {
+            error_setg(errp,
+                       "Cipher backend does not support DES RFB algorithm");
+            goto fail;
+        }
+    }
+
 #ifdef CONFIG_VNC_SASL
     if ((saslErr = sasl_server_init(NULL, "qemu")) != SASL_OK) {
         error_setg(errp, "Failed to initialize SASL auth: %s",
-- 
2.7.4