Re: [libvirt] [PATCH v2 2/2] selinux: add security selinux function to label tapfd

BZ:https://bugzilla.redhat.com/show_bug.cgi?id=851981 When using macvtap, a character device gets first created by kernel with name /dev/tapN, its selinux context is: system_u:object_r:device_t:s0 Shortly, when udev gets notification when new file is created in /dev, it will then jump in and relabel this file back to the expected default context: system_u:object_r:tun_tap_device_t:s0 There is a time gap happened. Sometimes, it will have migration failed, AVC error message: type=AVC msg=audit(1349858424.233:42507): avc: denied { read write } for pid=19926 comm="qemu-kvm" path="/dev/tap33" dev=devtmpfs ino=131524 scontext=unconfined_u:system_r:svirt_t:s0:c598,c908 tcontext=system_u:object_r:device_t:s0 tclass=chr_file This patch will label the tapfd device before qemu process starts: system_u:object_r:tun_tap_device_t:MCS(MCS from seclabel->label) --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 4 ++ src/security/security_apparmor.c | 10 ++++ src/security/security_dac.c | 9 +++ src/security/security_driver.h | 4 ++ src/security/security_manager.c | 11 ++++ src/security/security_manager.h | 3 + src/security/security_nop.c | 3 +- src/security/security_selinux.c | 118 +++++++++++++++++++++++++++++++-------- src/security/security_stack.c | 18 ++++++ 10 files changed, 156 insertions(+), 25 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6ea1308..1703f6d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1056,6 +1056,7 @@ virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; +virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; virSecurityManagerVerify; virSecurityManagerGetMountOptions; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d590df6..239592c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5412,6 +5412,10 @@ qemuBuildCommandLine(virConnectPtr conn, if (tapfd < 0) goto error; + if (virSecurityManagerSetTapFDLabel(driver->securityManager, + def, tapfd) < 0) + goto error; + last_good_net = i; virCommandTransferFD(cmd, tapfd); diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index d3f9d9e..1315fe1 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -872,6 +872,15 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, fd_path, true); } +/* TODO need code here */ +static int +AppArmorSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED) +{ + return 0; +} + virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, .name = SECURITY_APPARMOR_NAME, @@ -908,4 +917,5 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel, .domainSetSecurityImageFDLabel = AppArmorSetImageFDLabel, + .domainSetSecurityTapFDLabel = AppArmorSetTapFDLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index f126aa5..9dbf95d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1029,6 +1029,14 @@ virSecurityDACSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } +static int +virSecurityDACSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED) +{ + return 0; +} + static char *virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr vm ATTRIBUTE_UNUSED) { return NULL; @@ -1070,6 +1078,7 @@ virSecurityDriver virSecurityDriverDAC = { .domainRestoreSavedStateLabel = virSecurityDACRestoreSavedStateLabel, .domainSetSecurityImageFDLabel = virSecurityDACSetImageFDLabel, + .domainSetSecurityTapFDLabel = virSecurityDACSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 8f52ec5..d49b401 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -95,6 +95,9 @@ typedef int (*virSecurityDomainSecurityVerify) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainSetImageFDLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, int fd); +typedef int (*virSecurityDomainSetTapFDLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + int fd); typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr, virDomainDefPtr def); @@ -134,6 +137,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; virSecurityDomainSetImageFDLabel domainSetSecurityImageFDLabel; + virSecurityDomainSetTapFDLabel domainSetSecurityTapFDLabel; virSecurityDomainGetMountOptions domainGetSecurityMountOptions; }; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 40c8b7e..d446607 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -469,6 +469,17 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, return -1; } +int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + int fd) +{ + if (mgr->drv->domainSetSecurityTapFDLabel) + return mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd); + + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index b3bc191..1fdaf8e 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -105,6 +105,9 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr, int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, int fd); +int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + int fd); char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm); virSecurityManagerPtr* diff --git a/src/security/security_nop.c b/src/security/security_nop.c index b56971c..86f644b 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -204,7 +204,8 @@ virSecurityDriver virSecurityDriverNop = { .domainSetSavedStateLabel = virSecurityDomainSetSavedStateLabelNop, .domainRestoreSavedStateLabel = virSecurityDomainRestoreSavedStateLabelNop, - .domainSetSecurityImageFDLabel = virSecurityDomainSetFDLabelNop, + .domainSetSecurityImageFDLabel = virSecurityDomainSetFDLabelNop, + .domainSetSecurityTapFDLabel = virSecurityDomainSetFDLabelNop, .domainGetSecurityMountOptions = virSecurityDomainGetMountOptionsNop, }; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 10135ed..0a2a9fe 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -237,6 +237,46 @@ cleanup: return mcs; } +static char * +virSecuritySELinuxContextAddRange(security_context_t src, + security_context_t dst) +{ + char *str = NULL; + char *ret = NULL; + context_t srccon = NULL; + context_t dstcon = NULL; + + if (!src || !dst) + return ret; + + if (!(srccon = context_new(src)) || !(dstcon = context_new(dst))) { + virReportSystemError(errno, "%s", + _("unable to allocate security context")); + goto cleanup; + } + + if (context_range_set(dstcon, context_range_get(srccon)) == -1) { + virReportSystemError(errno, + _("unable to set security context range '%s'"), dst); + goto cleanup; + } + + if (!(str = context_str(dstcon))) { + virReportSystemError(errno, "%s", + _("Unable to format SELinux context")); + goto cleanup; + } + + if (!(ret = strdup(str))) { + virReportOOMError(); + goto cleanup; + } + +cleanup: + if (srccon) context_free(srccon); + if (dstcon) context_free(dstcon); + return ret; +} static char * virSecuritySELinuxGenNewContext(const char *basecontext, @@ -1597,6 +1637,7 @@ virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, context_t execcon = NULL; context_t proccon = NULL; security_context_t scon = NULL; + char *str = NULL; int rc = -1; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); @@ -1615,13 +1656,6 @@ virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, goto done; } - if ( !(execcon = context_new(secdef->label)) ) { - virReportSystemError(errno, - _("unable to allocate socket security context '%s'"), - secdef->label); - goto done; - } - if (getcon_raw(&scon) == -1) { virReportSystemError(errno, _("unable to get current process context '%s'"), @@ -1629,26 +1663,13 @@ virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, goto done; } - if ( !(proccon = context_new(scon)) ) { - virReportSystemError(errno, - _("unable to set socket security context '%s'"), - secdef->label); - goto done; - } - - if (context_range_set(proccon, context_range_get(execcon)) == -1) { - virReportSystemError(errno, - _("unable to set socket security context range '%s'"), - secdef->label); + if (!(str = virSecuritySELinuxContextAddRange(secdef->label, scon))) goto done; - } - VIR_DEBUG("Setting VM %s socket context %s", - def->name, context_str(proccon)); - if (setsockcreatecon_raw(context_str(proccon)) == -1) { + VIR_DEBUG("Setting VM %s socket context %s", def->name, str); + if (setsockcreatecon_raw(str) == -1) { virReportSystemError(errno, - _("unable to set socket security context '%s'"), - context_str(proccon)); + _("unable to set socket security context '%s'"), str); goto done; } @@ -1660,6 +1681,7 @@ done: if (execcon) context_free(execcon); if (proccon) context_free(proccon); freecon(scon); + VIR_FREE(str); return rc; } @@ -1869,6 +1891,53 @@ virSecuritySELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel); } +static int +virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + 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 == NULL) + return rc; + + if (secdef->label == NULL) + return 0; + + 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("/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 * virSecuritySELinuxGenImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) @@ -1969,6 +2038,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainRestoreSavedStateLabel = virSecuritySELinuxRestoreSavedStateLabel, .domainSetSecurityImageFDLabel = virSecuritySELinuxSetImageFDLabel, + .domainSetSecurityTapFDLabel = virSecuritySELinuxSetTapFDLabel, .domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 667448f..24de6f2 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -445,6 +445,23 @@ virSecurityStackSetImageFDLabel(virSecurityManagerPtr mgr, return rc; } +static int +virSecurityStackSetTapFDLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + int fd) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetTapFDLabel(item->securityManager, vm, fd) < 0) + rc = -1; + } + + return rc; +} + static char *virSecurityStackGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr vm ATTRIBUTE_UNUSED) { return NULL; @@ -509,6 +526,7 @@ virSecurityDriver virSecurityDriverStack = { .domainRestoreSavedStateLabel = virSecurityStackRestoreSavedStateLabel, .domainSetSecurityImageFDLabel = virSecurityStackSetImageFDLabel, + .domainSetSecurityTapFDLabel = virSecurityStackSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityStackGetMountOptions, };