diff mbox

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

Message ID 20120502193256.6508.86360.stgit@sifl
State New
Headers show

Commit Message

Paul Moore May 2, 2012, 7:32 p.m. UTC 
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 emit 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 VNC password auth is
not allowed in FIPS mode.

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

--
Changelog
* 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
---
 qemu-doc.texi |    8 +++++---
 ui/vnc.c      |   39 +++++++++++++++++++++++++++++++++++++++
 ui/vnc.h      |    1 +
 3 files changed, 45 insertions(+), 3 deletions(-)

Comments

Daniel P. Berrangé May 3, 2012, 8:29 a.m. UTC | #1 
On Wed, May 02, 2012 at 03:32:56PM -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 emit 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 VNC password auth is
> not allowed in FIPS mode.
> 
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> 
> --
> Changelog
> * 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
> ---
>  qemu-doc.texi |    8 +++++---
>  ui/vnc.c      |   39 +++++++++++++++++++++++++++++++++++++++
>  ui/vnc.h      |    1 +
>  3 files changed, 45 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..6162425 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -32,6 +32,9 @@
>  #include "acl.h"
>  #include "qemu-objects.h"
>  #include "qmp-commands.h"
> +#ifndef _WIN32
> +#include <syslog.h>
> +#endif
>  
>  #define VNC_REFRESH_INTERVAL_BASE 30
>  #define VNC_REFRESH_INTERVAL_INC  50
> @@ -48,6 +51,27 @@ static DisplayChangeListener *dcl;
>  static int vnc_cursor_define(VncState *vs);
>  static void vnc_release_modifiers(VncState *vs);
>  
> +static bool fips_enabled(void)
> +{
> +    bool enabled = false;
> +
> +#ifdef __linux__
> +    FILE *fds;
> +    char value;
> +
> +    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> +    if (fds == NULL) {
> +        return false;
> +    }
> +    if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
> +        enabled = true;
> +    }
> +    fclose(fds);
> +#endif /* __linux__ */
> +
> +    return enabled;
> +}
> +
>  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
>  {
>  #ifdef _VNC_DEBUG
> @@ -2748,6 +2772,14 @@ 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"));
> +#ifndef _WIN32
> +    if (vs->fips) {
> +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
> +    }
> +#endif /* _WIN32 */

I really think this should only be done if a password is actually set.
With the code as it is, then every single time you launch a VM you're
going to get this message in syslog, which makes it appear as if something
is trying to illegally use passwords in FIPS mode. I feel this will cause
admins/auditors to be worried about something being wrong, when in fact
everything is normal.

> +
>      vs->lsock = -1;
>  
>      vs->ds = ds;
> @@ -2892,6 +2924,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;
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a851ebd..d41631b 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -160,6 +160,7 @@ struct VncDisplay
>      char *display;
>      char *password;
>      time_t expires;
> +    bool fips;
>      int auth;
>      bool lossy;
>      bool non_adaptive;


Daniel
Alexander Graf May 3, 2012, 8:51 a.m. UTC | #2 
On 03.05.2012, at 10:29, Daniel P. Berrange wrote:

> On Wed, May 02, 2012 at 03:32:56PM -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.

So that means "no password" is more secure according to FIPS than "DES encrypted password"?


Alex
Daniel P. Berrangé May 3, 2012, 8:57 a.m. UTC | #3 
On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
> 
> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
> 
> > On Wed, May 02, 2012 at 03:32:56PM -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.
> 
> So that means "no password" is more secure according to FIPS than
> "DES encrypted password"?

No, FIPS is not making statements about the choice of auth methods.
FIPS is concerned with what encryption algorithms an application uses.
The requirements about whether authentication is required & what sort,
is upto other specifications (eg Common Criteria) to decide.

Daniel
Alexander Graf May 3, 2012, 9:01 a.m. UTC | #4 
On 03.05.2012, at 10:57, Daniel P. Berrange wrote:

> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
>> 
>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
>> 
>>> On Wed, May 02, 2012 at 03:32:56PM -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.
>> 
>> So that means "no password" is more secure according to FIPS than
>> "DES encrypted password"?
> 
> No, FIPS is not making statements about the choice of auth methods.
> FIPS is concerned with what encryption algorithms an application uses.
> The requirements about whether authentication is required & what sort,
> is upto other specifications (eg Common Criteria) to decide.

Hrm, so short-term this fixes things. But long-term, I think the better solution would be to implement the tight security model and use a real cipher:

  http://www.tigervnc.org/cgi-bin/rfbproto#tight-security-type


Alex
Daniel P. Berrangé May 3, 2012, 9:03 a.m. UTC | #5 
On Thu, May 03, 2012 at 11:01:29AM +0200, Alexander Graf wrote:
> 
> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
> 
> > On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
> >> 
> >> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
> >> 
> >>> On Wed, May 02, 2012 at 03:32:56PM -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.
> >> 
> >> So that means "no password" is more secure according to FIPS than
> >> "DES encrypted password"?
> > 
> > No, FIPS is not making statements about the choice of auth methods.
> > FIPS is concerned with what encryption algorithms an application uses.
> > The requirements about whether authentication is required & what sort,
> > is upto other specifications (eg Common Criteria) to decide.
> 
> Hrm, so short-term this fixes things. But long-term, I think the 
> better solution would be to implement the tight security model and
> use a real cipher:

That is certainly possible, but shouldn't have any bearing on whether
this patch is accepted. Note that QEMU already implements VeNCrypt
and SASL extensions both of which provide strong security


Daniel
Alexander Graf May 3, 2012, 9:04 a.m. UTC | #6 
On 03.05.2012, at 11:01, Alexander Graf wrote:

> 
> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
> 
>> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
>>> 
>>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
>>> 
>>>> On Wed, May 02, 2012 at 03:32:56PM -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.
>>> 
>>> So that means "no password" is more secure according to FIPS than
>>> "DES encrypted password"?
>> 
>> No, FIPS is not making statements about the choice of auth methods.
>> FIPS is concerned with what encryption algorithms an application uses.
>> The requirements about whether authentication is required & what sort,
>> is upto other specifications (eg Common Criteria) to decide.
> 
> Hrm, so short-term this fixes things. But long-term, I think the better solution would be to implement the tight security model and use a real cipher:
> 
>  http://www.tigervnc.org/cgi-bin/rfbproto#tight-security-type

In fact, there were even patches to get AES encryption into the tight auth:

  http://old.nabble.com/adding-AES-encryption-to-TightVNC-td14235009.html

Not sure what became of that though :).


Alex
Alexander Graf May 3, 2012, 9:06 a.m. UTC | #7 
On 03.05.2012, at 11:03, Daniel P. Berrange wrote:

> On Thu, May 03, 2012 at 11:01:29AM +0200, Alexander Graf wrote:
>> 
>> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
>> 
>>> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
>>>> 
>>>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
>>>> 
>>>>> On Wed, May 02, 2012 at 03:32:56PM -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.
>>>> 
>>>> So that means "no password" is more secure according to FIPS than
>>>> "DES encrypted password"?
>>> 
>>> No, FIPS is not making statements about the choice of auth methods.
>>> FIPS is concerned with what encryption algorithms an application uses.
>>> The requirements about whether authentication is required & what sort,
>>> is upto other specifications (eg Common Criteria) to decide.
>> 
>> Hrm, so short-term this fixes things. But long-term, I think the 
>> better solution would be to implement the tight security model and
>> use a real cipher:
> 
> That is certainly possible, but shouldn't have any bearing on whether
> this patch is accepted. Note that QEMU already implements VeNCrypt
> and SASL extensions both of which provide strong security

Hmm. Isn't the syslog message misleading then? Also, why would the whole password parameter be blocked then?


