security: make it possible to set SELinux label of child process from its binary

Normally when a child process is started by libvirt, the SELinux label
of that process is set to virtd_t (plus an MCS range). In at least one
case (passt) we need for the SELinux label of a child process label to
match the label that the binary would have transitioned to
automatically if it had been run standalone (in the case of passt,
that label is passt_t).

This patch modifies virSecuritySELinuxSetChildProcessLabel() (and all
the functions above it in the call chain) so that the toplevel
function can set a new argument "useBinarySpecificLabel" to true. If
it is true, then virSecuritySELinuxSetChildProcessLabel() will call
the new function virSecuritySELinuxContextSetFromFile(), which uses
the selinux library function security_compute_create() to determine
what would be the label of the new process if it had been run
standalone (rather than being run by libvirt) - the MCS range from the
normally-used label is added to this newly derived label, and that is
what is used for the new process rather than whatever is in the
domain's security label (which will usually be virtd_t).

In order to easily verify that nothing was broken by these changes to
the call chain, all callers currently set useBinarySpecificPath =
false, so all behavior should be completely unchanged. (The next
patch will set it to true only for the case of running passt.)

https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index a6dc802637bd1410f355151f5c6ea76a08b7f597..2e4067e704d6031f4b825a97f7ad1b4c7430b807 100644 (file)
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -217,7 +217,7 @@ qemuDBusStart(virQEMUDriver *driver,
     virCommandDaemonize(cmd);
     virCommandAddArgFormat(cmd, "--config-file=%s", configfile);
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
         goto cleanup;
 
     if (virPidFileReadPath(pidfile, &cpid) < 0) {

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 0afa8bdb3addefe4975fe28acc1d6ae5958e8b2f..fd0076077e852d3f570c2077421958d84b6b1825 100644 (file)
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -281,7 +281,7 @@ qemuPasstStart(virDomainObj *vm,
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
         return -1;
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
         goto error;
 
     return 0;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index deebd03717855adb6ef049b66b530bf0307cd577..be418ad8e65834dc9037085528a9e072536d8dbe 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7747,7 +7747,7 @@ qemuProcessLaunch(virConnectPtr conn,
 
     VIR_DEBUG("Setting up security labelling");
     if (qemuSecuritySetChildProcessLabel(driver->securityManager,
-                                         vm->def, cmd) < 0)
+                                         vm->def, false, cmd) < 0)
         goto cleanup;
 
     virCommandSetOutputFD(cmd, &logfile);

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index ee03e2225e495aaf4825e5c70c44af2b9c19a684..8bcef14d089d11e686e2e26b0eb2082f484100b8 100644 (file)
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -636,6 +636,7 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
                        virCommand *cmd,
                        uid_t uid,
                        gid_t gid,
+                       bool useBinarySpecificLabel,
                        int *exitstatus)
 {
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
@@ -643,8 +644,10 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
     int ret = -1;
 
     if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
-                                               vm->def, cmd) < 0)
+                                               vm->def, useBinarySpecificLabel,
+                                               cmd) < 0) {
         return -1;
+    }
 
     if (uid != (uid_t) -1)
         virCommandSetUID(cmd, uid);

diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index dc8e67cc81c805a11d9d2d3e2e0894503e825a09..10f11771b495933a81337fe6be3916c88c775c32 100644 (file)
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -115,6 +115,7 @@ int qemuSecurityCommandRun(virQEMUDriver *driver,
                            virCommand *cmd,
                            uid_t uid,
                            gid_t gid,
+                           bool useBinarySpecificLabel,
                            int *exitstatus);
 
 /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add

diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index 9697542cd3d83c8bb0b05cb4e374dd18efa34412..fdf0823d03be08a40f4275d26ff39a34083eac9e 100644 (file)
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -325,7 +325,7 @@ qemuSlirpStart(virDomainObj *vm,
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)
         goto error;
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
         goto error;
 
     rc = virPidFileReadPath(pidfile, &pid);

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index f4344de6638e89060f5ece0adb5bee55e414f886..788e81a655c4af1921df6e4eb8f3702221c9a227 100644 (file)
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -963,8 +963,9 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
         return -1;
 
     if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user,
-                               cfg->swtpm_group, NULL) < 0)
+                               cfg->swtpm_group, false, NULL) < 0) {
         goto error;
+    }
 
     if (virPidFileReadPath(pidfile, &pid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index 5b49ef4e28c7722a3b0ca657dff028755e72851d..ced41b04666840680aa9f477b6b9df10fa9f5042 100644 (file)
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -152,7 +152,7 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
             virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode);
     }
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
         goto error;
 
     rc = virPidFileReadPath(pidfile, &pid);

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index b63b2489752013b1d126fcd7488cb8743dd7d043..b5642c9a28f17214982ba8fa1e79fe1de6d6e710 100644 (file)
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -570,6 +570,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
 static int
 AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
                                      virDomainDef *def,
+                                     bool useBinarySpecificLabel G_GNUC_UNUSED,
                                      virCommand *cmd)
 {
     g_autofree char *profile_name = NULL;

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 9be8f458d1da16eae556b8129f262ba0e57f2650..ca3f4d2dc5b8e62f2f531fe2697b20608dcb7249 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -2273,6 +2273,7 @@ virSecurityDACSetProcessLabel(virSecurityManager *mgr,
 static int
 virSecurityDACSetChildProcessLabel(virSecurityManager *mgr,
                                    virDomainDef *def,
+                                   bool useBinarySpecificLabel G_GNUC_UNUSED,
                                    virCommand *cmd)
 {
     virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr);

diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index fe6982cecabbd4a8aaecbda02c40d9f83315e7da..aa1fb2125dd404650d0c09aa71464cf5c8061b59 100644 (file)
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -96,6 +96,7 @@ typedef int (*virSecurityDomainSetProcessLabel) (virSecurityManager *mgr,
                                                  virDomainDef *def);
 typedef int (*virSecurityDomainSetChildProcessLabel) (virSecurityManager *mgr,
                                                       virDomainDef *def,
+                                                      bool useBinarySpecificLabel,
                                                       virCommand *cmd);
 typedef int (*virSecurityDomainSecurityVerify) (virSecurityManager *mgr,
                                                 virDomainDef *def);

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 2f8e89cb04a3e9e21195fb21c9e7d0b902a5aebe..b0578d72097ceba1f4a50bfbc9b7fb2460827ac8 100644 (file)
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -885,10 +885,14 @@ virSecurityManagerSetProcessLabel(virSecurityManager *mgr,
 int
 virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr,
                                        virDomainDef *vm,
+                                       bool useBinarySpecificLabel,
                                        virCommand *cmd)
 {
-    if (mgr->drv->domainSetSecurityChildProcessLabel)
-       return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm, cmd);
+    if (mgr->drv->domainSetSecurityChildProcessLabel) {
+       return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm,
+                                                           useBinarySpecificLabel,
+                                                           cmd);
+    }
 
     virReportUnsupportedError();
     return -1;

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 4afdcc167b6728bfbd6418bddeb599af3d31a21d..97add3294d58fa2c80b112f00ee76bf503f6a43d 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -145,6 +145,7 @@ int virSecurityManagerSetProcessLabel(virSecurityManager *mgr,
                                       virDomainDef *def);
 int virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr,
                                            virDomainDef *def,
+                                           bool useBinarySpecificLabel,
                                            virCommand *cmd);
 int virSecurityManagerVerify(virSecurityManager *mgr,
                              virDomainDef *def);

diff --git a/src/security/security_nop.c b/src/security/security_nop.c
index 0dbc547feb511836006f45bc82b2b964d817e0da..1413f43d575175cc48bb33402d44d13a867cb2e5 100644 (file)
--- a/src/security/security_nop.c
+++ b/src/security/security_nop.c
@@ -152,6 +152,7 @@ virSecurityDomainSetProcessLabelNop(virSecurityManager *mgr G_GNUC_UNUSED,
 static int
 virSecurityDomainSetChildProcessLabelNop(virSecurityManager *mgr G_GNUC_UNUSED,
                                          virDomainDef *vm G_GNUC_UNUSED,
+                                         bool useBinarySpecificLabel G_GNUC_UNUSED,
                                          virCommand *cmd G_GNUC_UNUSED)
 {
     return 0;

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index cd1d9d14f715fe57c26e32a6a695c253050d78bd..7f409af525bf73448750f3a3ef4aeea3adad4187 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -560,6 +560,52 @@ virSecuritySELinuxContextAddRange(const char *src,
     return ret;
 }
 
+
+static char *
+virSecuritySELinuxContextSetFromFile(const char *origLabel,
+                                     const char *binaryPath)
+{
+    g_autofree char *currentCon = NULL;
+    g_autofree char *binaryCon = NULL;
+    g_autofree char *naturalLabel = NULL;
+    g_autofree char *updatedLabel = NULL;
+
+    /* First learn what would be the context set
+     * if binaryPath was exec'ed from this process.
+     */
+    if (getcon(&currentCon) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("unable to get SELinux context for current process"));
+        return NULL;
+    }
+
+    if (getfilecon(binaryPath, &binaryCon) < 0) {
+        virReportSystemError(errno, _("unable to get SELinux context for '%s'"),
+                             binaryPath);
+        return NULL;
+    }
+
+    if (security_compute_create(currentCon, binaryCon,
+                                string_to_security_class("process"),
+                                &naturalLabel) < 0) {
+        virReportSystemError(errno,
+                             _("unable create new SELinux label based on label '%s' and file '%s'"),
+                             origLabel, binaryPath);
+        return NULL;
+    }
+
+    /* now get the type from the original label
+     * (which already has proper MCS set) and add it to
+     * the new label
+     */
+    updatedLabel = virSecuritySELinuxContextAddRange(origLabel, naturalLabel);
+
+    VIR_DEBUG("original label: '%s' binary: '%s' binary-specific label: '%s'",
+              origLabel, binaryPath, NULLSTR(updatedLabel));
+    return g_steal_pointer(&updatedLabel);
+}
+
+
 static char *
 virSecuritySELinuxGenNewContext(const char *basecontext,
                                 const char *mcs,
@@ -2986,10 +3032,13 @@ virSecuritySELinuxSetProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
 static int
 virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
                                        virDomainDef *def,
+                                       bool useBinarySpecificLabel G_GNUC_UNUSED,
                                        virCommand *cmd)
 {
     /* TODO: verify DOI */
     virSecurityLabelDef *secdef;
+    g_autofree char *tmpLabel = NULL;
+    const char *label = NULL;
 
     secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
     if (!secdef || !secdef->label)
@@ -3006,8 +3055,30 @@ virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
             return -1;
     }
 
+    /* pick either the common label used by most binaries exec'ed by
+     * libvirt, or the specific label of this binary.
+     */
+    if (useBinarySpecificLabel) {
+        const char *binaryPath = virCommandGetBinaryPath(cmd);
+
+        if (!binaryPath)
+            return -1; /* error was already logged */
+
+        tmpLabel = virSecuritySELinuxContextSetFromFile(secdef->label,
+                                                        binaryPath);
+        if (!tmpLabel)
+            return -1;
+
+        label = tmpLabel;
+
+    } else {
+
+        label = secdef->label;
+
+    }
+
     /* save in cmd to be set after fork/before child process is exec'ed */
-    virCommandSetSELinuxLabel(cmd, secdef->label);
+    virCommandSetSELinuxLabel(cmd, label);
     return 0;
 }
 

diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 560f797030682470d91334ee40852cb289f8a4a1..369b5dd3a6d3c043e85e663b9a068d6bab4d27c3 100644 (file)
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -458,6 +458,7 @@ virSecurityStackSetProcessLabel(virSecurityManager *mgr,
 static int
 virSecurityStackSetChildProcessLabel(virSecurityManager *mgr,
                                      virDomainDef *vm,
+                                     bool useBinarySpecificLabel,
                                      virCommand *cmd)
 {
     virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr);
@@ -465,8 +466,10 @@ virSecurityStackSetChildProcessLabel(virSecurityManager *mgr,
     int rc = 0;
 
     for (; item; item = item->next) {
-        if (virSecurityManagerSetChildProcessLabel(item->securityManager, vm, cmd) < 0)
+        if (virSecurityManagerSetChildProcessLabel(item->securityManager, vm,
+                                                   useBinarySpecificLabel, cmd) < 0) {
             rc = -1;
+        }
     }
 
     return rc;