From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; This internal API sets two SELinux labels in the handle (the process label and the image label -- they are closely related). If using the libvirt attach-method with SELinux and sVirt, then this will cause the following XML to be added to the appliance definition: <seclabel type=static model=selinux relabel=yes> <label>[LABEL HERE]</label> <imagelabel>[IMAGELABEL HERE]</imagelabel> </seclabel> It is ignored by other attach-methods. --- generator/actions.ml | 12 ++++++++++++ src/guestfs-internal.h | 2 ++ src/handle.c | 2 ++ src/launch-libvirt.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/generator/actions.ml b/generator/actions.ml index 8a8e3ff..59e667d 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -2694,6 +2694,18 @@ the default. Else C</var/tmp> is the default." }; longdesc = "\ Get the directory used by the handle to store the appliance cache." }; + { defaults with + name = "internal_set_libvirt_selinux_label"; + style = RErr, [String "label"; String "imagelabel"], []; + blocking = false; + visibility = VInternal; + shortdesc = "set SELinux label used by the libvirt attach method"; + longdesc = "\ +This internal function sets the SELinux security label (in +reality, two labels: the process label and the image label) +used by the appliance when the libvirt attach method is selected +(it is ignored by other attach methods)." }; + ] (* daemon_functions are any functions which cause some action diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index e1a7d31..78e2bf5 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -322,6 +322,8 @@ struct guestfs_h virDomainPtr dom; /* libvirt domain */ } virt; #endif + char *virt_selinux_label; + char *virt_selinux_imagelabel; }; /* Per-filesystem data stored for inspect_os. */ diff --git a/src/handle.c b/src/handle.c index c630daf..2f44632 100644 --- a/src/handle.c +++ b/src/handle.c @@ -326,6 +326,8 @@ guestfs_close (guestfs_h *g) if (g->pda) hash_free (g->pda); + free (g->virt_selinux_label); + free (g->virt_selinux_imagelabel); free (g->tmpdir); free (g->env_tmpdir); free (g->int_tmpdir); diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 7db2ce5..68d875e 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -855,6 +855,31 @@ construct_libvirt_xml_seclabel (guestfs_h *g, BAD_CAST "none")); XMLERROR (-1, xmlTextWriterEndElement (xo)); } + else if (g->virt_selinux_label && g->virt_selinux_imagelabel) { + /* Enable sVirt and pass a custom <seclabel/> inherited from the + * original libvirt domain (when guestfs_add_domain was called). + * https://bugzilla.redhat.com/show_bug.cgi?id=912499#c7 + */ + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "seclabel")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "type", + BAD_CAST "static")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "model", + BAD_CAST "selinux")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "relabel", + BAD_CAST "yes")); + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "label")); + XMLERROR (-1, xmlTextWriterWriteString (xo, + BAD_CAST g->virt_selinux_label)); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "imagelabel")); + XMLERROR (-1, xmlTextWriterWriteString (xo, + BAD_CAST g->virt_selinux_imagelabel)); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + } return 0; } @@ -1603,3 +1628,14 @@ struct attach_ops attach_ops_libvirt = { }; #endif /* no libvirt or libxml2 at compile time */ + +int +guestfs__internal_set_libvirt_selinux_label (guestfs_h *g, const char *label, + const char *imagelabel) +{ + free (g->virt_selinux_label); + g->virt_selinux_label = safe_strdup (g, label); + free (g->virt_selinux_imagelabel); + g->virt_selinux_imagelabel = safe_strdup (g, imagelabel); + return 0; +} -- 1.8.1.2

On Thu, 2013-02-28 at 16:02 +0000, Richard W.M. Jones wrote: I remain convinced that this is going to bite us at some point in the future. The fact that it works now is essentially a quirk of the default SELinux policy. I still don't understand at all how the confined guest can access the underlying appliance image, which libvirt presumably doesn't relabel. Given that it does currently work I don't think we should hold up these patches for a solution. However, I think we should document the problem as we currently understand it somewhere in the source tree, perhaps the TODO file. I'd prefer documentation of the problem of SELinux and the appliance image, but otherwise ACK. Matt

On Fri, 2013-03-01 at 10:49 +0000, Richard W.M. Jones wrote: Actually, that doesn't clear it up. Firstly, the <shareable/> attribute uses an alternative labelling. This alternative labelling still requires support from the policy, so it remains policy-specific. Secondly, it doesn't explain if/how the backing file is relabelled. If it isn't, why is it accessible? Again, this would be a quirk of the default policy. Dan? Matt

On Thu, 2013-02-28 at 16:02 +0000, Richard W.M. Jones wrote: Do we need both internal apis? 1 seems to imply the other. gotlabel and gotimagelabel appear to be unused apart from assignment. Not terribly important, but I wouldn't silently bomb out here if nr_nodes > 1. If you're going to catch it at all I'd add a warning and continue. For this to happen would require bugs in both libvirt and xmlXPathEvalExpression(). For clarity and brevity I wouldn't bother error checking this. Alternatively: labelObj = xmlXPathEvalExpression (BAD_CAST "/domain/seclabel[1]/label", xpathCtx); imageLabelObj = xmlXPathEvalExpression (BAD_CAST "/domain/seclabel[1]/imagelabel", xpathCtx); if (labelObj == NULL || imageLabelObj == NULL) { error (g, _("unable to evaluate XPath expression")); return -1; } labelNodes = labelObj->nodesetval; imageLabelNodes = imageLabelObj->nodesetval; if (labelNodes->nodeNr == 0 || imageLabelNodes->nodeNr == 0) return 0; *label_rtn = (char *) xmlNodeGetContent (labelNodes->nodeTab[0]); *imagelabel_rtn = (char *) xmlNodeGetContent (imageLabelNodes->nodeTab[0]); Matt

