Re: [libvirt] [PATCH 3/6] security driver: Introduce transaction APIs

With our new qemu namespace code in place, the relabelling of devices is done not as good is it could: a child process is spawned, it enters the mount namespace of the qemu process and then runs desired API of the security driver.

Problem with this approach is that internal state transition of the security driver done in the child process is not reflected in the parent process. While currently it wouldn't matter that much, it is fairly easy to forge about that. We should take the extra

step now while this limitation is still freshly in our minds.

Three new APIs are introduced here: virSecurityManagerTransactionStart() virSecurityManagerTransactionCommit() virSecurityManagerTransactionAbort() The Start() is going to be used to let security driver know that we are starting a new transaction. During a transaction no security labels are actually touched, but rather recorded and only at Commit() phase they are actually updated. Should something go wrong Abort() aborts the transaction freeing up all memory allocated by transaction.

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 3 +++ src/security/security_driver.h | 9 ++++++++ src/security/security_manager.c | 38 ++++++++++++++++++++++++++++++++ src/security/security_manager.h | 5 +++++ src/security/security_stack.c | 49 +++++++++++++++++++++++++++++++++++++++++

5 files changed, 104 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4c0170c03..400295b7b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1172,6 +1172,9 @@ virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; +virSecurityManagerTransactionAbort; +virSecurityManagerTransactionCommit; +virSecurityManagerTransactionStart; virSecurityManagerVerify; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index b48f2aaed..fa65eb359 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -51,6 +51,11 @@ typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); +typedef int (*virSecurityDriverTransactionStart) (virSecurityManagerPtr mgr); +typedef int (*virSecurityDriverTransactionCommit) (virSecurityManagerPtr mgr, + pid_t pid); +typedef void (*virSecurityDriverTransactionAbort) (virSecurityManagerPtr mgr); + typedef int (*virSecurityDomainRestoreDiskLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk); @@ -135,6 +140,10 @@ struct _virSecurityDriver { virSecurityDriverPreFork preFork; + virSecurityDriverTransactionStart transactionStart; + virSecurityDriverTransactionCommit transactionCommit; + virSecurityDriverTransactionAbort transactionAbort; + virSecurityDomainSecurityVerify domainSecurityVerify; virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f98c7d2d1..4b6b4a926 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -235,6 +235,44 @@ virSecurityManagerPostFork(virSecurityManagerPtr mgr) virObjectUnlock(mgr); } + +int +virSecurityManagerTransactionStart(virSecurityManagerPtr mgr) +{ + int ret = 0; + + virObjectLock(mgr); + if (mgr->drv->transactionStart) + ret = mgr->drv->transactionStart(mgr); + virObjectUnlock(mgr); + return ret; +} + + +int +virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid) +{ + int ret = 0; + + virObjectLock(mgr); + if (mgr->drv->transactionCommit) + ret = mgr->drv->transactionCommit(mgr, pid); + virObjectUnlock(mgr); + return ret; +} + + +void +virSecurityManagerTransactionAbort(virSecurityManagerPtr mgr) +{ + virObjectLock(mgr); + if (mgr->drv->transactionAbort) + mgr->drv->transactionAbort(mgr); + virObjectUnlock(mgr); +} + + void * virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 80fceddc8..bae8493ee 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -76,6 +76,11 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, int virSecurityManagerPreFork(virSecurityManagerPtr mgr); void virSecurityManagerPostFork(virSecurityManagerPtr mgr); +int virSecurityManagerTransactionStart(virSecurityManagerPtr mgr); +int virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid); +void virSecurityManagerTransactionAbort(virSecurityManagerPtr mgr); + void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDriver(virSecurityManagerPtr mgr); diff --git a/src/security/security_stack.c b/src/security/security_stack.c index c123931a0..6056ae321 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -137,6 +137,51 @@ virSecurityStackPreFork(virSecurityManagerPtr mgr) return rc; } + +static int +virSecurityStackTransactionStart(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerTransactionStart(item->securityManager) < 0) + rc = -1; + } + + return rc; +} + + +static int +virSecurityStackTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerTransactionCommit(item->securityManager, pid) < 0) + rc = -1; + } + + return rc; +} + + +static void +virSecurityStackTransactionAbort(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + + for (; item; item = item->next) + virSecurityManagerTransactionAbort(item->securityManager); +} + + static int virSecurityStackVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) @@ -612,6 +657,10 @@ virSecurityDriver virSecurityDriverStack = { .preFork = virSecurityStackPreFork, + .transactionStart = virSecurityStackTransactionStart, + .transactionCommit = virSecurityStackTransactionCommit, + .transactionAbort = virSecurityStackTransactionAbort, + .domainSecurityVerify = virSecurityStackVerify, .domainSetSecurityDiskLabel = virSecurityStackSetDiskLabel,