qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (secur


From: Paul Moore
Subject: Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
Date: Wed, 02 May 2012 11:50:42 -0400
User-agent: KMail/4.8.2 (Linux/3.3.4-gentoo; KDE/4.8.2; x86_64; ; )

On Wednesday, May 02, 2012 10:18:50 AM Daniel P. Berrange wrote:
> On Tue, May 01, 2012 at 05:20:40PM -0400, Paul Moore wrote:
> > diff --git a/ui/vnc.c b/ui/vnc.c
> > index deb9ecd..620791e 100644
> > --- a/ui/vnc.c
> > +++ b/ui/vnc.c
> > @@ -32,6 +32,7 @@
> > 
> >  #include "acl.h"
> >  #include "qemu-objects.h"
> >  #include "qmp-commands.h"
> > 
> > +#include <syslog.h>
> > 
> >  #define VNC_REFRESH_INTERVAL_BASE 30
> >  #define VNC_REFRESH_INTERVAL_INC  50
> > 
> > @@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
> > 
> >  static int vnc_cursor_define(VncState *vs);
> >  static void vnc_release_modifiers(VncState *vs);
> > 
> > +static int fips_enabled(void)
> 
> s/int/bool/  and use true/false as values

Fixed.
 
> > +{
> > +    int enabled = 0;
> > +    char value;
> > +    FILE *fds;
> > +
> > +    fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> > +    if (fds == NULL) {
> > +        return 0;
> > +    }
> > +    if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
> > +        enabled = 1;
> > +    }
> > +    fclose(fds);
> > +
> > +    return enabled;
> > +}
> 
> As already pointed out,wWe should probably make this depend on
> __linux__, and 'return false' fo other platforms.

Yep, that makes sense.  Fixed.

> >  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
> >  {
> >  #ifdef _VNC_DEBUG
> > 
> > @@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
> > 
> >      dcl->idle = 1;
> >      vnc_display = vs;
> > 
> > +    vs->fips = fips_enabled();
> > +    VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
> > +    if (vs->fips) {
> > +        syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS
> > mode\n"); +    }
> 
> I think this syslog message is better placed in the next chunk of the
> patch where you actually test the vs->fips value.

My reasoning for placing it here is so that there would be some positive 
indication that VNC password auth is disabled even if the user isn't 
attempting to use the it.  Although I can understand the reasoning for moving 
it down as well so that things are quieter in the non-passwd-auth case.

With this in mind, do you still think it should be moved down?

Regardless, thanks again for your time and comments.
 
-- 
paul moore
security and virtualization @ redhat




reply via email to

[Prev in Thread] Current Thread [Next in Thread]