From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; This is just code motion. --- src/libvirt-domain.c | 138 +++++++++++++++++++++++++-------------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 768ed78..e2068dd 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -318,75 +318,6 @@ add_disk (guestfs_h *g, return guestfs__add_drive_opts (g, filename, &optargs); } -static int -connect_live (guestfs_h *g, virDomainPtr dom) -{ - int i; - CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL; - CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL; - CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL; - CLEANUP_FREE char *path = NULL, *attach_method = NULL; - xmlNodeSetPtr nodes; - - /* Domain XML. */ - if ((doc = get_domain_xml (g, dom)) == NULL) - return -1; - - xpathCtx = xmlXPathNewContext (doc); - if (xpathCtx == NULL) { - error (g, _("unable to create new XPath context")); - return -1; - } - - /* This gives us a set of all the <channel> nodes related to the - * guestfsd virtio-serial channel. - */ - xpathObj = xmlXPathEvalExpression (BAD_CAST - "//devices/channel[@type=\"unix\" and " - "./source/@mode=\"bind\" and " - "./source/@path and " - "./target/@type=\"virtio\" and " - &quot;./target/(a)name=\&quot;org.libguestfs.channel.0\&quot;]&quot;, - xpathCtx); - if (xpathObj == NULL) { - error (g, _("unable to evaluate XPath expression")); - return -1; - } - - nodes = xpathObj->nodesetval; - for (i = 0; i < nodes->nodeNr; ++i) { - CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL; - xmlAttrPtr attr; - - /* See note in function above. */ - xpathCtx->node = nodes->nodeTab[i]; - - /* The path is in <source path=..> attribute. */ - xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path", xpathCtx); - if (xppath == NULL || - xppath->nodesetval == NULL || - xppath->nodesetval->nodeNr == 0) { - xmlXPathFreeObject (xppath); - continue; /* no type attribute, skip it */ - } - assert (xppath->nodesetval->nodeTab[0]); - assert (xppath->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0]; - path = (char *) xmlNodeListGetString (doc, attr->children, 1); - break; - } - - if (path == NULL) { - error (g, _("this guest has no libvirt <channel> definition for guestfsd\n" - "See ATTACHING TO RUNNING DAEMONS in guestfs(3) for further information.")); - return -1; - } - - /* Got a path. */ - attach_method = safe_asprintf (g, "unix:%s", path); - return guestfs_set_attach_method (g, attach_method); -} - /* Find the <seclabel/> element in the libvirt XML, and if it exists * get the SELinux process label and image label from it. * @@ -580,6 +511,75 @@ for_each_disk (guestfs_h *g, return nr_added; } +static int +connect_live (guestfs_h *g, virDomainPtr dom) +{ + int i; + CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL; + CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL; + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL; + CLEANUP_FREE char *path = NULL, *attach_method = NULL; + xmlNodeSetPtr nodes; + + /* Domain XML. */ + if ((doc = get_domain_xml (g, dom)) == NULL) + return -1; + + xpathCtx = xmlXPathNewContext (doc); + if (xpathCtx == NULL) { + error (g, _("unable to create new XPath context")); + return -1; + } + + /* This gives us a set of all the <channel> nodes related to the + * guestfsd virtio-serial channel. + */ + xpathObj = xmlXPathEvalExpression (BAD_CAST + "//devices/channel[@type=\"unix\" and " + "./source/@mode=\"bind\" and " + "./source/@path and " + "./target/@type=\"virtio\" and " + &quot;./target/(a)name=\&quot;org.libguestfs.channel.0\&quot;]&quot;, + xpathCtx); + if (xpathObj == NULL) { + error (g, _("unable to evaluate XPath expression")); + return -1; + } + + nodes = xpathObj->nodesetval; + for (i = 0; i < nodes->nodeNr; ++i) { + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL; + xmlAttrPtr attr; + + /* See note in function above. */ + xpathCtx->node = nodes->nodeTab[i]; + + /* The path is in <source path=..> attribute. */ + xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path", xpathCtx); + if (xppath == NULL || + xppath->nodesetval == NULL || + xppath->nodesetval->nodeNr == 0) { + xmlXPathFreeObject (xppath); + continue; /* no type attribute, skip it */ + } + assert (xppath->nodesetval->nodeTab[0]); + assert (xppath->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); + attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0]; + path = (char *) xmlNodeListGetString (doc, attr->children, 1); + break; + } + + if (path == NULL) { + error (g, _("this guest has no libvirt <channel> definition for guestfsd\n" + "See ATTACHING TO RUNNING DAEMONS in guestfs(3) for further information.")); + return -1; + } + + /* Got a path. */ + attach_method = safe_asprintf (g, "unix:%s", path); + return guestfs_set_attach_method (g, attach_method); +} + static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom) { -- 1.8.1.2