10:29 a.m.
Previous discussion:
https://www.redhat.com/archives/libvir-list/2012-September/thread.html#01037
This adds <seclabel> to character devices' <source/> elements,
like this:
<serial type="unix">
<source mode="connect" path="/tmp/console.sock">
<seclabel model="selinux" relabel="no"/>
</source>
<target port="0"/>
</serial>
I tested it by controlling the labelling of the libguestfs console
socket (when unlabelled, SELinux prevents libguestfs from starting),
and it appears to work.
By the way, I could only get this to work by explicitly adding the
model="selinux" attribute. Looking at the code, it seems the same
would be true for disk-specific seclabels too, so the documentation is
wrong.
Rich.
10:29 a.m.
New subject: [libvirt] [PATCH v3 1/2] Make virSecurityDeviceLabelDefParseXML into generic device <seclabel> parser.
From: "Richard W.M. Jones" <rjones(a)redhat.com>
This is just code motion, allowing us to reuse the same function to
parse the <seclabel> from character devices too.
---
src/conf/domain_conf.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35814fb..06b3209 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3253,8 +3253,10 @@ error:
return -1;
}
+/* Parse the <seclabel> from a disk or serial device. */
static int
-virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
+ size_t *nseclabels_rtn,
virSecurityLabelDefPtr *vmSeclabels,
int nvmSeclabels, xmlXPathContextPtr ctxt)
{
@@ -3263,19 +3265,16 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
virSecurityLabelDefPtr vmDef = NULL;
char *model, *relabel, *label;
- if (def == NULL)
- return 0;
-
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
return 0;
- def->nseclabels = n;
- if (VIR_ALLOC_N(def->seclabels, n) < 0) {
+ *nseclabels_rtn = n;
+ if (VIR_ALLOC_N(*seclabels_rtn, n) < 0) {
virReportOOMError();
goto error;
}
for (i = 0; i < n; i++) {
- if (VIR_ALLOC(def->seclabels[i]) < 0) {
+ if (VIR_ALLOC((*seclabels_rtn)[i]) < 0) {
virReportOOMError();
goto error;
}
@@ -3292,7 +3291,7 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
break;
}
}
- def->seclabels[i]->model = model;
+ (*seclabels_rtn)[i]->model = model;
}
/* Can't use overrides if top-level doesn't allow relabeling. */
@@ -3306,9 +3305,9 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
relabel = virXMLPropString(list[i], "relabel");
if (relabel != NULL) {
if (STREQ(relabel, "yes")) {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
} else if (STREQ(relabel, "no")) {
- def->seclabels[i]->norelabel = true;
+ (*seclabels_rtn)[i]->norelabel = true;
} else {
virReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"),
@@ -3318,19 +3317,19 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
}
VIR_FREE(relabel);
} else {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
}
ctxt->node = list[i];
label = virXPathStringLimit("string(./label)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- def->seclabels[i]->label = label;
+ (*seclabels_rtn)[i]->label = label;
- if (label && def->seclabels[i]->norelabel) {
+ if (label && (*seclabels_rtn)[i]->norelabel) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot specify a label if relabelling is "
"turned off. model=%s"),
- NULLSTR(def->seclabels[i]->model));
+ NULLSTR((*seclabels_rtn)[i]->model));
goto error;
}
}
@@ -3339,10 +3338,11 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
error:
for (i = 0; i < n; i++) {
- virSecurityDeviceLabelDefFree(def->seclabels[i]);
+ virSecurityDeviceLabelDefFree((*seclabels_rtn)[i]);
}
- VIR_FREE(def->seclabels);
+ VIR_FREE(*seclabels_rtn);
VIR_FREE(list);
+ *nseclabels_rtn = 0;
return -1;
}
@@ -3834,7 +3834,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (sourceNode) {
xmlNodePtr saved_node = ctxt->node;
ctxt->node = sourceNode;
- if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
+ &def->nseclabels,
+ vmSeclabels,
nvmSeclabels,
ctxt) < 0)
goto error;
--
1.7.10.4
4:27 a.m.
New subject: [libvirt] [PATCH v3 1/2] Make virSecurityDeviceLabelDefParseXML into generic device <seclabel> parser.
On 20.09.2012 17:29, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones(a)redhat.com>
This is just code motion, allowing us to reuse the same function to
parse the <seclabel> from character devices too.
---
src/conf/domain_conf.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35814fb..06b3209 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3253,8 +3253,10 @@ error:
return -1;
}
+/* Parse the <seclabel> from a disk or serial device. */
static int
-virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
+ size_t *nseclabels_rtn,
virSecurityLabelDefPtr *vmSeclabels,
int nvmSeclabels, xmlXPathContextPtr ctxt)
{
@@ -3263,19 +3265,16 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
virSecurityLabelDefPtr vmDef = NULL;
char *model, *relabel, *label;
- if (def == NULL)
- return 0;
-
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
return 0;
- def->nseclabels = n;
- if (VIR_ALLOC_N(def->seclabels, n) < 0) {
+ *nseclabels_rtn = n;
+ if (VIR_ALLOC_N(*seclabels_rtn, n) < 0) {
I know it is not your fault, but this may lead to SIGSEGV. if this fails
then we proceed to error label where we dereference *seclabels_rtn.
I think the correct solution is:
- if VIR_ALLOC_N() fails, virReportOOMError(); return -1;
- set nseclabels_rtn = n; after alloc succeeds.
or check for *seclabels_rtn != NULL within error label.
virReportOOMError();
goto error;
}
for (i = 0; i < n; i++) {
- if (VIR_ALLOC(def->seclabels[i]) < 0) {
+ if (VIR_ALLOC((*seclabels_rtn)[i]) < 0) {
virReportOOMError();
goto error;
}
This is okay, since previous VIR_ALLOC zeroed the allocated memory.
@@ -3292,7 +3291,7 @@
virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
break;
}
}
- def->seclabels[i]->model = model;
+ (*seclabels_rtn)[i]->model = model;
}
/* Can't use overrides if top-level doesn't allow relabeling. */
@@ -3306,9 +3305,9 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
relabel = virXMLPropString(list[i], "relabel");
if (relabel != NULL) {
if (STREQ(relabel, "yes")) {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
} else if (STREQ(relabel, "no")) {
- def->seclabels[i]->norelabel = true;
+ (*seclabels_rtn)[i]->norelabel = true;
} else {
virReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"),
@@ -3318,19 +3317,19 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
}
VIR_FREE(relabel);
} else {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
}
ctxt->node = list[i];
label = virXPathStringLimit("string(./label)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- def->seclabels[i]->label = label;
+ (*seclabels_rtn)[i]->label = label;
- if (label && def->seclabels[i]->norelabel) {
+ if (label && (*seclabels_rtn)[i]->norelabel) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot specify a label if relabelling is "
"turned off. model=%s"),
- NULLSTR(def->seclabels[i]->model));
+ NULLSTR((*seclabels_rtn)[i]->model));
goto error;
}
}
@@ -3339,10 +3338,11 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
error:
for (i = 0; i < n; i++) {
- virSecurityDeviceLabelDefFree(def->seclabels[i]);
+ virSecurityDeviceLabelDefFree((*seclabels_rtn)[i]);
}
- VIR_FREE(def->seclabels);
+ VIR_FREE(*seclabels_rtn);
VIR_FREE(list);
+ *nseclabels_rtn = 0;
return -1;
}
@@ -3834,7 +3834,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (sourceNode) {
xmlNodePtr saved_node = ctxt->node;
ctxt->node = sourceNode;
- if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
+ &def->nseclabels,
+ vmSeclabels,
nvmSeclabels,
ctxt) < 0)
goto error;
ACK with the issue fixed.
Michal
10:29 a.m.
New subject: [libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
From: "Richard W.M. Jones" <rjones(a)redhat.com>
This allows the user to control labelling of each character device
separately (the default is to inherit from the VM).
---
docs/formatdomain.html.in | 8 ++++
src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 6 +++
src/security/security_selinux.c | 86 +++++++++++++++++++++++++++------------
4 files changed, 147 insertions(+), 30 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 51f897c..939312c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3434,6 +3434,14 @@ qemu-kvm -net nic,model=? /dev/null
</p>
<p>
+ The <code>source</code> element may contain an optional
+ <code>seclabel</code> to override the way that labelling
+ is done on the socket path. If this element is not present,
+ the <a href="#seclabel">security label is inherited from
+ the per-domain setting</a>.
+ </p>
+
+ <p>
Each character device element has an optional
sub-element <code><address></code> which can tie the
device to a
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06b3209..7023897 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1273,6 +1273,8 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
void virDomainChrDefFree(virDomainChrDefPtr def)
{
+ size_t i;
+
if (!def)
return;
@@ -1296,6 +1298,12 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
virDomainChrSourceDefClear(&def->source);
virDomainDeviceInfoClear(&def->info);
+ if (def->seclabels) {
+ for (i = 0; i < def->nseclabels; i++)
+ virSecurityDeviceLabelDefFree(def->seclabels[i]);
+ VIR_FREE(def->seclabels);
+ }
+
VIR_FREE(def);
}
@@ -5343,7 +5351,11 @@ error:
* <target>, which is used by <serial> but not <smartcard>). */
static int
virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
- xmlNodePtr cur, unsigned int flags)
+ xmlNodePtr cur, unsigned int flags,
+ virDomainChrDefPtr chr_def,
+ xmlXPathContextPtr ctxt,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels)
{
char *bindHost = NULL;
char *bindService = NULL;
@@ -5398,6 +5410,21 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
if (def->type == VIR_DOMAIN_CHR_TYPE_UDP)
VIR_FREE(mode);
}
+
+ /* Check for an optional seclabel override in <source/>. */
+ if (chr_def) {
+ xmlNodePtr saved_node = ctxt->node;
+ ctxt->node = cur;
+ if (virSecurityDeviceLabelDefParseXML (&chr_def->seclabels,
+ &chr_def->nseclabels,
+ vmSeclabels,
+ nvmSeclabels,
+ ctxt) < 0) {
+ ctxt->node = saved_node;
+ goto error;
+ }
+ ctxt->node = saved_node;
+ }
} else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) {
if (protocol == NULL)
protocol = virXMLPropString(cur, "type");
@@ -5591,7 +5618,10 @@ virDomainChrDefNew(void) {
static virDomainChrDefPtr
virDomainChrDefParseXML(virCapsPtr caps,
virDomainDefPtr vmdef,
+ xmlXPathContextPtr ctxt,
xmlNodePtr node,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels,
unsigned int flags)
{
xmlNodePtr cur;
@@ -5622,7 +5652,8 @@ virDomainChrDefParseXML(virCapsPtr caps,
}
cur = node->children;
- remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags);
+ remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags,
+ def, ctxt, vmSeclabels, nvmSeclabels);
if (remaining < 0)
goto error;
if (remaining) {
@@ -5759,7 +5790,8 @@ virDomainSmartcardDefParseXML(xmlNodePtr node,
}
cur = node->children;
- if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags) <
0)
+ if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags,
+ NULL, NULL, NULL, 0) < 0)
goto error;
if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
@@ -7240,7 +7272,8 @@ virDomainRedirdevDefParseXML(const xmlNodePtr node,
if (xmlStrEqual(cur->name, BAD_CAST "source")) {
int remaining;
- remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur,
flags);
+ remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur,
flags,
+ NULL, NULL, NULL, 0);
if (remaining != 0)
goto error;
}
@@ -9282,7 +9315,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9309,7 +9345,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9339,7 +9378,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
bool create_stub = true;
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9415,7 +9457,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -12359,6 +12404,7 @@ virDomainChrDefFormat(virBufferPtr buf,
const char *targetType = virDomainChrTargetTypeToString(def->deviceType,
def->targetType);
bool tty_compat;
+ size_t n;
int ret = 0;
@@ -12438,6 +12484,14 @@ virDomainChrDefFormat(virBufferPtr buf,
return -1;
}
+ /* Security label overrides, if any. */
+ if (def->seclabels && def->nseclabels > 0) {
+ virBufferAdjustIndent(buf, 2);
+ for (n = 0; n < def->nseclabels; n++)
+ virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
+ virBufferAdjustIndent(buf, -2);
+ }
+
virBufferAsprintf(buf, " </%s>\n", elementName);
return ret;
@@ -15262,6 +15316,21 @@ virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def,
const char *model)
return NULL;
}
+virSecurityDeviceLabelDefPtr
+virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
+{
+ int i;
+
+ if (def == NULL)
+ return NULL;
+
+ for (i = 0; i < def->nseclabels; i++) {
+ if (STREQ_NULLABLE(def->seclabels[i]->model, model))
+ return def->seclabels[i];
+ }
+ return NULL;
+}
+
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 510406a..84f566f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -966,6 +966,9 @@ struct _virDomainChrDef {
virDomainChrSourceDef source;
virDomainDeviceInfo info;
+
+ size_t nseclabels;
+ virSecurityDeviceLabelDefPtr *seclabels;
};
enum virDomainSmartcardType {
@@ -2123,6 +2126,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model);
virSecurityDeviceLabelDefPtr
virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
+
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a7e2420..c47c872 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1213,38 +1213,58 @@ done:
static int
virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
- virDomainChrSourceDefPtr dev)
+ virDomainChrDefPtr dev,
+ virDomainChrSourceDefPtr dev_source)
{
- virSecurityLabelDefPtr secdef;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
+ char *imagelabel;
char *in = NULL, *out = NULL;
int ret = -1;
- secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
- if (secdef == NULL)
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (seclabel == NULL)
return -1;
- if (secdef->norelabel)
+ if (dev)
+ chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
+ SECURITY_SELINUX_NAME);
+
+ if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel))
return 0;
- switch (dev->type) {
+ if (chr_seclabel)
+ imagelabel = chr_seclabel->label;
+ else
+ imagelabel = seclabel->imagelabel;
+
+ switch (dev_source->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- ret = virSecuritySELinuxSetFilecon(dev->data.file.path,
secdef->imagelabel);
+ ret = virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel);
+ break;
+
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ if (!dev_source->data.nix.listen) {
+ if (virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel)
< 0)
+ goto done;
+ }
+ ret = 0;
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
- if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) ||
- (virAsprintf(&out, "%s.out", dev->data.file.path) < 0))
{
+ if ((virAsprintf(&in, "%s.in", dev_source->data.file.path) <
0) ||
+ (virAsprintf(&out, "%s.out", dev_source->data.file.path)
< 0)) {
virReportOOMError();
goto done;
}
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecuritySELinuxSetFilecon(in, secdef->imagelabel) < 0) ||
- (virSecuritySELinuxSetFilecon(out, secdef->imagelabel) < 0)) {
+ if ((virSecuritySELinuxSetFilecon(in, imagelabel) < 0) ||
+ (virSecuritySELinuxSetFilecon(out, imagelabel) < 0)) {
goto done;
}
- } else if (virSecuritySELinuxSetFilecon(dev->data.file.path,
secdef->imagelabel) < 0) {
+ } else if (virSecuritySELinuxSetFilecon(dev_source->data.file.path,
imagelabel) < 0) {
goto done;
}
ret = 0;
@@ -1263,30 +1283,44 @@ done:
static int
virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def,
- virDomainChrSourceDefPtr dev)
+ virDomainChrDefPtr dev,
+ virDomainChrSourceDefPtr dev_source)
{
- virSecurityLabelDefPtr secdef;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
- secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
- if (secdef == NULL)
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (seclabel == NULL)
return -1;
- if (secdef->norelabel)
+ if (dev)
+ chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
+ SECURITY_SELINUX_NAME);
+ if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel))
return 0;
- switch (dev->type) {
+ switch (dev_source->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- if (virSecuritySELinuxRestoreSecurityFileLabel(dev->data.file.path) < 0)
+ if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path)
< 0)
goto done;
ret = 0;
break;
+
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ if (!dev_source->data.nix.listen) {
+ if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path)
< 0)
+ goto done;
+ }
+ ret = 0;
+ break;
+
case VIR_DOMAIN_CHR_TYPE_PIPE:
- if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0)
||
- (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) {
+ if ((virAsprintf(&out, "%s.out", dev_source->data.file.path)
< 0) ||
+ (virAsprintf(&in, "%s.in", dev_source->data.file.path) <
0)) {
virReportOOMError();
goto done;
}
@@ -1295,7 +1329,7 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def,
(virSecuritySELinuxRestoreSecurityFileLabel(in) < 0)) {
goto done;
}
- } else if (virSecuritySELinuxRestoreSecurityFileLabel(dev->data.file.path)
< 0) {
+ } else if
(virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) {
goto done;
}
ret = 0;
@@ -1323,7 +1357,7 @@ virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr
def,
dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
return 0;
- return virSecuritySELinuxRestoreSecurityChardevLabel(def, &dev->source);
+ return virSecuritySELinuxRestoreSecurityChardevLabel(def, dev, &dev->source);
}
@@ -1345,7 +1379,7 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr
def,
return virSecuritySELinuxRestoreSecurityFileLabel(database);
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
- return virSecuritySELinuxRestoreSecurityChardevLabel(def,
&dev->data.passthru);
+ return virSecuritySELinuxRestoreSecurityChardevLabel(def, NULL,
&dev->data.passthru);
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1703,7 +1737,7 @@ virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
return 0;
- return virSecuritySELinuxSetSecurityChardevLabel(def, &dev->source);
+ return virSecuritySELinuxSetSecurityChardevLabel(def, dev, &dev->source);
}
@@ -1727,7 +1761,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
return virSecuritySELinuxSetFilecon(database, data->content_context);
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
- return virSecuritySELinuxSetSecurityChardevLabel(def,
&dev->data.passthru);
+ return virSecuritySELinuxSetSecurityChardevLabel(def, NULL,
&dev->data.passthru);
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
--
1.7.10.4
4:27 a.m.
New subject: [libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
On 20.09.2012 17:29, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones(a)redhat.com>
This allows the user to control labelling of each character device
separately (the default is to inherit from the VM).
---
docs/formatdomain.html.in | 8 ++++
src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 6 +++
src/security/security_selinux.c | 86 +++++++++++++++++++++++++++------------
4 files changed, 147 insertions(+), 30 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 51f897c..939312c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3434,6 +3434,14 @@ qemu-kvm -net nic,model=? /dev/null
</p>
<p>
+ The <code>source</code> element may contain an optional
+ <code>seclabel</code> to override the way that labelling
+ is done on the socket path. If this element is not present,
+ the <a href="#seclabel">security label is inherited from
+ the per-domain setting</a>.
+ </p>
+
+ <p>
Each character device element has an optional
sub-element <code><address></code> which can tie the
device to a
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06b3209..7023897 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1273,6 +1273,8 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
void virDomainChrDefFree(virDomainChrDefPtr def)
{
+ size_t i;
+
if (!def)
return;
@@ -1296,6 +1298,12 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
virDomainChrSourceDefClear(&def->source);
virDomainDeviceInfoClear(&def->info);
+ if (def->seclabels) {
+ for (i = 0; i < def->nseclabels; i++)
+ virSecurityDeviceLabelDefFree(def->seclabels[i]);
+ VIR_FREE(def->seclabels);
+ }
+
VIR_FREE(def);
}
@@ -5343,7 +5351,11 @@ error:
* <target>, which is used by <serial> but not <smartcard>). */
static int
virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
- xmlNodePtr cur, unsigned int flags)
+ xmlNodePtr cur, unsigned int flags,
+ virDomainChrDefPtr chr_def,
+ xmlXPathContextPtr ctxt,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels)
{
char *bindHost = NULL;
char *bindService = NULL;
@@ -5398,6 +5410,21 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
if (def->type == VIR_DOMAIN_CHR_TYPE_UDP)
VIR_FREE(mode);
}
+
+ /* Check for an optional seclabel override in <source/>. */
+ if (chr_def) {
+ xmlNodePtr saved_node = ctxt->node;
+ ctxt->node = cur;
+ if (virSecurityDeviceLabelDefParseXML (&chr_def->seclabels,
+ &chr_def->nseclabels,
+ vmSeclabels,
+ nvmSeclabels,
+ ctxt) < 0) {
extra space; we don't type space between function name and (
+ ctxt->node = saved_node;
+ goto error;
+ }
+ ctxt->node = saved_node;
+ }
} else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) {
if (protocol == NULL)
protocol = virXMLPropString(cur, "type");
@@ -5591,7 +5618,10 @@ virDomainChrDefNew(void) {
static virDomainChrDefPtr
virDomainChrDefParseXML(virCapsPtr caps,
virDomainDefPtr vmdef,
+ xmlXPathContextPtr ctxt,
xmlNodePtr node,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels,
unsigned int flags)
{
xmlNodePtr cur;
@@ -5622,7 +5652,8 @@ virDomainChrDefParseXML(virCapsPtr caps,
}
cur = node->children;
- remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags);
+ remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags,
+ def, ctxt, vmSeclabels, nvmSeclabels);
long line
if (remaining < 0)
goto error;
if (remaining) {
@@ -5759,7 +5790,8 @@ virDomainSmartcardDefParseXML(xmlNodePtr node,
}
cur = node->children;
- if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags) <
0)
+ if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags,
+ NULL, NULL, NULL, 0) < 0)
goto error;
if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
@@ -7240,7 +7272,8 @@ virDomainRedirdevDefParseXML(const xmlNodePtr node,
if (xmlStrEqual(cur->name, BAD_CAST "source")) {
int remaining;
- remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur,
flags);
+ remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur,
flags,
+ NULL, NULL, NULL, 0);
if (remaining != 0)
goto error;
}
@@ -9282,7 +9315,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9309,7 +9345,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9339,7 +9378,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
bool create_stub = true;
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -9415,7 +9457,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
for (i = 0 ; i < n ; i++) {
virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
def,
+ ctxt,
nodes[i],
+ def->seclabels,
+ def->nseclabels,
flags);
if (!chr)
goto error;
@@ -12359,6 +12404,7 @@ virDomainChrDefFormat(virBufferPtr buf,
const char *targetType = virDomainChrTargetTypeToString(def->deviceType,
def->targetType);
bool tty_compat;
+ size_t n;
int ret = 0;
@@ -12438,6 +12484,14 @@ virDomainChrDefFormat(virBufferPtr buf,
return -1;
}
+ /* Security label overrides, if any. */
+ if (def->seclabels && def->nseclabels > 0) {
+ virBufferAdjustIndent(buf, 2);
+ for (n = 0; n < def->nseclabels; n++)
+ virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
+ virBufferAdjustIndent(buf, -2);
+ }
+
virBufferAsprintf(buf, " </%s>\n", elementName);
return ret;
@@ -15262,6 +15316,21 @@ virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def,
const char *model)
return NULL;
}
+virSecurityDeviceLabelDefPtr
+virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
+{
+ int i;
+
+ if (def == NULL)
+ return NULL;
+
+ for (i = 0; i < def->nseclabels; i++) {
+ if (STREQ_NULLABLE(def->seclabels[i]->model, model))
+ return def->seclabels[i];
+ }
+ return NULL;
+}
+
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 510406a..84f566f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -966,6 +966,9 @@ struct _virDomainChrDef {
virDomainChrSourceDef source;
virDomainDeviceInfo info;
+
+ size_t nseclabels;
+ virSecurityDeviceLabelDefPtr *seclabels;
};
enum virDomainSmartcardType {
@@ -2123,6 +2126,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model);
virSecurityDeviceLabelDefPtr
virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
+
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a7e2420..c47c872 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1213,38 +1213,58 @@ done:
static int
virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
- virDomainChrSourceDefPtr dev)
+ virDomainChrDefPtr dev,
+ virDomainChrSourceDefPtr dev_source)
{
- virSecurityLabelDefPtr secdef;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
+ char *imagelabel;
char *in = NULL, *out = NULL;
int ret = -1;
- secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
- if (secdef == NULL)
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (seclabel == NULL)
return -1;
- if (secdef->norelabel)
+ if (dev)
+ chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
+ SECURITY_SELINUX_NAME);
+
+ if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel))
return 0;
- switch (dev->type) {
+ if (chr_seclabel)
+ imagelabel = chr_seclabel->label;
+ else
+ imagelabel = seclabel->imagelabel;
+
+ switch (dev_source->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- ret = virSecuritySELinuxSetFilecon(dev->data.file.path,
secdef->imagelabel);
+ ret = virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel);
+ break;
+
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ if (!dev_source->data.nix.listen) {
+ if (virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel)
< 0)
again long line
+ goto done;
+ }
+ ret = 0;
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
- if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) ||
- (virAsprintf(&out, "%s.out", dev->data.file.path) < 0))
{
+ if ((virAsprintf(&in, "%s.in", dev_source->data.file.path) <
0) ||
+ (virAsprintf(&out, "%s.out", dev_source->data.file.path)
< 0)) {
virReportOOMError();
goto done;
}
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecuritySELinuxSetFilecon(in, secdef->imagelabel) < 0) ||
- (virSecuritySELinuxSetFilecon(out, secdef->imagelabel) < 0)) {
+ if ((virSecuritySELinuxSetFilecon(in, imagelabel) < 0) ||
+ (virSecuritySELinuxSetFilecon(out, imagelabel) < 0)) {
goto done;
}
- } else if (virSecuritySELinuxSetFilecon(dev->data.file.path,
secdef->imagelabel) < 0) {
+ } else if (virSecuritySELinuxSetFilecon(dev_source->data.file.path,
imagelabel) < 0) {
and again (there are more so I am stopping reporting them). Beside that
I haven't spotted anything bad with this patch. Hence ACK if you fix
coding style issues.
Michal
4:37 a.m.
New subject: [libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
On 21.09.2012 11:27, Michal Privoznik wrote:
On 20.09.2012 17:29, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones(a)redhat.com>
>
> This allows the user to control labelling of each character device
> separately (the default is to inherit from the VM).
> ---
> docs/formatdomain.html.in | 8 ++++
> src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++--
> src/conf/domain_conf.h | 6 +++
> src/security/security_selinux.c | 86 +++++++++++++++++++++++++++------------
> 4 files changed, 147 insertions(+), 30 deletions(-)
>
Ah, I gave premature ACK; you should have updated RNG as well. But I
believe you can handle it without any special review. So my ACK holds as
long as you update the schema and coding style raised by the first review.
Michal
4:52 a.m.
New subject: [libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
On Fri, Sep 21, 2012 at 11:37:08AM +0200, Michal Privoznik wrote:
On 21.09.2012 11:27, Michal Privoznik wrote:
> On 20.09.2012 17:29, Richard W.M. Jones wrote:
>> From: "Richard W.M. Jones" <rjones(a)redhat.com>
>>
>> This allows the user to control labelling of each character device
>> separately (the default is to inherit from the VM).
>> ---
>> docs/formatdomain.html.in | 8 ++++
>> src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++--
>> src/conf/domain_conf.h | 6 +++
>> src/security/security_selinux.c | 86 +++++++++++++++++++++++++++------------
>> 4 files changed, 147 insertions(+), 30 deletions(-)
>>
Ah, I gave premature ACK; you should have updated RNG as well. But I
believe you can handle it without any special review. So my ACK holds as
long as you update the schema and coding style raised by the first review.
I'll post an updated version anyway.
Are there style guidelines on long lines? I'm seeing a lot of
long lines in the existing code ..
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
4:56 a.m.
New subject: [libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
On Fri, Sep 21, 2012 at 10:52:08AM +0100, Richard W.M. Jones wrote:
On Fri, Sep 21, 2012 at 11:37:08AM +0200, Michal Privoznik wrote:
> On 21.09.2012 11:27, Michal Privoznik wrote:
> > On 20.09.2012 17:29, Richard W.M. Jones wrote:
> >> From: "Richard W.M. Jones" <rjones(a)redhat.com>
> >>
> >> This allows the user to control labelling of each character device
> >> separately (the default is to inherit from the VM).
> >> ---
> >> docs/formatdomain.html.in | 8 ++++
> >> src/conf/domain_conf.c | 77
+++++++++++++++++++++++++++++++++--
> >> src/conf/domain_conf.h | 6 +++
> >> src/security/security_selinux.c | 86
+++++++++++++++++++++++++++------------
> >> 4 files changed, 147 insertions(+), 30 deletions(-)
> >>
>
> Ah, I gave premature ACK; you should have updated RNG as well. But I
> believe you can handle it without any special review. So my ACK holds as
> long as you update the schema and coding style raised by the first review.
I'll post an updated version anyway.
Are there style guidelines on long lines? I'm seeing a lot of
long lines in the existing code ..
Informally, 80 characters, but as you see we've not really
enforced it.
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 :|
4447
days inactive
4448
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Richard W.M. Jones