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

[Daniel P. Berrange]

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.

> @@ -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


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|