Alex
Daniel P. Berrangé May 3, 2012, 9:09 a.m. UTC | #8 
On Thu, May 03, 2012 at 11:06:18AM +0200, Alexander Graf wrote:
> 
> On 03.05.2012, at 11:03, Daniel P. Berrange wrote:
> 
> > On Thu, May 03, 2012 at 11:01:29AM +0200, Alexander Graf wrote:
> >> 
> >> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
> >> 
> >>> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
> >>>> 
> >>>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
> >>>> 
> >>>>> On Wed, May 02, 2012 at 03:32:56PM -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.
> >>>> 
> >>>> So that means "no password" is more secure according to FIPS than
> >>>> "DES encrypted password"?
> >>> 
> >>> No, FIPS is not making statements about the choice of auth methods.
> >>> FIPS is concerned with what encryption algorithms an application uses.
> >>> The requirements about whether authentication is required & what sort,
> >>> is upto other specifications (eg Common Criteria) to decide.
> >> 
> >> Hrm, so short-term this fixes things. But long-term, I think the 
> >> better solution would be to implement the tight security model and
> >> use a real cipher:
> > 
> > That is certainly possible, but shouldn't have any bearing on whether
> > this patch is accepted. Note that QEMU already implements VeNCrypt
> > and SASL extensions both of which provide strong security
> 
> Hmm. Isn't the syslog message misleading then? Also, why would the
> whole password parameter be blocked then?

The password parameter is irrelevant for VeNCrypt & SASL authentication
types. They are configured via other parameters.


Daniel
Alexander Graf May 3, 2012, 9:11 a.m. UTC | #9 
On 03.05.2012, at 11:09, Daniel P. Berrange wrote:

> On Thu, May 03, 2012 at 11:06:18AM +0200, Alexander Graf wrote:
>> 
>> On 03.05.2012, at 11:03, Daniel P. Berrange wrote:
>> 
>>> On Thu, May 03, 2012 at 11:01:29AM +0200, Alexander Graf wrote:
>>>> 
>>>> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
>>>> 
>>>>> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
>>>>>> 
>>>>>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
>>>>>> 
>>>>>>> On Wed, May 02, 2012 at 03:32:56PM -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.
>>>>>> 
>>>>>> So that means "no password" is more secure according to FIPS than
>>>>>> "DES encrypted password"?
>>>>> 
>>>>> No, FIPS is not making statements about the choice of auth methods.
>>>>> FIPS is concerned with what encryption algorithms an application uses.
>>>>> The requirements about whether authentication is required & what sort,
>>>>> is upto other specifications (eg Common Criteria) to decide.
>>>> 
>>>> Hrm, so short-term this fixes things. But long-term, I think the 
>>>> better solution would be to implement the tight security model and
>>>> use a real cipher:
>>> 
>>> That is certainly possible, but shouldn't have any bearing on whether
>>> this patch is accepted. Note that QEMU already implements VeNCrypt
>>> and SASL extensions both of which provide strong security
>> 
>> Hmm. Isn't the syslog message misleading then? Also, why would the
>> whole password parameter be blocked then?
> 
> The password parameter is irrelevant for VeNCrypt & SASL authentication
> types. They are configured via other parameters.

Ah, an error message hinting to the alternatives would be nice then :).


Alex
Alexander Graf May 3, 2012, 2:54 p.m. UTC | #10 
On 02.05.2012, at 21:32, 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 emit 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 VNC password auth is
> not allowed in FIPS mode.

I just talked to Roman about this one and he had some comments :)


Alex

> 
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> 
> --
> Changelog
> * 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
> ---
> qemu-doc.texi |    8 +++++---
> ui/vnc.c      |   39 +++++++++++++++++++++++++++++++++++++++
> ui/vnc.h      |    1 +
> 3 files changed, 45 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..6162425 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -32,6 +32,9 @@
> #include "acl.h"
> #include "qemu-objects.h"
> #include "qmp-commands.h"
> +#ifndef _WIN32
> +#include <syslog.h>
> +#endif
> 
> #define VNC_REFRESH_INTERVAL_BASE 30
> #define VNC_REFRESH_INTERVAL_INC  50
> @@ -48,6 +51,27 @@ static DisplayChangeListener *dcl;
> static int vnc_cursor_define(VncState *vs);
> static void vnc_release_modifiers(VncState *vs);
> 
> +static bool fips_enabled(void)
> +{
> +    bool enabled = false;
> +
> +#ifdef __linux__
> +    FILE *fds;
> +    char value;
> +
> +    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> +    if (fds == NULL) {
> +        return false;
> +    }
> +    if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
> +        enabled = true;
> +    }
> +    fclose(fds);
> +#endif /* __linux__ */
> +
> +    return enabled;
> +}
> +
> static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
> {
> #ifdef _VNC_DEBUG
> @@ -2748,6 +2772,14 @@ 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"));
> +#ifndef _WIN32
> +    if (vs->fips) {
> +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
> +    }
> +#endif /* _WIN32 */
> +
>     vs->lsock = -1;
> 
>     vs->ds = ds;
> @@ -2892,6 +2924,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;
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a851ebd..d41631b 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -160,6 +160,7 @@ struct VncDisplay
>     char *display;
>     char *password;
>     time_t expires;
> +    bool fips;
>     int auth;
>     bool lossy;
>     bool non_adaptive;
> 
>
Paul Moore May 3, 2012, 8:51 p.m. UTC | #11 
On Thursday, May 03, 2012 09:29:15 AM Daniel P. Berrange wrote:
> On Wed, May 02, 2012 at 03:32:56PM -0400, Paul Moore wrote:
> >  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
> >  {
> >  #ifdef _VNC_DEBUG
> > 
> > @@ -2748,6 +2772,14 @@ 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"));
> > +#ifndef _WIN32
> > +    if (vs->fips) {
> > +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS
> > mode\n"); +    }
> > +#endif /* _WIN32 */
> 
> I really think this should only be done if a password is actually set.
> With the code as it is, then every single time you launch a VM you're
> going to get this message in syslog, which makes it appear as if something
> is trying to illegally use passwords in FIPS mode. I feel this will cause
> admins/auditors to be worried about something being wrong, when in fact
> everything is normal.

Yep.  I can see arguments for either location but I'll go ahead and move it in 
v3 which I will be posting shortly.
Paul Moore May 3, 2012, 8:54 p.m. UTC | #12 
On Thursday, May 03, 2012 04:54:42 PM Alexander Graf wrote:
> On 02.05.2012, at 21:32, 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 emit 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 VNC password auth is
> > not allowed in FIPS mode.
> 
> I just talked to Roman about this one and he had some comments :)

I'm sure he did :)
Paul Moore May 3, 2012, 8:58 p.m. UTC | #13 
On Thursday, May 03, 2012 11:11:16 AM Alexander Graf wrote:
> On 03.05.2012, at 11:09, Daniel P. Berrange wrote:
> > On Thu, May 03, 2012 at 11:06:18AM +0200, Alexander Graf wrote:
> >> On 03.05.2012, at 11:03, Daniel P. Berrange wrote:
> >>> On Thu, May 03, 2012 at 11:01:29AM +0200, Alexander Graf wrote:
> >>>> On 03.05.2012, at 10:57, Daniel P. Berrange wrote:
> >>>>> On Thu, May 03, 2012 at 10:51:15AM +0200, Alexander Graf wrote:
> >>>>>> On 03.05.2012, at 10:29, Daniel P. Berrange wrote:
> >>>>>>> On Wed, May 02, 2012 at 03:32:56PM -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.
> >>>>>> 
> >>>>>> So that means "no password" is more secure according to FIPS than
> >>>>>> "DES encrypted password"?
> >>>>> 
> >>>>> No, FIPS is not making statements about the choice of auth methods.
> >>>>> FIPS is concerned with what encryption algorithms an application uses.
> >>>>> The requirements about whether authentication is required & what sort,
> >>>>> is upto other specifications (eg Common Criteria) to decide.
> >>>> 
> >>>> Hrm, so short-term this fixes things. But long-term, I think the
> >>>> better solution would be to implement the tight security model and
> >>> 
> >>>> use a real cipher:
> >>> That is certainly possible, but shouldn't have any bearing on whether
> >>> this patch is accepted. Note that QEMU already implements VeNCrypt
> >>> and SASL extensions both of which provide strong security
> >> 
> >> Hmm. Isn't the syslog message misleading then? Also, why would the
> >> whole password parameter be blocked then?
> > 
> > The password parameter is irrelevant for VeNCrypt & SASL authentication
> > types. They are configured via other parameters.
> 
> Ah, an error message hinting to the alternatives would be nice then :).

Fair enough.  I'll make the stderr notice suggest both VeNCrypt and SASL.
Roman Drahtmueller May 4, 2012, 2:01 a.m. UTC | #14 
> > > 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 emit 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 VNC password auth is
> > > not allowed in FIPS mode.
> > 
> > I just talked to Roman about this one and he had some comments :)
> 
> I'm sure he did :)

