Patchwork [v5] vnc: disable VNC password authentication (security type 2) when in FIPS mode

login
register
mail settings
Submitter Paul Moore
Date Aug. 3, 2012, 6:39 p.m.
Message ID <20120803183920.4928.95381.stgit@sifl>
Download mbox | patch
Permalink /patch/175038/
State New
Headers show

Comments

Paul Moore - Aug. 3, 2012, 6:39 p.m.
FIPS 140-2 requires disabling certain ciphers, including DES, which is used
by VNC to obscure passwords when they are sent over the network.  The
solution for FIPS users is to disable the use of VNC password auth when the
host system is operating in FIPS compliance mode and the user has specified
'-enable-fips' on the QEMU command line.

This patch causes QEMU to emit a message to stderr when the host system is
running in FIPS mode and a VNC password was specified on the commend line.
If the system is not running in FIPS mode, or is running in FIPS mode but
VNC password authentication was not requested, QEMU operates normally.

Signed-off-by: Paul Moore <pmoore@redhat.com>

--
Changelog
* v5
- Added the '-enable-fips' command line option
* v4
- Removed the use of syslog
* v3
- Use fgetc() instead of fgets() in fips_enabled
- Only emit a syslog message if the caller tries to use VNC password auth
- Suggest alternative auth methods in the stderr notice
* v2
- Protected syslog with _WIN32
- Protected the guts of fips_enabled() with __linux__
- Converted fips_enabled() and the fips flag from int to bool
*v1
- Initial draft
---
 osdep.c         |   29 +++++++++++++++++++++++++++++
 osdep.h         |    4 ++++
 qemu-doc.texi   |    8 +++++---
 qemu-options.hx |   11 +++++++++++
 ui/vnc.c        |   10 ++++++++++
 vl.c            |    4 ++++
 6 files changed, 63 insertions(+), 3 deletions(-)
Anthony Liguori - Aug. 3, 2012, 8:41 p.m.
Paul Moore <pmoore@redhat.com> writes:

> FIPS 140-2 requires disabling certain ciphers, including DES, which is used
> by VNC to obscure passwords when they are sent over the network.  The
> solution for FIPS users is to disable the use of VNC password auth when the
> host system is operating in FIPS compliance mode and the user has specified
> '-enable-fips' on the QEMU command line.
>
> This patch causes QEMU to emit a message to stderr when the host system is
> running in FIPS mode and a VNC password was specified on the commend line.
> If the system is not running in FIPS mode, or is running in FIPS mode but
> VNC password authentication was not requested, QEMU operates normally.
>
> Signed-off-by: Paul Moore <pmoore@redhat.com>

Applied. Thanks for pushing through with this one.

Regards,

Anthony Liguori

