6:58 a.m.
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 forget about that. We should take the extra
step now while this limitation is still fresh 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>
---
src/libvirt_private.syms | 3 ++
src/security/security_driver.h | 9 ++++++
src/security/security_manager.c | 68 +++++++++++++++++++++++++++++++++++++++++
src/security/security_manager.h | 5 +++
src/security/security_stack.c | 49 +++++++++++++++++++++++++++++
5 files changed, 134 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5b9cee45..97c1123b3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1176,6 +1176,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..d8c6facc8 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -235,6 +235,74 @@ virSecurityManagerPostFork(virSecurityManagerPtr mgr)
virObjectUnlock(mgr);
}
+
+/**
+ * virSecurityManagerTransactionStart:
+ * @mgr: security manager
+ *
+ * Starts a new transaction. In transaction nothing is changed security
+ * label until virSecurityManagerTransactionCommit() is called.
+ *
+ * Returns 0 on success,
+ * -1 otherwise.
+ */
+int
+virSecurityManagerTransactionStart(virSecurityManagerPtr mgr)
+{
+ int ret = 0;
+
+ virObjectLock(mgr);
+ if (mgr->drv->transactionStart)
+ ret = mgr->drv->transactionStart(mgr);
+ virObjectUnlock(mgr);
+ return ret;
+}
+
+
+/**
+ * virSecurityManagerTransactionCommit:
+ * @mgr: security manager
+ * @pid: domain's PID
+ *
+ * Enters the @pid namespace (usually @pid refers to a domain) and
+ * performs all the operations on the transaction list. Note that the
+ * transaction is also freed, therefore new one has to be started after
+ * successful return from this function. Also it is considered as error
+ * if there's no transaction set and this function is called.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise.
+ */
+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;
+}
+
+
+/**
+ * virSecurityManagerTransactionAbort:
+ * @mgr: security manager
+ *
+ * Cancels and frees any out standing transaction.
+ */
+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,
--
2.11.0