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

[Anthony Liguori]

Paul Moore <address@hidden> 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 <address@hidden>

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
> address@hidden -enable-fips
> address@hidden -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);
>              }