security: framework for driver PreFork handler

https://bugzilla.redhat.com/show_bug.cgi?id=964358

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>
(cherry picked from commit fdb3bde31ccf8ff172abf00ef5aa974b87af2794)

Conflicts:
	src/security/security_manager.c - context from previous backport differences

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 01c68806ddca10e9ffe42a1858fb5e6ca00f8747..4fdad6ab7b166c7feb0f34945d77295cb2d8ace9 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3645,7 +3645,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 d49b401d4fefcc7c3a16f17a35a375f2ed155436..92bed3f02b397f7ee02607edb16475188c3bb7b0 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);
@@ -111,6 +113,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 fc7acff4d25e5833de2382ca3d378db2ed9152db..b138cc16126c0e691287744cb9135517378d23d6 100644 (file)
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -164,11 +164,21 @@ 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 ATTRIBUTE_UNUSED)
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr)
 {
+    int ret = 0;
+
     /* XXX Grab our own mutex here instead of relying on caller's mutex */
+    if (mgr->drv->preFork) {
+        ret = mgr->drv->preFork(mgr);
+    }
+
+    return ret;
 }
 
 

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index d1a59974ea9e9ecd7facb0867a83f6a7347a5168..be8774dcaccacf03e973080e96a03136e1bb3ae0 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -46,7 +46,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 91401c6c7c67aa10468cb2ab6fdae99963d4cc80..e8133c44ed070010532f49626260840d6bdf68ee 100644 (file)
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -113,6 +113,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)
@@ -500,6 +521,8 @@ virSecurityDriver virSecurityDriverStack = {
     .getModel                           = virSecurityStackGetModel,
     .getDOI                             = virSecurityStackGetDOI,
 
+    .preFork                            = virSecurityStackPreFork,
+
     .domainSecurityVerify               = virSecurityStackVerify,
 
     .domainSetSecurityImageLabel        = virSecurityStackSetSecurityImageLabel,