[libvirt PATCHv4 15/15] wip: SELinux integration for virtiofsd

Ján Tomko jtomko at redhat.com
Thu Feb 20 14:32:52 UTC 2020


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




More information about the libvir-list mailing list