*g* Thanks, Alex! :)

The purpose makes perfect sense, I think.

Some small glitch, though:

fips=1 on the kernel commandline turns on fips mode in the kernel crypto, 
and leaves "1" in /proc/sys/crypto/fips_enabled for userland to consume.
openssl starts up, reads the file and runs its fips initialization with 
"1" in the file. Typically...

Two problems:
1) openssl may not come with FIPS support. proc file is ignored.
2) openssl may run in FIPS mode for reasons other than fips=1 on the 
   kernel cmdline (environment, ...).

Suggested way to handle this:

1) compile-time check if <openssl/fips.h> exists.
   Ignore fips specifics if not, otherwise:
2) use int FIPS_mode(void) for what it's there:

#ifdef _QEMU_FIPS 		/* or whatever */
#include <openssl/fips.h>
  vs->fips = FIPS_mode();
#endif
 
and skip fips_enabled(void).

Much easier!

> paul moore
> security and virtualization @ redhat

Thanks, 
Roman. (don't know if list is subscribers-post only)
Paul Moore May 4, 2012, 12:39 p.m. UTC | #15 
On Friday, May 04, 2012 04:01:09 AM Roman Drahtmueller 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 emit 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 VNC password
> > > > auth is
> > > > not allowed in FIPS mode.
> > > 
> > > I just talked to Roman about this one and he had some comments :)
> > 
> > I'm sure he did :)
> 
> *g* Thanks, Alex! :)
> 
> The purpose makes perfect sense, I think.
> 
> Some small glitch, though:
> 
> fips=1 on the kernel commandline turns on fips mode in the kernel crypto,
> and leaves "1" in /proc/sys/crypto/fips_enabled for userland to consume.
> openssl starts up, reads the file and runs its fips initialization with
> "1" in the file. Typically...
> 
> Two problems:
> 1) openssl may not come with FIPS support. proc file is ignored.
> 2) openssl may run in FIPS mode for reasons other than fips=1 on the
>    kernel cmdline (environment, ...).
>
> Suggested way to handle this:
> 
> 1) compile-time check if <openssl/fips.h> exists.
>    Ignore fips specifics if not, otherwise:
> 2) use int FIPS_mode(void) for what it's there:
> 
> #ifdef _QEMU_FIPS 		/* or whatever */
> #include <openssl/fips.h>
>   vs->fips = FIPS_mode();
> #endif
> 
> and skip fips_enabled(void).
> 
> Much easier!

If QEMU's VNC implementation used OpenSSL's DES cipher for the password 
encryption I would agree with you, but QEMU uses its own implementation 
(ui/d3des.*) and because of this I think it makes the most sense to check the 
kernel setting directly.
Daniel P. Berrangé May 4, 2012, 12:42 p.m. UTC | #16 
On Fri, May 04, 2012 at 08:39:04AM -0400, Paul Moore wrote:
> On Friday, May 04, 2012 04:01:09 AM Roman Drahtmueller wrote:

> > Two problems:
> > 1) openssl may not come with FIPS support. proc file is ignored.
> > 2) openssl may run in FIPS mode for reasons other than fips=1 on the
> >    kernel cmdline (environment, ...).
> >
> > Suggested way to handle this:
> > 
> > 1) compile-time check if <openssl/fips.h> exists.
> >    Ignore fips specifics if not, otherwise:
> > 2) use int FIPS_mode(void) for what it's there:
> > 
> > #ifdef _QEMU_FIPS 		/* or whatever */
> > #include <openssl/fips.h>
> >   vs->fips = FIPS_mode();
> > #endif
> > 
> > and skip fips_enabled(void).
> > 
> > Much easier!

QEMU does not use OpenSSL for anything [1], it uses GNUTLS, so I would
not be in favour of using OpenSSL for this. 

Daniel

[1] Yes the libspice-server.so uses OpenSSL which is a shame, but that
    can be sorted out one day.
Anthony Liguori June 3, 2012, 12:55 a.m. UTC | #17 
On 05/03/2012 03:32 AM, 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 emit 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 VNC password auth is
> not allowed in FIPS mode.
>
> Signed-off-by: Paul Moore<pmoore@redhat.com>
>
> --
> Changelog
> * 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
> ---
>   qemu-doc.texi |    8 +++++---
>   ui/vnc.c      |   39 +++++++++++++++++++++++++++++++++++++++
>   ui/vnc.h      |    1 +
>   3 files changed, 45 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..6162425 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -32,6 +32,9 @@
>   #include "acl.h"
>   #include "qemu-objects.h"
>   #include "qmp-commands.h"
> +#ifndef _WIN32
> +#include<syslog.h>
> +#endif
>
>   #define VNC_REFRESH_INTERVAL_BASE 30
>   #define VNC_REFRESH_INTERVAL_INC  50
> @@ -48,6 +51,27 @@ static DisplayChangeListener *dcl;
>   static int vnc_cursor_define(VncState *vs);
>   static void vnc_release_modifiers(VncState *vs);
>
> +static bool fips_enabled(void)
> +{
> +    bool enabled = false;
> +
> +#ifdef __linux__
> +    FILE *fds;
> +    char value;
> +
> +    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> +    if (fds == NULL) {
> +        return false;
> +    }
> +    if (fread(&value, sizeof(value), 1, fds) == 1&&  value == '1') {
> +        enabled = true;
> +    }
> +    fclose(fds);
> +#endif /* __linux__ */
> +
> +    return enabled;
> +}
> +
>   static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
>   {
>   #ifdef _VNC_DEBUG
> @@ -2748,6 +2772,14 @@ 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"));
> +#ifndef _WIN32
> +    if (vs->fips) {
> +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
> +    }
> +#endif /* _WIN32 */

We don't log to syslog in QEMU and we shouldn't start doing it just for this 
feature.

This needs to be optional and disabled by default I think.  I strongly dislike 
disabling a feature when a user isn't asking for it.  You can introduce a global 
-enable-fips-mode or something like that.

If you plumb it through QemuOpts, then a distro can choose to set the option by 
default in /etc/qemu/target-x86_64.conf.

Regards,

Anthony Liguori

> +
>       vs->lsock = -1;
>
>       vs->ds = ds;
> @@ -2892,6 +2924,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;
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a851ebd..d41631b 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -160,6 +160,7 @@ struct VncDisplay
>       char *display;
>       char *password;
>       time_t expires;
> +    bool fips;
>       int auth;
>       bool lossy;
>       bool non_adaptive;
>
>
>
Paul Moore June 4, 2012, 6:16 p.m. UTC | #18 
On Sunday, June 03, 2012 08:55:42 AM Anthony Liguori wrote:
> This needs to be optional and disabled by default I think.  I strongly
> dislike  disabling a feature when a user isn't asking for it.  You can
> introduce a global -enable-fips-mode or something like that.

I'll resend the patch, but before I do I want to make sure the defaults are 
set to whatever you find acceptable to merging and the second sentence above 
has me a little confused; do you mean "... dislike _enabling_ a feature when a 
user isn't asking for it."?
Anthony Liguori June 4, 2012, 11:11 p.m. UTC | #19 
On 06/05/2012 02:16 AM, Paul Moore wrote:
> On Sunday, June 03, 2012 08:55:42 AM Anthony Liguori wrote:
>> This needs to be optional and disabled by default I think.  I strongly
>> dislike  disabling a feature when a user isn't asking for it.  You can
>> introduce a global -enable-fips-mode or something like that.
>
> I'll resend the patch, but before I do I want to make sure the defaults are
> set to whatever you find acceptable to merging and the second sentence above
> has me a little confused; do you mean "... dislike _enabling_ a feature when a
> user isn't asking for it."?

I dislike *removing* a feature unless a user has explicitly asked us too.

If a user isn't aware that fips mode is enabled, they will have no idea why VNC 
authentication doesn't work.  I think we should let a user choice whether they 
want QEMU to respect fips mode or not.

Regards,

Anthony Liguori

>
Alexander Graf June 4, 2012, 11:17 p.m. UTC | #20 
On 05.06.2012, at 01:11, Anthony Liguori wrote:

> On 06/05/2012 02:16 AM, Paul Moore wrote:
>> On Sunday, June 03, 2012 08:55:42 AM Anthony Liguori wrote:
>>> This needs to be optional and disabled by default I think.  I strongly
>>> dislike  disabling a feature when a user isn't asking for it.  You can
>>> introduce a global -enable-fips-mode or something like that.
>> 
>> I'll resend the patch, but before I do I want to make sure the defaults are
>> set to whatever you find acceptable to merging and the second sentence above
>> has me a little confused; do you mean "... dislike _enabling_ a feature when a
>> user isn't asking for it."?
> 
> I dislike *removing* a feature unless a user has explicitly asked us too.
> 
> If a user isn't aware that fips mode is enabled, they will have no idea why VNC authentication doesn't work.  I think we should let a user choice whether they want QEMU to respect fips mode or not.

While I agree in general, for FIPS chances are basically negligible that you accidentally enable it. And if you do, the rest of your system will have gone mad before you notice QEMU behaving differently anyways :)


