[libvirt] [PATCH] qemuProcessStart: Be tolerant to relabel errors for session mode

Daniel P. Berrange berrange at redhat.com
Mon Jul 20 13:50:26 UTC 2015


On Wed, Jul 15, 2015 at 03:02:13PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1124841
> 
> When the daemon is running under unprivileged user, that is under
> qemu:///session, there are plenty of operations we can't do. What
> we can do is to go with best effort. One of such cases is
> relabeling domain resources (be it disks, sockets, regular files,
> etc.) during domain startup process. While we may successfully set
> DAC labels, we can be fairly certain that any attempt to change
> SELinux labels will fail. Therefore we should tolerate relabelling
> errors and just let qemu to try access the resources. If it fails,
> our error reporting system is strong enough to articulate the
> exact error to the user anyway.

Errr, isn't it entirely the opposite to what you say. Running as
an unprivileged user ID has no bearing on whether you are allowed
to set SELinux labels. If the user acount is unconfined_t it can
set any SELinux labels it wants. It will only fail if the libvird
process is confined in some way.  IMHO we shold not be ignoring
such failures.

What *will* fail is any attempt to set DAC labels, since you need
CAP_CHOWN capability, but we shouldn't have the DAC security
maanger running when in session mode.

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1c0c734..58ed631 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4856,8 +4856,13 @@ int qemuProcessStart(virConnectPtr conn,
>  
>      VIR_DEBUG("Setting domain security labels");
>      if (virSecurityManagerSetAllLabel(driver->securityManager,
> -                                      vm->def, stdin_path) < 0)
> -        goto cleanup;
> +                                      vm->def, stdin_path) < 0) {
> +        /* Be tolerant to relabel errors if we are running unprivileged. */
> +        if (virQEMUDriverIsPrivileged(driver))
> +            goto cleanup;
> +        else
> +            VIR_DEBUG("Ignoring relabel errors for unprivileged daemon");
> +    }

I really don't think we should do this here as it affects all
security managers.

What is the failure you are actually seeing without this ?  SElinux
label changes should be succeeding in session mode and we should not
even be applying DAC labels

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




More information about the libvir-list mailing list