10:33 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Curently security labels can be of type 'dynamic' or 'static'.
If no security label is given, then 'dynamic' is assumed. The
current code takes advantage of this default, and avoids even
saving <seclabel> elements with type='dynamic' to disk. This
means if you temporarily change security driver, the guests
can all still start.
With the introduce of sVirt to LXC though, there needs to be
a new default of 'none' to allow unconfined LXC containers.
This patch introduces two new security label types
- default: the host configuration decides whether to run the
guest with type 'none' or 'dynamic' at guest start
- none: the guest will run unconfined by security policy
The 'none' label type will obviously be undesirable for some
deployments, so a new qemu.conf option allows a host admin to
mandate confined guests. It is also possible to turn off default
confinement
security_default_confined = 1|0 (default == 1)
security_require_confined = 1|0 (default == 0)
* src/conf/domain_conf.c, src/conf/domain_conf.h: Add new
seclabel types
* src/security/security_manager.c, src/security/security_manager.h:
Set default sec label types
* src/security/security_selinux.c: Handle 'none' seclabel type
* src/qemu/qemu.conf, src/qemu/qemu_conf.c, src/qemu/qemu_conf.h,
src/qemu/libvirtd_qemu.aug: New security config options
* src/qemu/qemu_driver.c: Tell security driver about default
config
---
src/conf/domain_conf.c | 81 +++++++++++++++++++++++----------------
src/conf/domain_conf.h | 2 +
src/qemu/libvirtd_qemu.aug | 2 +
src/qemu/qemu.conf | 8 ++++
src/qemu/qemu_conf.c | 11 +++++
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_driver.c | 7 +++-
src/security/security_manager.c | 51 ++++++++++++++++++++++--
src/security/security_manager.h | 8 +++-
src/security/security_selinux.c | 28 ++++++++++----
10 files changed, 152 insertions(+), 48 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4ae417e..a2b7430 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -551,6 +551,8 @@ VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST,
"unknown")
VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
+ "default",
+ "none",
"dynamic",
"static")
@@ -2547,13 +2549,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
"%s", _("missing security type"));
goto error;
}
+
def->type = virDomainSeclabelTypeFromString(p);
VIR_FREE(p);
- if (def->type < 0) {
+ if (def->type <= 0) {
virDomainReportError(VIR_ERR_XML_ERROR,
"%s", _("invalid security type"));
goto error;
}
+
p = virXPathStringLimit("string(./seclabel/@relabel)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p != NULL) {
@@ -2574,13 +2578,23 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
"%s", _("dynamic label type must use
resource relabeling"));
goto error;
}
+ if (def->type == VIR_DOMAIN_SECLABEL_NONE &&
+ !def->norelabel) {
+ virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("dynamic label type must use not
resource relabeling"));
+ goto error;
+ }
} else {
- if (def->type == VIR_DOMAIN_SECLABEL_STATIC)
+ if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
+ def->type == VIR_DOMAIN_SECLABEL_NONE)
def->norelabel = true;
else
def->norelabel = false;
}
+ if (def->type == VIR_DOMAIN_SECLABEL_NONE)
+ return 0;
+
/* Only parse label, if using static labels, or
* if the 'live' VM XML is requested
*/
@@ -9810,37 +9824,41 @@ virDomainLifecycleDefFormat(virBufferPtr buf,
}
-static int
-virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def,
- unsigned int flags)
+static void
+virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def)
{
const char *sectype = virDomainSeclabelTypeToString(def->type);
- int ret = -1;
if (!sectype)
- goto cleanup;
+ return;
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
- !def->baselabel &&
- (flags & VIR_DOMAIN_XML_INACTIVE)) {
- /* This is the default for inactive xml, so nothing to output. */
- } else {
- virBufferAsprintf(buf, "<seclabel type='%s' model='%s'
relabel='%s'>\n",
- sectype, def->model,
- def->norelabel ? "no" : "yes");
- virBufferEscapeString(buf, " <label>%s</label>\n",
- def->label);
- if (!def->norelabel)
- virBufferEscapeString(buf, "
<imagelabel>%s</imagelabel>\n",
- def->imagelabel);
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
- virBufferEscapeString(buf, "
<baselabel>%s</baselabel>\n",
- def->baselabel);
- virBufferAddLit(buf, "</seclabel>\n");
+ if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT)
+ return;
+
+ virBufferAsprintf(buf, "<seclabel type='%s'",
+ sectype);
+ if (def->model)
+ virBufferEscapeString(buf, " model='%s' ", def->model);
+
+ virBufferAsprintf(buf, " relabel='%s'",
+ def->norelabel ? "no" : "yes");
+
+ if (def->type == VIR_DOMAIN_SECLABEL_NONE) {
+ virBufferAddLit(buf, "/>\n");
+ return;
}
- ret = 0;
-cleanup:
- return ret;
+
+ virBufferAddLit(buf, ">\n");
+
+ virBufferEscapeString(buf, " <label>%s</label>\n",
+ def->label);
+ if (!def->norelabel)
+ virBufferEscapeString(buf, "
<imagelabel>%s</imagelabel>\n",
+ def->imagelabel);
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
+ virBufferEscapeString(buf, "
<baselabel>%s</baselabel>\n",
+ def->baselabel);
+ virBufferAddLit(buf, "</seclabel>\n");
}
@@ -11811,12 +11829,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAddLit(buf, " </devices>\n");
- if (def->seclabel.model) {
- virBufferAdjustIndent(buf, 2);
- if (virSecurityLabelDefFormat(buf, &def->seclabel, flags) < 0)
- goto cleanup;
- virBufferAdjustIndent(buf, -2);
- }
+ virBufferAdjustIndent(buf, 2);
+ virSecurityLabelDefFormat(buf, &def->seclabel);
+ virBufferAdjustIndent(buf, -2);
if (def->namespaceData && def->ns.format) {
if ((def->ns.format)(buf, def->namespaceData) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 25aae62..d3a1175 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -162,6 +162,8 @@ struct _virDomainDeviceInfo {
};
enum virDomainSeclabelType {
+ VIR_DOMAIN_SECLABEL_DEFAULT,
+ VIR_DOMAIN_SECLABEL_NONE,
VIR_DOMAIN_SECLABEL_DYNAMIC,
VIR_DOMAIN_SECLABEL_STATIC,
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 078e9c4..f6cec1f 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -33,6 +33,8 @@ module Libvirtd_qemu =
| bool_entry "vnc_sasl"
| str_entry "vnc_sasl_dir"
| str_entry "security_driver"
+ | bool_entry "security_default_confined"
+ | bool_entry "security_require_confined"
| str_entry "user"
| str_entry "group"
| bool_entry "dynamic_ownership"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 4ec5e6c..d32a4c3 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -138,6 +138,14 @@
#
# security_driver = "selinux"
+# If set to non-zero, then the default security labelling
+# will make guests confined. If set to zero, then guests
+# will be unconfined by default. Defaults to 1
+# security_default_confined = 1
+
+# If set to non-zero, then attempts to create unconfined
+# guests will be blocked. Defaults to zero.
+# security_require_confined = 1
# The user ID for QEMU processes run by the system instance.
#user = "root"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index bc0a646..e95c7a5 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -75,6 +75,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
int i;
/* Setup critical defaults */
+ driver->securityDefaultConfined = true;
+ driver->securityRequireConfined = false;
driver->dynamicOwnership = 1;
driver->clearEmulatorCapabilities = 1;
@@ -195,6 +197,15 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
}
}
+ p = virConfGetValue (conf, "security_default_confined");
+ CHECK_TYPE ("security_default_confined", VIR_CONF_LONG);
+ if (p) driver->securityDefaultConfined = p->l;
+
+ p = virConfGetValue (conf, "security_require_confined");
+ CHECK_TYPE ("security_require_confined", VIR_CONF_LONG);
+ if (p) driver->securityRequireConfined = p->l;
+
+
p = virConfGetValue (conf, "vnc_sasl");
CHECK_TYPE ("vnc_sasl", VIR_CONF_LONG);
if (p) driver->vncSASL = p->l;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d8d7915..fb98ef2 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -115,6 +115,8 @@ struct qemud_driver {
virDomainEventStatePtr domainEventState;
char *securityDriverName;
+ bool securityDefaultConfined;
+ bool securityRequireConfined;
virSecurityManagerPtr securityManager;
char *saveImageFormat;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 712f1fc..b697f43 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -208,7 +208,10 @@ static int
qemuSecurityInit(struct qemud_driver *driver)
{
virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName,
-
driver->allowDiskFormatProbing);
+ driver->allowDiskFormatProbing,
+
driver->securityDefaultConfined,
+
driver->securityRequireConfined);
+
if (!mgr)
goto error;
@@ -216,6 +219,8 @@ qemuSecurityInit(struct qemud_driver *driver)
virSecurityManagerPtr dac = virSecurityManagerNewDAC(driver->user,
driver->group,
driver->allowDiskFormatProbing,
+
driver->securityDefaultConfined,
+
driver->securityRequireConfined,
driver->dynamicOwnership);
if (!dac)
goto error;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 2e4956a..d0bafae 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -36,10 +36,14 @@
struct _virSecurityManager {
virSecurityDriverPtr drv;
bool allowDiskFormatProbing;
+ bool defaultConfined;
+ bool requireConfined;
};
static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv,
- bool allowDiskFormatProbing)
+ bool allowDiskFormatProbing,
+ bool defaultConfined,
+ bool requireConfined)
{
virSecurityManagerPtr mgr;
@@ -50,6 +54,8 @@ static virSecurityManagerPtr
virSecurityManagerNewDriver(virSecurityDriverPtr dr
mgr->drv = drv;
mgr->allowDiskFormatProbing = allowDiskFormatProbing;
+ mgr->defaultConfined = defaultConfined;
+ mgr->requireConfined = requireConfined;
if (drv->open(mgr) < 0) {
virSecurityManagerFree(mgr);
@@ -64,7 +70,9 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr
primary,
{
virSecurityManagerPtr mgr =
virSecurityManagerNewDriver(&virSecurityDriverStack,
-
virSecurityManagerGetAllowDiskFormatProbing(primary));
+
virSecurityManagerGetAllowDiskFormatProbing(primary),
+ virSecurityManagerGetDefaultConfined(primary),
+ virSecurityManagerGetRequireConfined(primary));
if (!mgr)
return NULL;
@@ -78,11 +86,15 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr
primary,
virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user,
gid_t group,
bool allowDiskFormatProbing,
+ bool defaultConfined,
+ bool requireConfined,
bool dynamicOwnership)
{
virSecurityManagerPtr mgr =
virSecurityManagerNewDriver(&virSecurityDriverDAC,
- allowDiskFormatProbing);
+ allowDiskFormatProbing,
+ defaultConfined,
+ requireConfined);
if (!mgr)
return NULL;
@@ -95,13 +107,18 @@ virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user,
}
virSecurityManagerPtr virSecurityManagerNew(const char *name,
- bool allowDiskFormatProbing)
+ bool allowDiskFormatProbing,
+ bool defaultConfined,
+ bool requireConfined)
{
virSecurityDriverPtr drv = virSecurityDriverLookup(name);
if (!drv)
return NULL;
- return virSecurityManagerNewDriver(drv, allowDiskFormatProbing);
+ return virSecurityManagerNewDriver(drv,
+ allowDiskFormatProbing,
+ defaultConfined,
+ requireConfined);
}
@@ -149,6 +166,16 @@ bool
virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr)
return mgr->allowDiskFormatProbing;
}
+bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr)
+{
+ return mgr->defaultConfined;
+}
+
+bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr)
+{
+ return mgr->requireConfined;
+}
+
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virDomainDiskDefPtr disk)
@@ -248,6 +275,20 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr
mgr,
int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm)
{
+ if (vm->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) {
+ if (mgr->defaultConfined)
+ vm->seclabel.type = VIR_DOMAIN_SECLABEL_DYNAMIC;
+ else
+ vm->seclabel.type = VIR_DOMAIN_SECLABEL_NONE;
+ }
+
+ if ((vm->seclabel.type == VIR_DOMAIN_SECLABEL_NONE) &&
+ mgr->requireConfined) {
+ virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unconfined guests are not allowed on this
host"));
+ return -1;
+ }
+
if (mgr->drv->domainGenSecurityLabel)
return mgr->drv->domainGenSecurityLabel(mgr, vm);
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 6731d59..32c8c3b 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -32,7 +32,9 @@ typedef struct _virSecurityManager virSecurityManager;
typedef virSecurityManager *virSecurityManagerPtr;
virSecurityManagerPtr virSecurityManagerNew(const char *name,
- bool allowDiskFormatProbing);
+ bool allowDiskFormatProbing,
+ bool defaultConfined,
+ bool requireConfined);
virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary,
virSecurityManagerPtr secondary);
@@ -40,6 +42,8 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr
primary,
virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user,
gid_t group,
bool allowDiskFormatProbing,
+ bool defaultConfined,
+ bool requireConfined,
bool dynamicOwnership);
void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr);
@@ -49,6 +53,8 @@ void virSecurityManagerFree(virSecurityManagerPtr mgr);
const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr);
const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr);
bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr);
+bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr);
+bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr);
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c2dceca..ade36f9 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -170,6 +170,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
int c1 = 0;
int c2 = 0;
context_t ctx = NULL;
+ const char *range;
if ((def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) &&
!def->seclabel.baselabel &&
@@ -200,7 +201,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return rc;
}
- if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) {
+ switch (def->seclabel.type) {
+ case VIR_DOMAIN_SECLABEL_STATIC:
if (!(ctx = context_new(def->seclabel.label)) ) {
virReportSystemError(errno,
_("unable to allocate socket security context
'%s'"),
@@ -208,13 +210,15 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return rc;
}
- const char *range = context_range_get(ctx);
+ range = context_range_get(ctx);
if (!range ||
!(mcs = strdup(range))) {
virReportOOMError();
goto cleanup;
}
- } else {
+ break;
+
+ case VIR_DOMAIN_SECLABEL_DYNAMIC:
do {
c1 = virRandom(1024);
c2 = virRandom(1024);
@@ -246,12 +250,20 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
_("cannot generate selinux context for %s"),
mcs);
goto cleanup;
}
+ break;
+
+ case VIR_DOMAIN_SECLABEL_NONE:
+ /* no op */
+ break;
}
- def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs);
- if (!def->seclabel.imagelabel) {
- virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot generate selinux context for %s"),
mcs);
- goto cleanup;
+
+ if (!def->seclabel.norelabel) {
+ def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs);
+ if (!def->seclabel.imagelabel) {
+ virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot generate selinux context for %s"),
mcs);
+ goto cleanup;
+ }
}
if (!def->seclabel.model &&
--
1.7.7.5