2:15 p.m.
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.)
---
Change from V1: rebased, Copyright dates simplified.
src/libvirt_private.syms | 1 +
src/security/security_apparmor.c | 42 +++++++++++++++++++++++++++++++++++++++-
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 | 32 ++++++++++++++++++++++++++++++
src/security/security_stack.c | 20 ++++++++++++++++++-
9 files changed, 147 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8241e6f..dcdcb67 100644
--- 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 532b21b..ddc1fe4 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
@@ -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 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..cc401e1 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 50962ba..c621366 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-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 8e8accf..711b354 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-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 b6eb3f6..2b9767e 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-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 8173b20..a61e0f0 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1859,6 +1859,37 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UN
}
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 e2d0b1d..14d757d 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-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