[libvirt] [PATCH] security_selinux: Don't relabel /dev/net/tun

[Richard W.M. Jones]

On Tue, Oct 07, 2014 at 04:53:24PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1147057
> 
> The code for relabelling the TAP FD is there due to a race. When
> libvirt creates a /dev/tapN device it's labeled as
> 'system_u:object_r:device_t:s0' by default. Later, when
> udev/systemd reacts to this device, it's relabelled to the
> expected label 'system_u:object_r:tun_tap_device_t:s0'. Hence, we
> have a code that relabels the device, to cut the race down. For
> more info see ae368ebfcc4.
> 
> But the problem is, the relabel function is called on all TUN/TAP
> devices. Yes, on /dev/net/tun too. This is however a special kind
> of device - other processes uses it too. We shouldn't touch it's
> label then.
> 
> Ideally, there would an API in SELinux that would label just the
> passed FD and not the underlying path. That way, we wouldn't need
> to care as we would be not labeling /dev/net/tun but the FD
> passed to the domain. Unfortunately, there's no such API so we
> have to workaround until then.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/security/security_selinux.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index b7c1015..25e8320 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2352,7 +2352,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
>      struct stat buf;
>      security_context_t fcon = NULL;
>      virSecurityLabelDefPtr secdef;
> -    char *str = NULL;
> +    char *str = NULL, *proc = NULL, *fd_path = NULL;
>      int rc = -1;
>  
>      secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> @@ -2370,6 +2370,23 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
>          goto cleanup;
>      }
>  
> +    /* Label /dev/tap.* devices only. Leave /dev/net/tun alone! */
> +    if (virAsprintf(&proc, "/proc/self/fd/%d", fd) == -1)
> +        goto cleanup;
> +
> +    if (virFileResolveLink(proc, &fd_path) < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to resolve link: %s"), proc);
> +        goto cleanup;
> +    }
> +
> +    if (!STRPREFIX(fd_path, "/dev/tap")) {
> +        VIR_DEBUG("fd=%d points to %s not setting SELinux label",
> +                  fd, fd_path);
> +        rc = 0;
> +        goto cleanup;
> +    }
> +
>      if (getContext(mgr, "/dev/tap.*", buf.st_mode, &fcon) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("cannot lookup default selinux label for tap fd %d"), fd);
> @@ -2384,6 +2401,8 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
>  
>   cleanup:
>      freecon(fcon);
> +    VIR_FREE(fd_path);
> +    VIR_FREE(proc);
>      VIR_FREE(str);
>      return rc;
>  }

I applied this patch (without any of Eric's suggested changes) to
libvirt on Rawhide to see if it would fix the relabelling problems
that are stopping libguestfs networking from working.

It does indeed appear to fix them.  Hence you can add:

  Tested-by: Richard W.M. Jones <rjones at redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top