Re: [libvirt] [PATCH] qemu: Copy SELinux labels for namespace too

On Fri, Jan 13, 2017 at 11:12:43AM +0100, Michal Privoznik wrote: > When creating new /dev/* for qemu, we do chown() and copy ACLs to > create the exact copy from the original /dev. I though that > copying SELinux labels is not necessary as SELinux will chose the > sane defaults. Surprisingly, it does not leaving namespace with > the following labels: > > crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 random > crw-------. root root system_u:object_r:tmpfs_t:s0 rtc0 > drwxrwxrwt. root root system_u:object_r:tmpfs_t:s0 shm > crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 urandom > > As a result, domain is unable to start: > > error: internal error: process exited while connecting to monitor: > Error in GnuTLS initialization: Failed to acquire random data. > qemu-kvm: cannot initialize crypto: Unable to initialize GNUTLS > library: Failed to acquire random data. > > The solution is to copy the SELinux labels as well. > > Reported-by: Andrea Bolognani <abologna(a)redhat.com&gt; > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 61 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 1399dee0d..a29866673 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -63,6 +63,9 @@ > #if defined(HAVE_SYS_MOUNT_H) > # include <sys/mount.h> > #endif > +#ifdef WITH_SELINUX > +# include <selinux/selinux.h> > +#endif > > #include <libxml/xpathInternals.h> > > @@ -6958,6 +6961,9 @@ qemuDomainCreateDevice(const char *device, > char *canonDevicePath = NULL; > struct stat sb; > int ret = -1; > +#ifdef WITH_SELINUX > + char *tcon = NULL; > +#endif > > if (virFileResolveAllLinks(device, &canonDevicePath) < 0) { > if (errno == ENOENT && allow_noent) { > @@ -7023,10 +7029,34 @@ qemuDomainCreateDevice(const char *device, > goto cleanup; > } > > +#ifdef WITH_SELINUX > + if (getfilecon_raw(canonDevicePath, &tcon) < 0 && > + (errno != ENOTSUP && errno != ENODATA)) { > + virReportSystemError(errno, > + _("Unable to get SELinux label on %s"), > canonDevicePath); > + goto cleanup; > + } > + > + if (tcon && > + setfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *) > tcon) < 0) { > + VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR > + if (errno != EOPNOTSUPP && errno != ENOTSUP) { > + VIR_WARNINGS_RESET > + virReportSystemError(errno, > + _("Unable to set SELinux label on %s"), > + devicePath); > + goto cleanup; > + } > + } > +#endif > + I think, instead of all the ifdefs, this should be a security driver API instead of being hardcoded in places. That way it will be processed properly by all the security drivers.