[libvirt] [PATCHv2 1/2] security: framework for driver PreFork handler

[Eric Blake]

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 at redhat.com>
---
 src/qemu/qemu_process.c         |  3 ++-
 src/security/security_driver.h  |  4 ++++
 src/security/security_manager.c | 16 ++++++++++++++--
 src/security/security_manager.h |  2 +-
 src/security/security_stack.c   | 23 +++++++++++++++++++++++
 5 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d5e8f6..a594cc6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3730,7 +3730,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 cc401e1..8735558 100644
--- 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 411a909..92fb504 100644
--- 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 711b354..9252830 100644
--- 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 1e229d7..ed69b9c 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -112,6 +112,27 @@ virSecurityStackGetDOI(virSecurityManagerPtr 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,
-- 
1.8.3.1