>
> --
> Changelog
> * v5
> - Added the '-enable-fips' command line option
> * v4
> - Removed the use of syslog
> * v3
> - Use fgetc() instead of fgets() in fips_enabled
> - Only emit a syslog message if the caller tries to use VNC password auth
> - Suggest alternative auth methods in the stderr notice
> * v2
> - Protected syslog with _WIN32
> - Protected the guts of fips_enabled() with __linux__
> - Converted fips_enabled() and the fips flag from int to bool
> *v1
> - Initial draft
> ---
>  osdep.c         |   29 +++++++++++++++++++++++++++++
>  osdep.h         |    4 ++++
>  qemu-doc.texi   |    8 +++++---
>  qemu-options.hx |   11 +++++++++++
>  ui/vnc.c        |   10 ++++++++++
>  vl.c            |    4 ++++
>  6 files changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/osdep.c b/osdep.c
> index 03817f0..c07faf5 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -24,6 +24,7 @@
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <stdarg.h>
> +#include <stdbool.h>
>  #include <string.h>
>  #include <errno.h>
>  #include <unistd.h>
> @@ -48,6 +49,8 @@ extern int madvise(caddr_t, size_t, int);
>  #include "trace.h"
>  #include "qemu_socket.h"
>  
> +static bool fips_enabled = false;
> +
>  static const char *qemu_version = QEMU_VERSION;
>  
>  int socket_set_cork(int fd, int v)
> @@ -253,3 +256,29 @@ const char *qemu_get_version(void)
>  {
>      return qemu_version;
>  }
> +
> +void fips_set_state(bool requested)
> +{
> +#ifdef __linux__
> +    if (requested) {
> +        FILE *fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> +        if (fds != NULL) {
> +            fips_enabled = (fgetc(fds) == '1');
> +            fclose(fds);
> +        }
> +    }
> +#else
> +    fips_enabled = false;
> +#endif /* __linux__ */
> +
> +#ifdef _FIPS_DEBUG
> +    fprintf(stderr, "FIPS mode %s (requested %s)\n",
> +	    (fips_enabled ? "enabled" : "disabled"),
> +	    (requested ? "enabled" : "disabled"));
> +#endif
> +}
> +
> +bool fips_get_state(void)
> +{
> +    return fips_enabled;
> +}
> diff --git a/osdep.h b/osdep.h
> index 1e15a4b..d4b887d 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -3,6 +3,7 @@
>  
>  #include <stdarg.h>
>  #include <stddef.h>
> +#include <stdbool.h>
>  #ifdef __OpenBSD__
>  #include <sys/types.h>
>  #include <sys/signal.h>
> @@ -154,4 +155,7 @@ void qemu_set_cloexec(int fd);
>  void qemu_set_version(const char *);
>  const char *qemu_get_version(void);
>  
> +void fips_set_state(bool requested);
> +bool fips_get_state(void);
> +
>  #endif
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 84dad19..f482fed 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -1124,9 +1124,11 @@ the protocol limits passwords to 8 characters it should not be considered
>  to provide high security. The password can be fairly easily brute-forced by
>  a client making repeat connections. For this reason, a VNC server using password
>  authentication should be restricted to only listen on the loopback interface
> -or UNIX domain sockets. Password authentication is requested with the @code{password}
> -option, and then once QEMU is running the password is set with the monitor. Until
> -the monitor is used to set the password all clients will be rejected.
> +or UNIX domain sockets. Password authentication is not supported when operating
> +in FIPS 140-2 compliance mode as it requires the use of the DES cipher. Password
> +authentication is requested with the @code{password} option, and then once QEMU
> +is running the password is set with the monitor. Until the monitor is used to
> +set the password all clients will be rejected.
>  
>  @example
>  qemu-system-i386 [...OPTIONS...] -vnc :1,password -monitor stdio
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dc68e15..1f114ad 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2783,6 +2783,17 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>      "-qtest-log LOG  specify tracing options\n",
>      QEMU_ARCH_ALL)
>  
> +#ifdef __linux__
> +DEF("enable-fips", 0, QEMU_OPTION_enablefips,
> +    "-enable-fips    enable FIPS 140-2 compliance\n",
> +    QEMU_ARCH_ALL)
> +#endif
> +STEXI
> +@item -enable-fips
> +@findex -enable-fips
> +Enable FIPS 140-2 compliance mode.
> +ETEXI
> +
>  HXCOMM This is the last statement. Insert new options before this line!
>  STEXI
>  @end table
> diff --git a/ui/vnc.c b/ui/vnc.c
> index cfc61a7..312ad7f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -32,6 +32,7 @@
>  #include "acl.h"
>  #include "qemu-objects.h"
>  #include "qmp-commands.h"
> +#include "osdep.h"
>  
>  #define VNC_REFRESH_INTERVAL_BASE 30
>  #define VNC_REFRESH_INTERVAL_INC  50
> @@ -2875,6 +2876,15 @@ int vnc_display_open(DisplayState *ds, const char *display)
>      while ((options = strchr(options, ','))) {
>          options++;
>          if (strncmp(options, "password", 8) == 0) {
> +            if (fips_get_state()) {
> +                fprintf(stderr,
> +                        "VNC password auth disabled due to FIPS mode, "
> +                        "consider using the VeNCrypt or SASL authentication "
> +                        "methods as an alternative\n");
> +                g_free(vs->display);
> +                vs->display = NULL;
> +                return -1;
> +            }
>              password = 1; /* Require password auth */
>          } else if (strncmp(options, "reverse", 7) == 0) {
>              reverse = 1;
> diff --git a/vl.c b/vl.c
> index 9fea320..c9bd7b7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -159,6 +159,7 @@ int main(int argc, char **argv)
>  #include "qemu-queue.h"
>  #include "cpus.h"
>  #include "arch_init.h"
> +#include "osdep.h"
>  
>  #include "ui/qemu-spice.h"
>  
> @@ -3198,6 +3199,9 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_qtest_log:
>                  qtest_log = optarg;
>                  break;
> +            case QEMU_OPTION_enablefips:
> +                fips_set_state(true);
> +                break;
>              default:
>                  os_parse_cmd_args(popt->index, optarg);
>              }

Patch

diff --git a/osdep.c b/osdep.c
index 03817f0..c07faf5 100644
--- a/osdep.c
+++ b/osdep.c
@@ -24,6 +24,7 @@ 
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
@@ -48,6 +49,8 @@  extern int madvise(caddr_t, size_t, int);
 #include "trace.h"
 #include "qemu_socket.h"
 
+static bool fips_enabled = false;
+
 static const char *qemu_version = QEMU_VERSION;
 
 int socket_set_cork(int fd, int v)
@@ -253,3 +256,29 @@  const char *qemu_get_version(void)
 {
     return qemu_version;
 }
+
+void fips_set_state(bool requested)
+{
+#ifdef __linux__
+    if (requested) {
+        FILE *fds = fopen("/proc/sys/crypto/fips_enabled", "r");
+        if (fds != NULL) {
+            fips_enabled = (fgetc(fds) == '1');
+            fclose(fds);
+        }
+    }
+#else
+    fips_enabled = false;
+#endif /* __linux__ */
+
+#ifdef _FIPS_DEBUG
+    fprintf(stderr, "FIPS mode %s (requested %s)\n",
+	    (fips_enabled ? "enabled" : "disabled"),
+	    (requested ? "enabled" : "disabled"));
+#endif
+}
+
+bool fips_get_state(void)
+{
+    return fips_enabled;
+}
diff --git a/osdep.h b/osdep.h
index 1e15a4b..d4b887d 100644
--- a/osdep.h
+++ b/osdep.h
@@ -3,6 +3,7 @@ 
 
 #include <stdarg.h>
 #include <stddef.h>
+#include <stdbool.h>
 #ifdef __OpenBSD__
 #include <sys/types.h>
 #include <sys/signal.h>
@@ -154,4 +155,7 @@  void qemu_set_cloexec(int fd);
 void qemu_set_version(const char *);
 const char *qemu_get_version(void);
 
+void fips_set_state(bool requested);
+bool fips_get_state(void);
+
 #endif
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 84dad19..f482fed 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1124,9 +1124,11 @@  the protocol limits passwords to 8 characters it should not be considered
 to provide high security. The password can be fairly easily brute-forced by
 a client making repeat connections. For this reason, a VNC server using password
 authentication should be restricted to only listen on the loopback interface
-or UNIX domain sockets. Password authentication is requested with the @code{password}
-option, and then once QEMU is running the password is set with the monitor. Until
-the monitor is used to set the password all clients will be rejected.
+or UNIX domain sockets. Password authentication is not supported when operating
+in FIPS 140-2 compliance mode as it requires the use of the DES cipher. Password
+authentication is requested with the @code{password} option, and then once QEMU
+is running the password is set with the monitor. Until the monitor is used to
+set the password all clients will be rejected.
 
 @example
 qemu-system-i386 [...OPTIONS...] -vnc :1,password -monitor stdio
diff --git a/qemu-options.hx b/qemu-options.hx
index dc68e15..1f114ad 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2783,6 +2783,17 @@  DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+#ifdef __linux__
+DEF("enable-fips", 0, QEMU_OPTION_enablefips,
+    "-enable-fips    enable FIPS 140-2 compliance\n",
+    QEMU_ARCH_ALL)
+#endif
+STEXI
+@item -enable-fips
+@findex -enable-fips
+Enable FIPS 140-2 compliance mode.
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/ui/vnc.c b/ui/vnc.c
index cfc61a7..312ad7f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -32,6 +32,7 @@ 
 #include "acl.h"
 #include "qemu-objects.h"
 #include "qmp-commands.h"
+#include "osdep.h"
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -2875,6 +2876,15 @@  int vnc_display_open(DisplayState *ds, const char *display)
     while ((options = strchr(options, ','))) {
         options++;
         if (strncmp(options, "password", 8) == 0) {
+            if (fips_get_state()) {
+                fprintf(stderr,
+                        "VNC password auth disabled due to FIPS mode, "
+                        "consider using the VeNCrypt or SASL authentication "
+                        "methods as an alternative\n");
+                g_free(vs->display);
+                vs->display = NULL;
+                return -1;
+            }
             password = 1; /* Require password auth */
         } else if (strncmp(options, "reverse", 7) == 0) {
             reverse = 1;
diff --git a/vl.c b/vl.c
index 9fea320..c9bd7b7 100644
--- a/vl.c
+++ b/vl.c
@@ -159,6 +159,7 @@  int main(int argc, char **argv)
 #include "qemu-queue.h"
 #include "cpus.h"
 #include "arch_init.h"
+#include "osdep.h"
 
 #include "ui/qemu-spice.h"
 
@@ -3198,6 +3199,9 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+            case QEMU_OPTION_enablefips:
+                fips_set_state(true);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }