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 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@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 82adf7ecb7afb648b3331c0e7c5dac16aa197f74..b69e8bcc25b310fe40c15f23175b4b298835f761 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1183,6 +1183,9 @@ virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
 virSecurityManagerSetTapFDLabel;
 virSecurityManagerStackAddNested;
+virSecurityManagerTransactionAbort;
+virSecurityManagerTransactionCommit;
+virSecurityManagerTransactionStart;
 virSecurityManagerVerify;
 
 

diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index b48f2aaed8621ea7103a0b2a2040522c5827157e..fa65eb3595bf1b7e811ef571c9bc47d02ad557ee 100644 (file)
--- 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 f98c7d2d1cff2e2566d3e1d5b22750eb2e384a1b..d8c6facc81f9903d59a0d5e462136f2b6572c5c3 100644 (file)
--- 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 80fceddc84e3260645362a78e8df238db4e2d4eb..bae8493eea57255fc13e3558aae2f75f15ea4639 100644 (file)
--- 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 c123931a0bd0f72d84d4c43c45e13f239ad162a8..6056ae321d2e2185c1e367e4ca6197472022532e 100644 (file)
--- 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,