Re: [libvirt] [PATCH] qemuProcessStart: Be tolerant to relabel errors for session mode
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 :|