From 75c9ad56f08bfa0d86737f8872ea7cf7a5426bad Mon Sep 17 00:00:00 2001

Message-Id: <75c9ad56f08bfa0d86737f8872ea7cf7a5426bad@dist-git>

From: Laine Stump <laine@redhat.com>

Date: Wed, 1 Mar 2023 15:34:32 -0500

Subject: [PATCH] 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>

(cherry picked from commit 75056f61f12d6efec51f699f2b901f8d02cd075c)

Conflicts:

	src/qemu/qemu_dbus.c

	src/qemu/qemu_passt.c

	src/qemu/qemu_security.c

	src/qemu/qemu_security.h

	src/qemu/qemu_slirp.c

	src/qemu/qemu_tpm.c

	src/qemu/qemu_vhost_user_gpu.c

  The argument list for qemuSecurityCommandRun changed upstream to

  remove one of the arguments, but that changeset has not been

  backported to the rhel-9.2.0 branch. (see the 4 commits starting at

  upstream commit 0634d640)

https://bugzilla.redhat.com/2172267

Signed-off-by: Laine Stump <laine@redhat.com>

---

 src/qemu/qemu_dbus.c             |  5 ++-

 src/qemu/qemu_passt.c            |  4 +-

 src/qemu/qemu_process.c          |  2 +-

 src/qemu/qemu_security.c         |  5 ++-

 src/qemu/qemu_security.h         |  1 +

 src/qemu/qemu_slirp.c            |  4 +-

 src/qemu/qemu_tpm.c              |  3 +-

 src/qemu/qemu_vhost_user_gpu.c   |  4 +-

 src/security/security_apparmor.c |  1 +

 src/security/security_dac.c      |  1 +

 src/security/security_driver.h   |  1 +

 src/security/security_manager.c  |  8 +++-

 src/security/security_manager.h  |  1 +

 src/security/security_nop.c      |  1 +

 src/security/security_selinux.c  | 73 +++++++++++++++++++++++++++++++-

 src/security/security_stack.c    |  5 ++-

 16 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c

index cb2694795e..f13c792956 100644

--- a/src/qemu/qemu_dbus.c

+++ b/src/qemu/qemu_dbus.c

@@ -219,9 +219,10 @@ qemuDBusStart(virQEMUDriver *driver,

     virCommandDaemonize(cmd);

     virCommandAddArgFormat(cmd, "--config-file=%s", configfile);

-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1,

-                               &exitstatus, &cmdret) < 0)

+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false,

+                               &exitstatus, &cmdret) < 0) {

         goto cleanup;

+    }

     if (cmdret < 0 || exitstatus != 0) {

         virReportError(VIR_ERR_INTERNAL_ERROR,

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c

index 8d28a55455..ed7b518212 100644

--- a/src/qemu/qemu_passt.c

+++ b/src/qemu/qemu_passt.c

@@ -285,8 +285,10 @@ qemuPasstStart(virDomainObj *vm,

     if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)

         return -1;

-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)

+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false,

+                               &exitstatus, &cmdret) < 0) {

         goto error;

+    }

     if (cmdret < 0 || exitstatus != 0) {

         virReportError(VIR_ERR_INTERNAL_ERROR,

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c

index 298904fe2e..e5c438aa26 100644

--- a/src/qemu/qemu_process.c

+++ b/src/qemu/qemu_process.c

@@ -7764,7 +7764,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 beada669f7..a5c05b86a9 100644

--- a/src/qemu/qemu_security.c

+++ b/src/qemu/qemu_security.c

@@ -637,6 +637,7 @@ qemuSecurityCommandRun(virQEMUDriver *driver,

                        virCommand *cmd,

                        uid_t uid,

                        gid_t gid,

+                       bool useBinarySpecificLabel,

                        int *exitstatus,

                        int *cmdret)

 {

@@ -644,8 +645,10 @@ qemuSecurityCommandRun(virQEMUDriver *driver,

     qemuDomainObjPrivate *priv = vm->privateData;

     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 8d1c6b38c3..a7ba16e076 100644

--- 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,

                            int *cmdret);

diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c

index 3f83db03bf..e22d86b521 100644

--- a/src/qemu/qemu_slirp.c

+++ b/src/qemu/qemu_slirp.c

@@ -329,8 +329,10 @@ qemuSlirpStart(virDomainObj *vm,

     if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)

         goto error;

-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)

+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false,

+                               &exitstatus, &cmdret) < 0) {

         goto error;

+    }

     if (cmdret < 0 || exitstatus != 0) {

         virReportError(VIR_ERR_INTERNAL_ERROR,

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c

index 5831ffc32e..d4a87921d3 100644

--- 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, &cmdret) < 0)

+                               cfg->swtpm_group, false, NULL, &cmdret) < 0) {

         goto error;

+    }

     if (cmdret < 0) {

         /* virCommandRun() hidden in qemuSecurityCommandRun()

diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c

index bc5a1dc3ec..7909fffe64 100644

--- a/src/qemu/qemu_vhost_user_gpu.c

+++ b/src/qemu/qemu_vhost_user_gpu.c

@@ -153,8 +153,10 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,

             virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode);

     }

-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)

+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false,

+                               &exitstatus, &cmdret) < 0) {

         goto error;

+    }

     if (cmdret < 0 || exitstatus != 0) {

         virReportError(VIR_ERR_INTERNAL_ERROR,

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c

index b63b248975..b5642c9a28 100644

--- 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 9be8f458d1..ca3f4d2dc5 100644

--- 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 fe6982ceca..aa1fb2125d 100644

--- 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 2f8e89cb04..b0578d7209 100644

--- 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 4afdcc167b..97add3294d 100644

--- 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 0dbc547feb..1413f43d57 100644

--- 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 a0b3a5e147..7ea4ff5c1a 100644

--- 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,

@@ -2984,10 +3030,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)

@@ -3004,8 +3053,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 560f797030..369b5dd3a6 100644

--- 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;

-- 

2.40.0