From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53084) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPbpI-0002Bd-H6 for qemu-devel@nongnu.org; Wed, 02 May 2012 11:50:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SPbpG-00022g-Fa for qemu-devel@nongnu.org; Wed, 02 May 2012 11:50:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58611) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPbpG-00022P-2U for qemu-devel@nongnu.org; Wed, 02 May 2012 11:50:46 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q42FoiK7019707 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 2 May 2012 11:50:44 -0400 From: Paul Moore Date: Wed, 02 May 2012 11:50:42 -0400 Message-ID: <1644942.8Ul4qYdc8c@sifl> In-Reply-To: <20120502091850.GK13336@redhat.com> References: <20120501212040.27850.27184.stgit@sifl> <20120502091850.GK13336@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org 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 > > > > #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