security: don't relabel chardev source if virtlogd is used as stdio handler

In the case that virtlogd is used as stdio handler we pass to QEMU
only FD to a PIPE connected to virtlogd instead of the file itself.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430988Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

src/lxc/lxc_process.c

@@ -852,7 +852,7 @@ int virLXCProcessStop(virLXCDriverPtr driver,
     }
 
     virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                      vm->def, false);
+                                      vm->def, false, false);
     virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
     /* Clear out dynamically assigned labels */
     if (vm->def->nseclabels &&
@@ -1349,7 +1349,7 @@ int virLXCProcessStart(virConnectPtr conn,
 
     VIR_DEBUG("Setting domain security labels");
     if (virSecurityManagerSetAllLabel(driver->securityManager,
-                                      vm->def, NULL) < 0)
+                                      vm->def, NULL, false) < 0)
         goto cleanup;
 
     VIR_DEBUG("Setting up consoles");
@@ -1578,7 +1578,7 @@ int virLXCProcessStart(virConnectPtr conn,
             virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
         } else {
             virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                              vm->def, false);
+                                              vm->def, false, false);
             virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
             /* Clear out dynamically assigned labels */
             if (vm->def->nseclabels &&

src/qemu/qemu_security.c

@@ -38,6 +38,7 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
                         const char *stdin_path)
 {
     int ret = -1;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
 
     if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
         virSecurityManagerTransactionStart(driver->securityManager) < 0)
@@ -45,7 +46,8 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
 
     if (virSecurityManagerSetAllLabel(driver->securityManager,
                                       vm->def,
-                                      stdin_path) < 0)
+                                      stdin_path,
+                                      priv->chardevStdioLogd) < 0)
         goto cleanup;
 
     if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
@@ -65,6 +67,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             bool migrated)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
     /* In contrast to qemuSecuritySetAllLabel, do not use
      * secdriver transactions here. This function is called from
      * qemuProcessStop() which is meant to do cleanup after qemu
@@ -73,7 +77,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
      * in entering the namespace then. */
     virSecurityManagerRestoreAllLabel(driver->securityManager,
                                       vm->def,
-                                      migrated);
+                                      migrated,
+                                      priv->chardevStdioLogd);
 }
 
 

src/security/security_apparmor.c

@@ -489,7 +489,9 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 
 static int
 AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
-                            virDomainDefPtr def, const char *stdin_path)
+                            virDomainDefPtr def,
+                            const char *stdin_path,
+                            bool chardevStdioLogd ATTRIBUTE_UNUSED)
 {
     virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
                                                     SECURITY_APPARMOR_NAME);
@@ -567,7 +569,8 @@ AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 static int
 AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                 virDomainDefPtr def,
-                                bool migrated ATTRIBUTE_UNUSED)
+                                bool migrated ATTRIBUTE_UNUSED,
+                                bool chardevStdioLogd ATTRIBUTE_UNUSED)
 {
     int rc = 0;
     virSecurityLabelDefPtr secdef =

src/security/security_dac.c

@@ -1159,7 +1159,8 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr def,
-                              virDomainChrSourceDefPtr dev_source)
+                              virDomainChrSourceDefPtr dev_source,
+                              bool chardevStdioLogd)
 
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
@@ -1178,6 +1179,9 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
+    if (!chr_seclabel && chardevStdioLogd)
+        return 0;
+
     if (chr_seclabel && chr_seclabel->label) {
         if (virParseOwnershipIds(chr_seclabel->label, &user, &group) < 0)
             return -1;
@@ -1243,7 +1247,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr def ATTRIBUTE_UNUSED,
-                                  virDomainChrSourceDefPtr dev_source)
+                                  virDomainChrSourceDefPtr dev_source,
+                                  bool chardevStdioLogd)
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
@@ -1256,6 +1261,9 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
+    if (!chr_seclabel && chardevStdioLogd)
+        return 0;
+
     switch ((virDomainChrType) dev_source->type) {
     case VIR_DOMAIN_CHR_TYPE_DEV:
     case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -1298,14 +1306,21 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
 }
 
 