Alex
Anthony Liguori June 4, 2012, 11:54 p.m. UTC | #21 
On 06/05/2012 07:17 AM, Alexander Graf wrote:
>
> On 05.06.2012, at 01:11, Anthony Liguori wrote:
>
>> On 06/05/2012 02:16 AM, Paul Moore wrote:
>>> On Sunday, June 03, 2012 08:55:42 AM Anthony Liguori wrote:
>>>> This needs to be optional and disabled by default I think.  I strongly
>>>> dislike  disabling a feature when a user isn't asking for it.  You can
>>>> introduce a global -enable-fips-mode or something like that.
>>>
>>> I'll resend the patch, but before I do I want to make sure the defaults are
>>> set to whatever you find acceptable to merging and the second sentence above
>>> has me a little confused; do you mean "... dislike _enabling_ a feature when a
>>> user isn't asking for it."?
>>
>> I dislike *removing* a feature unless a user has explicitly asked us too.
>>
>> If a user isn't aware that fips mode is enabled, they will have no idea why VNC authentication doesn't work.  I think we should let a user choice whether they want QEMU to respect fips mode or not.
>
> While I agree in general, for FIPS chances are basically negligible that you accidentally enable it. And if you do, the rest of your system will have gone mad before you notice QEMU behaving differently anyways :)

Have you ever experienced a random failure on an SELinux box that made no 
logical sense?  Out of desperation, you setenforce 0 and magically, thinks work 
again.

Even if the user enabled fips mode, they may not understand that this means VNC 
authentication will stop working.  Providing an option (1) allows the user to 
discover what the problem is (2) makes the behavior much more clear.

Removing features based on a magic procfs variable with no input from the user 
is a bad idea IMHO.

Regards,

Anthony Liguori

>
> Alex
>
Alexander Graf June 5, 2012, 12:55 a.m. UTC | #22 
On 05.06.2012, at 01:54, Anthony Liguori wrote:

> On 06/05/2012 07:17 AM, Alexander Graf wrote:
>> 
>> On 05.06.2012, at 01:11, Anthony Liguori wrote:
>> 
>>> On 06/05/2012 02:16 AM, Paul Moore wrote:
>>>> On Sunday, June 03, 2012 08:55:42 AM Anthony Liguori wrote:
>>>>> This needs to be optional and disabled by default I think.  I strongly
>>>>> dislike  disabling a feature when a user isn't asking for it.  You can
>>>>> introduce a global -enable-fips-mode or something like that.
>>>> 
>>>> I'll resend the patch, but before I do I want to make sure the defaults are
>>>> set to whatever you find acceptable to merging and the second sentence above
>>>> has me a little confused; do you mean "... dislike _enabling_ a feature when a
>>>> user isn't asking for it."?
>>> 
>>> I dislike *removing* a feature unless a user has explicitly asked us too.
>>> 
>>> If a user isn't aware that fips mode is enabled, they will have no idea why VNC authentication doesn't work.  I think we should let a user choice whether they want QEMU to respect fips mode or not.
>> 
>> While I agree in general, for FIPS chances are basically negligible that you accidentally enable it. And if you do, the rest of your system will have gone mad before you notice QEMU behaving differently anyways :)
> 
> Have you ever experienced a random failure on an SELinux box that made no logical sense?  Out of desperation, you setenforce 0 and magically, thinks work again.

Yeah - I never understood how anyone thought it makes sense to enable SELinux globally be default.... Either way, FIPS hopefully isn't something you find enabled by accident anywhere.

> Even if the user enabled fips mode, they may not understand that this means VNC authentication will stop working.  Providing an option (1) allows the user to discover what the problem is (2) makes the behavior much more clear.

Where would you want the option to live? Compile time would be useless - users don't recompile QEMU, they take binary packages. A runtime option? Who would enable that runtime option then? Libvirt by default I suppose? So you're back in the same hell. RH would patch libvirt to always pass in -enable-fips and nothing would be different.

> Removing features based on a magic procfs variable with no input from the user is a bad idea IMHO.

But it's the design of the Linux FIPS model.

The thing that I definitely do think we should do is tell the user about it. So if we find FIPS mode, let the user know that his option just got dropped because of it, so that he has a chance to debug why his great command line didn't work just now.


Alex
Anthony Liguori June 5, 2012, 1:03 a.m. UTC | #23 
On 06/05/2012 08:55 AM, Alexander Graf wrote:
>
> On 05.06.2012, at 01:54, Anthony Liguori wrote:
>
>> Have you ever experienced a random failure on an SELinux box that made no logical sense?  Out of desperation, you setenforce 0 and magically, thinks work again.
>
> Yeah - I never understood how anyone thought it makes sense to enable SELinux globally be default.... Either way, FIPS hopefully isn't something you find enabled by accident anywhere.
>
>> Even if the user enabled fips mode, they may not understand that this means VNC authentication will stop working.  Providing an option (1) allows the user to discover what the problem is (2) makes the behavior much more clear.
>
> Where would you want the option to live? Compile time would be useless - users don't recompile QEMU, they take binary packages. A runtime option? Who would enable that runtime option then? Libvirt by default I suppose? So you're back in the same hell. RH would patch libvirt to always pass in -enable-fips and nothing would be different.

A QemuOpts option that is disabled by default but can be enabled through 
/etc/qemu/target-x86_64.conf

If any distribution wants to enable it as part of the default configuration, 
they certainly can.  But a user can override it if they want to.

Likewise, libvirt can enable it by default if they are so inclined.  At least 
the qemu logs from libvirt will show -enable-fips-mode

>
>> Removing features based on a magic procfs variable with no input from the user is a bad idea IMHO.
>
> But it's the design of the Linux FIPS model.

Just because someone made a bad choice, that doesn't mean we have to continue to 
make bad choices ourselves.

This whole feature is ridiculous from a technical perspective.  As you said, 
disabling VNC auth but allowing no-password to be used is simply moronic.

I understand why we have to support these things, but it should not be the 
default behavior.

Regards,

Anthony Liguori
Alexander Graf June 5, 2012, 1:08 a.m. UTC | #24 
On 05.06.2012, at 03:03, Anthony Liguori wrote:

> On 06/05/2012 08:55 AM, Alexander Graf wrote:
>> 
>> On 05.06.2012, at 01:54, Anthony Liguori wrote:
>> 
>>> Have you ever experienced a random failure on an SELinux box that made no logical sense?  Out of desperation, you setenforce 0 and magically, thinks work again.
>> 
>> Yeah - I never understood how anyone thought it makes sense to enable SELinux globally be default.... Either way, FIPS hopefully isn't something you find enabled by accident anywhere.
>> 
>>> Even if the user enabled fips mode, they may not understand that this means VNC authentication will stop working.  Providing an option (1) allows the user to discover what the problem is (2) makes the behavior much more clear.
>> 
>> Where would you want the option to live? Compile time would be useless - users don't recompile QEMU, they take binary packages. A runtime option? Who would enable that runtime option then? Libvirt by default I suppose? So you're back in the same hell. RH would patch libvirt to always pass in -enable-fips and nothing would be different.
> 
> A QemuOpts option that is disabled by default but can be enabled through /etc/qemu/target-x86_64.conf
> 
> If any distribution wants to enable it as part of the default configuration, they certainly can.  But a user can override it if they want to.
> 
> Likewise, libvirt can enable it by default if they are so inclined.  At least the qemu logs from libvirt will show -enable-fips-mode
> 
>> 
>>> Removing features based on a magic procfs variable with no input from the user is a bad idea IMHO.
>> 
>> But it's the design of the Linux FIPS model.
> 
> Just because someone made a bad choice, that doesn't mean we have to continue to make bad choices ourselves.
> 
> This whole feature is ridiculous from a technical perspective.  As you said, disabling VNC auth but allowing no-password to be used is simply moronic.
> 
> I understand why we have to support these things, but it should not be the default behavior.

