Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[Daniel P. Berrange]

On Tue, May 01, 2012 at 05:20:40PM -0400, Paul Moore wrote:
> 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 mode.
> 
> This patch causes qemu to emits a syslog entry indicating that VNC password
> auth is disabled when it detects the host is running in FIPS mode, and
> unless a VNC password was specified on the command line it continues
> normally.  However, if a VNC password was given on the command line, qemu
> fails with an error message to stderr explaining that that VNC password
> auth is not allowed in FIPS mode.
> 
> Signed-off-by: Paul Moore <address@hidden>
> ---
>  qemu-doc.texi |    8 +++++---
>  ui/vnc.c      |   32 ++++++++++++++++++++++++++++++++
>  ui/vnc.h      |    1 +
>  3 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index e5d7ac4..f9b113e 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 [...OPTIONS...] -vnc :1,password -monitor stdio
> diff --git a/ui/vnc.c b/ui/vnc.c
> index deb9ecd..620791e 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 <syslog.h>
>  
>  #define VNC_REFRESH_INTERVAL_BASE 30
>  #define VNC_REFRESH_INTERVAL_INC  50
> @@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
>  static int vnc_cursor_define(VncState *vs);
>  static void vnc_release_modifiers(VncState *vs);
>  
> +static int fips_enabled(void)

s/int/bool/  and use true/false as values

> +{
> +    int enabled = 0;
> +    char value;
> +    FILE *fds;
> +
> +    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> +    if (fds == NULL) {
> +        return 0;
> +    }
> +    if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
> +        enabled = 1;
> +    }
> +    fclose(fds);
> +
> +    return enabled;
> +}

As already pointed out,wWe should probably make this depend on
__linux__, and 'return false' fo other platforms.

> +
>  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
>  {
>  #ifdef _VNC_DEBUG
> @@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
>      dcl->idle = 1;
>      vnc_display = vs;
>  
> +    vs->fips = fips_enabled();
> +    VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
> +    if (vs->fips) {
> +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
> +    }

I think this syslog message is better placed in the next chunk of the
patch where you actually test the vs->fips value.

> +
>      vs->lsock = -1;
>  
>      vs->ds = ds;
> @@ -2892,6 +2917,13 @@ int vnc_display_open(DisplayState *ds, const char 
> *display)
>      while ((options = strchr(options, ','))) {
>          options++;
>          if (strncmp(options, "password", 8) == 0) {
> +            if (vs->fips) {
> +                fprintf(stderr,
> +                        "VNC password auth disabled due to FIPS mode\n");
> +                g_free(vs->display);
> +                vs->display = NULL;
> +                return -1;
> +            }
>              password = 1; /* Require password auth */
>          } else if (strncmp(options, "reverse", 7) == 0) {
>              reverse = 1;

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|