The virSecurityDomainSetTPMLabels() and virSecurityDomainRestoreTPMLabels() APIs set/restore label on two files/directories: 1) the TPM state (tpm->data.emulator.storagepath), and 2) the TPM log file (tpm->data.emulator.logfile). Soon there will be a need to set the label on the log file but not on the state. Therefore, extend these APIs for a boolean flag that when set does both, but when unset does only 2). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_security.c | 6 ++--- src/security/security_driver.h | 6 +++-- src/security/security_manager.c | 10 +++++---- src/security/security_manager.h | 6 +++-- src/security/security_selinux.c | 40 +++++++++++++++++++++------------ src/security/security_stack.c | 12 +++++----- 6 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 5b7d5f30c2..d9a1ee5f56 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -535,7 +535,7 @@ qemuSecurityStartTPMEmulator(virQEMUDriver *driver, transactionStarted = true; if (virSecurityManagerSetTPMLabels(driver->securityManager, - vm->def) < 0) { + vm->def, true) < 0) { virSecurityManagerTransactionAbort(driver->securityManager); return -1; } @@ -560,7 +560,7 @@ qemuSecurityStartTPMEmulator(virQEMUDriver *driver, virSecurityManagerTransactionStart(driver->securityManager) >= 0) transactionStarted = true; - virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def); + virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def, true); if (transactionStarted && virSecurityManagerTransactionCommit(driver->securityManager, @@ -583,7 +583,7 @@ qemuSecurityCleanupTPMEmulator(virQEMUDriver *driver, if (virSecurityManagerTransactionStart(driver->securityManager) >= 0) transactionStarted = true; - virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def); + virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def, true); if (transactionStarted && virSecurityManagerTransactionCommit(driver->securityManager, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index a1fc23be38..fe6982ceca 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -154,9 +154,11 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManager *mgr, virDomainChrSourceDef *dev_source, bool chardevStdioLogd); typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManager *mgr, - virDomainDef *def); + virDomainDef *def, + bool setTPMStateLabel); typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManager *mgr, - virDomainDef *def); + virDomainDef *def, + bool restoreTPMStateLabel); typedef int (*virSecurityDomainSetNetdevLabel) (virSecurityManager *mgr, virDomainDef *def, virDomainNetDef *net); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 572e400a48..2f8e89cb04 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1188,27 +1188,29 @@ virSecurityManagerRestoreChardevLabel(virSecurityManager *mgr, int virSecurityManagerSetTPMLabels(virSecurityManager *mgr, - virDomainDef *vm) + virDomainDef *vm, + bool setTPMStateLabel) { VIR_LOCK_GUARD lock = virObjectLockGuard(mgr); if (!mgr->drv->domainSetSecurityTPMLabels) return 0; - return mgr->drv->domainSetSecurityTPMLabels(mgr, vm); + return mgr->drv->domainSetSecurityTPMLabels(mgr, vm, setTPMStateLabel); } int virSecurityManagerRestoreTPMLabels(virSecurityManager *mgr, - virDomainDef *vm) + virDomainDef *vm, + bool restoreTPMStateLabel) { VIR_LOCK_GUARD lock = virObjectLockGuard(mgr); if (!mgr->drv->domainRestoreSecurityTPMLabels) return 0; - return mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm); + return mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm, restoreTPMStateLabel); } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index bb3855efef..60597ffc0a 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -214,10 +214,12 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManager *mgr, bool chardevStdioLogd); int virSecurityManagerSetTPMLabels(virSecurityManager *mgr, - virDomainDef *vm); + virDomainDef *vm, + bool setTPMStateLabel); int virSecurityManagerRestoreTPMLabels(virSecurityManager *mgr, - virDomainDef *vm); + virDomainDef *vm, + bool restoreTPMStateLabel); int virSecurityManagerSetNetdevLabel(virSecurityManager *mgr, virDomainDef *vm, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 92e85c92e0..415a26a386 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3526,7 +3526,8 @@ virSecuritySELinuxRestoreFileLabels(virSecurityManager *mgr, static int virSecuritySELinuxSetTPMLabels(virSecurityManager *mgr, - virDomainDef *def) + virDomainDef *def, + bool setTPMStateLabel) { int ret = 0; size_t i; @@ -3540,13 +3541,18 @@ virSecuritySELinuxSetTPMLabels(virSecurityManager *mgr, if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - ret = virSecuritySELinuxSetFileLabels( - mgr, def->tpms[i]->data.emulator.storagepath, - seclabel); - if (ret == 0 && def->tpms[i]->data.emulator.logfile) - ret = virSecuritySELinuxSetFileLabels( - mgr, def->tpms[i]->data.emulator.logfile, - seclabel); + if (setTPMStateLabel) { + ret = virSecuritySELinuxSetFileLabels(mgr, + def->tpms[i]->data.emulator.storagepath, + seclabel); + } + + if (ret == 0 && + def->tpms[i]->data.emulator.logfile) { + ret = virSecuritySELinuxSetFileLabels(mgr, + def->tpms[i]->data.emulator.logfile, + seclabel); + } } return ret; @@ -3555,7 +3561,8 @@ virSecuritySELinuxSetTPMLabels(virSecurityManager *mgr, static int virSecuritySELinuxRestoreTPMLabels(virSecurityManager *mgr, - virDomainDef *def) + virDomainDef *def, + bool restoreTPMStateLabel) { int ret = 0; size_t i; @@ -3564,11 +3571,16 @@ virSecuritySELinuxRestoreTPMLabels(virSecurityManager *mgr, if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - ret = virSecuritySELinuxRestoreFileLabels( - mgr, def->tpms[i]->data.emulator.storagepath); - if (ret == 0 && def->tpms[i]->data.emulator.logfile) - ret = virSecuritySELinuxRestoreFileLabels( - mgr, def->tpms[i]->data.emulator.logfile); + if (restoreTPMStateLabel) { + ret = virSecuritySELinuxRestoreFileLabels(mgr, + def->tpms[i]->data.emulator.storagepath); + } + + if (ret == 0 && + def->tpms[i]->data.emulator.logfile) { + ret = virSecuritySELinuxRestoreFileLabels(mgr, + def->tpms[i]->data.emulator.logfile); + } } return ret; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 0c72f93a20..560f797030 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -916,14 +916,15 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManager *mgr, static int virSecurityStackSetTPMLabels(virSecurityManager *mgr, - virDomainDef *vm) + virDomainDef *vm, + bool setTPMStateLabel) { virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackItem *item = priv->itemsHead; for (; item; item = item->next) { if (virSecurityManagerSetTPMLabels(item->securityManager, - vm) < 0) + vm, setTPMStateLabel) < 0) goto rollback; } @@ -932,7 +933,7 @@ virSecurityStackSetTPMLabels(virSecurityManager *mgr, rollback: for (item = item->prev; item; item = item->prev) { if (virSecurityManagerRestoreTPMLabels(item->securityManager, - vm) < 0) { + vm, setTPMStateLabel) < 0) { VIR_WARN("Unable to restore TPM label after failed set label " "call virDriver=%s driver=%s domain=%s", virSecurityManagerGetVirtDriver(mgr), @@ -946,7 +947,8 @@ virSecurityStackSetTPMLabels(virSecurityManager *mgr, static int virSecurityStackRestoreTPMLabels(virSecurityManager *mgr, - virDomainDef *vm) + virDomainDef *vm, + bool restoreTPMStateLabel) { virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackItem *item = priv->itemsHead; @@ -954,7 +956,7 @@ virSecurityStackRestoreTPMLabels(virSecurityManager *mgr, for (; item; item = item->next) { if (virSecurityManagerRestoreTPMLabels(item->securityManager, - vm) < 0) + vm, restoreTPMStateLabel) < 0) rc = -1; } -- 2.37.4

Recently, the QEMU driver gained support for migration with TPM state on a shared volume (e.g. NFS). As a part of that, the destination side avoids setting seclabels on it to avoid cutting off the source while it is still using it. Makes sense, except for a wee bit: the secdriver API does a bit more - it also sets label on the swtpm log file. And this one definitely needs to be labeled (it lives under /var/log/swtpm/libvirt/qemu/..., i.e. not on a shared volume). Previously, qemuSecurityStartTPMEmulator() took care of that. But during rework to shared volume migration, the code was changed so now plain qemuSecurityCommandRun() would be run (i.e. no relabelling). But after previous commits, we can now chose whether the TPM state should be relabelled or just the log file. Fixes: 2e669ec789231d39e0d5f5f6a201d2a661b8070c Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2130192#c7 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_tpm.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 8dba716ef2..0939f64e4e 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -926,6 +926,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, g_autofree char *pidfile = NULL; virTimeBackOffVar timebackoff; const unsigned long long timeout = 1000; /* ms */ + bool setTPMStateLabel = true; int cmdret = 0; pid_t pid = -1; @@ -955,14 +956,12 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath) == 1) { /* security labels must have been set up on source already */ - if (qemuSecurityCommandRun(driver, vm, cmd, - cfg->swtpm_user, cfg->swtpm_group, - NULL, &cmdret) < 0) { - goto error; - } - } else if (qemuSecurityStartTPMEmulator(driver, vm, cmd, - cfg->swtpm_user, cfg->swtpm_group, - true, NULL, &cmdret) < 0) { + setTPMStateLabel = false; + } + + if (qemuSecurityStartTPMEmulator(driver, vm, cmd, + cfg->swtpm_user, cfg->swtpm_group, + setTPMStateLabel, NULL, &cmdret) < 0) { goto error; } @@ -1133,13 +1132,16 @@ qemuExtTPMStop(virQEMUDriver *driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *shortName = virDomainDefGetShortName(vm->def); + bool restoreTPMStateLabel = true; if (!shortName) return; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - if (!(outgoingMigration && qemuTPMHasSharedStorage(vm->def))) - qemuSecurityCleanupTPMEmulator(driver, vm, true); + if (outgoingMigration || qemuTPMHasSharedStorage(vm->def)) + restoreTPMStateLabel = false; + + qemuSecurityCleanupTPMEmulator(driver, vm, restoreTPMStateLabel); } -- 2.37.4