8:12 a.m.
This is an update of
https://www.redhat.com/archives/libvir-list/2012-January/msg00418.html
Changes since v1:
- Pushed the first 2 patches which passed review
- Update to include all Eric's suggested changes
- Rebase to latest GIT master
8:12 a.m.
New subject: [libvirt] [PATCH v2 1/5] Revert changes to sec label parsing
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Revert parsing changes:
commit 302fe95ffa1bc5f1c61c0beb31a1adfbc38c668e
Author: Eric Blake <eblake(a)redhat.com>
Date: Wed Jan 4 16:01:24 2012 -0700
seclabel: fix regression in libvirtd restart
commit b43432931aef92325920953ff92beabfbe5224c8
Author: Eric Blake <eblake(a)redhat.com>
Date: Thu Dec 22 17:47:50 2011 -0700
seclabel: allow a seclabel override on a disk src
These two commits changed the sec label parsing code so that
the same code dealt with both the VM level sec label, and the
per device label. Unfortunately, as we add more options to the
VM level sec label, the logic required to use the same parsing
code for the per device label becomes unintelligible.
* src/conf/domain_conf.c: Remove support for parsing per
device sec labels
---
src/conf/domain_conf.c | 190 ++++++++++++-----------------------------------
1 files changed, 49 insertions(+), 141 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e872396..6c67cd8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2011 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -809,15 +809,6 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def)
VIR_FREE(def->baselabel);
}
-static void
-virSecurityLabelDefFree(virSecurityLabelDefPtr def)
-{
- if (!def)
- return;
- virSecurityLabelDefClear(def);
- VIR_FREE(def);
-}
-
void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
{
int ii;
@@ -887,7 +878,6 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def->serial);
VIR_FREE(def->src);
- virSecurityLabelDefFree(def->seclabel);
VIR_FREE(def->dst);
VIR_FREE(def->driverName);
VIR_FREE(def->driverType);
@@ -2508,33 +2498,31 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr
def)
return 0;
}
-/* Parse the portion of a SecurityLabel that is common to both the
- * top-level <seclabel> and to a per-device override.
- * default_seclabel is NULL for top-level, or points to the top-level
- * when parsing an override. */
static int
-virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
- xmlNodePtr node,
- xmlXPathContextPtr ctxt,
- virSecurityLabelDefPtr default_seclabel,
- unsigned int flags)
+virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
{
char *p;
- xmlNodePtr save_ctxt = ctxt->node;
- int ret = -1;
- int type = default_seclabel ? default_seclabel->type : def->type;
- ctxt->node = node;
+ if (virXPathNode("./seclabel", ctxt) == NULL)
+ return 0;
- /* Can't use overrides if top-level doesn't allow relabeling. */
- if (default_seclabel && default_seclabel->norelabel) {
- virDomainReportError(VIR_ERR_XML_ERROR, "%s",
- _("label overrides require relabeling to be "
- "enabled at the domain level"));
- goto cleanup;
+ p = virXPathStringLimit("string(./seclabel/@type)",
+ VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+ if (p == NULL) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing security type"));
+ goto error;
}
-
- p = virXPathStringLimit("string(./@relabel)",
+ def->type = virDomainSeclabelTypeFromString(p);
+ VIR_FREE(p);
+ 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) {
if (STREQ(p, "yes")) {
@@ -2545,77 +2533,38 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
virDomainReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"), p);
VIR_FREE(p);
- goto cleanup;
+ goto error;
}
VIR_FREE(p);
- if (!default_seclabel &&
- type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
def->norelabel) {
- virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("dynamic label type must use resource "
- "relabeling"));
- goto cleanup;
+ virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("dynamic label type must use
resource relabeling"));
+ goto error;
}
} else {
- if (!default_seclabel && type == VIR_DOMAIN_SECLABEL_STATIC)
+ if (def->type == VIR_DOMAIN_SECLABEL_STATIC)
def->norelabel = true;
else
def->norelabel = false;
}
/* Only parse label, if using static labels, or
- * if the 'live' VM XML is requested, or if this is a device override
+ * if the 'live' VM XML is requested
*/
- if (type == VIR_DOMAIN_SECLABEL_STATIC ||
- !(flags & VIR_DOMAIN_XML_INACTIVE) ||
- (default_seclabel && !def->norelabel)) {
- p = virXPathStringLimit("string(./label[1])",
+ if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
+ !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- if (p == NULL && !(default_seclabel && def->norelabel)) {
+ if (p == NULL) {
virDomainReportError(VIR_ERR_XML_ERROR,
"%s", _("security label is
missing"));
- goto cleanup;
+ goto error;
}
def->label = p;
}
- ret = 0;
-cleanup:
- ctxt->node = save_ctxt;
- return ret;
-}
-
-/* Parse the top-level <seclabel>, if present. */
-static int
-virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
- xmlXPathContextPtr ctxt,
- unsigned int flags)
-{
- char *p;
- xmlNodePtr node = virXPathNode("./seclabel", ctxt);
-
- if (node == NULL)
- return 0;
-
- p = virXPathStringLimit("string(./seclabel/@type)",
- VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- if (p == NULL) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security type"));
- goto error;
- }
- def->type = virDomainSeclabelTypeFromString(p);
- VIR_FREE(p);
- if (def->type < 0) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("invalid security type"));
- goto error;
- }
-
- if (virSecurityLabelDefParseXMLHelper(def, node, ctxt, NULL, flags) < 0)
- goto error;
-
/* Only parse imagelabel, if requested live XML with relabeling */
if (!def->norelabel &&
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
@@ -2741,7 +2690,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
virBitmapPtr bootMap,
- virSecurityLabelDefPtr default_seclabel,
unsigned int flags)
{
virDomainDiskDefPtr def;
@@ -3051,16 +2999,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
goto error;
}
- /* If source is present, check for an optional seclabel override. */
- if (source) {
- xmlNodePtr seclabel = virXPathNode("./source/seclabel", ctxt);
- if (seclabel &&
- (VIR_ALLOC(def->seclabel) < 0 ||
- virSecurityLabelDefParseXMLHelper(def->seclabel, seclabel, ctxt,
- default_seclabel, flags) < 0))
- goto error;
- }
-
if (target == NULL) {
virDomainReportError(VIR_ERR_NO_TARGET,
source ? "%s" : NULL, source);
@@ -6408,8 +6346,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
if (xmlStrEqual(node->name, BAD_CAST "disk")) {
dev->type = VIR_DOMAIN_DEVICE_DISK;
if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt,
- NULL, &def->seclabel,
- flags)))
+ NULL, flags)))
goto error;
} else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
dev->type = VIR_DOMAIN_DEVICE_LEASE;
@@ -7511,7 +7448,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
nodes[i],
ctxt,
bootMap,
- &def->seclabel,
flags);
if (!disk)
goto error;
@@ -9832,32 +9768,23 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr
def,
if (!sectype)
goto cleanup;
- if (def->model &&
- def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+ 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 {
- virBufferAddLit(buf, "<seclabel");
- if (def->model)
- virBufferAsprintf(buf, " type='%s' model='%s'",
- sectype, def->model);
- virBufferAsprintf(buf, " relabel='%s'",
+ virBufferAsprintf(buf, "<seclabel type='%s' model='%s'
relabel='%s'>\n",
+ sectype, def->model,
def->norelabel ? "no" : "yes");
- if (def->label || def->baselabel) {
- 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");
- } else {
- 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");
}
ret = 0;
cleanup:
@@ -9980,36 +9907,17 @@ virDomainDiskDefFormat(virBufferPtr buf,
def->startupPolicy) {
switch (def->type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
- virBufferAddLit(buf, " <source");
+ virBufferAsprintf(buf," <source");
if (def->src)
virBufferEscapeString(buf, " file='%s'", def->src);
if (def->startupPolicy)
virBufferEscapeString(buf, " startupPolicy='%s'",
startupPolicy);
- if (def->seclabel) {
- virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 8);
- if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0)
- return -1;
- virBufferAdjustIndent(buf, -8);
- virBufferAddLit(buf, " </source>\n");
- } else {
- virBufferAddLit(buf, "/>\n");
- }
+ virBufferAsprintf(buf, "/>\n");
break;
case VIR_DOMAIN_DISK_TYPE_BLOCK:
- if (def->src && def->seclabel) {
- virBufferEscapeString(buf, " <source
dev='%s'>\n",
- def->src);
- virBufferAdjustIndent(buf, 8);
- if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0)
- return -1;
- virBufferAdjustIndent(buf, -8);
- virBufferAddLit(buf, " </source>\n");
- } else {
- virBufferEscapeString(buf, " <source
dev='%s'/>\n",
- def->src);
- }
+ virBufferEscapeString(buf, " <source
dev='%s'/>\n",
+ def->src);
break;
case VIR_DOMAIN_DISK_TYPE_DIR:
virBufferEscapeString(buf, " <source
dir='%s'/>\n",
--
1.7.7.5
11:59 a.m.
New subject: [libvirt] [PATCH v2 1/5] Revert changes to sec label parsing
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Revert parsing changes:
commit 302fe95ffa1bc5f1c61c0beb31a1adfbc38c668e
Author: Eric Blake <eblake(a)redhat.com>
Date: Wed Jan 4 16:01:24 2012 -0700
seclabel: fix regression in libvirtd restart
commit b43432931aef92325920953ff92beabfbe5224c8
Author: Eric Blake <eblake(a)redhat.com>
Date: Thu Dec 22 17:47:50 2011 -0700
seclabel: allow a seclabel override on a disk src
These two commits changed the sec label parsing code so that
the same code dealt with both the VM level sec label, and the
per device label. Unfortunately, as we add more options to the
VM level sec label, the logic required to use the same parsing
code for the per device label becomes unintelligible.
* src/conf/domain_conf.c: Remove support for parsing per
device sec labels
---
src/conf/domain_conf.c | 190 ++++++++++++-----------------------------------
1 files changed, 49 insertions(+), 141 deletions(-)
ACK. The test suite fails on this patch (which could affect 'git
bisect'), but that is to be expected, and remedied shortly by the next
patch.
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2011 Red Hat, Inc.
This particular hunk should either be omitted from this patch, or
restored in the next patch.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:12 a.m.
New subject: [libvirt] [PATCH v2 2/5] Re-add domain device seclabel parsing / formatting
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This re-introduces parsing & formatting for per device seclabels.
There is a new virDomainDeviceSeclabelPtr struct and corresponding
APIs for parsing/formatting.
---
src/conf/domain_conf.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 12 ++++-
2 files changed, 139 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c67cd8..8eacc1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -809,6 +809,17 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def)
VIR_FREE(def->baselabel);
}
+
+static void
+virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
+{
+ if (!def)
+ return;
+ VIR_FREE(def->label);
+ VIR_FREE(def);
+}
+
+
void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
{
int ii;
@@ -887,6 +898,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
virStorageEncryptionFree(def->encryption);
virDomainDeviceInfoClear(&def->info);
+ virSecurityDeviceLabelDefFree(def->seclabel);
+
for (i = 0 ; i < def->nhosts ; i++)
virDomainDiskHostDefFree(&def->hosts[i]);
VIR_FREE(def->hosts);
@@ -2609,6 +2622,67 @@ error:
return -1;
}
+
+static int
+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
+ virSecurityLabelDefPtr vmDef,
+ xmlXPathContextPtr ctxt)
+{
+ char *p;
+
+ *def = NULL;
+
+ if (virXPathNode("./seclabel", ctxt) == NULL)
+ return 0;
+
+ /* Can't use overrides if top-level doesn't allow relabeling. */
+ if (vmDef && vmDef->norelabel) {
+ virDomainReportError(VIR_ERR_XML_ERROR, "%s",
+ _("label overrides require relabeling to be "
+ "enabled at the domain level"));
+ return -1;
+ }
+
+ if (VIR_ALLOC(*def) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ p = virXPathStringLimit("string(./seclabel/@relabel)",
+ VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+ if (p != NULL) {
+ if (STREQ(p, "yes")) {
+ (*def)->norelabel = false;
+ } else if (STREQ(p, "no")) {
+ (*def)->norelabel = true;
+ } else {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("invalid security relabel value %s"), p);
+ VIR_FREE(p);
+ VIR_FREE(*def);
+ return -1;
+ }
+ VIR_FREE(p);
+ } else {
+ (*def)->norelabel = false;
+ }
+
+ p = virXPathStringLimit("string(./seclabel/label[1])",
+ VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+ (*def)->label = p;
+
+ if ((*def)->label && (*def)->norelabel) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("Cannot specify a label if relabelling is turned
off"));
+ VIR_FREE((*def)->label);
+ VIR_FREE(*def);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/* Parse the XML definition for a lease
*/
static virDomainLeaseDefPtr
@@ -2690,9 +2764,11 @@ virDomainDiskDefParseXML(virCapsPtr caps,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
virBitmapPtr bootMap,
+ virSecurityLabelDefPtr vmSeclabel,
unsigned int flags)
{
virDomainDiskDefPtr def;
+ xmlNodePtr sourceNode = NULL;
xmlNodePtr cur, child;
xmlNodePtr save_ctxt = ctxt->node;
char *type = NULL;
@@ -2748,6 +2824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if ((source == NULL && hosts == NULL) &&
(xmlStrEqual(cur->name, BAD_CAST "source"))) {
+ sourceNode = cur;
+
switch (def->type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
source = virXMLPropString(cur, "file");
@@ -2999,6 +3077,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
goto error;
}
+ /* If source is present, check for an optional seclabel override. */
+ if (sourceNode) {
+ xmlNodePtr saved_node = ctxt->node;
+ ctxt->node = sourceNode;
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabel,
+ vmSeclabel,
+ ctxt) < 0)
+ goto error;
+ ctxt->node = saved_node;
+ }
+
if (target == NULL) {
virDomainReportError(VIR_ERR_NO_TARGET,
source ? "%s" : NULL, source);
@@ -6346,7 +6435,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
if (xmlStrEqual(node->name, BAD_CAST "disk")) {
dev->type = VIR_DOMAIN_DEVICE_DISK;
if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt,
- NULL, flags)))
+ NULL, &def->seclabel,
flags)))
goto error;
} else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
dev->type = VIR_DOMAIN_DEVICE_LEASE;
@@ -7448,6 +7537,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
nodes[i],
ctxt,
bootMap,
+ &def->seclabel,
flags);
if (!disk)
goto error;
@@ -9792,6 +9882,23 @@ cleanup:
}
+static void
+virSecurityDeviceLabelDefFormat(virBufferPtr buf,
+ virSecurityDeviceLabelDefPtr def)
+{
+ virBufferAsprintf(buf, "<seclabel relabel='%s'",
+ def->norelabel ? "no" : "yes");
+ if (def->label) {
+ virBufferAddLit(buf, ">\n");
+ virBufferEscapeString(buf, " <label>%s</label>\n",
+ def->label);
+ virBufferAddLit(buf, "</seclabel>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
+}
+
+
static int
virDomainLeaseDefFormat(virBufferPtr buf,
virDomainLeaseDefPtr def)
@@ -9907,17 +10014,34 @@ virDomainDiskDefFormat(virBufferPtr buf,
def->startupPolicy) {
switch (def->type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
- virBufferAsprintf(buf," <source");
+ virBufferAddLit(buf," <source");
if (def->src)
virBufferEscapeString(buf, " file='%s'", def->src);
if (def->startupPolicy)
virBufferEscapeString(buf, " startupPolicy='%s'",
startupPolicy);
- virBufferAsprintf(buf, "/>\n");
+ if (def->seclabel) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 8);
+ virSecurityDeviceLabelDefFormat(buf, def->seclabel);
+ virBufferAdjustIndent(buf, -8);
+ virBufferAddLit(buf, " </source>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
break;
case VIR_DOMAIN_DISK_TYPE_BLOCK:
- virBufferEscapeString(buf, " <source
dev='%s'/>\n",
+ virBufferEscapeString(buf, " <source dev='%s'",
def->src);
+ if (def->seclabel) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 8);
+ virSecurityDeviceLabelDefFormat(buf, def->seclabel);
+ virBufferAdjustIndent(buf, -8);
+ virBufferAddLit(buf, " </source>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
break;
case VIR_DOMAIN_DISK_TYPE_DIR:
virBufferEscapeString(buf, " <source
dir='%s'/>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3b522a9..2d31fd3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -180,6 +180,16 @@ struct _virSecurityLabelDef {
bool norelabel;
};
+
+/* Security configuration for domain */
+typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef;
+typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr;
+struct _virSecurityDeviceLabelDef {
+ char *label; /* image label string */
+ bool norelabel;
+};
+
+
typedef struct _virDomainHostdevOrigStates virDomainHostdevOrigStates;
typedef virDomainHostdevOrigStates *virDomainHostdevOrigStatesPtr;
struct _virDomainHostdevOrigStates {
@@ -367,7 +377,7 @@ struct _virDomainDiskDef {
int device;
int bus;
char *src;
- virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr seclabel;
char *dst;
int protocol;
int nhosts;
--
1.7.7.5
11:59 a.m.
New subject: [libvirt] [PATCH v2 2/5] Re-add domain device seclabel parsing / formatting
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This re-introduces parsing & formatting for per device seclabels.
There is a new virDomainDeviceSeclabelPtr struct and corresponding
APIs for parsing/formatting.
---
src/conf/domain_conf.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 12 ++++-
2 files changed, 139 insertions(+), 5 deletions(-)
This time it passed 'make check', so you definitely resolved my findings
on v1.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:12 a.m.
New subject: [libvirt] [PATCH v2 3/5] Add two new security label types
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 introduction 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
---
docs/formatdomain.html.in | 24 +++++++++----
docs/schemas/domaincommon.rng | 5 +++
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 70 ++++++++++++++++++++++++--------------
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 | 32 ++++++++++++++----
tests/seclabeltest.c | 2 +-
14 files changed, 177 insertions(+), 48 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dfb010d..2d4f4cf 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null
<p>
The <code>seclabel</code> element allows control over the
- operation of the security drivers. There are two basic
- modes of operation, dynamic where libvirt automatically
- generates a unique security label, or static where the
- application/administrator chooses the labels. With dynamic
+ operation of the security drivers. There are three basic
+ modes of operation, 'dynamic' where libvirt automatically
+ generates a unique security label, 'static' where the
+ application/administrator chooses the labels, or 'none'
+ where confinement is disabled. With dynamic
label generation, libvirt will always automatically
relabel any resources associated with the virtual machine.
With static label assignment, by default, the administrator
@@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null
<seclabel type='static' model='selinux'
relabel='yes'>
<label>system_u:system_r:svirt_t:s0:c392,c662</label>
</seclabel>
+
+ <seclabel type='none'/>
</pre>
<p>
+ If no 'type' attribute is provided in the input XML, then
+ the security driver default setting will be used, which
+ may be either 'none' or 'static'. If a 'baselabel' is set
+ but no 'type' is set, then the type is presumed to be 'dynamic'
+ </p>
+
+ <p>
When viewing the XML for a running guest with automatic
resource relabeling active, an additional XML element,
<code>imagelabel</code>, will be included. This is an
@@ -3526,9 +3536,9 @@ qemu-kvm -net nic,model=? /dev/null
</p>
<dl>
<dt><code>type</code></dt>
- <dd>Either <code>static</code> or
<code>dynamic</code> to determine
- whether libvirt automatically generates a unique security label
- or not.
+ <dd>Either <code>static</code>, <code>dynamic</code>
or <code>none</code>
+ to determine whether libvirt automatically generates a unique security
+ label or not.
</dd>
<dt><code>model</code></dt>
<dd>A valid security model name, matching the currently
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4fa968d..6094643 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -116,6 +116,11 @@
</optional>
</interleave>
</group>
+ <group>
+ <attribute name='type'>
+ <value>none</value>
+ </attribute>
+ </group>
</choice>
</element>
</define>
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0126320..42e80d2 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -90,6 +90,7 @@ src/secret/secret_driver.c
src/security/security_apparmor.c
src/security/security_dac.c
src/security/security_driver.c
+src/security/security_manager.c
src/security/security_selinux.c
src/security/virt-aa-helper.c
src/storage/parthelper.c
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8eacc1f..cb6c4fd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -562,6 +562,8 @@ VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST,
"unknown")
VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
+ "default",
+ "none",
"dynamic",
"static")
@@ -2528,13 +2530,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) {
@@ -2555,8 +2559,15 @@ 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", _("resource relabeling is not
compatible with 'none' label type"));
+ 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;
@@ -2591,12 +2602,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
def->imagelabel = p;
}
- /* Only parse baselabel, for dynamic label */
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+ /* Only parse baselabel, for dynamic or none label types */
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
+ def->type == VIR_DOMAIN_SECLABEL_NONE) {
p = virXPathStringLimit("string(./seclabel/baselabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p != NULL)
def->baselabel = p;
+ /* Forces none type to dynamic for back compat */
+ def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
}
/* Only parse model, if static labelling, or a base
@@ -9848,24 +9862,32 @@ 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_DEFAULT)
+ return;
+
+ virBufferAsprintf(buf, "<seclabel type='%s'",
+ sectype);
+ 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;
+ }
+
+ if (def->label || def->imagelabel || def->baselabel) {
+ virBufferAddLit(buf, ">\n");
- 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)
@@ -9875,10 +9897,9 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr
def,
virBufferEscapeString(buf, "
<baselabel>%s</baselabel>\n",
def->baselabel);
virBufferAddLit(buf, "</seclabel>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
}
- ret = 0;
-cleanup:
- return ret;
}
@@ -11887,12 +11908,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 2d31fd3..38730ab 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..95428c1 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 labeling
+# 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 0.
+# 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 7d79823..e85017f 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 ab69dca..f44e852 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -210,7 +210,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;
@@ -218,6 +221,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..7522b06 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,14 +250,28 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
_("cannot generate selinux context for %s"),
mcs);
goto cleanup;
}
- }
- def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs);
- if (!def->seclabel.imagelabel) {
+ break;
+
+ case VIR_DOMAIN_SECLABEL_NONE:
+ /* no op */
+ break;
+
+ default:
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot generate selinux context for %s"),
mcs);
+ _("unexpected security label type
'%s'"),
+ virDomainSeclabelTypeToString(def->seclabel.type));
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 &&
!(def->seclabel.model = strdup(SECURITY_SELINUX_NAME))) {
virReportOOMError();
diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c
index 5d87789..1898c3e 100644
--- a/tests/seclabeltest.c
+++ b/tests/seclabeltest.c
@@ -13,7 +13,7 @@ main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
virSecurityManagerPtr mgr;
const char *doi, *model;
- mgr = virSecurityManagerNew(NULL, false);
+ mgr = virSecurityManagerNew(NULL, false, true, true);
if (mgr == NULL) {
fprintf (stderr, "Failed to start security driver");
exit (-1);
--
1.7.7.5
12:27 p.m.
New subject: [libvirt] [PATCH v2 3/5] Add two new security label types
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Curently security labels can be of type 'dynamic' or 'static'.
s/Curently/Currently/
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 introduction 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
---
docs/formatdomain.html.in | 24 +++++++++----
docs/schemas/domaincommon.rng | 5 +++
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 70 ++++++++++++++++++++++++--------------
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 | 32 ++++++++++++++----
tests/seclabeltest.c | 2 +-
14 files changed, 177 insertions(+), 48 deletions(-)
Just glancing at this diffstat, it looks like you hit my major concerns
from v1
(https://www.redhat.com/archives/libvir-list/2012-January/msg00940.html)
@@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null
<p>
The <code>seclabel</code> element allows control over the
- operation of the security drivers. There are two basic
- modes of operation, dynamic where libvirt automatically
- generates a unique security label, or static where the
- application/administrator chooses the labels. With dynamic
+ operation of the security drivers. There are three basic
+ modes of operation, 'dynamic' where libvirt automatically
+ generates a unique security label, 'static' where the
+ application/administrator chooses the labels, or 'none'
+ where confinement is disabled. With dynamic
label generation, libvirt will always automatically
relabel any resources associated with the virtual machine.
With static label assignment, by default, the administrator
Probably want to also document with a <span class="since"> that
'none'
was introduced in 0.9.10.
@@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null
<seclabel type='static' model='selinux'
relabel='yes'>
<label>system_u:system_r:svirt_t:s0:c392,c662</label>
</seclabel>
+
+ <seclabel type='none'/>
</pre>
<p>
+ If no 'type' attribute is provided in the input XML, then
+ the security driver default setting will be used, which
+ may be either 'none' or 'static'.
Actually, it is either 'none' or 'dynamic'; the only way to get
'static'
is with explicit type attribute.
@@ -2591,12 +2602,15 @@
virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
def->imagelabel = p;
}
- /* Only parse baselabel, for dynamic label */
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+ /* Only parse baselabel, for dynamic or none label types */
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
+ def->type == VIR_DOMAIN_SECLABEL_NONE) {
p = virXPathStringLimit("string(./seclabel/baselabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p != NULL)
def->baselabel = p;
+ /* Forces none type to dynamic for back compat */
+ def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
Missing braces. This should be:
if (p != NULL) {
def->baselabel = p;
/* Force none to dynamic for back compat */
def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
}
ACK with those items fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:43 a.m.
New subject: [libvirt] [PATCH v2 3/5] Add two new security label types
Not sure if related, but after syncing libvirt to latest master branch I
see following errors:
2012-02-04 10:38:00.119+0000: 18828: error :
virSecurityLabelDefParseXML:2646 : XML error: security label is
missing
2012-02-04 10:38:00.129+0000: 18828: error :
virSecurityLabelDefParseXML:2646 : XML error: security label is missing
And virt-manager does not want to start anymore. Is this
backward-compatibility related issue?
Thanks,
Ansis
On Wed, Feb 1, 2012 at 8:27 PM, Eric Blake <eblake(a)redhat.com> wrote:
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> Curently security labels can be of type 'dynamic' or 'static'.
s/Curently/Currently/
> 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 introduction 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
> ---
> docs/formatdomain.html.in | 24 +++++++++----
> docs/schemas/domaincommon.rng | 5 +++
> po/POTFILES.in | 1 +
> src/conf/domain_conf.c | 70
++++++++++++++++++++++++--------------
> 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 | 32 ++++++++++++++----
> tests/seclabeltest.c | 2 +-
> 14 files changed, 177 insertions(+), 48 deletions(-)
Just glancing at this diffstat, it looks like you hit my major concerns
from v1
(https://www.redhat.com/archives/libvir-list/2012-January/msg00940.html)
> @@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null
>
> <p>
> The <code>seclabel</code> element allows control over the
> - operation of the security drivers. There are two basic
> - modes of operation, dynamic where libvirt automatically
> - generates a unique security label, or static where the
> - application/administrator chooses the labels. With dynamic
> + operation of the security drivers. There are three basic
> + modes of operation, 'dynamic' where libvirt automatically
> + generates a unique security label, 'static' where the
> + application/administrator chooses the labels, or 'none'
> + where confinement is disabled. With dynamic
> label generation, libvirt will always automatically
> relabel any resources associated with the virtual machine.
> With static label assignment, by default, the administrator
Probably want to also document with a <span class="since"> that
'none'
was introduced in 0.9.10.
> @@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null
> <seclabel type='static' model='selinux'
relabel='yes'>
>
<label>system_u:system_r:svirt_t:s0:c392,c662</label>
> </seclabel>
> +
> + <seclabel type='none'/>
> </pre>
>
> <p>
> + If no 'type' attribute is provided in the input XML, then
> + the security driver default setting will be used, which
> + may be either 'none' or 'static'.
Actually, it is either 'none' or 'dynamic'; the only way to get
'static'
is with explicit type attribute.
> @@ -2591,12 +2602,15 @@
virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> def->imagelabel = p;
> }
>
> - /* Only parse baselabel, for dynamic label */
> - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> + /* Only parse baselabel, for dynamic or none label types */
> + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
> + def->type == VIR_DOMAIN_SECLABEL_NONE) {
> p = virXPathStringLimit("string(./seclabel/baselabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p != NULL)
> def->baselabel = p;
> + /* Forces none type to dynamic for back compat */
> + def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
Missing braces. This should be:
if (p != NULL) {
def->baselabel = p;
/* Force none to dynamic for back compat */
def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
}
ACK with those items fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:17 a.m.
New subject: [libvirt] [PATCH v2 3/5] Add two new security label types
[please don't top-post on technical lists]
On 02/04/2012 03:43 AM, Ansis Atteka wrote:
Not sure if related, but after syncing libvirt to latest master
branch I
see following errors:
2012-02-04 10:38:00.119+0000: 18828: error :
> virSecurityLabelDefParseXML:2646 : XML error: security label is missing
> 2012-02-04 10:38:00.129+0000: 18828: error :
> virSecurityLabelDefParseXML:2646 : XML error: security label is missing
And virt-manager does not want to start anymore. Is this
backward-compatibility related issue?
Probably a bug in our parser that needs fixing. I haven't yet been able
to reproduce it, though, so I need to know more about your setup. Is
your domain running or inactive when you get that message? Can you post
the domain's XML (here, looking at /etc/libvirt/qemu/dom.xml and
/var/run/libvirt/qemu/dom.xml might be appropriate)?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:31 a.m.
New subject: [libvirt] [PATCH v2 3/5] Add two new security label types
On 02/04/2012 06:17 AM, Eric Blake wrote:
On 02/04/2012 03:43 AM, Ansis Atteka wrote:
> Not sure if related, but after syncing libvirt to latest master branch I
> see following errors:
>
> 2012-02-04 10:38:00.119+0000: 18828: error :
>> virSecurityLabelDefParseXML:2646 : XML error: security label is missing
>> 2012-02-04 10:38:00.129+0000: 18828: error :
>> virSecurityLabelDefParseXML:2646 : XML error: security label is missing
>
>
> And virt-manager does not want to start anymore. Is this
> backward-compatibility related issue?
Probably a bug in our parser that needs fixing. I haven't yet been able
to reproduce it, though, so I need to know more about your setup.
Actually, I just reproduced it. I changed an offline domain to have
<seclabel type='none'/>, started the domain, then restarted libvirtd. I
also see a problem where dumpxml doesn't revalidate under our RNG
schema. I'm preparing a patch.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:10 a.m.
New subject: [libvirt] [PATCH] seclabel: make code and RNG match
Commit b170eb99 introduced a bug: domains that had an explicit
<seclabel type='none'/> when started would not be reparsed if
libvirtd restarted. It turns out that our testsuite was not
exercising this because it never tried anything but inactive
parsing. Additionally, the live XML for such a domain failed
to re-validate. Applying just the tests/ portion of this patch
will expose the bugs that are fixed by the other two files.
* docs/schemas/domaincommon.rng (seclabel): Allow relabel under
type='none'.
* src/conf/domain_conf.c (virSecurityLabelDefParseXML): Per RNG,
presence of <seclabel> with no type implies dynamic. Don't
require sub-elements for type='none'.
* tests/qemuxml2xmltest.c (mymain): Add test.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml: Add file.
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args: Add file.
Reported by Ansis Atteka.
---
docs/schemas/domaincommon.rng | 6 +++
src/conf/domain_conf.c | 40 +++++++++-----------
.../qemuxml2argv-seclabel-none.args | 4 ++
.../qemuxml2argv-seclabel-none.xml | 26 +++++++++++++
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmltest.c | 29 +++++++++-----
6 files changed, 74 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8111045..724d7d0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -130,9 +130,15 @@
</interleave>
</group>
<group>
+ <!-- with none, relabel must be no if present -->
<attribute name='type'>
<value>none</value>
</attribute>
+ <optional>
+ <attribute name='relabel'>
+ <value>no</value>
+ </attribute>
+ </optional>
</group>
</choice>
</element>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aa4b32d..6949ece 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2583,17 +2583,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
p = virXPathStringLimit("string(./seclabel/@type)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security type"));
- goto error;
- }
-
- def->type = virDomainSeclabelTypeFromString(p);
- VIR_FREE(p);
- if (def->type <= 0) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("invalid security type"));
- goto error;
+ def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
+ } else {
+ def->type = virDomainSeclabelTypeFromString(p);
+ VIR_FREE(p);
+ if (def->type <= 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("invalid security type"));
+ goto error;
+ }
}
p = virXPathStringLimit("string(./seclabel/@relabel)",
@@ -2634,7 +2632,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
* if the 'live' VM XML is requested
*/
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
@@ -2648,7 +2647,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
/* Only parse imagelabel, if requested live XML with relabeling */
if (!def->norelabel &&
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/imagelabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
@@ -2659,16 +2659,11 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
def->imagelabel = p;
}
- /* Only parse baselabel, for dynamic or none label types */
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
- def->type == VIR_DOMAIN_SECLABEL_NONE) {
+ /* Only parse baselabel for dynamic label type */
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
p = virXPathStringLimit("string(./seclabel/baselabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- if (p != NULL) {
- def->baselabel = p;
- /* Forces none type to dynamic for back compat */
- def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
- }
+ def->baselabel = p;
}
/* Only parse model, if static labelling, or a base
@@ -2676,7 +2671,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
*/
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
def->baselabel ||
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/@model)",
VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
if (p == NULL) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
new file mode 100644
index 0000000..651793d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
@@ -0,0 +1,4 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor unix:/tmp/test-monitor,\
+server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \
+none -parallel none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
new file mode 100644
index 0000000..1ef97ce
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
@@ -0,0 +1,26 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219100</memory>
+ <currentMemory>219100</currentMemory>
+ <vcpu cpuset='1-4,8-20,525'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='none' relabel='no'/>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8ce77f..fcffc27 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -728,6 +728,7 @@ mymain(void)
DO_TEST("seclabel-dynamic-override", false, QEMU_CAPS_NAME);
DO_TEST("seclabel-static", false, QEMU_CAPS_NAME);
DO_TEST("seclabel-static-relabel", false, QEMU_CAPS_NAME);
+ DO_TEST("seclabel-none", false, QEMU_CAPS_NAME);
DO_TEST("pseries-basic", false,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b0f80d3..ddc7cae 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -19,7 +19,7 @@
static struct qemud_driver driver;
static int
-testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
{
char *inXmlData = NULL;
char *outXmlData = NULL;
@@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
if (!(def = virDomainDefParseString(driver.caps, inXmlData,
QEMU_EXPECTED_VIRT_TYPES,
- VIR_DOMAIN_XML_INACTIVE)))
+ live ? 0 : VIR_DOMAIN_XML_INACTIVE)))
goto fail;
if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE)))
@@ -58,6 +58,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
struct testInfo {
const char *name;
int different;
+ bool inactive_only;
};
static int
@@ -75,9 +76,16 @@ testCompareXMLToXMLHelper(const void *data)
goto cleanup;
if (info->different) {
- ret = testCompareXMLToXMLFiles(xml_in, xml_out);
+ ret = testCompareXMLToXMLFiles(xml_in, xml_out, false);
} else {
- ret = testCompareXMLToXMLFiles(xml_in, xml_in);
+ ret = testCompareXMLToXMLFiles(xml_in, xml_in, false);
+ }
+ if (!info->inactive_only) {
+ if (info->different) {
+ ret = testCompareXMLToXMLFiles(xml_in, xml_out, true);
+ } else {
+ ret = testCompareXMLToXMLFiles(xml_in, xml_in, true);
+ }
}
cleanup:
@@ -95,19 +103,19 @@ mymain(void)
if ((driver.caps = testQemuCapsInit()) == NULL)
return (EXIT_FAILURE);
-# define DO_TEST_FULL(name, is_different) \
+# define DO_TEST_FULL(name, is_different, inactive) \
do { \
- const struct testInfo info = {name, is_different}; \
+ const struct testInfo info = {name, is_different, inactive}; \
if (virtTestRun("QEMU XML-2-XML " name, \
1, testCompareXMLToXMLHelper, &info) < 0) \
ret = -1; \
} while (0)
# define DO_TEST(name) \
- DO_TEST_FULL(name, 0)
+ DO_TEST_FULL(name, 0, false)
# define DO_TEST_DIFFERENT(name) \
- DO_TEST_FULL(name, 1)
+ DO_TEST_FULL(name, 1, false)
/* Unset or set all envvars here that are copied in qemudBuildCommandLine
* using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
@@ -200,9 +208,10 @@ mymain(void)
DO_TEST("usb-redir");
DO_TEST("blkdeviotune");
- DO_TEST("seclabel-dynamic-baselabel");
- DO_TEST("seclabel-dynamic-override");
+ DO_TEST_FULL("seclabel-dynamic-baselabel", false, true);
+ DO_TEST_FULL("seclabel-dynamic-override", false, true);
DO_TEST("seclabel-static");
+ DO_TEST("seclabel-none");
/* These tests generate different XML */
DO_TEST_DIFFERENT("balloon-device-auto");
--
1.7.7.6
2:18 p.m.
New subject: [libvirt] [PATCH] seclabel: make code and RNG match
Hmm, this patch does not seem to fix my issue. See the both xml files in
the attachment.
On Sat, Feb 4, 2012 at 4:10 PM, Eric Blake <eblake(a)redhat.com> wrote:
Commit b170eb99 introduced a bug: domains that had an explicit
<seclabel type='none'/> when started would not be reparsed if
libvirtd restarted. It turns out that our testsuite was not
exercising this because it never tried anything but inactive
parsing. Additionally, the live XML for such a domain failed
to re-validate. Applying just the tests/ portion of this patch
will expose the bugs that are fixed by the other two files.
* docs/schemas/domaincommon.rng (seclabel): Allow relabel under
type='none'.
* src/conf/domain_conf.c (virSecurityLabelDefParseXML): Per RNG,
presence of <seclabel> with no type implies dynamic. Don't
require sub-elements for type='none'.
* tests/qemuxml2xmltest.c (mymain): Add test.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml: Add file.
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args: Add file.
Reported by Ansis Atteka.
---
docs/schemas/domaincommon.rng | 6 +++
src/conf/domain_conf.c | 40
+++++++++-----------
.../qemuxml2argv-seclabel-none.args | 4 ++
.../qemuxml2argv-seclabel-none.xml | 26 +++++++++++++
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmltest.c | 29 +++++++++-----
6 files changed, 74 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8111045..724d7d0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -130,9 +130,15 @@
</interleave>
</group>
<group>
+ <!-- with none, relabel must be no if present -->
<attribute name='type'>
<value>none</value>
</attribute>
+ <optional>
+ <attribute name='relabel'>
+ <value>no</value>
+ </attribute>
+ </optional>
</group>
</choice>
</element>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aa4b32d..6949ece 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2583,17 +2583,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
def,
p = virXPathStringLimit("string(./seclabel/@type)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security type"));
- goto error;
- }
-
- def->type = virDomainSeclabelTypeFromString(p);
- VIR_FREE(p);
- if (def->type <= 0) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- "%s", _("invalid security type"));
- goto error;
+ def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
+ } else {
+ def->type = virDomainSeclabelTypeFromString(p);
+ VIR_FREE(p);
+ if (def->type <= 0) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("invalid security type"));
+ goto error;
+ }
}
p = virXPathStringLimit("string(./seclabel/@relabel)",
@@ -2634,7 +2632,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
def,
* if the 'live' VM XML is requested
*/
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
@@ -2648,7 +2647,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
def,
/* Only parse imagelabel, if requested live XML with relabeling */
if (!def->norelabel &&
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/imagelabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
@@ -2659,16 +2659,11 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
def,
def->imagelabel = p;
}
- /* Only parse baselabel, for dynamic or none label types */
- if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
- def->type == VIR_DOMAIN_SECLABEL_NONE) {
+ /* Only parse baselabel for dynamic label type */
+ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
p = virXPathStringLimit("string(./seclabel/baselabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- if (p != NULL) {
- def->baselabel = p;
- /* Forces none type to dynamic for back compat */
- def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
- }
+ def->baselabel = p;
}
/* Only parse model, if static labelling, or a base
@@ -2676,7 +2671,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
def,
*/
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
def->baselabel ||
- !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+ (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
+ def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./seclabel/@model)",
VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
if (p == NULL) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
new file mode 100644
index 0000000..651793d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
@@ -0,0 +1,4 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu
-S -M \
+pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor
unix:/tmp/test-monitor,\
+server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none
-serial \
+none -parallel none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
new file mode 100644
index 0000000..1ef97ce
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
@@ -0,0 +1,26 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219100</memory>
+ <currentMemory>219100</currentMemory>
+ <vcpu cpuset='1-4,8-20,525'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+ <seclabel type='none' relabel='no'/>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8ce77f..fcffc27 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -728,6 +728,7 @@ mymain(void)
DO_TEST("seclabel-dynamic-override", false, QEMU_CAPS_NAME);
DO_TEST("seclabel-static", false, QEMU_CAPS_NAME);
DO_TEST("seclabel-static-relabel", false, QEMU_CAPS_NAME);
+ DO_TEST("seclabel-none", false, QEMU_CAPS_NAME);
DO_TEST("pseries-basic", false,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b0f80d3..ddc7cae 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -19,7 +19,7 @@
static struct qemud_driver driver;
static int
-testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
{
char *inXmlData = NULL;
char *outXmlData = NULL;
@@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml)
if (!(def = virDomainDefParseString(driver.caps, inXmlData,
QEMU_EXPECTED_VIRT_TYPES,
- VIR_DOMAIN_XML_INACTIVE)))
+ live ? 0 :
VIR_DOMAIN_XML_INACTIVE)))
goto fail;
if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE)))
@@ -58,6 +58,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml)
struct testInfo {
const char *name;
int different;
+ bool inactive_only;
};
static int
@@ -75,9 +76,16 @@ testCompareXMLToXMLHelper(const void *data)
goto cleanup;
if (info->different) {
- ret = testCompareXMLToXMLFiles(xml_in, xml_out);
+ ret = testCompareXMLToXMLFiles(xml_in, xml_out, false);
} else {
- ret = testCompareXMLToXMLFiles(xml_in, xml_in);
+ ret = testCompareXMLToXMLFiles(xml_in, xml_in, false);
+ }
+ if (!info->inactive_only) {
+ if (info->different) {
+ ret = testCompareXMLToXMLFiles(xml_in, xml_out, true);
+ } else {
+ ret = testCompareXMLToXMLFiles(xml_in, xml_in, true);
+ }
}
cleanup:
@@ -95,19 +103,19 @@ mymain(void)
if ((driver.caps = testQemuCapsInit()) == NULL)
return (EXIT_FAILURE);
-# define DO_TEST_FULL(name, is_different) \
+# define DO_TEST_FULL(name, is_different, inactive) \
do { \
- const struct testInfo info = {name, is_different}; \
+ const struct testInfo info = {name, is_different, inactive}; \
if (virtTestRun("QEMU XML-2-XML " name, \
1, testCompareXMLToXMLHelper, &info) < 0) \
ret = -1; \
} while (0)
# define DO_TEST(name) \
- DO_TEST_FULL(name, 0)
+ DO_TEST_FULL(name, 0, false)
# define DO_TEST_DIFFERENT(name) \
- DO_TEST_FULL(name, 1)
+ DO_TEST_FULL(name, 1, false)
/* Unset or set all envvars here that are copied in
qemudBuildCommandLine
* using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
@@ -200,9 +208,10 @@ mymain(void)
DO_TEST("usb-redir");
DO_TEST("blkdeviotune");
- DO_TEST("seclabel-dynamic-baselabel");
- DO_TEST("seclabel-dynamic-override");
+ DO_TEST_FULL("seclabel-dynamic-baselabel", false, true);
+ DO_TEST_FULL("seclabel-dynamic-override", false, true);
DO_TEST("seclabel-static");
+ DO_TEST("seclabel-none");
/* These tests generate different XML */
DO_TEST_DIFFERENT("balloon-device-auto");
--
1.7.7.6
9:42 a.m.
New subject: [libvirt] [PATCH] seclabel: make code and RNG match
My bad, it actually seems that your patch fixed my issue after I manually
removed
the domain XML file that was stuck in /var/run. Nevertheless, it seems that
the
issue when virt-manager exited preamturely could be caused by something
else:
2012-02-05 15:39:28.395+0000: 29540: error : virNetSocketReadWire:999 : End
of file while reading data: Input/output error
On Sat, Feb 4, 2012 at 10:18 PM, Ansis Atteka <aatteka(a)nicira.com> wrote:
Hmm, this patch does not seem to fix my issue. See the both xml files
in
the attachment.
On Sat, Feb 4, 2012 at 4:10 PM, Eric Blake <eblake(a)redhat.com> wrote:
> Commit b170eb99 introduced a bug: domains that had an explicit
> <seclabel type='none'/> when started would not be reparsed if
> libvirtd restarted. It turns out that our testsuite was not
> exercising this because it never tried anything but inactive
> parsing. Additionally, the live XML for such a domain failed
> to re-validate. Applying just the tests/ portion of this patch
> will expose the bugs that are fixed by the other two files.
>
> * docs/schemas/domaincommon.rng (seclabel): Allow relabel under
> type='none'.
> * src/conf/domain_conf.c (virSecurityLabelDefParseXML): Per RNG,
> presence of <seclabel> with no type implies dynamic. Don't
> require sub-elements for type='none'.
> * tests/qemuxml2xmltest.c (mymain): Add test.
> * tests/qemuxml2argvtest.c (mymain): Likewise.
> * tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml: Add file.
> * tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args: Add file.
> Reported by Ansis Atteka.
> ---
> docs/schemas/domaincommon.rng | 6 +++
> src/conf/domain_conf.c | 40
> +++++++++-----------
> .../qemuxml2argv-seclabel-none.args | 4 ++
> .../qemuxml2argv-seclabel-none.xml | 26 +++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmltest.c | 29 +++++++++-----
> 6 files changed, 74 insertions(+), 32 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8111045..724d7d0 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -130,9 +130,15 @@
> </interleave>
> </group>
> <group>
> + <!-- with none, relabel must be no if present -->
> <attribute name='type'>
> <value>none</value>
> </attribute>
> + <optional>
> + <attribute name='relabel'>
> + <value>no</value>
> + </attribute>
> + </optional>
> </group>
> </choice>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index aa4b32d..6949ece 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2583,17 +2583,15 @@
> virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> p = virXPathStringLimit("string(./seclabel/@type)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> - virDomainReportError(VIR_ERR_XML_ERROR,
> - "%s", _("missing security type"));
> - goto error;
> - }
> -
> - def->type = virDomainSeclabelTypeFromString(p);
> - VIR_FREE(p);
> - if (def->type <= 0) {
> - virDomainReportError(VIR_ERR_XML_ERROR,
> - "%s", _("invalid security type"));
> - goto error;
> + def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
> + } else {
> + def->type = virDomainSeclabelTypeFromString(p);
> + VIR_FREE(p);
> + if (def->type <= 0) {
> + virDomainReportError(VIR_ERR_XML_ERROR,
> + "%s", _("invalid security
type"));
> + goto error;
> + }
> }
>
> p = virXPathStringLimit("string(./seclabel/@relabel)",
> @@ -2634,7 +2632,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
> def,
> * if the 'live' VM XML is requested
> */
> if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
> - !(flags & VIR_DOMAIN_XML_INACTIVE)) {
> + (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
> + def->type != VIR_DOMAIN_SECLABEL_NONE)) {
> p = virXPathStringLimit("string(./seclabel/label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> @@ -2648,7 +2647,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
> def,
>
> /* Only parse imagelabel, if requested live XML with relabeling */
> if (!def->norelabel &&
> - !(flags & VIR_DOMAIN_XML_INACTIVE)) {
> + (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
> + def->type != VIR_DOMAIN_SECLABEL_NONE)) {
> p = virXPathStringLimit("string(./seclabel/imagelabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> @@ -2659,16 +2659,11 @@
> virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> def->imagelabel = p;
> }
>
> - /* Only parse baselabel, for dynamic or none label types */
> - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
> - def->type == VIR_DOMAIN_SECLABEL_NONE) {
> + /* Only parse baselabel for dynamic label type */
> + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> p = virXPathStringLimit("string(./seclabel/baselabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> - if (p != NULL) {
> - def->baselabel = p;
> - /* Forces none type to dynamic for back compat */
> - def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
> - }
> + def->baselabel = p;
> }
>
> /* Only parse model, if static labelling, or a base
> @@ -2676,7 +2671,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr
> def,
> */
> if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
> def->baselabel ||
> - !(flags & VIR_DOMAIN_XML_INACTIVE)) {
> + (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
> + def->type != VIR_DOMAIN_SECLABEL_NONE)) {
> p = virXPathStringLimit("string(./seclabel/@model)",
> VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
> b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
> new file mode 100644
> index 0000000..651793d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args
> @@ -0,0 +1,4 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu
> -S -M \
> +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor
> unix:/tmp/test-monitor,\
> +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none
> -serial \
> +none -parallel none -usb
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
> b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
> new file mode 100644
> index 0000000..1ef97ce
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml
> @@ -0,0 +1,26 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory>219100</memory>
> + <currentMemory>219100</currentMemory>
> + <vcpu cpuset='1-4,8-20,525'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='block' device='disk'>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0'
unit='0'/>
> + </disk>
> + <controller type='ide' index='0'/>
> + <memballoon model='virtio'/>
> + </devices>
> + <seclabel type='none' relabel='no'/>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index c8ce77f..fcffc27 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -728,6 +728,7 @@ mymain(void)
> DO_TEST("seclabel-dynamic-override", false, QEMU_CAPS_NAME);
> DO_TEST("seclabel-static", false, QEMU_CAPS_NAME);
> DO_TEST("seclabel-static-relabel", false, QEMU_CAPS_NAME);
> + DO_TEST("seclabel-none", false, QEMU_CAPS_NAME);
>
> DO_TEST("pseries-basic", false,
> QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index b0f80d3..ddc7cae 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -19,7 +19,7 @@
> static struct qemud_driver driver;
>
> static int
> -testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
> +testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool
> live)
> {
> char *inXmlData = NULL;
> char *outXmlData = NULL;
> @@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
> *outxml)
>
> if (!(def = virDomainDefParseString(driver.caps, inXmlData,
> QEMU_EXPECTED_VIRT_TYPES,
> - VIR_DOMAIN_XML_INACTIVE)))
> + live ? 0 :
> VIR_DOMAIN_XML_INACTIVE)))
> goto fail;
>
> if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE)))
> @@ -58,6 +58,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
> *outxml)
> struct testInfo {
> const char *name;
> int different;
> + bool inactive_only;
> };
>
> static int
> @@ -75,9 +76,16 @@ testCompareXMLToXMLHelper(const void *data)
> goto cleanup;
>
> if (info->different) {
> - ret = testCompareXMLToXMLFiles(xml_in, xml_out);
> + ret = testCompareXMLToXMLFiles(xml_in, xml_out, false);
> } else {
> - ret = testCompareXMLToXMLFiles(xml_in, xml_in);
> + ret = testCompareXMLToXMLFiles(xml_in, xml_in, false);
> + }
> + if (!info->inactive_only) {
> + if (info->different) {
> + ret = testCompareXMLToXMLFiles(xml_in, xml_out, true);
> + } else {
> + ret = testCompareXMLToXMLFiles(xml_in, xml_in, true);
> + }
> }
>
> cleanup:
> @@ -95,19 +103,19 @@ mymain(void)
> if ((driver.caps = testQemuCapsInit()) == NULL)
> return (EXIT_FAILURE);
>
> -# define DO_TEST_FULL(name, is_different) \
> +# define DO_TEST_FULL(name, is_different, inactive) \
> do { \
> - const struct testInfo info = {name, is_different}; \
> + const struct testInfo info = {name, is_different, inactive}; \
> if (virtTestRun("QEMU XML-2-XML " name, \
> 1, testCompareXMLToXMLHelper, &info) < 0) \
> ret = -1; \
> } while (0)
>
> # define DO_TEST(name) \
> - DO_TEST_FULL(name, 0)
> + DO_TEST_FULL(name, 0, false)
>
> # define DO_TEST_DIFFERENT(name) \
> - DO_TEST_FULL(name, 1)
> + DO_TEST_FULL(name, 1, false)
>
> /* Unset or set all envvars here that are copied in
> qemudBuildCommandLine
> * using ADD_ENV_COPY, otherwise these tests may fail due to
> unexpected
> @@ -200,9 +208,10 @@ mymain(void)
> DO_TEST("usb-redir");
> DO_TEST("blkdeviotune");
>
> - DO_TEST("seclabel-dynamic-baselabel");
> - DO_TEST("seclabel-dynamic-override");
> + DO_TEST_FULL("seclabel-dynamic-baselabel", false, true);
> + DO_TEST_FULL("seclabel-dynamic-override", false, true);
> DO_TEST("seclabel-static");
> + DO_TEST("seclabel-none");
>
> /* These tests generate different XML */
> DO_TEST_DIFFERENT("balloon-device-auto");
> --
> 1.7.7.6
>
>
1:13 p.m.
New subject: [libvirt] [PATCH] seclabel: make code and RNG match
On 02/05/2012 08:42 AM, Ansis Atteka wrote:
My bad, it actually seems that your patch fixed my issue after I
manually
removed
the domain XML file that was stuck in /var/run.
I've gone ahead and pushed the patch, since it helped.
Nevertheless, it seems that
the
issue when virt-manager exited preamturely could be caused by something
else:
2012-02-05 15:39:28.395+0000: 29540: error : virNetSocketReadWire:999 : End
> of file while reading data: Input/output error
virt-manager can't connect if libvirtd is in a weird state; and perhaps
the parse error was causing things to get in a weird state to begin
with. At any rate, for the XML you attached, the config XML had no
<seclabel>, and the live XML had:
<seclabel type='dynamic' relabel='yes'/>
with no mode='selinux' or any <label> sub-element to say what had
actually been labeled. I think that all stems back to the parsing error
in the first place, and that with the patch in place, things should be
working better.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:12 a.m.
New subject: [libvirt] [PATCH v2 4/5] Add support for sVirt in the LXC driver
From: "Daniel P. Berrange" <berrange(a)redhat.com>
For the sake of backwards compat, LXC guests are *not*
confined by default. This is because it is not practical
to dynamically relabel containers using large filesystem
trees. Applications can create confined containers though,
by giving suitable XML configs
* src/Makefile.am: Link libvirt_lxc to security drivers
* src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h,
src/lxc/lxc_conf.c, src/lxc/lxc.conf,
src/lxc/test_libvirtd_lxc.aug: Config file handling for
security driver
* src/lxc/lxc_driver.c: Wire up security driver functions
* src/lxc/lxc_controller.c: Add a '--security' flag to
specify which security driver to activate
* src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set
the process label just before exec'ing init.
---
src/Makefile.am | 13 +++
src/lxc/libvirtd_lxc.aug | 15 +++-
src/lxc/lxc.conf | 18 ++++
src/lxc/lxc_conf.c | 62 ++++++++++++--
src/lxc/lxc_conf.h | 8 ++-
src/lxc/lxc_container.c | 9 ++-
src/lxc/lxc_container.h | 2 +
src/lxc/lxc_controller.c | 28 +++++-
src/lxc/lxc_driver.c | 193 ++++++++++++++++++++++++++++++++++++++++-
src/lxc/test_libvirtd_lxc.aug | 2 +
10 files changed, 335 insertions(+), 15 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index a44446f..7f1c80f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1503,7 +1503,14 @@ libvirt_lxc_SOURCES = \
$(DOMAIN_CONF_SOURCES) \
$(SECRET_CONF_SOURCES) \
$(CPU_CONF_SOURCES) \
+ $(SECURITY_DRIVER_SOURCES) \
$(NWFILTER_PARAM_CONF_SOURCES)
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES)
+endif
+if WITH_SECDRIVER_APPARMOR
+libvirt_lxc_SOURCES += $(SECURITY_DRIVER_APPARMOR_SOURCES)
+endif
libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS)
libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \
@@ -1513,6 +1520,9 @@ libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
if WITH_DTRACE
libvirt_lxc_LDADD += probes.o
endif
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_LDADD += $(SELINUX_LIBS)
+endif
libvirt_lxc_CFLAGS = \
$(LIBPARTED_CFLAGS) \
$(NUMACTL_CFLAGS) \
@@ -1525,6 +1535,9 @@ if HAVE_LIBBLKID
libvirt_lxc_CFLAGS += $(BLKID_CFLAGS)
libvirt_lxc_LDADD += $(BLKID_LIBS)
endif
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_CFLAGS += $(SELINUX_CFLAGS)
+endif
endif
endif
EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
diff --git a/src/lxc/libvirtd_lxc.aug b/src/lxc/libvirtd_lxc.aug
index 10f25e4..be6402c 100644
--- a/src/lxc/libvirtd_lxc.aug
+++ b/src/lxc/libvirtd_lxc.aug
@@ -7,13 +7,26 @@ module Libvirtd_lxc =
let value_sep = del /[ \t]*=[ \t]*/ " = "
let indent = del /[ \t]*/ ""
+ let array_sep = del /,[ \t\n]*/ ", "
+ let array_start = del /\[[ \t\n]*/ "[ "
+ let array_end = del /\]/ "]"
+
+ let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/
"\""
let bool_val = store /0|1/
+ let int_val = store /[0-9]+/
+ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/
""
+ let str_array_val = counter "el" . array_start . ( str_array_element . (
array_sep . str_array_element ) * ) ? . array_end
+ let str_entry (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry (kw:string) = [ key kw . value_sep . bool_val ]
-
+ let int_entry (kw:string) = [ key kw . value_sep . int_val ]
+ let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
(* Config entry grouped by function - same order as example config *)
let log_entry = bool_entry "log_with_libvirtd"
+ | str_entry "security_driver"
+ | bool_entry "security_default_confined"
+ | bool_entry "security_require_confined"
(* Each enty in the config is one of the following three ... *)
let entry = log_entry
diff --git a/src/lxc/lxc.conf b/src/lxc/lxc.conf
index 7a5066f..09dc95f 100644
--- a/src/lxc/lxc.conf
+++ b/src/lxc/lxc.conf
@@ -11,3 +11,21 @@
# This is disabled by default, uncomment below to enable it.
#
# log_with_libvirtd = 1
+
+
+# The default security driver is SELinux. If SELinux is disabled
+# on the host, then the security driver will automatically disable
+# itself. If you wish to disable QEMU SELinux security driver while
+# leaving SELinux enabled for the host in general, then set this
+# to 'none' instead.
+#
+# security_driver = "selinux"
+
+# If set to non-zero, then the default security labeling
+# will make guests confined. If set to zero, then guests
+# will be unconfined by default. Defaults to 0.
+# security_default_confined = 1
+
+# If set to non-zero, then attempts to create unconfined
+# guests will be blocked. Defaults to 0.
+# security_require_confined = 1
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index e842736..72547c4 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -49,7 +49,7 @@ static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED)
/* Functions */
-virCapsPtr lxcCapsInit(void)
+virCapsPtr lxcCapsInit(lxc_driver_t *driver)
{
struct utsname utsname;
virCapsPtr caps;
@@ -127,8 +127,30 @@ virCapsPtr lxcCapsInit(void)
/* LXC Requires an emulator in the XML */
virCapabilitiesSetEmulatorRequired(caps);
+ if (driver) {
+ /* Security driver data */
+ const char *doi, *model;
+
+ doi = virSecurityManagerGetDOI(driver->securityManager);
+ model = virSecurityManagerGetModel(driver->securityManager);
+ if (STRNEQ(model, "none")) {
+ if (!(caps->host.secModel.model = strdup(model)))
+ goto no_memory;
+ if (!(caps->host.secModel.doi = strdup(doi)))
+ goto no_memory;
+ }
+
+ VIR_DEBUG("Initialized caps for security driver \"%s\" with
"
+ "DOI \"%s\"", model, doi);
+ } else {
+ VIR_INFO("No driver, not initializing security driver");
+ }
+
return caps;
+no_memory:
+ virReportOOMError();
+
error:
virCapabilitiesFree(caps);
return NULL;
@@ -140,6 +162,9 @@ int lxcLoadDriverConfig(lxc_driver_t *driver)
virConfPtr conf;
virConfValuePtr p;
+ driver->securityDefaultConfined = false;
+ driver->securityRequireConfined = false;
+
/* Set the container configuration directory */
if ((driver->configDir = strdup(LXC_CONFIG_DIR)) == NULL)
goto no_memory;
@@ -161,14 +186,39 @@ int lxcLoadDriverConfig(lxc_driver_t *driver)
if (!conf)
goto done;
+#define CHECK_TYPE(name,typ) if (p && p->type != (typ)) { \
+ lxcError(VIR_ERR_INTERNAL_ERROR, \
+ "%s: %s: expected type " #typ, \
+ filename, (name)); \
+ virConfFree(conf); \
+ return -1; \
+ }
+
p = virConfGetValue(conf, "log_with_libvirtd");
- if (p) {
- if (p->type != VIR_CONF_LONG)
- VIR_WARN("lxcLoadDriverConfig: invalid setting:
log_with_libvirtd");
- else
- driver->log_libvirtd = p->l;
+ CHECK_TYPE ("log_with_libvirtd", VIR_CONF_LONG);
+ if (p) driver->log_libvirtd = p->l;
+
+ p = virConfGetValue (conf, "security_driver");
+ CHECK_TYPE ("security_driver", VIR_CONF_STRING);
+ if (p && p->str) {
+ if (!(driver->securityDriverName = strdup(p->str))) {
+ virReportOOMError();
+ virConfFree(conf);
+ return -1;
+ }
}
+ 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;
+
+
+#undef CHECK_TYPE
+
virConfFree(conf);
done:
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index b124330..ebdc173 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -33,6 +33,7 @@
# include "capabilities.h"
# include "threads.h"
# include "cgroup.h"
+# include "security/security_manager.h"
# include "configmake.h"
# define LXC_CONFIG_DIR SYSCONFDIR "/libvirt/lxc"
@@ -57,6 +58,11 @@ struct __lxc_driver {
virDomainEventStatePtr domainEventState;
+ char *securityDriverName;
+ bool securityDefaultConfined;
+ bool securityRequireConfined;
+ virSecurityManagerPtr securityManager;
+
/* Mapping of 'char *uuidstr' -> virConnectPtr
* of guests which will be automatically killed
* when the virConnectPtr is closed*/
@@ -64,7 +70,7 @@ struct __lxc_driver {
};
int lxcLoadDriverConfig(lxc_driver_t *driver);
-virCapsPtr lxcCapsInit(void);
+virCapsPtr lxcCapsInit(lxc_driver_t *driver);
# define lxcError(code, ...) \
virReportErrorHelper(VIR_FROM_LXC, code, __FILE__, \
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index dcd65ef..b0493fd 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -91,6 +91,7 @@ typedef char lxc_message_t;
typedef struct __lxc_child_argv lxc_child_argv_t;
struct __lxc_child_argv {
virDomainDefPtr config;
+ virSecurityManagerPtr securityDriver;
unsigned int nveths;
char **veths;
int monitor;
@@ -1296,6 +1297,10 @@ static int lxcContainerChild( void *data )
goto cleanup;
}
+ VIR_DEBUG("Setting up security labeling");
+ if (virSecurityManagerSetProcessLabel(argv->securityDriver, vmDef) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
VIR_FREE(ttyPath);
@@ -1358,6 +1363,7 @@ const char *lxcContainerGetAlt32bitArch(const char *arch)
* Returns PID of container on success or -1 in case of error
*/
int lxcContainerStart(virDomainDefPtr def,
+ virSecurityManagerPtr securityDriver,
unsigned int nveths,
char **veths,
int control,
@@ -1369,7 +1375,8 @@ int lxcContainerStart(virDomainDefPtr def,
int cflags;
int stacksize = getpagesize() * 4;
char *stack, *stacktop;
- lxc_child_argv_t args = { def, nveths, veths, control,
+ lxc_child_argv_t args = { def, securityDriver,
+ nveths, veths, control,
ttyPaths, nttyPaths, handshakefd};
/* allocate a stack for the container */
diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h
index ffeda5e..77fb9b2 100644
--- a/src/lxc/lxc_container.h
+++ b/src/lxc/lxc_container.h
@@ -25,6 +25,7 @@
# define LXC_CONTAINER_H
# include "lxc_conf.h"
+# include "security/security_manager.h"
enum {
LXC_CONTAINER_FEATURE_NET = (1 << 0),
@@ -49,6 +50,7 @@ int lxcContainerSendContinue(int control);
int lxcContainerWaitForContinue(int control);
int lxcContainerStart(virDomainDefPtr def,
+ virSecurityManagerPtr securityDriver,
unsigned int nveths,
char **veths,
int control,
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 49727dd..cb04b08 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1361,6 +1361,7 @@ cleanup:
static int
lxcControllerRun(virDomainDefPtr def,
+ virSecurityManagerPtr securityDriver,
unsigned int nveths,
char **veths,
int monitor,
@@ -1515,6 +1516,7 @@ lxcControllerRun(virDomainDefPtr def,
goto cleanup;
if ((container = lxcContainerStart(def,
+ securityDriver,
nveths,
veths,
control[1],
@@ -1623,11 +1625,13 @@ int main(int argc, char *argv[])
{ "veth", 1, NULL, 'v' },
{ "console", 1, NULL, 'c' },
{ "handshakefd", 1, NULL, 's' },
+ { "security", 1, NULL, 'S' },
{ "help", 0, NULL, 'h' },
{ 0, 0, 0, 0 },
};
int *ttyFDs = NULL;
size_t nttyFDs = 0;
+ virSecurityManagerPtr securityDriver = NULL;
if (setlocale(LC_ALL, "") == NULL ||
bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
@@ -1639,7 +1643,7 @@ int main(int argc, char *argv[])
while (1) {
int c;
- c = getopt_long(argc, argv, "dn:v:m:c:s:h",
+ c = getopt_long(argc, argv, "dn:v:m:c:s:h:S:",
options, NULL);
if (c == -1)
@@ -1687,6 +1691,14 @@ int main(int argc, char *argv[])
}
break;
+ case 'S':
+ if (!(securityDriver = virSecurityManagerNew(optarg, false, false, false)))
{
+ fprintf(stderr, "Cannot create security manager '%s'",
+ optarg);
+ goto cleanup;
+ }
+ break;
+
case 'h':
case '?':
fprintf(stderr, "\n");
@@ -1699,12 +1711,20 @@ int main(int argc, char *argv[])
fprintf(stderr, " -c FD, --console FD\n");
fprintf(stderr, " -v VETH, --veth VETH\n");
fprintf(stderr, " -s FD, --handshakefd FD\n");
+ fprintf(stderr, " -S NAME, --security NAME\n");
fprintf(stderr, " -h, --help\n");
fprintf(stderr, "\n");
goto cleanup;
}
}
+ if (securityDriver == NULL) {
+ if (!(securityDriver = virSecurityManagerNew("none", false, false,
false))) {
+ fprintf(stderr, "%s: cannot initialize nop security manager",
argv[0]);
+ goto cleanup;
+ }
+ }
+
if (name == NULL) {
fprintf(stderr, "%s: missing --name argument for configuration\n",
argv[0]);
@@ -1724,7 +1744,7 @@ int main(int argc, char *argv[])
virEventRegisterDefaultImpl();
- if ((caps = lxcCapsInit()) == NULL)
+ if ((caps = lxcCapsInit(NULL)) == NULL)
goto cleanup;
if ((configFile = virDomainConfigFile(LXC_STATE_DIR,
@@ -1790,10 +1810,10 @@ int main(int argc, char *argv[])
goto cleanup;
}
- rc = lxcControllerRun(def, nveths, veths, monitor, client,
+ rc = lxcControllerRun(def, securityDriver,
+ nveths, veths, monitor, client,
ttyFDs, nttyFDs, handshakefd);
-
cleanup:
if (def)
virPidFileDelete(LXC_STATE_DIR, def->name);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e6cc90d..b712da4 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -441,6 +441,9 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char
*xml)
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
+ if (virSecurityManagerVerify(driver->securityManager, def) < 0)
+ goto cleanup;
+
if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0)
goto cleanup;
@@ -1394,7 +1397,21 @@ static int lxcMonitorClient(lxc_driver_t * driver,
return -1;
}
- if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
+ if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0)
{
+ VIR_ERROR(_("Failed to set security context for monitor for %s"),
+ vm->def->name);
+ goto error;
+ }
+
+ fd = socket(PF_UNIX, SOCK_STREAM, 0);
+
+ if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) <
0) {
+ VIR_ERROR(_("Failed to clear security context for monitor for %s"),
+ vm->def->name);
+ goto error;
+ }
+
+ if (fd < 0) {
virReportSystemError(errno, "%s",
_("Failed to create client socket"));
goto error;
@@ -1437,6 +1454,16 @@ static int lxcVmTerminate(lxc_driver_t *driver,
return -1;
}
+ virSecurityManagerRestoreAllLabel(driver->securityManager,
+ vm->def, false);
+ virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
+ /* Clear out dynamically assigned labels */
+ if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+ VIR_FREE(vm->def->seclabel.model);
+ VIR_FREE(vm->def->seclabel.label);
+ VIR_FREE(vm->def->seclabel.imagelabel);
+ }
+
if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) == 0)
{
rc = virCgroupKillPainfully(group);
if (rc < 0) {
@@ -1567,6 +1594,10 @@ lxcBuildControllerCmd(lxc_driver_t *driver,
virCommandAddArgFormat(cmd, "%d", ttyFDs[i]);
virCommandPreserveFD(cmd, ttyFDs[i]);
}
+
+ if (driver->securityDriverName)
+ virCommandAddArgPair(cmd, "--security",
driver->securityDriverName);
+
virCommandAddArg(cmd, "--handshake");
virCommandAddArgFormat(cmd, "%d", handshakefd);
virCommandAddArg(cmd, "--background");
@@ -1761,6 +1792,24 @@ static int lxcVmStart(virConnectPtr conn,
virReportOOMError();
goto cleanup;
}
+
+ /* If you are using a SecurityDriver with dynamic labelling,
+ then generate a security label for isolation */
+ VIR_DEBUG("Generating domain security label (if required)");
+ if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT)
+ vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_NONE;
+
+ if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) {
+ virDomainAuditSecurityLabel(vm, false);
+ goto cleanup;
+ }
+ virDomainAuditSecurityLabel(vm, true);
+
+ VIR_DEBUG("Setting domain security labels");
+ if (virSecurityManagerSetAllLabel(driver->securityManager,
+ vm->def, NULL) < 0)
+ goto cleanup;
+
for (i = 0 ; i < vm->def->nconsoles ; i++)
ttyFDs[i] = -1;
@@ -1916,6 +1965,16 @@ cleanup:
if (rc != 0) {
VIR_FORCE_CLOSE(priv->monitor);
virDomainConfVMNWFilterTeardown(vm);
+
+ virSecurityManagerRestoreAllLabel(driver->securityManager,
+ vm->def, false);
+ virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
+ /* Clear out dynamically assigned labels */
+ if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+ VIR_FREE(vm->def->seclabel.model);
+ VIR_FREE(vm->def->seclabel.label);
+ VIR_FREE(vm->def->seclabel.imagelabel);
+ }
}
for (i = 0 ; i < nttyFDs ; i++)
VIR_FORCE_CLOSE(ttyFDs[i]);
@@ -2040,6 +2099,9 @@ lxcDomainCreateAndStart(virConnectPtr conn,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
+ if (virSecurityManagerVerify(driver->securityManager, def) < 0)
+ goto cleanup;
+
if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
goto cleanup;
@@ -2084,6 +2146,102 @@ cleanup:
}
+static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel)
+{
+ lxc_driver_t *driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ int ret = -1;
+
+ lxcDriverLock(driver);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+
+ memset(seclabel, 0, sizeof(*seclabel));
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ lxcError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
+ goto cleanup;
+ }
+
+ if (!virDomainVirtTypeToString(vm->def->virtType)) {
+ lxcError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown virt type in domain definition '%d'"),
+ vm->def->virtType);
+ goto cleanup;
+ }
+
+ /*
+ * Theoretically, the pid can be replaced during this operation and
+ * return the label of a different process. If atomicity is needed,
+ * further validation will be required.
+ *
+ * Comment from Dan Berrange:
+ *
+ * Well the PID as stored in the virDomainObjPtr can't be changed
+ * because you've got a locked object. The OS level PID could have
+ * exited, though and in extreme circumstances have cycled through all
+ * PIDs back to ours. We could sanity check that our PID still exists
+ * after reading the label, by checking that our FD connecting to the
+ * LXC monitor hasn't seen SIGHUP/ERR on poll().
+ */
+ if (virDomainObjIsActive(vm)) {
+ if (virSecurityManagerGetProcessLabel(driver->securityManager,
+ vm->def, vm->pid, seclabel) < 0)
{
+ lxcError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Failed to get security label"));
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ if (vm)
+ virDomainObjUnlock(vm);
+ lxcDriverUnlock(driver);
+ return ret;
+}
+
+static int lxcNodeGetSecurityModel(virConnectPtr conn,
+ virSecurityModelPtr secmodel)
+{
+ lxc_driver_t *driver = conn->privateData;
+ int ret = 0;
+
+ lxcDriverLock(driver);
+ memset(secmodel, 0, sizeof(*secmodel));
+
+ /* NULL indicates no driver, which we treat as
+ * success, but simply return no data in *secmodel */
+ if (driver->caps->host.secModel.model == NULL)
+ goto cleanup;
+
+ if (!virStrcpy(secmodel->model, driver->caps->host.secModel.model,
+ VIR_SECURITY_MODEL_BUFLEN)) {
+ lxcError(VIR_ERR_INTERNAL_ERROR,
+ _("security model string exceeds max %d bytes"),
+ VIR_SECURITY_MODEL_BUFLEN - 1);
+ ret = -1;
+ goto cleanup;
+ }
+
+ if (!virStrcpy(secmodel->doi, driver->caps->host.secModel.doi,
+ VIR_SECURITY_DOI_BUFLEN)) {
+ lxcError(VIR_ERR_INTERNAL_ERROR,
+ _("security DOI string exceeds max %d bytes"),
+ VIR_SECURITY_DOI_BUFLEN-1);
+ ret = -1;
+ goto cleanup;
+ }
+
+cleanup:
+ lxcDriverUnlock(driver);
+ return ret;
+}
+
+
static int
lxcDomainEventRegister(virConnectPtr conn,
virConnectDomainEventCallback callback,
@@ -2332,6 +2490,10 @@ lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED,
void *opaque)
lxcMonitorEvent,
vm, NULL)) < 0)
goto error;
+
+ if (virSecurityManagerReserveLabel(driver->securityManager,
+ vm->def, vm->pid) < 0)
+ goto error;
} else {
vm->def->id = -1;
VIR_FORCE_CLOSE(priv->monitor);
@@ -2348,6 +2510,27 @@ error:
}
+static int
+lxcSecurityInit(lxc_driver_t *driver)
+{
+ virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName,
+ false,
+
driver->securityDefaultConfined,
+
driver->securityRequireConfined);
+ if (!mgr)
+ goto error;
+
+ driver->securityManager = mgr;
+
+ return 0;
+
+error:
+ VIR_ERROR(_("Failed to initialize security drivers"));
+ virSecurityManagerFree(mgr);
+ return -1;
+}
+
+
static int lxcStartup(int privileged)
{
char *ld;
@@ -2408,7 +2591,10 @@ static int lxcStartup(int privileged)
if (lxcLoadDriverConfig(lxc_driver) < 0)
goto cleanup;
- if ((lxc_driver->caps = lxcCapsInit()) == NULL)
+ if (lxcSecurityInit(lxc_driver) < 0)
+ goto cleanup;
+
+ if ((lxc_driver->caps = lxcCapsInit(lxc_driver)) == NULL)
goto cleanup;
lxc_driver->caps->privateDataAllocFunc = lxcDomainObjPrivateAlloc;
@@ -2500,6 +2686,7 @@ static int lxcShutdown(void)
lxcProcessAutoDestroyShutdown(lxc_driver);
virCapabilitiesFree(lxc_driver->caps);
+ virSecurityManagerFree(lxc_driver->securityManager);
VIR_FREE(lxc_driver->configDir);
VIR_FREE(lxc_driver->autostartDir);
VIR_FREE(lxc_driver->stateDir);
@@ -3671,6 +3858,8 @@ static virDriver lxcDriver = {
.domainGetBlkioParameters = lxcDomainGetBlkioParameters, /* 0.9.8 */
.domainGetInfo = lxcDomainGetInfo, /* 0.4.2 */
.domainGetState = lxcDomainGetState, /* 0.9.2 */
+ .domainGetSecurityLabel = lxcDomainGetSecurityLabel, /* 0.9.10 */
+ .nodeGetSecurityModel = lxcNodeGetSecurityModel, /* 0.9.10 */
.domainGetXMLDesc = lxcDomainGetXMLDesc, /* 0.4.2 */
.listDefinedDomains = lxcListDefinedDomains, /* 0.4.2 */
.numOfDefinedDomains = lxcNumDefinedDomains, /* 0.4.2 */
diff --git a/src/lxc/test_libvirtd_lxc.aug b/src/lxc/test_libvirtd_lxc.aug
index e757b82..f8544a0 100644
--- a/src/lxc/test_libvirtd_lxc.aug
+++ b/src/lxc/test_libvirtd_lxc.aug
@@ -13,6 +13,7 @@ module Test_libvirtd_lxc =
# This is disabled by default, uncomment below to enable it.
#
log_with_libvirtd = 1
+security_driver = \"selinux\"
"
test Libvirtd_lxc.lns get conf =
@@ -29,3 +30,4 @@ log_with_libvirtd = 1
{ "#comment" = "This is disabled by default, uncomment below to enable
it." }
{ "#comment" = "" }
{ "log_with_libvirtd" = "1" }
+{ "security_driver" = "selinux" }
--
1.7.7.5
12:39 p.m.
New subject: [libvirt] [PATCH v2 4/5] Add support for sVirt in the LXC driver
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
For the sake of backwards compat, LXC guests are *not*
confined by default. This is because it is not practical
to dynamically relabel containers using large filesystem
trees. Applications can create confined containers though,
by giving suitable XML configs
* src/Makefile.am: Link libvirt_lxc to security drivers
* src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h,
src/lxc/lxc_conf.c, src/lxc/lxc.conf,
src/lxc/test_libvirtd_lxc.aug: Config file handling for
security driver
* src/lxc/lxc_driver.c: Wire up security driver functions
* src/lxc/lxc_controller.c: Add a '--security' flag to
specify which security driver to activate
* src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set
the process label just before exec'ing init.
---
src/Makefile.am | 13 +++
src/lxc/libvirtd_lxc.aug | 15 +++-
src/lxc/lxc.conf | 18 ++++
src/lxc/lxc_conf.c | 62 ++++++++++++--
src/lxc/lxc_conf.h | 8 ++-
src/lxc/lxc_container.c | 9 ++-
src/lxc/lxc_container.h | 2 +
src/lxc/lxc_controller.c | 28 +++++-
src/lxc/lxc_driver.c | 193 ++++++++++++++++++++++++++++++++++++++++-
src/lxc/test_libvirtd_lxc.aug | 2 +
10 files changed, 335 insertions(+), 15 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:12 a.m.
New subject: [libvirt] [PATCH v2 5/5] Set a security context on /dev and /dev/pts mounts
From: "Daniel P. Berrange" <berrange(a)redhat.com>
To allow the container to access /dev and /dev/pts when under
sVirt, set an explicit mount option. Also set a max size on
the /dev mount to prevent DOS on memory usage
* src/lxc/lxc_container.c: Set /dev mount context
* src/lxc/lxc_controller.c: Set /dev/pts mount context
---
src/lxc/lxc_container.c | 72 +++++++++++++++++++++++++++++++++++-----------
src/lxc/lxc_controller.c | 32 ++++++++++++++++++--
2 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b0493fd..c2ece84 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -36,6 +36,10 @@
#include <unistd.h>
#include <mntent.h>
+#if HAVE_SELINUX
+# include <selinux/selinux.h>
+#endif
+
/* Yes, we want linux private one, for _syscall2() macro */
#include <linux/unistd.h>
@@ -419,7 +423,6 @@ err:
static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot)
{
const struct {
- bool onlyPivotRoot;
bool needPrefix;
const char *src;
const char *dst;
@@ -433,16 +436,19 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool
pivotRoot)
* mount point in the main OS becomes readonly too which is not what
* we want. Hence some things have two entries here.
*/
- { true, false, "devfs", "/dev", "tmpfs",
"mode=755", MS_NOSUID },
- { false, false, "proc", "/proc", "proc", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { false, false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND
},
- { false, false, "/proc/sys", "/proc/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { false, true, "/sys", "/sys", NULL, NULL, MS_BIND },
- { false, true, "/sys", "/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { false, true, "/selinux", "/selinux", NULL, NULL, MS_BIND
},
- { false, true, "/selinux", "/selinux", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { false, "proc", "/proc", "proc", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND },
+ { false, "/proc/sys", "/proc/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { true, "/sys", "/sys", NULL, NULL, MS_BIND },
+ { true, "/sys", "/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { true, "/selinux", "/selinux", NULL, NULL, MS_BIND },
+ { true, "/selinux", "/selinux", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
};
int i, rc = -1;
+ char *opts = NULL;
+#if HAVE_SELINUX
+ security_context_t con;
+#endif
VIR_DEBUG("Mounting basic filesystems %s pivotRoot=%d", NULLSTR(srcprefix),
pivotRoot);
@@ -450,10 +456,8 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool
pivotRoot)
char *src = NULL;
const char *srcpath = NULL;
- VIR_DEBUG("Consider %s onlyPivotRoot=%d",
- mnts[i].src, mnts[i].onlyPivotRoot);
- if (mnts[i].onlyPivotRoot && !pivotRoot)
- continue;
+ VIR_DEBUG("Process %s -> %s",
+ mnts[i].src, mnts[i].dst);
if (virFileMakePath(mnts[i].dst) < 0) {
virReportSystemError(errno,
@@ -474,8 +478,10 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool
pivotRoot)
/* Skip if mount doesn't exist in source */
if ((srcpath[0] == '/') &&
- (access(srcpath, R_OK) < 0))
+ (access(srcpath, R_OK) < 0)) {
+ VIR_FREE(src);
continue;
+ }
VIR_DEBUG("Mount %s on %s type=%s flags=%x, opts=%s",
srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags, mnts[i].opts);
@@ -489,15 +495,47 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool
pivotRoot)
VIR_FREE(src);
}
+ if (pivotRoot) {
+#if HAVE_SELINUX
+ if (getfilecon("/", &con) < 0 &&
+ errno != ENOTSUP) {
+ virReportSystemError(errno, "%s",
+ _("Failed to query file context on /"));
+ goto cleanup;
+ }
+#endif
+ /*
+ * tmpfs is limited to 64kb, since we only have device nodes in there
+ * and don't want to DOS the entire OS RAM usage
+ */
+ if (virAsprintf(&opts, "mode=755,size=65536%s%s%s",
+ con ? ",context=\"" : "",
+ con ? (const char *)con : "",
+ con ? "\"" : "") < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Mount devfs on /dev type=tmpfs flags=%x, opts=%s",
+ MS_NOSUID, opts);
+ if (mount("devfs", "/dev", "tmpfs", MS_NOSUID,
opts) < 0) {
+ virReportSystemError(errno,
+ _("Failed to mount %s on %s type %s"),
+ "devfs", "/dev",
"tmpfs");
+ goto cleanup;
+ }
+ }
+
rc = 0;
cleanup:
VIR_DEBUG("rc=%d", rc);
+ VIR_FREE(opts);
return rc;
}
-static int lxcContainerMountDevFS(virDomainFSDefPtr root)
+static int lxcContainerMountFSDevPTS(virDomainFSDefPtr root)
{
char *devpts = NULL;
int rc = -1;
@@ -1069,8 +1107,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef,
if (lxcContainerMountBasicFS("/.oldroot", true) < 0)
return -1;
- /* Mounts /dev and /dev/pts */
- if (lxcContainerMountDevFS(root) < 0)
+ /* Mounts /dev/pts */
+ if (lxcContainerMountFSDevPTS(root) < 0)
return -1;
/* Populates device nodes in /dev/ */
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index cb04b08..fedb7c8 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -52,6 +52,9 @@
# define NUMA_VERSION1_COMPATIBILITY 1
# include <numa.h>
#endif
+#if HAVE_SELINUX
+# include <selinux/selinux.h>
+#endif
#include "virterror_internal.h"
#include "logging.h"
@@ -1433,6 +1436,10 @@ lxcControllerRun(virDomainDefPtr def,
* marked as shared
*/
if (root) {
+#if HAVE_SELINUX
+ security_context_t con;
+#endif
+ char *opts;
VIR_DEBUG("Setting up private /dev/pts");
if (!virFileExists(root->src)) {
@@ -1467,16 +1474,35 @@ lxcControllerRun(virDomainDefPtr def,
goto cleanup;
}
+#if HAVE_SELINUX
+ if (getfilecon(root->src, &con) < 0 &&
+ errno != ENOTSUP) {
+ virReportSystemError(errno,
+ _("Failed to query file context on %s"),
+ root->src);
+ goto cleanup;
+ }
+#endif
/* XXX should we support gid=X for X!=5 for distros which use
* a different gid for tty? */
- VIR_DEBUG("Mounting 'devpts' on %s", devpts);
- if (mount("devpts", devpts, "devpts", 0,
- "newinstance,ptmxmode=0666,mode=0620,gid=5") < 0) {
+ if (virAsprintf(&opts,
"newinstance,ptmxmode=0666,mode=0620,gid=5%s%s%s",
+ con ? ",context=\"" : "",
+ con ? (const char *)con : "",
+ con ? "\"" : "") < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Mount devpts on %s type=tmpfs flags=%x, opts=%s",
+ devpts, MS_NOSUID, opts);
+ if (mount("devpts", devpts, "devpts", MS_NOSUID, opts) <
0) {
+ VIR_FREE(opts);
virReportSystemError(errno,
_("Failed to mount devpts on %s"),
devpts);
goto cleanup;
}
+ VIR_FREE(opts);
if (access(devptmx, R_OK) < 0) {
VIR_WARN("Kernel does not support private devpts, using shared
devpts");
--
1.7.7.5
12:45 p.m.
New subject: [libvirt] [PATCH v2 5/5] Set a security context on /dev and /dev/pts mounts
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
To allow the container to access /dev and /dev/pts when under
sVirt, set an explicit mount option. Also set a max size on
the /dev mount to prevent DOS on memory usage
* src/lxc/lxc_container.c: Set /dev mount context
* src/lxc/lxc_controller.c: Set /dev/pts mount context
---
src/lxc/lxc_container.c | 72 +++++++++++++++++++++++++++++++++++-----------
src/lxc/lxc_controller.c | 32 ++++++++++++++++++--
2 files changed, 84 insertions(+), 20 deletions(-)
@@ -450,10 +456,8 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool
pivotRoot)
char *src = NULL;
const char *srcpath = NULL;
- VIR_DEBUG("Consider %s onlyPivotRoot=%d",
- mnts[i].src, mnts[i].onlyPivotRoot);
- if (mnts[i].onlyPivotRoot && !pivotRoot)
- continue;
+ VIR_DEBUG("Process %s -> %s",
+ mnts[i].src, mnts[i].dst);
That threw me; I read it as the noun, and expected a pid to be the next
word. Even though it's only a debug statement, I'd do
s/Process/Processing/ to make it obvious that you were using it as a verb.
ACK with that nit fixed. Yay - we have LXC sVirt approved for 0.9.10!
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:45 p.m.
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
This is an update of
https://www.redhat.com/archives/libvir-list/2012-January/msg00418.html
Changes since v1:
- Pushed the first 2 patches which passed review
- Update to include all Eric's suggested changes
- Rebase to latest GIT master
I've gone ahead and pushed this series, after fixing the nits I pointed
out in my review.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:27 a.m.
On Thu, Feb 02, 2012 at 05:45:48PM -0700, Eric Blake wrote:
On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
> This is an update of
>
> https://www.redhat.com/archives/libvir-list/2012-January/msg00418.html
>
> Changes since v1:
>
> - Pushed the first 2 patches which passed review
> - Update to include all Eric's suggested changes
> - Rebase to latest GIT master
I've gone ahead and pushed this series, after fixing the nits I pointed
out in my review.
Thanks for taking care of this.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4707
days inactive
4719
days old
19 comments
3 participants
participants (3)
-
Ansis Atteka -
Daniel P. Berrange -
Eric Blake