External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-22 03:12:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

security: Allow skipping locking when labeling lock files

Browse Source 
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>
This commit is contained in:
Andrea Bolognani 2024-08-12 17:07:54 +02:00
parent 8fe803247e
commit 454219ad6c
7 changed files with 83 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/qemu/qemu_security.c
View File

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

12
src/security/security_dac.c
View File

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

3
src/security/security_driver.h
View File

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

21
src/security/security_manager.c
View File

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

6
src/security/security_manager.h
View File

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

12
src/security/security_selinux.c
View File

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

6
src/security/security_stack.c
View File

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