Fair enough, but I don't think a

### log file ###

qemu-kvm <a lot of options> -enable-fips <a lot of other options>

### end of log file ###

vs

### log file ###

qemu-kvm <even more options> <option that sets vnc password>
WARNING: your VNC password has just been deactivated

### end of log file ###

make too much of a difference.

Which gets me to a new idea. Why not exit(1) when we detect FIPS and a password is set? I agree with the assessment that we should never silently drop features. So the best way to make sure that the user knows he did something stupid (enable FIPS, but require a non-FIPS compliant authentication method) would be to just quit, no?

That way everyone should be happy. Users can't run VMs with FIPS && vnc password, we don't have to enable new command line options and whoever does the UI can also check the same thing and just not expose the vnc password field.


Alex
Anthony Liguori June 5, 2012, 1:23 a.m. UTC | #25 
On 06/05/2012 09:08 AM, Alexander Graf wrote:
>
> On 05.06.2012, at 03:03, Anthony Liguori wrote:
>
>> On 06/05/2012 08:55 AM, Alexander Graf wrote:
>>>
>>> On 05.06.2012, at 01:54, Anthony Liguori wrote:
>>>
>>>> Have you ever experienced a random failure on an SELinux box that made no logical sense?  Out of desperation, you setenforce 0 and magically, thinks work again.
>>>
>>> Yeah - I never understood how anyone thought it makes sense to enable SELinux globally be default.... Either way, FIPS hopefully isn't something you find enabled by accident anywhere.
>>>
>>>> Even if the user enabled fips mode, they may not understand that this means VNC authentication will stop working.  Providing an option (1) allows the user to discover what the problem is (2) makes the behavior much more clear.
>>>
>>> Where would you want the option to live? Compile time would be useless - users don't recompile QEMU, they take binary packages. A runtime option? Who would enable that runtime option then? Libvirt by default I suppose? So you're back in the same hell. RH would patch libvirt to always pass in -enable-fips and nothing would be different.
>>
>> A QemuOpts option that is disabled by default but can be enabled through /etc/qemu/target-x86_64.conf
>>
>> If any distribution wants to enable it as part of the default configuration, they certainly can.  But a user can override it if they want to.
>>
>> Likewise, libvirt can enable it by default if they are so inclined.  At least the qemu logs from libvirt will show -enable-fips-mode
>>
>>>
>>>> Removing features based on a magic procfs variable with no input from the user is a bad idea IMHO.
>>>
>>> But it's the design of the Linux FIPS model.
>>
>> Just because someone made a bad choice, that doesn't mean we have to continue to make bad choices ourselves.
>>
>> This whole feature is ridiculous from a technical perspective.  As you said, disabling VNC auth but allowing no-password to be used is simply moronic.
>>
>> I understand why we have to support these things, but it should not be the default behavior.
>
> Fair enough, but I don't think a
>
> ### log file ###
>
> qemu-kvm<a lot of options>  -enable-fips<a lot of other options>
>
> ### end of log file ###
>
> vs
>
> ### log file ###
>
> qemu-kvm<even more options>  <option that sets vnc password>
> WARNING: your VNC password has just been deactivated
>
> ### end of log file ###
>
> make too much of a difference.
>
> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a password is set? I agree with the assessment that we should never silently drop features. So the best way to make sure that the user knows he did something stupid (enable FIPS, but require a non-FIPS compliant authentication method) would be to just quit, no?

I think my primary requirement is: allow a user to use vnc authentication even 
when fips mode is active by using some command line option.

Regards,

Anthony Liguori

>
> That way everyone should be happy. Users can't run VMs with FIPS&&  vnc password, we don't have to enable new command line options and whoever does the UI can also check the same thing and just not expose the vnc password field.
>
>
> Alex
>
>
>
Alexander Graf June 5, 2012, 1:29 a.m. UTC | #26 
On 05.06.2012, at 03:23, Anthony Liguori wrote:

> On 06/05/2012 09:08 AM, Alexander Graf wrote:
>> 
>> On 05.06.2012, at 03:03, Anthony Liguori wrote:
>> 
>>> On 06/05/2012 08:55 AM, Alexander Graf wrote:
>>>> 
>>>> On 05.06.2012, at 01:54, Anthony Liguori wrote:
>>>> 
>>>>> Have you ever experienced a random failure on an SELinux box that made no logical sense?  Out of desperation, you setenforce 0 and magically, thinks work again.
>>>> 
>>>> Yeah - I never understood how anyone thought it makes sense to enable SELinux globally be default.... Either way, FIPS hopefully isn't something you find enabled by accident anywhere.
>>>> 
>>>>> Even if the user enabled fips mode, they may not understand that this means VNC authentication will stop working.  Providing an option (1) allows the user to discover what the problem is (2) makes the behavior much more clear.
>>>> 
>>>> Where would you want the option to live? Compile time would be useless - users don't recompile QEMU, they take binary packages. A runtime option? Who would enable that runtime option then? Libvirt by default I suppose? So you're back in the same hell. RH would patch libvirt to always pass in -enable-fips and nothing would be different.
>>> 
>>> A QemuOpts option that is disabled by default but can be enabled through /etc/qemu/target-x86_64.conf
>>> 
>>> If any distribution wants to enable it as part of the default configuration, they certainly can.  But a user can override it if they want to.
>>> 
>>> Likewise, libvirt can enable it by default if they are so inclined.  At least the qemu logs from libvirt will show -enable-fips-mode
>>> 
>>>> 
>>>>> Removing features based on a magic procfs variable with no input from the user is a bad idea IMHO.
>>>> 
>>>> But it's the design of the Linux FIPS model.
>>> 
>>> Just because someone made a bad choice, that doesn't mean we have to continue to make bad choices ourselves.
>>> 
>>> This whole feature is ridiculous from a technical perspective.  As you said, disabling VNC auth but allowing no-password to be used is simply moronic.
>>> 
>>> I understand why we have to support these things, but it should not be the default behavior.
>> 
>> Fair enough, but I don't think a
>> 
>> ### log file ###
>> 
>> qemu-kvm<a lot of options>  -enable-fips<a lot of other options>
>> 
>> ### end of log file ###
>> 
>> vs
>> 
>> ### log file ###
>> 
>> qemu-kvm<even more options>  <option that sets vnc password>
>> WARNING: your VNC password has just been deactivated
>> 
>> ### end of log file ###
>> 
>> make too much of a difference.
>> 
>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a password is set? I agree with the assessment that we should never silently drop features. So the best way to make sure that the user knows he did something stupid (enable FIPS, but require a non-FIPS compliant authentication method) would be to just quit, no?
> 
> I think my primary requirement is: allow a user to use vnc authentication even when fips mode is active by using some command line option.

That's where we disagree.

To me, fips mode enabled in /proc/somewhere is as good as passing in -enable-fips on the command line. So it's almost the same as -enable-kvm. I don't want -enable-kvm to silently not use KVM, I want QEMU to bail out because I've asked it to do something it can't do.

If I pass -enable-fips -vnc-passwd foo, I want to have QEMU bail out. Since /proc/foo/fips_mode==1 is equivalent to -enable-fips, I want the same effect there.

But then again, most people in this world won't care, because they won't enable FIPS either way ;). So we've probably already wasted more time discussing this than any user would have trying to debug why his password authentication wouldn't work.


Alex
Gerd Hoffmann June 5, 2012, 7:23 a.m. UTC | #27 
Hi,

>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
>> password is set? I agree with the assessment that we should never
>> silently drop features. So the best way to make sure that the user
>> knows he did something stupid (enable FIPS, but require a non-FIPS
>> compliant authentication method) would be to just quit, no?
> 
> I think my primary requirement is: allow a user to use vnc
> authentication even when fips mode is active by using some command line
> option.