+struct _virSecuritySELinuxChardevCallbackData {
+    virSecurityManagerPtr mgr;
+    bool chardevStdioLogd;
+};
+
+
 static int
 virSecurityDACRestoreChardevCallback(virDomainDefPtr def,
                                      virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                      void *opaque)
 {
-    virSecurityManagerPtr mgr = opaque;
+    struct _virSecuritySELinuxChardevCallbackData *data = opaque;
 
-    return virSecurityDACRestoreChardevLabel(mgr, def, dev->source);
+    return virSecurityDACRestoreChardevLabel(data->mgr, def, dev->source,
+                                             data->chardevStdioLogd);
 }
 
 
@@ -1319,7 +1334,8 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
     switch (tpm->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
         ret = virSecurityDACSetChardevLabel(mgr, def,
-                                            &tpm->data.passthrough.source);
+                                            &tpm->data.passthrough.source,
+                                            false);
         break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
@@ -1339,7 +1355,8 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
     switch (tpm->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
         ret = virSecurityDACRestoreChardevLabel(mgr, def,
-                                                &tpm->data.passthrough.source);
+                                                &tpm->data.passthrough.source,
+                                                false);
         break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
@@ -1436,7 +1453,8 @@ virSecurityDACRestoreMemoryLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr def,
-                              bool migrated)
+                              bool migrated,
+                              bool chardevStdioLogd)
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityLabelDefPtr secdef;
@@ -1479,10 +1497,15 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
             rc = -1;
     }
 
+    struct _virSecuritySELinuxChardevCallbackData chardevData = {
+        .mgr = mgr,
+        .chardevStdioLogd = chardevStdioLogd,
+    };
+
     if (virDomainChrDefForeach(def,
                                false,
                                virSecurityDACRestoreChardevCallback,
-                               mgr) < 0)
+                               &chardevData) < 0)
         rc = -1;
 
     if (def->tpm) {
@@ -1505,9 +1528,10 @@ virSecurityDACSetChardevCallback(virDomainDefPtr def,
                                  virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                  void *opaque)
 {
-    virSecurityManagerPtr mgr = opaque;
+    struct _virSecuritySELinuxChardevCallbackData *data = opaque;
 
-    return virSecurityDACSetChardevLabel(mgr, def, dev->source);
+    return virSecurityDACSetChardevLabel(data->mgr, def, dev->source,
+                                         data->chardevStdioLogd);
 }
 
 
@@ -1549,7 +1573,8 @@ virSecurityDACSetMemoryLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
                           virDomainDefPtr def,
-                          const char *stdin_path ATTRIBUTE_UNUSED)
+                          const char *stdin_path ATTRIBUTE_UNUSED,
+                          bool chardevStdioLogd)
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityLabelDefPtr secdef;
@@ -1592,10 +1617,15 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
             return -1;
     }
 
+    struct _virSecuritySELinuxChardevCallbackData chardevData = {
+        .mgr = mgr,
+        .chardevStdioLogd = chardevStdioLogd,
+    };
+
     if (virDomainChrDefForeach(def,
                                true,
                                virSecurityDACSetChardevCallback,
-                               mgr) < 0)
+                               &chardevData) < 0)
         return -1;
 
     if (def->tpm) {

src/security/security_driver.h

@@ -91,10 +91,12 @@ typedef int (*virSecurityDomainReleaseLabel) (virSecurityManagerPtr mgr,
                                               virDomainDefPtr sec);
 typedef int (*virSecurityDomainSetAllLabel) (virSecurityManagerPtr mgr,
                                              virDomainDefPtr sec,
-                                             const char *stdin_path);
+                                             const char *stdin_path,
+                                             bool chardevStdioLogd);
 typedef int (*virSecurityDomainRestoreAllLabel) (virSecurityManagerPtr mgr,
                                                  virDomainDefPtr def,
-                                                 bool migrated);
+                                                 bool migrated,
+                                                 bool chardevStdioLogd);
 typedef int (*virSecurityDomainGetProcessLabel) (virSecurityManagerPtr mgr,
                                                  virDomainDefPtr def,
                                                  pid_t pid,

src/security/security_manager.c

@@ -856,12 +856,14 @@ int virSecurityManagerCheckAllLabel(virSecurityManagerPtr mgr,
 int
 virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr vm,
-                              const char *stdin_path)
+                              const char *stdin_path,
+                              bool chardevStdioLogd)
 {
     if (mgr->drv->domainSetSecurityAllLabel) {
         int ret;
         virObjectLock(mgr);
-        ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path);
+        ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path,
+                                                  chardevStdioLogd);
         virObjectUnlock(mgr);
         return ret;
     }
@@ -874,12 +876,14 @@ virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr,
 int
 virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr vm,
-                                  bool migrated)
+                                  bool migrated,
+                                  bool chardevStdioLogd)
 {
     if (mgr->drv->domainRestoreSecurityAllLabel) {
         int ret;
         virObjectLock(mgr);
-        ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated);
+        ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated,
+                                                      chardevStdioLogd);
         virObjectUnlock(mgr);
         return ret;
     }

src/security/security_manager.h

@@ -130,10 +130,12 @@ int virSecurityManagerCheckAllLabel(virSecurityManagerPtr mgr,
                                     virDomainDefPtr sec);
 int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr sec,
-                                  const char *stdin_path);
+                                  const char *stdin_path,
+                                  bool chardevStdioLogd);
 int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr def,
-                                      bool migrated);
+                                      bool migrated,
+                                      bool chardevStdioLogd);
 int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr def,
                                       pid_t pid,

src/security/security_nop.c

@@ -151,7 +151,8 @@ virSecurityDomainReleaseLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 static int
 virSecurityDomainSetAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                 virDomainDefPtr sec ATTRIBUTE_UNUSED,
-                                const char *stdin_path ATTRIBUTE_UNUSED)
+                                const char *stdin_path ATTRIBUTE_UNUSED,
+                                bool chardevStdioLogd ATTRIBUTE_UNUSED)
 {
     return 0;
 }
@@ -159,7 +160,8 @@ virSecurityDomainSetAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 static int
 virSecurityDomainRestoreAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                     virDomainDefPtr vm ATTRIBUTE_UNUSED,
-                                    bool migrated ATTRIBUTE_UNUSED)
+                                    bool migrated ATTRIBUTE_UNUSED,
+                                    bool chardevStdioLogd ATTRIBUTE_UNUSED)
 {
     return 0;
 }

src/security/security_selinux.c

@@ -2179,7 +2179,8 @@ virSecuritySELinuxRestoreHostdevLabel(virSecurityManagerPtr mgr,
 static int
 virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr def,
-                                  virDomainChrSourceDefPtr dev_source)
+                                  virDomainChrSourceDefPtr dev_source,
+                                  bool chardevStdioLogd)
 
 {
     virSecurityLabelDefPtr seclabel;
@@ -2198,6 +2199,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
+    if (!chr_seclabel && chardevStdioLogd)
+        return 0;
+
     if (chr_seclabel)
         imagelabel = chr_seclabel->label;
     if (!imagelabel)
@@ -2252,7 +2256,8 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
 static int
 virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr def,
-                                      virDomainChrSourceDefPtr dev_source)
+                                      virDomainChrSourceDefPtr dev_source,
+                                      bool chardevStdioLogd)
 
 {
     virSecurityLabelDefPtr seclabel;
@@ -2269,6 +2274,9 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
+    if (!chr_seclabel && chardevStdioLogd)
+        return 0;
+
     switch (dev_source->type) {
     case VIR_DOMAIN_CHR_TYPE_DEV:
     case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -2312,14 +2320,21 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
 }
 
 
+struct _virSecuritySELinuxChardevCallbackData {
+    virSecurityManagerPtr mgr;
+    bool chardevStdioLogd;
+};
+
+
 static int
 virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
                                                  virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                                  void *opaque)
 {
-    virSecurityManagerPtr mgr = opaque;
+    struct _virSecuritySELinuxChardevCallbackData *data = opaque;
 
-    return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->source);
+    return virSecuritySELinuxRestoreChardevLabel(data->mgr, def, dev->source,
+                                                 data->chardevStdioLogd);
 }
 
 
