[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior
On 09/18/2014 10:20 AM, Michal Privoznik wrote: > 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 ACK - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior
On 09/18/2014 10:20 AM, Michal Privoznik wrote: > 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 > --- > src/security/security_selinux.c | 36 +--- > 1 file changed, 33 insertions(+), 3 deletions(-) > Probably should note that this also reverts a4431931393aeb1ac5893f121151fa3df4fde612 (in 1.2.8) and b635b7a1af0e64754016d758376f382470bc11e7 (post 1.2.8) Although neither really matters with this change - it just points out the trail for the "secdef->imagelabel -> secdef->label" change that isn't present in cf976d... ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSecuritySELinuxSetTapFDLabel: Temporarily revert to old behavior
On 18.09.2014 18:36, John Ferlan wrote: On 09/18/2014 10:20 AM, Michal Privoznik wrote: 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 --- src/security/security_selinux.c | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) Probably should note that this also reverts a4431931393aeb1ac5893f121151fa3df4fde612 (in 1.2.8) and b635b7a1af0e64754016d758376f382470bc11e7 (post 1.2.8) Although neither really matters with this change - it just points out the trail for the "secdef->imagelabel -> secdef->label" change that isn't present in cf976d... ACK John Okay, I've fixed the commit message and pushed. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list