That doesn't make sense to me at all.  If fips is enabled by accident
just disable it.  If fips is enabled intentionally I don't think qemu
should ignore it and allow to use weak vnc auth.  Fips users should
setup sasl instead I guess ...

cheers,
  Gerd
Paul Moore June 5, 2012, 9:45 p.m. UTC | #28 
On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
> password is set? I agree with the assessment that we should never silently
> drop features. So the best way to make sure that the user knows he did
> something stupid (enable FIPS, but require a non-FIPS compliant
> authentication method) would be to just quit, no?

That is basically what the patch does now.  In vnc_display_open() if it 
detects that the user has supplied a VNC password it prints an error to stderr 
and returns an error which causes QEMU to exit.

The error message displayed is shown below:

 "VNC password auth disabled due to FIPS mode, consider using the VeNCrypt
  or SASL authentication methods as an alernative"

... which seems pretty obvious to me.  If anyone would prefer something 
different, let me know.

On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
> I think my primary requirement is: allow a user to use vnc authentication
> even when fips mode is active by using some command line option.

I'll agree that FIPS mode can be a bit silly in the case of QEMU and VNC but 
to be honest, that requirement above seems just as silly to me, if not more 
so.  However, if making this behavior optional is what it takes to get the 
patch accepted, so be it.

I'll start working on v4 of the patch tomorrow.
Alexander Graf June 5, 2012, 9:51 p.m. UTC | #29 
On 05.06.2012, at 23:45, Paul Moore wrote:

> On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
>> password is set? I agree with the assessment that we should never silently
>> drop features. So the best way to make sure that the user knows he did
>> something stupid (enable FIPS, but require a non-FIPS compliant
>> authentication method) would be to just quit, no?
> 
> That is basically what the patch does now.  In vnc_display_open() if it 
> detects that the user has supplied a VNC password it prints an error to stderr 
> and returns an error which causes QEMU to exit.
> 
> The error message displayed is shown below:
> 
> "VNC password auth disabled due to FIPS mode, consider using the VeNCrypt
>  or SASL authentication methods as an alernative"
> 
> ... which seems pretty obvious to me.  If anyone would prefer something 
> different, let me know.

No, as long as the spelling is actually correct and not the one above, that's perfectly fine. I just have a habit of not reading the patches I comment on :).

> 
> On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
>> I think my primary requirement is: allow a user to use vnc authentication
>> even when fips mode is active by using some command line option.
> 
> I'll agree that FIPS mode can be a bit silly in the case of QEMU and VNC but 
> to be honest, that requirement above seems just as silly to me, if not more 
> so.  However, if making this behavior optional is what it takes to get the 
> patch accepted, so be it.
> 
> I'll start working on v4 of the patch tomorrow.

Let's just wait for Anthony to reply. I'm sure he'll find it reasonable to just quit when the environment dictates something that can't be fulfilled. After all, enabling FIPS mode globally is like setting a global ulimit, or like setting a disk quota, or like starting QEMU with strace. We don't randomly ignore what our parents dictate on us ;).


Alex
Paul Moore June 5, 2012, 10:06 p.m. UTC | #30 
On Tuesday, June 05, 2012 11:51:40 PM Alexander Graf wrote:
> On 05.06.2012, at 23:45, Paul Moore wrote:
> > On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
> >> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
> >> password is set? I agree with the assessment that we should never
> >> silently drop features. So the best way to make sure that the user knows
> >> he did something stupid (enable FIPS, but require a non-FIPS compliant
> >> authentication method) would be to just quit, no?
> > 
> > That is basically what the patch does now.  In vnc_display_open() if it
> > detects that the user has supplied a VNC password it prints an error to
> > stderr and returns an error which causes QEMU to exit.
> > 
> > The error message displayed is shown below:
> > 
> > "VNC password auth disabled due to FIPS mode, consider using the VeNCrypt
> >  or SASL authentication methods as an alernative"
> > 
> > ... which seems pretty obvious to me.  If anyone would prefer something
> > different, let me know.
> 
> No, as long as the spelling is actually correct and not the one above,
> that's perfectly fine.

What, not a fan of my "alernative" spelling?  Fixed in the next version of the 
patch :)

> I just have a habit of not reading the patches I comment on :).

If nothing else, it makes the discussions much more interesting :)

> > On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
> >> I think my primary requirement is: allow a user to use vnc authentication
> >> even when fips mode is active by using some command line option.
> > 
> > I'll agree that FIPS mode can be a bit silly in the case of QEMU and VNC
> > but to be honest, that requirement above seems just as silly to me, if
> > not more so.  However, if making this behavior optional is what it takes
> > to get the patch accepted, so be it.
> > 
> > I'll start working on v4 of the patch tomorrow.
> 
> Let's just wait for Anthony to reply ...

Fine with me, I've got plenty else to do in the meantime and I don't think 
this is 1.1 material anyway.
Anthony Liguori June 5, 2012, 11:07 p.m. UTC | #31 
On 06/06/2012 06:06 AM, Paul Moore wrote:
> On Tuesday, June 05, 2012 11:51:40 PM Alexander Graf wrote:
>> On 05.06.2012, at 23:45, Paul Moore wrote:
>>> On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
>>>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
>>>> password is set? I agree with the assessment that we should never
>>>> silently drop features. So the best way to make sure that the user knows
>>>> he did something stupid (enable FIPS, but require a non-FIPS compliant
>>>> authentication method) would be to just quit, no?
>>>
>>> That is basically what the patch does now.  In vnc_display_open() if it
>>> detects that the user has supplied a VNC password it prints an error to
>>> stderr and returns an error which causes QEMU to exit.
>>>
>>> The error message displayed is shown below:
>>>
>>> "VNC password auth disabled due to FIPS mode, consider using the VeNCrypt
>>>   or SASL authentication methods as an alernative"
>>>
>>> ... which seems pretty obvious to me.  If anyone would prefer something
>>> different, let me know.
>>
>> No, as long as the spelling is actually correct and not the one above,
>> that's perfectly fine.
>
> What, not a fan of my "alernative" spelling?  Fixed in the next version of the
> patch :)
>
>> I just have a habit of not reading the patches I comment on :).
>
> If nothing else, it makes the discussions much more interesting :)
>
>>> On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
>>>> I think my primary requirement is: allow a user to use vnc authentication
>>>> even when fips mode is active by using some command line option.
>>>
>>> I'll agree that FIPS mode can be a bit silly in the case of QEMU and VNC
>>> but to be honest, that requirement above seems just as silly to me, if
>>> not more so.  However, if making this behavior optional is what it takes
>>> to get the patch accepted, so be it.
>>>
>>> I'll start working on v4 of the patch tomorrow.
>>
>> Let's just wait for Anthony to reply ...
>
> Fine with me, I've got plenty else to do in the meantime and I don't think
> this is 1.1 material anyway.

What's the actual requirement from FIPS for applications?

Regards,

Anthony Liguori
Alexander Graf June 5, 2012, 11:56 p.m. UTC | #32 
On 06.06.2012, at 01:07, Anthony Liguori wrote:

> On 06/06/2012 06:06 AM, Paul Moore wrote:
>> On Tuesday, June 05, 2012 11:51:40 PM Alexander Graf wrote:
>>> On 05.06.2012, at 23:45, Paul Moore wrote:
>>>> On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
>>>>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
>>>>> password is set? I agree with the assessment that we should never
>>>>> silently drop features. So the best way to make sure that the user knows
>>>>> he did something stupid (enable FIPS, but require a non-FIPS compliant
>>>>> authentication method) would be to just quit, no?
>>>> 
>>>> That is basically what the patch does now.  In vnc_display_open() if it
>>>> detects that the user has supplied a VNC password it prints an error to
>>>> stderr and returns an error which causes QEMU to exit.
>>>> 
>>>> The error message displayed is shown below:
>>>> 
>>>> "VNC password auth disabled due to FIPS mode, consider using the VeNCrypt
>>>>  or SASL authentication methods as an alernative"
>>>> 
>>>> ... which seems pretty obvious to me.  If anyone would prefer something
>>>> different, let me know.
>>> 
>>> No, as long as the spelling is actually correct and not the one above,
>>> that's perfectly fine.
>> 
>> What, not a fan of my "alernative" spelling?  Fixed in the next version of the
>> patch :)
>> 
>>> I just have a habit of not reading the patches I comment on :).
>> 
>> If nothing else, it makes the discussions much more interesting :)
>> 
>>>> On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
>>>>> I think my primary requirement is: allow a user to use vnc authentication
>>>>> even when fips mode is active by using some command line option.
>>>> 
>>>> I'll agree that FIPS mode can be a bit silly in the case of QEMU and VNC
>>>> but to be honest, that requirement above seems just as silly to me, if
>>>> not more so.  However, if making this behavior optional is what it takes
>>>> to get the patch accepted, so be it.
>>>> 
>>>> I'll start working on v4 of the patch tomorrow.
>>> 
>>> Let's just wait for Anthony to reply ...
>> 
>> Fine with me, I've got plenty else to do in the meantime and I don't think
>> this is 1.1 material anyway.
> 
> What's the actual requirement from FIPS for applications?

If I understood Roman correctly, there are 2 puzzle pieces to this. One (whose name I forgot) is responsible for making sure you use encryption at all, which authentication methods (retina scan, fingerprint, etc) are allowed and so forth.

The other one (FIPS) is basically a list of encryption algorithms that are deemed OK and not crackable within seconds by anyone.

Only one of the 2 doesn't help much. In combination they actually enhance security. This patch is only about FIPS though.


Alex
Paul Moore June 6, 2012, 10:56 p.m. UTC | #33 
On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
> On 06.06.2012, at 01:07, Anthony Liguori wrote:
> > On 06/06/2012 06:06 AM, Paul Moore wrote:
> >> On Tuesday, June 05, 2012 11:51:40 PM Alexander Graf wrote:
> >>> On 05.06.2012, at 23:45, Paul Moore wrote:
> >>>> On Tuesday, June 05, 2012 03:08:26 AM Alexander Graf wrote:
> >>>>> Which gets me to a new idea. Why not exit(1) when we detect FIPS and a
> >>>>> password is set? I agree with the assessment that we should never
> >>>>> silently drop features. So the best way to make sure that the user
> >>>>> knows
> >>>>> he did something stupid (enable FIPS, but require a non-FIPS compliant
> >>>>> authentication method) would be to just quit, no?
> >>>> 
> >>>> That is basically what the patch does now.  In vnc_display_open() if it
> >>>> detects that the user has supplied a VNC password it prints an error to
> >>>> stderr and returns an error which causes QEMU to exit.
> >>>> 
> >>>> The error message displayed is shown below:
> >>>> 
> >>>> "VNC password auth disabled due to FIPS mode, consider using the
> >>>> VeNCrypt
> >>>> 
> >>>>  or SASL authentication methods as an alernative"
> >>>> 
> >>>> ... which seems pretty obvious to me.  If anyone would prefer something
> >>>> different, let me know.
> >>> 
> >>> No, as long as the spelling is actually correct and not the one above,
> >>> that's perfectly fine.
> >> 
> >> What, not a fan of my "alernative" spelling?  Fixed in the next version
> >> of the patch :)
> >> 
> >>> I just have a habit of not reading the patches I comment on :).
> >> 
> >> If nothing else, it makes the discussions much more interesting :)
> >> 
> >>>> On Tuesday, June 05, 2012 09:23:04 AM Anthony Liguori wrote:
> >>>>> I think my primary requirement is: allow a user to use vnc
> >>>>> authentication
> >>>>> even when fips mode is active by using some command line option.
> >>>> 
> >>>> I'll agree that FIPS mode can be a bit silly in the case of QEMU and
> >>>> VNC
> >>>> but to be honest, that requirement above seems just as silly to me, if
> >>>> not more so.  However, if making this behavior optional is what it
> >>>> takes
> >>>> to get the patch accepted, so be it.
> >>>> 
> >>>> I'll start working on v4 of the patch tomorrow.
> >>> 
> >>> Let's just wait for Anthony to reply ...
> >> 
> >> Fine with me, I've got plenty else to do in the meantime and I don't
> >> think
> >> this is 1.1 material anyway.
> > 
> > What's the actual requirement from FIPS for applications?
> 
> If I understood Roman correctly, there are 2 puzzle pieces to this. One
> (whose name I forgot) is responsible for making sure you use encryption at
> all, which authentication methods (retina scan, fingerprint, etc) are
> allowed and so forth.
> 
> The other one (FIPS) is basically a list of encryption algorithms that are
> deemed OK and not crackable within seconds by anyone.
> 
> Only one of the 2 doesn't help much. In combination they actually enhance
> security. This patch is only about FIPS though.

I don't have much to add beyond what Alex already posted.  FIPS 140-2 outlines 
a set of security requirements for systems implementing cryptography in a 
variety of forms; the full requirements are likely beyond the scope here but 
you can always read the full specification (Google knows where to find the 
document).

The relevant portion appears to be annex A which lists the approved ciphers 
and their approved uses; DES is not listed as an approved cipher and that is 
the main problem we are trying to solve right now.
Anthony Liguori June 7, 2012, 3:10 a.m. UTC | #34 
On 06/07/2012 06:56 AM, Paul Moore wrote:
> On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
>> The other one (FIPS) is basically a list of encryption algorithms that are
>> deemed OK and not crackable within seconds by anyone.
>>
>> Only one of the 2 doesn't help much. In combination they actually enhance
>> security. This patch is only about FIPS though.
>
> I don't have much to add beyond what Alex already posted.  FIPS 140-2 outlines
> a set of security requirements for systems implementing cryptography in a
> variety of forms; the full requirements are likely beyond the scope here but
> you can always read the full specification (Google knows where to find the
> document).
>
> The relevant portion appears to be annex A which lists the approved ciphers
> and their approved uses; DES is not listed as an approved cipher and that is
> the main problem we are trying to solve right now.

But does FIPS mandate that it's impossible for a user to use an unapproved cipher?

IOW, is just having this feature implemented at the libvirt level good enough to 
satisfy FIPS?  Do we really need to do this in QEMU?

Regards,

Anthony Liguori

>
Alexander Graf June 7, 2012, 10:31 a.m. UTC | #35 
On 07.06.2012, at 05:10, Anthony Liguori wrote:

> On 06/07/2012 06:56 AM, Paul Moore wrote:
>> On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
>>> The other one (FIPS) is basically a list of encryption algorithms that are
>>> deemed OK and not crackable within seconds by anyone.
>>> 
>>> Only one of the 2 doesn't help much. In combination they actually enhance
>>> security. This patch is only about FIPS though.
>> 
>> I don't have much to add beyond what Alex already posted.  FIPS 140-2 outlines
>> a set of security requirements for systems implementing cryptography in a
>> variety of forms; the full requirements are likely beyond the scope here but
>> you can always read the full specification (Google knows where to find the
>> document).
>> 
>> The relevant portion appears to be annex A which lists the approved ciphers
>> and their approved uses; DES is not listed as an approved cipher and that is
>> the main problem we are trying to solve right now.
> 
> But does FIPS mandate that it's impossible for a user to use an unapproved cipher?
> 
> IOW, is just having this feature implemented at the libvirt level good enough to satisfy FIPS?  Do we really need to do this in QEMU?

What would implementing it in libvirt buy us? That only stacks using libvirt can be FIPS certified? That any time a management stack that does not use libvirt they need to duplicate that code to be FIPS certified?

