This patch attempts to run virtiofsd with libvirtd's label and the category of the currently ran domain, which obviously does nothing without a corresponding policy.
Per: https://www.redhat.com/archives/libvir-list/2020-February/msg00108.html a new type might be needed. TODO: file a SELinux policy bug --- src/libvirt_private.syms | 1 + src/qemu/qemu_security.c | 40 +++++++++++++++++++ src/qemu/qemu_security.h | 7 ++++ src/qemu/qemu_virtiofs.c | 4 +- src/security/security_dac.c | 20 ++++++++++ src/security/security_driver.h | 2 + src/security/security_manager.c | 12 ++++++ src/security/security_manager.h | 4 ++ src/security/security_nop.c | 1 + src/security/security_selinux.c | 69 +++++++++++++++++++++++++++++++++ src/security/security_stack.c | 19 +++++++++ 11 files changed, 178 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 49eb8ab7cb..aaba15297a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1504,6 +1504,7 @@ virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; virSecurityManagerSetTPMLabels; +virSecurityManagerSetVirtioFSProcessLabel; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 2aa2b5b9c6..834556c910 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -475,6 +475,46 @@ qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver, } +/* + * qemuSecurityStartVhostUserGPU: + * + * @driver: the QEMU driver + * @vm: the domain object + * @cmd: the command to run + * @existstatus: pointer to int returning exit status of process + * @cmdret: pointer to int returning result of virCommandRun + * + * Start the vhost-user-gpu process with approriate labels. + * This function returns -1 on security setup error, 0 if all the + * setup was done properly. In case the virCommand failed to run + * 0 is returned but cmdret is set appropriately with the process + * exitstatus also set. + */ +int +qemuSecurityStartVirtioFS(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret) +{ + if (virSecurityManagerSetVirtioFSProcessLabel(driver->securityManager, + vm->def, cmd) < 0) + return -1; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + return -1; + + *cmdret = virCommandRun(cmd, exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (*cmdret < 0) + return -1; + + return 0; +} + + /* * qemuSecurityStartTPMEmulator: * diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index a8c648ece1..dbb3bdbc9b 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -84,6 +84,13 @@ int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver, int *exitstatus, int *cmdret); +int qemuSecurityStartVirtioFS(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret); + + int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, virCommandPtr cmd, diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index b53e5b0806..17afe27535 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -34,6 +34,7 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +VIR_LOG_INIT("qemu.virtiofs"); char * qemuVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg, @@ -227,7 +228,8 @@ qemuVirtioFSStart(virLogManagerPtr logManager, if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0) goto cleanup; - rc = virCommandRun(cmd, NULL); + if (qemuSecurityStartVirtioFS(driver, vm, cmd, NULL, &rc) < 0) + goto error; if (rc < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/security/security_dac.c b/src/security/security_dac.c index d75b18170b..64e00b0127 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -2263,6 +2263,24 @@ virSecurityDACSetChildProcessLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL) < 0) return -1; + VIR_DEBUG("Setting child to drop privileges to %u:%u", + (unsigned int)user, (unsigned int)group); + + virCommandSetUID(cmd, user); + virCommandSetGID(cmd, group); + return 0; +} + + +static int +virSecurityDACSetVirtioFSProcessLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED, + virDomainDefPtr def G_GNUC_UNUSED, + virCommandPtr cmd) +{ + /* only running as root:root is supported */ + uid_t user = 0; + gid_t group = 0; + VIR_DEBUG("Setting child to drop privileges to %u:%u", (unsigned int)user, (unsigned int)group); @@ -2581,4 +2599,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityChardevLabel = virSecurityDACSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecurityDACRestoreChardevLabel, + + .domainSetSecurityVirtioFSProcessLabel = virSecurityDACSetVirtioFSProcessLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 3353955813..d145922198 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -217,6 +217,8 @@ struct _virSecurityDriver { virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels; virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels; + + virSecurityDomainSetChildProcessLabel domainSetSecurityVirtioFSProcessLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index fe9def7fb9..13e6380ed6 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -945,6 +945,18 @@ virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr, } +int virSecurityManagerSetVirtioFSProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virCommandPtr cmd) +{ + if (mgr->drv->domainSetSecurityVirtioFSProcessLabel) + return mgr->drv->domainSetSecurityVirtioFSProcessLabel(mgr, def, cmd); + + virReportUnsupportedError(); + return -1; +} + + int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) diff --git a/src/security/security_manager.h b/src/security/security_manager.h index f835356b7e..0465f99203 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -137,6 +137,10 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, int virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virCommandPtr cmd); +int virSecurityManagerSetVirtioFSProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virCommandPtr cmd); + int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def); int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index c1856eb421..f783bfa7ee 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -328,4 +328,5 @@ virSecurityDriver virSecurityDriverNop = { .domainSetSecurityChardevLabel = virSecurityDomainSetChardevLabelNop, .domainRestoreSecurityChardevLabel = virSecurityDomainRestoreChardevLabelNop, + .domainSetSecurityVirtioFSProcessLabel = virSecurityDomainSetChildProcessLabelNop, }; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 3f6968a57a..853f5beb67 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2941,6 +2941,73 @@ virSecuritySELinuxSetChildProcessLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED, return 0; } +static int +virSecuritySELinuxSetVirtioFSProcessLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED, + virDomainDefPtr def, + virCommandPtr cmd) +{ + virSecurityLabelDefPtr secdef; + security_context_t daemon_ctx; + g_auto(GStrv) daemon_label = NULL; + g_auto(GStrv) domain_label = NULL; + g_autofree char *frankenlabel = NULL; + + VIR_ERROR("WE got here"); + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!secdef || !secdef->label) + return 0; + + if (getcon_raw(&daemon_ctx) == -1) { + virReportSystemError(errno, "%s", + _("unable to get security context")); + return -1; + } + + VIR_ERROR("domain_label=%s daemon_label=%s", secdef->label, (char *)daemon_ctx); + + daemon_label = g_strsplit((char *)daemon_ctx, ":", 6); + domain_label = g_strsplit((char *)secdef->label, ":", 6); + + freecon(daemon_ctx); + + if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("security label driver mismatch: " + "'%s' model configured for domain, but " + "hypervisor driver is '%s'."), + secdef->model, SECURITY_SELINUX_NAME); + if (security_getenforce() == 1) + return -1; + } + + if (!domain_label || !daemon_label || + g_strv_length(domain_label) != 5 || g_strv_length(daemon_label) != 5) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed security label")); + return -1; + } + + frankenlabel = g_strdup_printf("%s:%s:%s:%s:%s", + daemon_label[0], + daemon_label[1], + daemon_label[2], + domain_label[3], + domain_label[4]); + + VIR_ERROR("frankenlabel=%s", frankenlabel); + + if (strlen(frankenlabel) >= VIR_SECURITY_LABEL_BUFLEN) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("security label exceeds maximum length: %d"), + VIR_SECURITY_LABEL_BUFLEN - 1); + return -1; + } + + virCommandSetSELinuxLabel(cmd, frankenlabel); + return 0; +} + static int virSecuritySELinuxSetDaemonSocketLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED, virDomainDefPtr def) @@ -3576,4 +3643,6 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityTPMLabels = virSecuritySELinuxSetTPMLabels, .domainRestoreSecurityTPMLabels = virSecuritySELinuxRestoreTPMLabels, + + .domainSetSecurityVirtioFSProcessLabel = virSecuritySELinuxSetVirtioFSProcessLabel, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 073876daff..28982b4ba1 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -476,6 +476,23 @@ virSecurityStackSetChildProcessLabel(virSecurityManagerPtr mgr, return rc; } +static int +virSecurityStackSetVirtioFSProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virCommandPtr cmd) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetVirtioFSProcessLabel(item->securityManager, vm, cmd) < 0) + rc = -1; + } + + return rc; +} + static int virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, @@ -991,4 +1008,6 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels, .domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels, + + .domainSetSecurityVirtioFSProcessLabel = virSecurityStackSetVirtioFSProcessLabel, }; -- 2.24.1
