security: add new virSecurityManagerSetChildProcessLabel API

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 security driver, it calls
virCommandSetSELinuxLabel() to save a copy of the char* that will be
sent to setexeccon_raw() *after forking the child process*.

3) for the AppArmor security drivers, it calls
virCommandSetAppArmorProfile() to save a copy of the char* that will
be sent to aa_change_profile() *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.)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8241e6f81ff24ba8c3afccb07ce72290dd108cd0..dcdcb672a0bc9d954637a07dd191382684441851 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1068,6 +1068,7 @@ virSecurityManagerRestoreHostdevLabel;
 virSecurityManagerRestoreImageLabel;
 virSecurityManagerRestoreSavedStateLabel;
 virSecurityManagerSetAllLabel;
+virSecurityManagerSetChildProcessLabel;
 virSecurityManagerSetDaemonSocketLabel;
 virSecurityManagerSetHostdevLabel;
 virSecurityManagerSetHugepages;

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 532b21b7e9d4adaa6371c683728663f64576e277..ddc1fe4b4b6cba597fb049b87506bba756070a2d 100644 (file)
--- 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
@@ -627,6 +627,45 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
     return rc;
 }
 
+/* Called directly by API user prior to virCommandRun().
+ * virCommandRun() will then call aa_change_profile() (if a
+ * cmd->appArmorProfile has been set) *after forking the child
+ * process*.
+ */
+static int
+AppArmorSetSecurityChildProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+                                     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(SECURITY_APPARMOR_NAME, secdef->model)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("security label driver mismatch: "
+                         "\'%s\' model configured for domain, but "
+                         "hypervisor driver is \'%s\'."),
+                       secdef->model, SECURITY_APPARMOR_NAME);
+        if (use_apparmor() > 0)
+            goto cleanup;
+    }
+
+    if ((profile_name = get_profile_name(def)) == NULL)
+        goto cleanup;
+
+    virCommandSetAppArmorProfile(cmd, profile_name);
+    rc = 0;
+
+  cleanup:
+    VIR_FREE(profile_name);
+    return rc;
+}
+
 static int
 AppArmorSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                      virDomainDefPtr vm ATTRIBUTE_UNUSED)
@@ -927,6 +966,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 ae489e2f0696e5f4638a49d66aea261988bbdd03..b115bb0157cf99afe9216c152ee022a3e51c9dde 100644 (file)
--- 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
@@ -877,6 +877,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 6b775ab343cf51999e0a5b7e00c678b32b1a6211..cc401e116b9976b3558aba7c3f6990203431210b 100644 (file)
--- 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 50962bafb420f63f9d5810aa0526773847ca3d3e..c621366439c57a9508c069dc630ee82e5a7384dc 100644 (file)
--- 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-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
@@ -571,6 +571,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 8e8accf51fc10dfbf9de063a8f2b9098309a0148..711b354e8ab8914af665b847c9c0ad1df8b02be8 100644 (file)
--- 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-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;
@@ -103,6 +104,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 b6eb3f696f39846a849555a6d77f53cc65562021..2b9767ec29e626e686b81b6d92ba7099a61b1fac 100644 (file)
--- 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-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 8173b20667086f15dab9b436e926e74c008006b6..a61e0f0e3a018d610c7ff7e09bb6c7824f71c553 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1858,6 +1858,37 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN
     return 0;
 }
 
+static int
+virSecuritySELinuxSetSecurityChildProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+                                               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(SECURITY_SELINUX_NAME, secdef->model)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("security label driver mismatch: "
+                         "'%s' model configured for domain, but "
+                         "hypervisor driver is '%s'."),
+                       secdef->model, SECURITY_SELINUX_NAME);
+        if (security_getenforce() == 1)
+            return -1;
+    }
+
+    /* save in cmd to be set after fork/before child process is exec'ed */
+    virCommandSetSELinuxLabel(cmd, secdef->label);
+    return 0;
+}
+
 static int
 virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
                                                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 e2d0b1db75acb656e915800e9c766678d71bef1c..14d757dba64dece71ee2497e183428f374a579b7 100644 (file)
--- 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-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
@@ -366,6 +366,23 @@ virSecurityStackSetProcessLabel(virSecurityManagerPtr mgr,
     return rc;
 }
 
+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,
@@ -540,6 +557,7 @@ virSecurityDriver virSecurityDriverStack = {
 
     .domainGetSecurityProcessLabel      = virSecurityStackGetProcessLabel,
     .domainSetSecurityProcessLabel      = virSecurityStackSetProcessLabel,
+    .domainSetSecurityChildProcessLabel = virSecurityStackSetChildProcessLabel,
 
     .domainSetSecurityAllLabel          = virSecurityStackSetSecurityAllLabel,
     .domainRestoreSecurityAllLabel      = virSecurityStackRestoreSecurityAllLabel,