[libvirt] [PATCH 10/15] security: add new virSecurityManagerSetChildProcessLabel API

Laine Stump laine at laine.org
Thu Feb 7 21:37:51 UTC 2013


The existing virSecurityManagerSetProcessLabel() API is designed so
that it must be called after forking the child process, but before
exec'ing the child. Due to the way the virCommand API works, that
means it needs to be put in a "hook" function that virCommand is told
to call out to at that time.

Setting the child process label is a basic enough need when executing
any process that virCommand should have a method of doing that. But
virCommand must be told what label to set, and only the security
driver knows the answer to that question.

The new virSecurityManagerSet*Child*ProcessLabel() API is the way to
transfer the knowledge about what label to set from the security
driver to the virCommand object. It is given a virCommandPtr, and each
security driver calls the appropriate virCommand* API to tell
virCommand what to do between fork and exec.

1) in the case of the DAC security driver, it calls
virCommandSetUID/GID() to set a uid and gid that must be set for the
child process.

2) for the SELinux and AppArmor security drivers, it calls
virCommandSetSecLabel() to save a copy of the char* that will be sent
to each driver's respective "SetProcessLabel" API *after forking
the child process*.

With this new API in place, we will be able to remove
virSecurityManagerSetProcessLabel() from any virCommand pre-exec
hooks.

(Unfortunately, the LXC driver uses clone() rather than virCommand, so
it can't take advantage of this new security driver API, meaning that
we need to keep around the older virSecurityManagerSetProcessLabel(),
at least for now.)
---
 src/libvirt_private.syms         |  1 +
 src/security/security_apparmor.c | 41 +++++++++++++++++++++++++++++++++++++++-
 src/security/security_dac.c      | 24 ++++++++++++++++++++++-
 src/security/security_driver.h   |  6 +++++-
 src/security/security_manager.c  | 13 ++++++++++++-
 src/security/security_manager.h  |  6 +++++-
 src/security/security_nop.c      | 10 +++++++++-
 src/security/security_selinux.c  | 34 ++++++++++++++++++++++++++++++++-
 src/security/security_stack.c    | 20 +++++++++++++++++++-
 9 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4ac2d52..e50b63d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1066,6 +1066,7 @@ virSecurityManagerRestoreHostdevLabel;
 virSecurityManagerRestoreImageLabel;
 virSecurityManagerRestoreSavedStateLabel;
 virSecurityManagerSetAllLabel;
+virSecurityManagerSetChildProcessLabel;
 virSecurityManagerSetDaemonSocketLabel;
 virSecurityManagerSetHostdevLabel;
 virSecurityManagerSetHugepages;
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index bf795b0..4a81118 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -1,7 +1,7 @@
 /*
  * AppArmor security driver for libvirt
  *
- * Copyright (C) 2011 Red Hat, Inc.
+ * Copyright (C) 2011, 2013 Red Hat, Inc.
  * Copyright (C) 2009-2010 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or
@@ -626,6 +626,44 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def)
     return rc;
 }
 
+/* Called directly by API user prior to virCommandRun(). virCommandRun()
+ * will then call aa_change_profile() (if a cmd->seclabel has been set)
+ * *after forking the child process*.
+ */
+static int
+AppArmorSetSecurityChildProcessLabel(virSecurityManagerPtr mgr,
+                                     virDomainDefPtr def,
+                                     virCommandPtr cmd)
+{
+    int rc = -1;
+    char *profile_name = NULL;
+    const virSecurityLabelDefPtr secdef =
+        virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
+
+    if (!secdef)
+        goto cleanup;
+
+    if (STRNEQ(virSecurityManagerGetModel(mgr), secdef->model)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("security label driver mismatch: "
+                         "\'%s\' model configured for domain, but "
+                         "hypervisor driver is \'%s\'."),
+                       secdef->model, virSecurityManagerGetModel(mgr));
+        if (use_apparmor() > 0)
+            goto cleanup;
+    }
+
+    if ((profile_name = get_profile_name(def)) == NULL)
+        goto cleanup;
+
+    virCommandSetSecLabel(cmd, profile_name);
+    rc = 0;
+
+  cleanup:
+    VIR_FREE(profile_name);
+    return rc;
+}
+
 static int
 AppArmorSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                      virDomainDefPtr vm ATTRIBUTE_UNUSED)
