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

[Daniel P. Berrange]

On Fri, Sep 14, 2012 at 03:07:12PM +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.

So I've finally remembered the reason why we skip this. In the case
where QEMU is responsible for creating the listening socket, obviously
we don't have anything that can be labelled as this point, as discussed
previously. In the case where a 3rd party is creating the listening
socket, we can't know whether that socket is intended solely for one
QEMU instance to connect to, or whether it is shared amongst many
QEMU instances.

In the libguestfs case, IIUC, the socket is intended for just one QEMU
to use, so labelling it makes sense.

If it was for multiple QEMU to connect to, we can't relabel in libvirt,
because that would break things for other QEMU instances already
connected.

Finally, libguestfs can't label the socket itself, upfront, because
it doesn't know what random MCS category pair will be picked. So
clearly we need to solve this in libvirt

I see two options here to enable the behaviour you require

 1. Add virSecurityDeviceLabelDefPtr to virDomainCharDefPtr struct,
    as we have already done for virDomainDiskDefPtr. This lets us
    control whether relabelling is done on a per-device basis in the
    XML.

 2. Add a <shareable/> element inside all the char device schemas,
    to indicate that the source is a shared resource. When relabelling
    skip any devices marked sharable.

I think probably option 1 is the best bet, since it also gives us
the flexibility to control just what labels are used per device,
as well as the boolean relabel yes/no flag.

> ---
>  src/security/security_selinux.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index a7e2420..4214105 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1233,6 +1233,14 @@ virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
>          ret = virSecuritySELinuxSetFilecon(dev->data.file.path, secdef->imagelabel);
>          break;
>  
> +    case VIR_DOMAIN_CHR_TYPE_UNIX:
> +        if (!dev->data.nix.listen) {
> +            if (virSecuritySELinuxSetFilecon(dev->data.file.path, secdef->imagelabel) < 0)
> +                goto done;
> +        }
> +        ret = 0;
> +        break;
> +
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
>          if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) ||
>              (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) {
> @@ -1284,6 +1292,15 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def,
>              goto done;
>          ret = 0;
>          break;
> +
> +    case VIR_DOMAIN_CHR_TYPE_UNIX:
> +        if (!dev->data.nix.listen) {
> +            if (virSecuritySELinuxRestoreSecurityFileLabel(dev->data.file.path) < 0)
> +                goto done;
> +        }
> +        ret = 0;
> +        break;
> +
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
>          if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) ||
>              (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) {

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