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

[Martin Kletzander]

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 at redhat.com>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> 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.

For the purpose of this release, let's say I noticed this patch right
after the release and hence didn't have time to NACK it properly on
time.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170113/45bed3c5/attachment-0001.sig>