Libguestfs - May 2018 - [PATCH for discussion only] lib: libvirt: If root, run qemu subprocess as root.root.

libvirt doesn't have a concept of "session qemu" for root:

  https://bugzilla.redhat.com/show_bug.cgi?id=890291

When a libguestfs-using process runs as root, and libvirt runs a qemu
subprocess, the qemu subprocess is run as a non-root user (typically
qemu.qemu).  This causes various problems, for example if we try to
open a file which is readable by root but unreadable by qemu.qemu then
the operation will fail.

This can be changed globally via a configuration file, but it can also
be changed by using a <seclabel/> clause in the XML (although I think
that's not the only effect):

  <seclabel type="static" model="dac"
relabel="no">
    <label>0:0</label>
  </seclabel>

This patch makes that change.

I notice that after this change, qemu is indeed running as root.
However the file being examined still gets relabelled by SELinux (to
virt_content_t IIRC).  Maybe this relabelling is in fact desirable.

Also as you can see from the patch there are cases where we use
another <seclabel model='selinux'/> element to set labels to a
known
value.  It's not clear if we can include both <seclabel/> elements.
The patch as shown overrides the selinux seclabel if running as root.

Rich.

If root, this adds the following label to the libvirt XML which causes
libvirt to run the qemu subprocess as root and not to do any SELinux
relabelling of disks:

  <seclabel type="static" model="dac"
relabel="no">
    <label>0:0</label>
  </seclabel>

Partial mitigation for problems caused by
https://bugzilla.redhat.com/show_bug.cgi?id=890291
---
 lib/launch-libvirt.c | 59 ++++++++++------------------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 844023b80..0b8ccb03a 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -528,53 +528,6 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
 
   clear_socket_create_context (g);
 
-  /* libvirt, if running as root, will run the qemu process as
-   * qemu.qemu, which means it won't be able to access the socket.
-   * There are roughly three things that get in the way:
-   *
-   * (1) Permissions of the socket.
-   *
-   * (2) Permissions of the parent directory(-ies).  Remember this if
-   *     $TMPDIR is located in your home directory.
-   *
-   * (3) SELinux/sVirt will prevent access.  libvirt ought to label
-   *     the socket.
-   *
-   * Note that the 'current_proc_is_root' flag here just means that we
-   * are root.  It's also possible for non-root user to try to use the
-   * system libvirtd by specifying a qemu:///system URI (RHBZ#913774)
-   * but there's no sane way to test for that.
-   */
-  if (params.current_proc_is_root) {
-    /* Current process is root, so try to create sockets that are
-     * owned by root.qemu with mode 0660 and hence accessible to qemu.
-     */
-    struct group *grp;
-
-    if (chmod (data->guestfsd_path, 0660) == -1) {
-      perrorf (g, "chmod: %s", data->guestfsd_path);
-      goto cleanup;
-    }
-
-    if (chmod (data->console_path, 0660) == -1) {
-      perrorf (g, "chmod: %s", data->console_path);
-      goto cleanup;
-    }
-
-    grp = getgrnam ("qemu");
-    if (grp != NULL) {
-      if (chown (data->guestfsd_path, 0, grp->gr_gid) == -1) {
-        perrorf (g, "chown: %s", data->guestfsd_path);
-        goto cleanup;
-      }
-      if (chown (data->console_path, 0, grp->gr_gid) == -1) {
-        perrorf (g, "chown: %s", data->console_path);
-        goto cleanup;
-      }
-    } else
-      debug (g, "cannot find group 'qemu'");
-  }
-
   /* Store any secrets in libvirtd, keeping a mapping from the secret
    * to its UUID.
    */
@@ -1257,7 +1210,17 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
                                 const struct libvirt_xml_params *params,
                                 xmlTextWriterPtr xo)
 {
-  if (!params->enable_svirt) {
+  if (params->current_proc_is_root) {
+    start_element ("seclabel") {
+      attribute ("type", "static");
+      attribute ("model", "dac");
+      attribute ("relabel", "no");
+      start_element ("label") {
+        string ("0:0");
+      } end_element ();
+    } end_element ();
+  }
+  else if (!params->enable_svirt) {
     /* This disables SELinux/sVirt confinement. */
     start_element ("seclabel") {
       attribute ("type", "none");
-- 
2.16.2

On Mon, May 21, 2018 at 06:22:06PM +0100, Richard W.M. Jones
wrote:
> libvirt doesn't have a concept of "session qemu" for root:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=890291
> 
> When a libguestfs-using process runs as root, and libvirt runs a qemu
> subprocess, the qemu subprocess is run as a non-root user (typically
> qemu.qemu).  This causes various problems, for example if we try to
> open a file which is readable by root but unreadable by qemu.qemu then
> the operation will fail.
> 
> This can be changed globally via a configuration file, but it can also
> be changed by using a <seclabel/> clause in the XML (although I think
> that's not the only effect):
> 
>   <seclabel type="static" model="dac"
relabel="no">
>     <label>0:0</label>
>   </seclabel>
> 
> This patch makes that change.
> 
> I notice that after this change, qemu is indeed running as root.
> However the file being examined still gets relabelled by SELinux (to
> virt_content_t IIRC).  Maybe this relabelling is in fact desirable.
If you turn off relabelling you'll certainly get SELinux denials,
unless the files already have the virt_content_t label or equiv.
So if you wanted to prevent SELinux labelling, you would probably
have to turn off SELinux confinement entirely. 
> Also as you can see from the patch there are cases where we use
> another <seclabel model='selinux'/> element to set labels to
a known
> value.  It's not clear if we can include both <seclabel/>
elements.
> The patch as shown overrides the selinux seclabel if running as root.
Yes, you can set multiple <seclabel> elements in the same guest - in
fact if you look at a running guest you'll see two present. You just
need to make sure "model" attribute is unique on each.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|