[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior



https://bugzilla.redhat.com/show_bug.cgi?id=1141879

A long time ago I've implemented support for so called multiqueue
net.  The idea was to let guest network traffic be processed by
multiple host CPUs and thus increasing performance. However, this
behavior is enabled by QEMU via special ioctl() iterated over the
all tap FDs passed in by libvirt. Unfortunately, SELinux comes in
and disallows the ioctl() call because the /dev/net/tun has label
system_u:object_r:tun_tap_device_t:s0 and 'attach_queue' ioctl()
is not allowed on tun_tap_device_t type. So after discussion with
a SELinux developer we've decided that the FDs passed to the QEMU
should be labelled with svirt_t type and SELinux policy will
allow the ioctl(). Therefore I've made a patch
(cf976d9dcf4e592261b14f03572) that does exactly this. However,
things are not that easy - even though the API to label FD is
called (fsetfilecon_raw) the underlying file is labelled too! So
effectively we are mangling /dev/net/tun label. Yes, that broke
dozen of other application from openvpn, or boxes, to qemu
running other domains.

The best solution would be if SELinux provides a way to label an
FD only, which could be then labeled when passed to the qemu.
However that's a long path to go and we should fix this
regression AQAP. So I went to talk to the SELinux developer again
and we agreed on temporary solution that:

1) my patch is reverted
2) SELinux temporarily allows 'attach_queue' on the
tun_tap_device_t

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
 src/security/security_selinux.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 3db2b27..c69eeb9 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2343,17 +2343,47 @@ virSecuritySELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 }
 
 static int
-virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
                                 virDomainDefPtr def,
                                 int fd)
 {
+    struct stat buf;
+    security_context_t fcon = NULL;
     virSecurityLabelDefPtr secdef;
+    char *str = NULL;
+    int rc = -1;
 
     secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-    if (!secdef || !secdef->imagelabel)
+    if (!secdef || !secdef->label)
         return 0;
 
-    return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel);
+    if (fstat(fd, &buf) < 0) {
+        virReportSystemError(errno, _("cannot stat tap fd %d"), fd);
+        goto cleanup;
+    }
+
+    if ((buf.st_mode & S_IFMT) != S_IFCHR) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("tap fd %d is not character device"), fd);
+        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);
+        goto cleanup;
+    }
+
+    if (!(str = virSecuritySELinuxContextAddRange(secdef->label, fcon))) {
+        goto cleanup;
+    } else {
+        rc = virSecuritySELinuxFSetFilecon(fd, str);
+    }
+
+ cleanup:
+    freecon(fcon);
+    VIR_FREE(str);
+    return rc;
 }
 
 static char *
-- 
1.8.5.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]