[Libguestfs] [PATCH v2 4/5] add-domain: Pass SELinux label from guest to appliance (RHBZ#912499).

[Matthew Booth]

On Thu, 2013-02-28 at 16:02 +0000, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones at redhat.com>
> 
> When adding a domain (ie. guestfs_add_domain), read the SELinux
> <label/> and <imagelabel/> from the guest and use them for the
> appliance.  The appliance is statically labelled the same as the
> guest, so it is able to read its disks.
> 
> However tell libvirt not to try relabelling the disks, to prevent
> libvirt from disturbing the existing labels on the disks (in
> particular when the libvirt connection is closed, we don't want
> libvirt to try to restore some other label on the disks).
> ---
>  src/libvirt-domain.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 3f5b1ed..768ed78 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -20,6 +20,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <stdbool.h>
>  #include <assert.h>
>  
>  #ifdef HAVE_LIBVIRT
> @@ -42,6 +43,7 @@
>  
>  static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
>  static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, void *data), void *data);
> +static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn);
>  
>  static void
>  ignore_errors (void *ignore, virErrorPtr ignore2)
> @@ -169,6 +171,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
>    size_t ckp;
>    struct add_disk_data data;
>    CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
> +  CLEANUP_FREE char *label = NULL, *imagelabel = NULL;
>  
>    readonly =
>      optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK
> @@ -234,6 +237,16 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
>    if ((doc = get_domain_xml (g, dom)) == NULL)
>      return -1;
>  
> +  /* Find and pass the SELinux security label to the libvirt back end. */
> +  if (libvirt_selinux_label (g, doc, &label, &imagelabel) == -1)
> +    return -1;
> +  if (label && imagelabel) {
> +    guestfs_internal_set_libvirt_selinux_label (g, label, imagelabel);
> +    guestfs_internal_set_libvirt_selinux_norelabel_disks (g, 1);

Do we need both internal apis? 1 seems to imply the other.

> +  }
> +  else
> +    guestfs_internal_set_libvirt_selinux_norelabel_disks (g, 0);
> +
>    /* Add the disks. */
>    data.optargs.bitmask = 0;
>    data.readonly = readonly;
> @@ -374,6 +387,66 @@ connect_live (guestfs_h *g, virDomainPtr dom)
>    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.
> + *
> + * The reason for all this is because of sVirt:
> + * https://bugzilla.redhat.com/show_bug.cgi?id=912499#c7
> + */
> +static int
> +libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc,
> +                       char **label_rtn, char **imagelabel_rtn)
> +{
> +  CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
> +  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
> +  xmlNodeSetPtr nodes;
> +  size_t nr_nodes;
> +  xmlNodePtr node, child;
> +  bool gotlabel = 0, gotimagelabel = 0;

gotlabel and gotimagelabel appear to be unused apart from assignment.

> +  xpathCtx = xmlXPathNewContext (doc);
> +  if (xpathCtx == NULL) {
> +    error (g, _("unable to create new XPath context"));
> +    return -1;
> +  }
> +
> +  /* This gives us a set of all the <seclabel/> nodes, hopefully only one. */
> +  xpathObj = xmlXPathEvalExpression (BAD_CAST "/domain/seclabel",
> +                                     xpathCtx);
> +  if (xpathObj == NULL) {
> +    error (g, _("unable to evaluate XPath expression"));
> +    return -1;
> +  }
> +
> +  nodes = xpathObj->nodesetval;
> +  nr_nodes = nodes->nodeNr;
> +
> +  if (nr_nodes == 0 || nr_nodes > 1)
> +    return 0;

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.

> +  node = nodes->nodeTab[0];
> +  if (node->type != XML_ELEMENT_NODE) {
> +    error (g, _("expected <seclabel/> to be an XML element"));
> +    return -1;
> +  }

For this to happen would require bugs in both libvirt and
xmlXPathEvalExpression(). For clarity and brevity I wouldn't bother
error checking this.

> +
> +  /* Find the <label/> and <imagelabel/> child nodes. */
> +  for (child = node->children; child != NULL; child = child->next) {
> +    if (!gotlabel && STREQ ((char *) child->name, "label")) {
> +      /* Get the label content. */
> +      *label_rtn = (char *) xmlNodeGetContent (child);
> +      gotlabel = 1;
> +    }
> +    if (!gotimagelabel && STREQ ((char *) child->name, "imagelabel")) {
> +      /* Get the imagelabel content. */
> +      *imagelabel_rtn = (char *) xmlNodeGetContent (child);
> +      gotimagelabel = 1;
> +    }
> +  }

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]);

> +
> +  return 0;
> +}
> +
>  /* The callback function 'f' is called once for each disk.
>   *
>   * Returns number of disks, or -1 if there was an error.

Matt