@@ -2342,7 +2357,8 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
         return virSecuritySELinuxRestoreFileLabel(mgr, database);
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->data.passthru);
+        return virSecuritySELinuxRestoreChardevLabel(mgr, def,
+                                                     dev->data.passthru, false);
 
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2369,7 +2385,8 @@ virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
 static int
 virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr def,
-                                  bool migrated)
+                                  bool migrated,
+                                  bool chardevStdioLogd)
 {
     virSecurityLabelDefPtr secdef;
     virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
@@ -2414,10 +2431,15 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr,
             rc = -1;
     }
 
+    struct _virSecuritySELinuxChardevCallbackData chardevData = {
+        .mgr = mgr,
+        .chardevStdioLogd = chardevStdioLogd
+    };
+
     if (virDomainChrDefForeach(def,
                                false,
                                virSecuritySELinuxRestoreSecurityChardevCallback,
-                               mgr) < 0)
+                               &chardevData) < 0)
         rc = -1;
 
     if (virDomainSmartcardDefForeach(def,
@@ -2706,9 +2728,10 @@ virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
                                              virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                              void *opaque)
 {
-    virSecurityManagerPtr mgr = opaque;
+    struct _virSecuritySELinuxChardevCallbackData *data = opaque;
 
-    return virSecuritySELinuxSetChardevLabel(mgr, def, dev->source);
+    return virSecuritySELinuxSetChardevLabel(data->mgr, def, dev->source,
+                                             data->chardevStdioLogd);
 }
 
 
@@ -2733,7 +2756,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
         return virSecuritySELinuxSetChardevLabel(mgr, def,
-                                                 dev->data.passthru);
+                                                 dev->data.passthru, false);
 
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2749,7 +2772,8 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
 static int
 virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr def,
-                              const char *stdin_path)
+                              const char *stdin_path,
+                              bool chardevStdioLogd)
 {
     size_t i;
     virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
@@ -2797,10 +2821,15 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
             return -1;
     }
 
+    struct _virSecuritySELinuxChardevCallbackData chardevData = {
+        .mgr = mgr,
+        .chardevStdioLogd = chardevStdioLogd
+    };
+
     if (virDomainChrDefForeach(def,
                                true,
                                virSecuritySELinuxSetSecurityChardevCallback,
-                               mgr) < 0)
+                               &chardevData) < 0)
         return -1;
 
     if (virDomainSmartcardDefForeach(def,

src/security/security_stack.c

@@ -350,14 +350,16 @@ virSecurityStackRestoreHostdevLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityStackSetAllLabel(virSecurityManagerPtr mgr,
                             virDomainDefPtr vm,
-                            const char *stdin_path)
+                            const char *stdin_path,
+                            bool chardevStdioLogd)
 {
     virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityStackItemPtr item = priv->itemsHead;
     int rc = 0;
 
     for (; item; item = item->next) {
-        if (virSecurityManagerSetAllLabel(item->securityManager, vm, stdin_path) < 0)
+        if (virSecurityManagerSetAllLabel(item->securityManager, vm,
+                                          stdin_path, chardevStdioLogd) < 0)
             rc = -1;
     }
 
@@ -368,14 +370,16 @@ virSecurityStackSetAllLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityStackRestoreAllLabel(virSecurityManagerPtr mgr,
                                 virDomainDefPtr vm,
-                                bool migrated)
+                                bool migrated,
+                                bool chardevStdioLogd)
 {
     virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityStackItemPtr item = priv->itemsHead;
     int rc = 0;
 
     for (; item; item = item->next) {
-        if (virSecurityManagerRestoreAllLabel(item->securityManager, vm, migrated) < 0)
+        if (virSecurityManagerRestoreAllLabel(item->securityManager, vm,
+                                              migrated, chardevStdioLogd) < 0)
             rc = -1;
     }
 

tests/securityselinuxlabeltest.c

@@ -313,7 +313,7 @@ testSELinuxLabeling(const void *opaque)
     if (!(def = testSELinuxLoadDef(testname)))
         goto cleanup;
 
-    if (virSecurityManagerSetAllLabel(mgr, def, NULL) < 0)
+    if (virSecurityManagerSetAllLabel(mgr, def, NULL, false) < 0)
         goto cleanup;
 
     if (testSELinuxCheckLabels(files, nfiles) < 0)