security: framework for driver PreFork handler

A future patch wants the DAC security manager to be able to safely
get the supplemental group list for a given uid, but at the time
of a fork rather than during initialization so as to pick up on
live changes to the system's group database.  This patch adds the
framework, including the possibility of a pre-fork callback
failing.

For now, any driver that implements a prefork callback must be
robust against the possibility of being part of a security stack
where a later element in the chain fails prefork.  This means
that drivers cannot do any action that requires a call to postfork
for proper cleanup (no grabbing a mutex, for example).  If this
is too prohibitive in the future, we would have to switch to a
transactioning sequence, where each driver has (up to) 3 callbacks:
PreForkPrepare, PreForkCommit, and PreForkAbort, to either clean
up or commit changes made during prepare.

* src/security/security_driver.h (virSecurityDriverPreFork): New
callback.
* src/security/security_manager.h (virSecurityManagerPreFork):
Change signature.
* src/security/security_manager.c (virSecurityManagerPreFork):
Optionally call into driver, and allow returning failure.
* src/security/security_stack.c (virSecurityDriverStack):
Wrap the handler for the stack driver.
* src/qemu/qemu_process.c (qemuProcessStart): Adjust caller.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 68e727ec0d2dd49190327dd68535b111c36d5ff7..ef81536791015a193effd63abca577507e92d4af 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3764,7 +3764,8 @@ int qemuProcessStart(virConnectPtr conn,
     virCommandDaemonize(cmd);
     virCommandRequireHandshake(cmd);
 
-    virSecurityManagerPreFork(driver->securityManager);
+    if (virSecurityManagerPreFork(driver->securityManager) < 0)
+        goto cleanup;
     ret = virCommandRun(cmd, NULL);
     virSecurityManagerPostFork(driver->securityManager);
 

diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index cc401e116b9976b3558aba7c3f6990203431210b..87355589556d9b34cf55fc51dab31418f4b0caf6 100644 (file)
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -47,6 +47,8 @@ typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr);
 typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr);
 typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr);
 
+typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr);
+
 typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
                                                    virDomainDefPtr def,
                                                    virDomainDiskDefPtr disk);
@@ -119,6 +121,8 @@ struct _virSecurityDriver {
     virSecurityDriverGetModel getModel;
     virSecurityDriverGetDOI getDOI;
 
+    virSecurityDriverPreFork preFork;
+
     virSecurityDomainSecurityVerify domainSecurityVerify;
 
     virSecurityDomainSetImageLabel domainSetSecurityImageLabel;

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 411a9097cd573372b754e4d8f8a3a91e5f305210..92fb504f7543b0347ab919b50f295a80559a5a7f 100644 (file)
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -193,11 +193,23 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name,
 
 /*
  * Must be called before fork()'ing to ensure mutex state
- * is sane for the child to use
+ * is sane for the child to use. A negative return means the
+ * child must not be forked; a successful return must be
+ * followed by a call to virSecurityManagerPostFork() in both
+ * parent and child.
  */
-void virSecurityManagerPreFork(virSecurityManagerPtr mgr)
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr)
 {
+    int ret = 0;
+
     virObjectLock(mgr);
+    if (mgr->drv->preFork) {
+        ret = mgr->drv->preFork(mgr);
+        if (ret < 0)
+            virObjectUnlock(mgr);
+    }
+
+    return ret;
 }
 
 

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 711b354e8ab8914af665b847c9c0ad1df8b02be8..92528306135667e130ceeee35a376fdf32cea809 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -47,7 +47,7 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver,
                                                bool requireConfined,
                                                bool dynamicOwnership);
 
-void virSecurityManagerPreFork(virSecurityManagerPtr mgr);
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr);
 void virSecurityManagerPostFork(virSecurityManagerPtr mgr);
 
 void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr);

diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 1e229d7e768932011a085850e2f77d103e902a45..ed69b9c256fbd4b0ba9eca31544237123c9f6de7 100644 (file)
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -111,6 +111,27 @@ virSecurityStackGetDOI(virSecurityManagerPtr mgr)
     return virSecurityManagerGetDOI(virSecurityStackGetPrimary(mgr));
 }
 
+static int
+virSecurityStackPreFork(virSecurityManagerPtr mgr)
+{
+    virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+    virSecurityStackItemPtr item = priv->itemsHead;
+    int rc = 0;
+
+    /* XXX For now, we rely on no driver having any state that requires
+     * rollback if a later driver in the stack fails; if this changes,
+     * we'd need to split this into transaction semantics by dividing
+     * the work into prepare/commit/abort.  */
+    for (; item; item = item->next) {
+        if (virSecurityManagerPreFork(item->securityManager) < 0) {
+            rc = -1;
+            break;
+        }
+    }
+
+    return rc;
+}
+
 static int
 virSecurityStackVerify(virSecurityManagerPtr mgr,
                        virDomainDefPtr def)
@@ -539,6 +560,8 @@ virSecurityDriver virSecurityDriverStack = {
     .getModel                           = virSecurityStackGetModel,
     .getDOI                             = virSecurityStackGetDOI,
 
+    .preFork                            = virSecurityStackPreFork,
+
     .domainSecurityVerify               = virSecurityStackVerify,
 
     .domainSetSecurityImageLabel        = virSecurityStackSetSecurityImageLabel,