I would rather have the "FIPS certified" stamp on QEMU than on libvirt ;). The same way you would usually certify openssl and any user of it (usually) inherits the certification. So by having QEMU FIPS certified, we could reasonably assume libvirt to be FIPS certified. And Ganesi (or whatever it's called). And other stacks that don't go through libvirt hell.


Alex
Paul Moore June 7, 2012, 1:21 p.m. UTC | #36 
On Thursday, June 07, 2012 12:31:25 PM Alexander Graf wrote:
> On 07.06.2012, at 05:10, Anthony Liguori wrote:
> > On 06/07/2012 06:56 AM, Paul Moore wrote:
> >> On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
> >>> The other one (FIPS) is basically a list of encryption algorithms that
> >>> are deemed OK and not crackable within seconds by anyone.
> >>> 
> >>> Only one of the 2 doesn't help much. In combination they actually
> >>> enhance security. This patch is only about FIPS though.
> >> 
> >> I don't have much to add beyond what Alex already posted.  FIPS 140-2
> >> outlines a set of security requirements for systems implementing
> >> cryptography in a variety of forms; the full requirements are likely
> >> beyond the scope here but you can always read the full specification
> >> (Google knows where to find the document).
> >> 
> >> The relevant portion appears to be annex A which lists the approved
> >> ciphers and their approved uses; DES is not listed as an approved cipher
> >> and that is the main problem we are trying to solve right now.
> > 
> > But does FIPS mandate that it's impossible for a user to use an unapproved
> > cipher?
> > 
> > IOW, is just having this feature implemented at the libvirt level good
> > enough to satisfy FIPS?  Do we really need to do this in QEMU?
>
> What would implementing it in libvirt buy us? That only stacks using libvirt
> can be FIPS certified? That any time a management stack that does not use
> libvirt they need to duplicate that code to be FIPS certified?

Once again, I think Alex summed it up nicely.

While most users probably use QEMU via libvirt, the fact remains that you can 
always run QEMU directly so simply disallowing VNC's password authentication 
doesn't really solve the FIPS problem.
Paul Moore June 8, 2012, 9:37 p.m. UTC | #37 
On Thursday, June 07, 2012 09:21:12 AM Paul Moore wrote:
> On Thursday, June 07, 2012 12:31:25 PM Alexander Graf wrote:
> > On 07.06.2012, at 05:10, Anthony Liguori wrote:
> > > On 06/07/2012 06:56 AM, Paul Moore wrote:
> > >> On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
> > >>> The other one (FIPS) is basically a list of encryption algorithms that
> > >>> are deemed OK and not crackable within seconds by anyone.
> > >>> 
> > >>> Only one of the 2 doesn't help much. In combination they actually
> > >>> enhance security. This patch is only about FIPS though.
> > >> 
> > >> I don't have much to add beyond what Alex already posted.  FIPS 140-2
> > >> outlines a set of security requirements for systems implementing
> > >> cryptography in a variety of forms; the full requirements are likely
> > >> beyond the scope here but you can always read the full specification
> > >> (Google knows where to find the document).
> > >> 
> > >> The relevant portion appears to be annex A which lists the approved
> > >> ciphers and their approved uses; DES is not listed as an approved
> > >> cipher
> > >> and that is the main problem we are trying to solve right now.
> > > 
> > > But does FIPS mandate that it's impossible for a user to use an
> > > unapproved
> > > cipher?
> > > 
> > > IOW, is just having this feature implemented at the libvirt level good
> > > enough to satisfy FIPS?  Do we really need to do this in QEMU?
> > 
> > What would implementing it in libvirt buy us? That only stacks using
> > libvirt can be FIPS certified? That any time a management stack that does
> > not use libvirt they need to duplicate that code to be FIPS certified?
> 
> Once again, I think Alex summed it up nicely.
> 
> While most users probably use QEMU via libvirt, the fact remains that you
> can always run QEMU directly so simply disallowing VNC's password
> authentication doesn't really solve the FIPS problem.

I haven't seen any more discussion about this so I'm going to go ahead and 
post a v4 patch with the syslog bits removed.
Roman Drahtmueller June 11, 2012, 1:33 p.m. UTC | #38 
I was "beached", sorry for the late reply.


On Thu, 7 Jun 2012, Alexander Graf wrote:
> On 07.06.2012, at 05:10, Anthony Liguori wrote:
> > On 06/07/2012 06:56 AM, Paul Moore wrote:
> >> On Wednesday, June 06, 2012 01:56:52 AM Alexander Graf wrote:
> >>> The other one (FIPS) is basically a list of encryption algorithms that are
> >>> deemed OK and not crackable within seconds by anyone.
> >>> 
> >>> Only one of the 2 doesn't help much. In combination they actually enhance
> >>> security. This patch is only about FIPS though.
> >> 
> >> I don't have much to add beyond what Alex already posted.  FIPS 140-2 outlines
> >> a set of security requirements for systems implementing cryptography in a
> >> variety of forms; the full requirements are likely beyond the scope here but
> >> you can always read the full specification (Google knows where to find the
> >> document).
> >> 
> >> The relevant portion appears to be annex A which lists the approved ciphers
> >> and their approved uses; DES is not listed as an approved cipher and that is
> >> the main problem we are trying to solve right now.
> > 
> > But does FIPS mandate that it's impossible for a user to use an unapproved cipher?
> > 
> > IOW, is just having this feature implemented at the libvirt level good enough to satisfy FIPS?  Do we really need to do this in QEMU?
> 
> What would implementing it in libvirt buy us? That only stacks using 
> libvirt can be FIPS certified? That any time a management stack that 
> does not use libvirt they need to duplicate that code to be FIPS 
> certified?
> 
> I would rather have the "FIPS certified" stamp on QEMU than on libvirt 
> ;). The same way you would usually certify openssl and any user of it 
> (usually) inherits the certification. So by having QEMU FIPS certified, 
> we could reasonably assume libvirt to be FIPS certified. And Ganesi (or 
> whatever it's called). And other stacks that don't go through libvirt 
> hell.
> 

Just to add some small bits and pieces about FIPS-140-2 that may turn out 
to be useful, and clear up some misunderstandings.

It is a crypto certification, which has not much to do with security, but 
rather with making sure that the algorythms used are implemented in such 
way that they produce testable results. FIPS defines a so-called module, 
which is a region of code in a program, library or whatever that contains 
the cryptographic routines. The software must make use of these 
cryptographic routines exclusively in order to be able to make a
claim about FIPS compliance - or something similar. You'd want to 
minimize the code contained in the crypto boundary to be able to reduce 
the testing work for FIPS, but still have sufficient functionality.

About such claims: There is a distinction between
* The software makes use of a library that is FIPS-140-2 certified.
* The software is FIPS-140-2 certified.
There may be someone who is willing to pay for a FIPS-140-2 validation of 
qemu, but this is very unlikely. The weaker claim, "makes use of 
FIPS-140-2 certified crypto" is enough for most cases.

About enforcement of algo use:
FIPS-140-2 has four levels, of which 3 and 4 can only be achieved by 
hardware implementations (smartcards, crypto coprocessors, ...), 1 and 2 
also by software. Level 1 says that the solution must only use approved 
algorythms, but there is no requirement about enforcing this. Level 2 
requires enforcement.

As a consequence, for qemu to be able to run with a crypto library that 
is FIPS-140-2 certified, it should be aware of the FIPS mode if there is 
any, and refrain from initializing cryptography that is not allowable 
(crypt(3) aka DES, but also MD5 (is a no-go)). In addition to that, qemu 
must not have any own algorythm implementations (convenient...).

> Alex

Thanks,
Roman.
diff mbox

Patch

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..6162425 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -32,6 +32,9 @@ 
 #include "acl.h"
 #include "qemu-objects.h"
 #include "qmp-commands.h"
+#ifndef _WIN32
+#include <syslog.h>
+#endif
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -48,6 +51,27 @@  static DisplayChangeListener *dcl;
 static int vnc_cursor_define(VncState *vs);
 static void vnc_release_modifiers(VncState *vs);
 
+static bool fips_enabled(void)
+{
+    bool enabled = false;
+
+#ifdef __linux__
+    FILE *fds;
+    char value;
+
+    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
+    if (fds == NULL) {
+        return false;
+    }
+    if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
+        enabled = true;
+    }
+    fclose(fds);
+#endif /* __linux__ */
+
+    return enabled;
+}
+
 static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
 {
 #ifdef _VNC_DEBUG
@@ -2748,6 +2772,14 @@  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"));
+#ifndef _WIN32
+    if (vs->fips) {
+        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
+    }
+#endif /* _WIN32 */
+
     vs->lsock = -1;
 
     vs->ds = ds;
@@ -2892,6 +2924,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;
diff --git a/ui/vnc.h b/ui/vnc.h
index a851ebd..d41631b 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -160,6 +160,7 @@  struct VncDisplay
     char *display;
     char *password;
     time_t expires;
+    bool fips;
     int auth;
     bool lossy;
     bool non_adaptive;