@@ -926,6 +964,7 @@ virSecurityDriver virAppArmorSecurityDriver = {
 
     .domainGetSecurityProcessLabel      = AppArmorGetSecurityProcessLabel,
     .domainSetSecurityProcessLabel      = AppArmorSetSecurityProcessLabel,
+    .domainSetSecurityChildProcessLabel = AppArmorSetSecurityChildProcessLabel,
 
     .domainSetSecurityAllLabel          = AppArmorSetSecurityAllLabel,
     .domainRestoreSecurityAllLabel      = AppArmorRestoreSecurityAllLabel,
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index ae489e2..b115bb0 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -878,6 +878,27 @@ virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr,
 
 
 static int
+virSecurityDACSetChildProcessLabel(virSecurityManagerPtr mgr,
+                                   virDomainDefPtr def ATTRIBUTE_UNUSED,
+                                   virCommandPtr cmd)
+{
+    uid_t user;
+    gid_t group;
+    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+    if (virSecurityDACGetIds(def, priv, &user, &group))
+        return -1;
+
+    VIR_DEBUG("Setting child to drop privileges of DEF to %u:%u",
+              (unsigned int) user, (unsigned int) group);
+
+    virCommandSetUID(cmd, user);
+    virCommandSetGID(cmd, group);
+    return 0;
+}
+
+
+static int
 virSecurityDACVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                      virDomainDefPtr def ATTRIBUTE_UNUSED)
 {
@@ -1072,6 +1093,7 @@ virSecurityDriver virSecurityDriverDAC = {
 
     .domainGetSecurityProcessLabel      = virSecurityDACGetProcessLabel,
     .domainSetSecurityProcessLabel      = virSecurityDACSetProcessLabel,
+    .domainSetSecurityChildProcessLabel = virSecurityDACSetChildProcessLabel,
 
     .domainSetSecurityAllLabel          = virSecurityDACSetSecurityAllLabel,
     .domainRestoreSecurityAllLabel      = virSecurityDACRestoreSecurityAllLabel,
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 6b775ab..73d4d3b 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008, 2010 Red Hat, Inc.
+ * Copyright (C) 2008, 2010, 2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -92,6 +92,9 @@ typedef int (*virSecurityDomainGetProcessLabel) (virSecurityManagerPtr mgr,
                                                  virSecurityLabelPtr sec);
 typedef int (*virSecurityDomainSetProcessLabel) (virSecurityManagerPtr mgr,
                                                  virDomainDefPtr def);
+typedef int (*virSecurityDomainSetChildProcessLabel) (virSecurityManagerPtr mgr,
+                                                      virDomainDefPtr def,
+                                                      virCommandPtr cmd);
 typedef int (*virSecurityDomainSecurityVerify) (virSecurityManagerPtr mgr,
                                                 virDomainDefPtr def);
 typedef int (*virSecurityDomainSetImageFDLabel) (virSecurityManagerPtr mgr,
@@ -131,6 +134,7 @@ struct _virSecurityDriver {
 
     virSecurityDomainGetProcessLabel domainGetSecurityProcessLabel;
     virSecurityDomainSetProcessLabel domainSetSecurityProcessLabel;
+    virSecurityDomainSetChildProcessLabel domainSetSecurityChildProcessLabel;
 
     virSecurityDomainSetAllLabel domainSetSecurityAllLabel;
     virSecurityDomainRestoreAllLabel domainRestoreSecurityAllLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 593c00b..7fb75dd 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1,7 +1,7 @@
 /*
  * security_manager.c: Internal security manager API
  *
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2011, 2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -437,6 +437,17 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr,
     return -1;
 }
 
+int virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr,
+                                           virDomainDefPtr vm,
+                                           virCommandPtr cmd)
+{
+    if (mgr->drv->domainSetSecurityChildProcessLabel)
+       return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm, cmd);
+
+    virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    return -1;
+}
+
 int virSecurityManagerVerify(virSecurityManagerPtr mgr,
                              virDomainDefPtr def)
 {
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index dc09c7c..f0fdd15 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -1,7 +1,7 @@
 /*
  * security_manager.h: Internal security manager API
  *
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2011, 2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -24,6 +24,7 @@
 # define VIR_SECURITY_MANAGER_H__
 
 # include "domain_conf.h"
+# include "vircommand.h"
 
 typedef struct _virSecurityManager virSecurityManager;
 typedef virSecurityManager *virSecurityManagerPtr;
@@ -102,6 +103,9 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
                                       virSecurityLabelPtr sec);
 int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr def);
+int virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr,
+                                           virDomainDefPtr def,
+                                           virCommandPtr cmd);
 int virSecurityManagerVerify(virSecurityManagerPtr mgr,
                              virDomainDefPtr def);
 int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr,
diff --git a/src/security/security_nop.c b/src/security/security_nop.c
index b6eb3f6..29ead09 100644
--- a/src/security/security_nop.c
+++ b/src/security/security_nop.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2011, 2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -157,6 +157,13 @@ static int virSecurityDomainSetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBU
     return 0;
 }
 
+static int virSecurityDomainSetChildProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+                                                    virDomainDefPtr vm ATTRIBUTE_UNUSED,
+                                                    virCommandPtr cmd ATTRIBUTE_UNUSED)
+{
+    return 0;
+}
+
 static int virSecurityDomainVerifyNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                       virDomainDefPtr def ATTRIBUTE_UNUSED)
 {
@@ -207,6 +214,7 @@ virSecurityDriver virSecurityDriverNop = {
 
     .domainGetSecurityProcessLabel      = virSecurityDomainGetProcessLabelNop,
     .domainSetSecurityProcessLabel      = virSecurityDomainSetProcessLabelNop,
+    .domainSetSecurityChildProcessLabel = virSecurityDomainSetChildProcessLabelNop,
 
     .domainSetSecurityAllLabel          = virSecurityDomainSetAllLabelNop,
     .domainRestoreSecurityAllLabel      = virSecurityDomainRestoreAllLabelNop,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2f5012d..2f25bc8 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2012 Red Hat, Inc.
+ * Copyright (C) 2008-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1859,6 +1859,37 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr,
 }
 
 static int
+virSecuritySELinuxSetSecurityChildProcessLabel(virSecurityManagerPtr mgr,
+                                               virDomainDefPtr def,
+                                               virCommandPtr cmd)
+{
+    /* TODO: verify DOI */
+    virSecurityLabelDefPtr secdef;
+
+    secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+    if (secdef == NULL)
+        return -1;
+
+    if (secdef->label == NULL)
+        return 0;
+
+    VIR_DEBUG("label=%s", secdef->label);
+    if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("security label driver mismatch: "
+                         "'%s' model configured for domain, but "
+                         "hypervisor driver is '%s'."),
+                       secdef->model, virSecurityManagerGetModel(mgr));
+        if (security_getenforce() == 1)
+            return -1;
+    }
+
+    /* save in cmd to be set after fork/before child process is exec'ed */
+    virCommandSetSecLabel(cmd, secdef->label);
+    return 0;
+}
+
+static int
 virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr,
                                                virDomainDefPtr def)
 {
@@ -2261,6 +2292,7 @@ virSecurityDriver virSecurityDriverSELinux = {
 
     .domainGetSecurityProcessLabel      = virSecuritySELinuxGetSecurityProcessLabel,
     .domainSetSecurityProcessLabel      = virSecuritySELinuxSetSecurityProcessLabel,
+    .domainSetSecurityChildProcessLabel = virSecuritySELinuxSetSecurityChildProcessLabel,
 
     .domainSetSecurityAllLabel          = virSecuritySELinuxSetSecurityAllLabel,
     .domainRestoreSecurityAllLabel      = virSecuritySELinuxRestoreSecurityAllLabel,
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 8e1e5f9..dd321f7 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2011, 2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -367,6 +367,23 @@ virSecurityStackSetProcessLabel(virSecurityManagerPtr mgr,
 }
 
 static int
+virSecurityStackSetChildProcessLabel(virSecurityManagerPtr mgr,
+                                     virDomainDefPtr vm,
+                                     virCommandPtr cmd)
+{
+    virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+    virSecurityStackItemPtr item = priv->itemsHead;
+    int rc = 0;
+
+    for (; item; item = item->next) {
+        if (virSecurityManagerSetChildProcessLabel(item->securityManager, vm, cmd) < 0)
+            rc = -1;
+    }
+
+    return rc;
+}
+
+static int
 virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr,
                                 virDomainDefPtr vm,
                                 pid_t pid,
@@ -540,6 +557,7 @@ virSecurityDriver virSecurityDriverStack = {
 
     .domainGetSecurityProcessLabel      = virSecurityStackGetProcessLabel,
     .domainSetSecurityProcessLabel      = virSecurityStackSetProcessLabel,
+    .domainSetSecurityChildProcessLabel = virSecurityStackSetChildProcessLabel,
 
     .domainSetSecurityAllLabel          = virSecurityStackSetSecurityAllLabel,
     .domainRestoreSecurityAllLabel      = virSecurityStackRestoreSecurityAllLabel,
-- 
1.8.1




More information about the libvir-list mailing list