[libvirt] [PATCH] svirt: Label serial sockets (RHBZ#853393).

[Richard W.M. Jones]

On Fri, Sep 14, 2012 at 02:40:39PM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 14, 2012 at 02:31:44PM +0100, Richard W.M. Jones wrote:
> > From: "Richard W.M. Jones" <rjones at redhat.com>
> > 
> > libvirt skips labelling these, for unknown reasons.  This breaks
> > libguestfs.  Adding this and some SELinux rules (RHBZ#857453) fixes
> > everything for me.
> > ---
> >  src/security/security_selinux.c |   12 ++----------
> >  1 file changed, 2 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> > index a7e2420..c3b33f8 100644
> > --- a/src/security/security_selinux.c
> > +++ b/src/security/security_selinux.c
> > @@ -1230,6 +1230,7 @@ virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
> >      switch (dev->type) {
> >      case VIR_DOMAIN_CHR_TYPE_DEV:
> >      case VIR_DOMAIN_CHR_TYPE_FILE:
> > +    case VIR_DOMAIN_CHR_TYPE_UNIX:
> >          ret = virSecuritySELinuxSetFilecon(dev->data.file.path, secdef->imagelabel);
> >          break;
> >  
> > @@ -1280,6 +1281,7 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def,
> >      switch (dev->type) {
> >      case VIR_DOMAIN_CHR_TYPE_DEV:
> >      case VIR_DOMAIN_CHR_TYPE_FILE:
> > +    case VIR_DOMAIN_CHR_TYPE_UNIX:
> >          if (virSecuritySELinuxRestoreSecurityFileLabel(dev->data.file.path) < 0)
> >              goto done;
> >          ret = 0;
> 
> This needs a slight tweak I think. There are two usage scenarios for
> type=unix. One where a 3rd party has created the UNIX socket and you
> are telling QEMU to connect to it, which I assume is what you're
> using this for. The other mode is whre QEMU creates/listens on the
> socket and the 3rd party does the connection. In the latter case I
> think your addition will cause an error because the socket path
> won't exist until QEMU actually runs.

Agreed -- fixed in v2, posting shortly.

> > @@ -1318,11 +1320,6 @@ virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
> >                                                   virDomainChrDefPtr dev,
> >                                                   void *opaque ATTRIBUTE_UNUSED)
> >  {
> > -    /* This is taken care of by processing of def->serials */
> > -    if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
> > -        dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
> > -        return 0;
> > -
> >      return virSecuritySELinuxRestoreSecurityChardevLabel(def, &dev->source);
> >  }
> >  
> > @@ -1698,11 +1695,6 @@ virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
> >                                               virDomainChrDefPtr dev,
> >                                               void *opaque ATTRIBUTE_UNUSED)
> >  {
> > -    /* This is taken care of by processing of def->serials */
> > -    if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
> > -        dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
> > -        return 0;
> 
> Hmm, the idea here is that any <console> which has a <target type=serial/>
> already has a corresponding <serial> element in the XML which would have
> been labelled. So I'm not sure why you need to remove this. Can you capture
> a debug log with
> 
>   log_filters="1:security_selinux"
>   log_outputs="1:file:/var/log/libvirtd.log"
> 
> and post that along with your XML so we can see what's being labelled

Right -- it turns out these hunks are not necessary at all.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw