security: Allow skipping locking when labeling lock files

This is needed when migrating a guest that has persistent TPM
state: relabeling (which implies locking) needs to happen
before the swtpm process is started on the destination host,
but the lock file won't be released by the swtpm process
running on the source host before a handshake with the target
process has happened, creating a catch-22 scenario.

In order to make migration possible, make it so that locking
for lock files can be explicitly skipped. All other state
files are handled as usual.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 996c95acc0d89c23eac7a12923cd2e8b4928c502..5e815ba2a0212abc9d7e4c69bba3e2c7118a1913 100644 (file)
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -57,7 +57,8 @@ qemuSecuritySetAllLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -94,7 +95,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriver *driver,
 
     if (transactionStarted &&
         virSecurityManagerTransactionCommit(driver->securityManager,
-                                            -1, priv->rememberOwner) < 0)
+                                            -1, priv->rememberOwner,
+                                            false) < 0)
         VIR_WARN("Unable to run security manager transaction");
 
     virSecurityManagerTransactionAbort(driver->securityManager);
@@ -133,7 +135,8 @@ qemuSecuritySetImageLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -171,7 +174,8 @@ qemuSecurityRestoreImageLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -227,7 +231,8 @@ qemuSecuritySetHostdevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -261,7 +266,8 @@ qemuSecurityRestoreHostdevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -294,7 +300,8 @@ qemuSecuritySetMemoryLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -327,7 +334,8 @@ qemuSecurityRestoreMemoryLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -360,7 +368,8 @@ qemuSecuritySetInputLabel(virDomainObj *vm,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -393,7 +402,8 @@ qemuSecurityRestoreInputLabel(virDomainObj *vm,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -427,7 +437,8 @@ qemuSecuritySetChardevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -461,7 +472,8 @@ qemuSecurityRestoreChardevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -492,7 +504,8 @@ qemuSecuritySetNetdevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -524,7 +537,8 @@ qemuSecurityRestoreNetdevLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -552,7 +566,8 @@ qemuSecuritySetTPMLabels(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            -1, priv->rememberOwner) < 0)
+                                            -1, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -580,7 +595,8 @@ qemuSecurityRestoreTPMLabels(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            -1, priv->rememberOwner) < 0)
+                                            -1, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -615,7 +631,8 @@ qemuSecurityDomainSetPathLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -648,7 +665,8 @@ qemuSecurityDomainRestorePathLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            pid, priv->rememberOwner) < 0)
+                                            pid, priv->rememberOwner,
+                                            false) < 0)
         goto cleanup;
 
     ret = 0;
@@ -694,7 +712,7 @@ qemuSecurityDomainSetMountNSPathLabel(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            vm->pid, false) < 0)
+                                            vm->pid, false, false) < 0)
         goto cleanup;
 
     ret = 0;

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index fdc11876c9924175d609d14b7ef407b89a479962..a179378a7803f702c6d4920ff76d1300b0ea9a1e 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -83,6 +83,7 @@ struct _virSecurityDACChownList {
     virSecurityDACChownItem **items;
     size_t nItems;
     bool lock;
+    bool lockMetadataException;
 };
 
 
@@ -232,7 +233,8 @@ virSecurityDACTransactionRun(pid_t pid G_GNUC_UNUSED,
 
         if (!(state = virSecurityManagerMetadataLock(list->manager,
                                                      list->sharedFilesystems,
-                                                     paths, npaths)))
+                                                     paths, npaths,
+                                                     list->lockMetadataException)))
             return -1;
 
         for (i = 0; i < list->nItems; i++) {
@@ -580,6 +582,7 @@ virSecurityDACTransactionStart(virSecurityManager *mgr,
  * @mgr: security manager
  * @pid: domain's PID
  * @lock: lock and unlock paths that are relabeled
+ * @lockMetadataException: don't lock metadata for lock files
  *
  * If @pid is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the chown()-s on the list. If @pid is -1
@@ -599,7 +602,8 @@ virSecurityDACTransactionStart(virSecurityManager *mgr,
 static int
 virSecurityDACTransactionCommit(virSecurityManager *mgr G_GNUC_UNUSED,
                                 pid_t pid,
-                                bool lock)
+                                bool lock,
+                                bool lockMetadataException)
 {
     g_autoptr(virSecurityDACChownList) list = NULL;
     int rc;
@@ -618,6 +622,7 @@ virSecurityDACTransactionCommit(virSecurityManager *mgr G_GNUC_UNUSED,
     }
 
     list->lock = lock;
+    list->lockMetadataException = lockMetadataException;
 
     if (pid != -1) {
         rc = virProcessRunInMountNamespace(pid,
@@ -1084,7 +1089,8 @@ virSecurityDACMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
 
     if (!(state = virSecurityManagerMetadataLock(data->mgr,
                                                  data->sharedFilesystems,
-                                                 paths, G_N_ELEMENTS(paths))))
+                                                 paths, G_N_ELEMENTS(paths),
+                                                 false)))
         return -1;
 
     ret = virSecurityMoveRememberedLabel(SECURITY_DAC_NAME, data->src, data->dst);

diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 2956e002ff2876aaa13f9905a984e81d73deeb26..5ab4d6ca1e09bc5729c9ad4dceb1941d3b744304 100644 (file)
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -50,7 +50,8 @@ typedef int (*virSecurityDriverTransactionStart) (virSecurityManager *mgr,
                                                   char *const *sharedFilesystems);
 typedef int (*virSecurityDriverTransactionCommit) (virSecurityManager *mgr,
                                                    pid_t pid,
-                                                   bool lock);
+                                                   bool lock,
+                                                   bool lockMetadataException);
 typedef void (*virSecurityDriverTransactionAbort) (virSecurityManager *mgr);
 
 typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManager *mgr,

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 65b173e6702f5c54e3ef44dc6a4fbf3a8d45399c..c2460eae379d23a9b8af1bdc879f3b0286f22995 100644 (file)
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -31,6 +31,7 @@
 #include "virobject.h"
 #include "virlog.h"
 #include "virfile.h"
+#include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECURITY
 
@@ -270,6 +271,7 @@ virSecurityManagerTransactionStart(virSecurityManager *mgr,
  * @mgr: security manager
  * @pid: domain's PID
  * @lock: lock and unlock paths that are relabeled
+ * @lockMetadataException: don't lock metadata for lock files
  *
  * If @pid is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the operations on the transaction list.
@@ -290,14 +292,15 @@ virSecurityManagerTransactionStart(virSecurityManager *mgr,
 int
 virSecurityManagerTransactionCommit(virSecurityManager *mgr,
                                     pid_t pid,
-                                    bool lock)
+                                    bool lock,
+                                    bool lockMetadataException)
 {
     VIR_LOCK_GUARD lockguard = virObjectLockGuard(mgr);
 
     if (!mgr->drv->transactionCommit)
         return 0;
 
-    return mgr->drv->transactionCommit(mgr, pid, lock);
+    return mgr->drv->transactionCommit(mgr, pid, lock, lockMetadataException);
 }
 
 
@@ -1310,6 +1313,7 @@ cmpstringp(const void *p1,
  * @sharedFilesystems: list of filesystem to consider shared
  * @paths: paths to lock
  * @npaths: number of items in @paths array
+ * @lockMetadataException: don't lock metadata for lock files
  *
  * Lock passed @paths for metadata change. The returned state
  * should be passed to virSecurityManagerMetadataUnlock.
@@ -1325,7 +1329,8 @@ virSecurityManagerMetadataLockState *
 virSecurityManagerMetadataLock(virSecurityManager *mgr G_GNUC_UNUSED,
                                char *const *sharedFilesystems,
                                const char **paths,
-                               size_t npaths)
+                               size_t npaths,
+                               bool lockMetadataException)
 {
     size_t i = 0;
     size_t nfds = 0;
@@ -1371,6 +1376,16 @@ virSecurityManagerMetadataLock(virSecurityManager *mgr G_GNUC_UNUSED,
         if (i != j)
             continue;
 
+        /* Any attempt to lock a lock file is likely to go very
+         * poorly. This is a problem for TPM persistent storage,
+         * since on migration we need to relabel it while the swtpm
+         * process is still holding on to its lock file. As a way to
+         * resolve the conundrum, skip metadata locking for paths
+         * that look like they might be referring to lock files, if
+         * we have also been explicitly asked to make this exception */
+        if (lockMetadataException && virStringHasSuffix(p, ".lock"))
+            continue;
+
         if (stat(p, &sb) < 0)
             continue;
 

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index bf0059b2e06ad5377dd05f658b698333f3bb4900..068ca4e2902fa66b2661d518a96730ace20651f2 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -85,7 +85,8 @@ int virSecurityManagerTransactionStart(virSecurityManager *mgr,
                                        char *const *sharedFilesystems);
 int virSecurityManagerTransactionCommit(virSecurityManager *mgr,
                                         pid_t pid,
-                                        bool lock);
+                                        bool lock,
+                                        bool lockMetadataException);
 void virSecurityManagerTransactionAbort(virSecurityManager *mgr);
 
 void *virSecurityManagerGetPrivateData(virSecurityManager *mgr);
@@ -254,7 +255,8 @@ virSecurityManagerMetadataLockState *
 virSecurityManagerMetadataLock(virSecurityManager *mgr,
                                char *const *sharedFilesystems,
                                const char **paths,
-                               size_t npaths);
+                               size_t npaths,
+                               bool lockMetadataException);
 
 void
 virSecurityManagerMetadataUnlock(virSecurityManager *mgr,

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 05f54cc9f5479aad7795fdec5658286cd122fae5..8f567d54880b05469374aa2fc907f389708ce7c3 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -81,6 +81,7 @@ struct _virSecuritySELinuxContextList {
     virSecuritySELinuxContextItem **items;
     size_t nItems;
     bool lock;
+    bool lockMetadataException;
 };
 
 #define SECURITY_SELINUX_VOID_DOI       "0"
@@ -300,7 +301,8 @@ virSecuritySELinuxTransactionRun(pid_t pid G_GNUC_UNUSED,
 
         if (!(state = virSecurityManagerMetadataLock(list->manager,
                                                      list->sharedFilesystems,
-                                                     paths, npaths)))
+                                                     paths, npaths,
+                                                     list->lockMetadataException)))
             goto cleanup;
 
         for (i = 0; i < list->nItems; i++) {
@@ -1193,6 +1195,7 @@ virSecuritySELinuxTransactionStart(virSecurityManager *mgr,
  * @mgr: security manager
  * @pid: domain's PID
  * @lock: lock and unlock paths that are relabeled
+ * @lockMetadataException: don't lock metadata for lock files
  *
  * If @pis is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the sefilecon()-s on the list. If @pid
@@ -1213,7 +1216,8 @@ virSecuritySELinuxTransactionStart(virSecurityManager *mgr,
 static int
 virSecuritySELinuxTransactionCommit(virSecurityManager *mgr G_GNUC_UNUSED,
                                     pid_t pid,
-                                    bool lock)
+                                    bool lock,
+                                    bool lockMetadataException)
 {
     virSecuritySELinuxContextList *list;
     int rc;
@@ -1233,6 +1237,7 @@ virSecuritySELinuxTransactionCommit(virSecurityManager *mgr G_GNUC_UNUSED,
     }
 
     list->lock = lock;
+    list->lockMetadataException = lockMetadataException;
 
     if (pid != -1) {
         rc = virProcessRunInMountNamespace(pid,
@@ -2091,7 +2096,8 @@ virSecuritySELinuxMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
 
     if (!(state = virSecurityManagerMetadataLock(data->mgr,
                                                  data->sharedFilesystems,
-                                                 paths, G_N_ELEMENTS(paths))))
+                                                 paths, G_N_ELEMENTS(paths),
+                                                 false)))
         return -1;
 
     ret = virSecurityMoveRememberedLabel(SECURITY_SELINUX_NAME, data->src, data->dst);

diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 11800535b9d2db30657cecd3a5b4afe69acbd46a..99a68a605323da4b4427a48ce29fd0fe9b6242fc 100644 (file)
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -164,13 +164,15 @@ virSecurityStackTransactionStart(virSecurityManager *mgr,
 static int
 virSecurityStackTransactionCommit(virSecurityManager *mgr,
                                   pid_t pid,
-                                  bool lock)
+                                  bool lock,
+                                  bool lockMetadataException)
 {
     virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityStackItem *item = priv->itemsHead;
 
     for (; item; item = item->next) {
-        if (virSecurityManagerTransactionCommit(item->securityManager, pid, lock) < 0)
+        if (virSecurityManagerTransactionCommit(item->securityManager, pid,
+                                                lock, lockMetadataException) < 0)
             goto rollback;
     }