Libguestfs - May 2014 - SELinux relabel API

[
    I realized that we were discussing adding this feature, in various
    private email, IRC, and this long bugzilla thread:
    https://bugzilla.redhat.com/show_bug.cgi?id=1060423
    That's not how we should do things.  Let's discuss it on the
    mailing list.
  ]

One thing that virt-customize/virt-sysprep/virt-builder have to do is
relabel SELinux guests.

What we do at the moment is run:

      if load_policy && fixfiles restore; then
        rm -f /.autorelabel
      else
        touch /.autorelabel
        echo '%s: SELinux relabelling failed, will relabel at boot
instead.'
      fi

while chrooted into the guest (using the 'guestfs_sh' API).

This has a number of problems:

 - It has to load the policy using 'load_policy', but this doesn't
   work sometimes:

    * RHEL 5 load_policy takes a parameter.

    * Doesn't work if appliance kernel is significantly different from
      guest kernel version, because the binary policy format changes
      irregularly and is not backwards compatible.

    * Requires the appliance [host] kernel to be compiled with
      LSM/SELinux support.

 - Touching /.autorelabel is often broken, eg. it's broken in Fedora 20
   because of systemd (RHBZ#1049656).

 - /etc/resolv.conf will not be relabelled if guestfs network is on,
   because of resolv.conf shenanigans in libguestfs.git/daemon/command.c

 - It requires running guest code, which we'd like to avoid.

What would be nice would be to have an API to just do this
relabelling.  Libguestfs could change this API as required to handle
different guests.

Dan Walsh helpfully pointed out to us that we've been doing it wrong
all along :-)  A much better way to relabel is to run:

  setfiles /etc/selinux/targeted/contexts/files/file_contexts DIR

where 'file_contexts' is a file which contains the default labels for
files (a set of regexps), and 'DIR' is the directory at which
relabelling starts.  Note that 'setfiles' would be the libguestfs
appliance binary, so no guest binary needs to be run.

A simple API could just look like this:

  guestfs_selinux_relabel (g);

which would always use the 'targeted' policy from the guest, and
always start relabelling at the root.  This would work fine for
virt-builder.

For Colin's requirements for Project Atomic, I suspect he will want to
be able to set the file_contexts file and the root directory, but I'll
leave him to describe what would be useful.

A couple of notes:

 - I'd like to avoid baking in assumptions from the 'setfiles'
command
   as far as possible.  libguestfs APIs last for many years and some
   have caused us many years of regret (but that's our job) :-/

 - Is it a good idea to tie this into inspection in some way -- for
   example, inspection could provide us with the path to the current or
   default SELinux policy.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

> Dan Walsh helpfully pointed out to us that we've been doing it wrong
> all along :-)  A much better way to relabel is to run:
> 
>   setfiles /etc/selinux/targeted/contexts/files/file_contexts DIR
Yes, this is what I'm doing with OSTree.  However in the general cross
labeling case it has a subtle issue with PCRE:
http://comments.gmane.org/gmane.comp.security.selinux/20214

There is of course always the potential issue for incompatible future
changes in the file_contexts format.

My current workaround is:
https://github.com/cgwalters/rpm-ostree/commit/0cb346b798aead0fd544e2c9ef45f1817ba19434

On Saturday 24 May 2014 16:25:14 Richard W.M. Jones
wrote:
> What would be nice would be to have an API to just do this
> relabelling.  Libguestfs could change this API as required to handle
> different guests.
> 
> Dan Walsh helpfully pointed out to us that we've been doing it wrong
> all along :-)  A much better way to relabel is to run:
> 
>   setfiles /etc/selinux/targeted/contexts/files/file_contexts DIR
> 
> where 'file_contexts' is a file which contains the default labels
for
> files (a set of regexps), and 'DIR' is the directory at which
> relabelling starts.  Note that 'setfiles' would be the libguestfs
> appliance binary, so no guest binary needs to be run.
> 
> A simple API could just look like this:
> 
>   guestfs_selinux_relabel (g);
> 
> which would always use the 'targeted' policy from the guest, and
> always start relabelling at the root.  This would work fine for
> virt-builder.
> 
> For Colin's requirements for Project Atomic, I suspect he will want to
> be able to set the file_contexts file and the root directory, but
> I'll leave him to describe what would be useful.
I have been experiment/working on this last week (fighting against two 
SELinux, the one to make happy in the guest, and the active-but-not-
enforcing one eventually present in the appliance), and now I have 
something working.

As you said, there are various possibilities in configuring such 
selinux_relabel API; in my first implementation I chose to have it take 
as mandatory argument the root of the guest to relabel, and it is able 
to discover the configured policy there. Of course I am open to 
suggestions/complains/etc about it.

-- 
Pino Toscano

Move the current logic of the SELinux relabel to a new daemon function,
so
a) within the daemon it is much easier to do changes without having
   files touched or moved as side effects of other daemon calls
b) clients can just call this new API and be done with it, with no need
   to think by themselves about what to do to proper do the relabel
c) the logic can be updated anytime at once within libguestfs
---
 customize/customize_run.ml | 10 +---------
 daemon/selinux.c           | 27 +++++++++++++++++++++++++++
 generator/actions.ml       | 12 ++++++++++++
 src/MAX_PROC_NR            |  2 +-
 4 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 4d83e90..2650e19 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -300,15 +300,7 @@ exec >>%s 2>&1
 
   if ops.flags.selinux_relabel then (
     msg (f_"SELinux relabelling");
-    let cmd = sprintf "
-      if load_policy && fixfiles restore; then
-        rm -f /.autorelabel
-      else
-        touch /.autorelabel
-        echo '%s: SELinux relabelling failed, will relabel at boot
instead.'
-      fi
-    " prog in
-    do_run ~display:"load_policy && fixfiles restore" cmd
+    g#selinux_relabel "/"
   );
 
   (* Clean up the log file:
diff --git a/daemon/selinux.c b/daemon/selinux.c
index 1c1446d..d0a3832 100644
--- a/daemon/selinux.c
+++ b/daemon/selinux.c
@@ -90,3 +90,30 @@ do_getcon (void)
 OPTGROUP_SELINUX_NOT_AVAILABLE
 
 #endif /* !HAVE_LIBSELINUX */
+
+int
+do_selinux_relabel (const char *root)
+{
+  CLEANUP_FREE char *cmd = NULL, *out = NULL;
+  const char cmd_fmt[] +    "if load_policy && fixfiles restore;
then\n"
+    "  rm -f %.*s/.autorelabel\n"
+    "else\n"
+    "  touch %.*s/.autorelabel\n"
+    "  echo 'SELinux relabelling failed, will relabel at boot
instead.'\n"
+    "fi\n";
+  int len = strlen (root);
+
+  if (root[len - 1] == '/')
+    --len;
+
+  if (asprintf (&cmd, cmd_fmt, len, root, len, root) == -1) {
+    reply_with_perror ("asprintf");
+    return -1;
+  }
+
+  out = do_sh (cmd);
+  if (verbose)
+    fprintf (stderr, "%s\n", out);
+  return out == NULL ? -1 : 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index ed65c6e..a59fe31 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11924,6 +11924,18 @@ New (SVR4) portable format with a checksum.
 
 =back" };
 
+  { defaults with
+    name = "selinux_relabel";
+    style = RErr, [Pathname "root"], [];
+    proc_nr = Some 420;
+    shortdesc = "do the SELinux relabel of the files";
+    longdesc = "\
+This does a relabel of the files of the system under the specified
+C<root> according to the SELinux policy in the system mounted in that
+C<root>.
+
+See the documentation about SELINUX in L<guestfs(3)>." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 7b53aa0..816d01b 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-419
+420
-- 
1.9.3

Rewrite the relabel API to read the policy configured in the guest,
invoking setfiles (added as part of the appliance, as part of
policycoreutils) to relabel the specified root. In case of failure at
any point of the process, a touch of .autorelabel in the root is tried
as last-attempt measure to do the relabel.

Considering that running SELinux tools in the appliance might be
affected by the SELinux state (leading to wrong results),
selinux_relabel now bails out if SELinux is enabled in the appliance.
As a result of this, virt-builder and virt-customize explicitly disable
it if the relabel is enabled.
---
 appliance/packagelist.in    |   1 +
 builder/builder.ml          |   5 +-
 customize/customize_main.ml |   5 +-
 daemon/selinux.c            | 198 ++++++++++++++++++++++++++++++++++++++++----
 generator/actions.ml        |   7 ++
 5 files changed, 197 insertions(+), 19 deletions(-)

diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index e4a9c8b..6336048 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -217,6 +217,7 @@ dnl Enabling this pulls out 140 extra packages
 dnl into the appliance:
 dnl ocfs2-tools
 parted
+policycoreutils
 procps
 procps-ng
 psmisc
diff --git a/builder/builder.ml b/builder/builder.ml
index e7be901..a0ef6d7 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -585,7 +585,10 @@ let main ()      (match smp with None -> () | Some smp
-> g#set_smp smp);
     g#set_network network;
 
-    g#set_selinux ops.flags.selinux_relabel;
+    (* If a relabel is needed, make sure to turn SELinux off to avoid
+     * awkward interactions with the relabel process.
+     *)
+    if ops.flags.selinux_relabel then g#set_selinux false;
 
     (* The output disk is being created, so use cache=unsafe here. *)
     g#add_drive_opts ~format:output_format ~cachemode:"unsafe"
output_filename;
diff --git a/customize/customize_main.ml b/customize/customize_main.ml
index 00d5bae..a6c6643 100644
--- a/customize/customize_main.ml
+++ b/customize/customize_main.ml
@@ -193,7 +193,10 @@ read the man page virt-customize(1).
     (match memsize with None -> () | Some memsize -> g#set_memsize
memsize);
     (match smp with None -> () | Some smp -> g#set_smp smp);
     g#set_network network;
-    g#set_selinux ops.flags.selinux_relabel;
+    (* If a relabel is needed, make sure to turn SELinux off to avoid
+     * awkward interactions with the relabel process.
+     *)
+    if ops.flags.selinux_relabel then g#set_selinux false;
 
     (* Add disks. *)
     add g dryrun;
diff --git a/daemon/selinux.c b/daemon/selinux.c
index d0a3832..879bc61 100644
--- a/daemon/selinux.c
+++ b/daemon/selinux.c
@@ -21,6 +21,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 
 #ifdef HAVE_SELINUX_SELINUX_H
 #include <selinux/selinux.h>
@@ -31,6 +33,8 @@
 #include "actions.h"
 #include "optgroups.h"
 
+GUESTFSD_EXT_CMD(str_setfiles, setfiles);
+
 #if defined(HAVE_LIBSELINUX)
 
 int
@@ -92,28 +96,188 @@ OPTGROUP_SELINUX_NOT_AVAILABLE
 #endif /* !HAVE_LIBSELINUX */
 
 int
-do_selinux_relabel (const char *root)
+optgroup_selinuxtools_available (void)
+{
+  return prog_exists (str_setfiles);
+}
+
+#define SELINUXTYPE "SELINUXTYPE"
+
+static int
+has_selinux_mountpoint (void)
+{
+  static int has_mp = -1;
+  if (has_mp == -1) {
+    struct stat sb;
+    dev_t root_dev;
+
+    if (stat ("/", &sb) == -1)
+      return -1;
+    root_dev = sb.st_dev;
+
+    if (stat ("/sys/fs/selinux", &sb) == 0 && sb.st_dev
!= root_dev) {
+      has_mp = 1;
+      return has_mp;
+    }
+
+    if (stat ("/selinux", &sb) == 0 && sb.st_dev !=
root_dev) {
+      has_mp = 1;
+      return has_mp;
+    }
+  }
+  return has_mp;
+}
+
+static int
+read_selinux_policy (const char *file, char **policy)
+{
+  FILE *f;
+  ssize_t r;
+  CLEANUP_FREE char *line = NULL;
+  size_t len;
+
+  f = fopen (file, "r");
+  if (f == NULL)
+    return -1;
+
+  while ((r = getline (&line, &len, f)) != -1) {
+    if ((size_t) r >= sizeof (SELINUXTYPE) + 1
+        && STRPREFIX (line, SELINUXTYPE "=")) {
+      *policy = strndup (line + sizeof (SELINUXTYPE), r - sizeof (SELINUXTYPE)
- 1);
+      if (*policy == NULL) {
+        int errno_save = errno;
+        fclose (f);
+        errno = errno_save;
+        return -1;
+      }
+      trim (*policy);
+      break;
+    }
+  }
+
+  fclose (f);
+
+  return 0;
+}
+
+static int
+length_without_training_slash (const char *path)
 {
-  CLEANUP_FREE char *cmd = NULL, *out = NULL;
-  const char cmd_fmt[] -    "if load_policy && fixfiles restore;
then\n"
-    "  rm -f %.*s/.autorelabel\n"
-    "else\n"
-    "  touch %.*s/.autorelabel\n"
-    "  echo 'SELinux relabelling failed, will relabel at boot
instead.'\n"
-    "fi\n";
-  int len = strlen (root);
-
-  if (root[len - 1] == '/')
+  int len = strlen (path);
+  if (path[len - 1] == '/')
     --len;
+  return len;
+}
+
+int
+do_selinux_relabel (const char *root)
+{
+  CLEANUP_FREE char *selinux_config = NULL;
+  CLEANUP_FREE char *policy = NULL;
+  CLEANUP_FREE char *sys_rootpath = NULL;
+  CLEANUP_FREE char *sys_filecontextpath = NULL;
+  CLEANUP_FREE char *sys_autorelabelpath = NULL;
+  CLEANUP_FREE char *err = NULL;
+  CLEANUP_FREE char *out = NULL;
+  int r;
+  int len;
+  const size_t MAX_ARGS = 16;
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
 
-  if (asprintf (&cmd, cmd_fmt, len, root, len, root) == -1) {
-    reply_with_perror ("asprintf");
+  if (has_selinux_mountpoint () > 0) {
+    reply_with_error ("cannot work when SELinux is enabled in the "
+                      "appliance\n");
     return -1;
   }
 
-  out = do_sh (cmd);
+  len = length_without_training_slash (root);
+
+  if (asprintf (&selinux_config, "%s%.*s/etc/selinux/config",
+                sysroot, len, root) == -1) {
+    if (verbose)
+      fprintf (stderr, "asprintf/selinux_config failed\n");
+    goto do_autorelabel;
+  }
+
+  r = read_selinux_policy (selinux_config, &policy);
+  if (r == -1) {
+    if (verbose)
+      fprintf (stderr, "cannot read policy from %s\n",
selinux_config);
+    goto do_autorelabel;
+  }
+  if (verbose)
+    fprintf (stderr, "policy in %s: %s\n", root, policy);
+
+  if (policy[0] == '\0')
+    goto do_autorelabel;
+
+  /* Make the root path relative to /sysroot. */
+  sys_rootpath = sysroot_path (root);
+  if (sys_rootpath == NULL) {
+    if (verbose)
+      fprintf (stderr, "sysroot_path failed\n");
+    goto do_autorelabel;
+  }
+
+  len = length_without_training_slash (sys_rootpath);
+
+  if (asprintf (&sys_filecontextpath,
+                "%.*s/etc/selinux/%s/contexts/files/file_contexts",
+                len, sys_rootpath, policy) == -1) {
+    if (verbose)
+      fprintf (stderr, "asprintf/file_contexts failed\n");
+    goto do_autorelabel;
+  }
+
+  ADD_ARG (argv, i, str_setfiles);
+  ADD_ARG (argv, i, "-W");
   if (verbose)
-    fprintf (stderr, "%s\n", out);
-  return out == NULL ? -1 : 0;
+    ADD_ARG (argv, i, "-v");
+  ADD_ARG (argv, i, "-r");
+  ADD_ARG (argv, i, sys_rootpath);
+  ADD_ARG (argv, i, sys_filecontextpath);
+  ADD_ARG (argv, i, sys_rootpath);
+  ADD_ARG (argv, i, NULL);
+
+  r = commandv (verbose ? &out : NULL, &err, argv);
+  if (r == -1) {
+    if (verbose)
+      fprintf (stderr, "failed setfiles %s for %s: %s\n", policy,
root, err);
+    goto do_autorelabel;
+  }
+  if (verbose)
+    fprintf (stderr, "output from setfiles:\n%s\n", out);
+
+  if (asprintf (&sys_autorelabelpath, "%.*s/.autorelabel",
+                len, sys_rootpath) == -1) {
+    if (verbose)
+      fprintf (stderr, "asprintf/autorelabelpath failed\n");
+    /* We can safely exit in this case as well, since at most there is a
+     * ~/.autorelabel left in the guest.
+     */
+    return 0;
+  }
+
+  unlink (sys_autorelabelpath);
+
+  return 0;
+
+ do_autorelabel:
+  /* All of the above failed -- try to touch .autorelabel in the root
+   * and hope for the best.
+   */
+  if (verbose)
+    fprintf (stderr, "trying /.autorelabel\n");
+
+  len = length_without_training_slash (root);
+
+  if (asprintf (&sys_autorelabelpath, "%.*s/.autorelabel",
+                len, root) == -1) {
+    if (verbose)
+      fprintf (stderr, "asprintf/autorelabelpath failed\n");
+    return -1;
+  }
+
+  return do_touch (sys_autorelabelpath);
 }
diff --git a/generator/actions.ml b/generator/actions.ml
index a59fe31..3bb91c8 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11928,12 +11928,19 @@ New (SVR4) portable format with a checksum.
     name = "selinux_relabel";
     style = RErr, [Pathname "root"], [];
     proc_nr = Some 420;
+    optional = Some "selinuxtools";
     shortdesc = "do the SELinux relabel of the files";
     longdesc = "\
 This does a relabel of the files of the system under the specified
 C<root> according to the SELinux policy in the system mounted in that
 C<root>.
 
+I<Note:> the relabel does not work when SELinux is enabled (see
+C<guestfs_set_selinux>), as the kernel and the policy in the
+appliance might not be compatible with the policy found inside the
+guest mounted at C<root>; hence, this function will error out in this
+case.
+
 See the documentation about SELINUX in L<guestfs(3)>." };
 
 ]
-- 
1.9.3

Hi,

Im trying to expand an linux image but it does not seem to get bigger after 
everything is completed.
The thing i noticed was, i am trying to expand a /dev/vda1 within the image 
but virt-resize keeps referring it as /dev/sda1. See below:

# virt-resize --dry-run --expand /dev/vda1 test.qcow2 test-new.qcow2
Examining test.qcow2 ...
**********
Summary of changes:

/dev/sda1: This partition will be resized from 13.4G to 23.4G.  The 
    filesystem ext3 on /dev/sda1 will be expanded using the 'resize2fs' 
    method.
/dev/sda2: This partition will be left alone.
**********

Is this a bug or am i doing something wrong?

Thx!

On Mon, May 26, 2014 at 10:59:26PM +0200, Info || Vertixo B.V.
wrote:
> Hi,
> 
> Im trying to expand an linux image but it does not seem to get bigger after
> everything is completed.
When you say "it does not seem to get bigger" how do you determine
that?  What precise commands did you use to determine the size of the
filesystem?
>From the log below it appears everything is working fine.
> The thing i noticed was, i am trying to expand a /dev/vda1 within the image
> but virt-resize keeps referring it as /dev/sda1. See below:
libguestfs uses a canonical naming scheme for devices, where
"/dev/sda1" means "the first partition on the first device".
It has
nothing to do with how the guest refers to devices -- eg. a Windows
guest might refer to this partition as "C:".

See:
http://libguestfs.org/guestfs.3.html#block-device-naming
> # virt-resize --dry-run --expand /dev/vda1 test.qcow2 test-new.qcow2
> Examining test.qcow2 ...
> **********
> Summary of changes:
> 
> /dev/sda1: This partition will be resized from 13.4G to 23.4G.  The 
>     filesystem ext3 on /dev/sda1 will be expanded using the
'resize2fs'
>     method.
> /dev/sda2: This partition will be left alone.
> **********
> 
> Is this a bug or am i doing something wrong?
> 
> Thx!
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Please keep all replies on the list.

On Tue, May 27, 2014 at 11:22:17AM +0200, Info || Vertixo B.V.
wrote:
> Hi,
> 
> Ah, ok.
> Well, i shutdown the VM, follow the instructions and it says that the disk
is
> successfully resized.
> I start the machine and do an df -h and everything is just the same as
before,
> nothing got bigger.
Are you booting the new disk or the old disk?

What is the actual df -h output?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Hi,

Sorry, my bad.

Well, i'm mounting the new disk, to be exact: i'm copying the image to
the
machine and i'm replacing the old disk image with the newly created and 
resized one.

When i boot the vm, i do a df -h and see:

Filesystem            Size  Used Avail Use% Mounted on
/dev/vda1              14G  9.6G  3.0G  77% /
tmpfs                1007M     0 1007M   0% /lib/init/rw
udev                 1002M  120K 1002M   1% /dev
tmpfs                1007M     0 1007M   0% /dev/shm

It's exactly the same as before the resize... 

=====================Richard schreef.
=====================Please keep all replies on the list.

On Tue, May 27, 2014 at 11:22:17AM +0200, Info || Vertixo B.V.
wrote:
> Hi,
> 
> Ah, ok.
> Well, i shutdown the VM, follow the instructions and it says that the disk 
is 
> successfully resized.
> I start the machine and do an df -h and everything is just the same as 
before, 
> nothing got bigger.
Are you booting the new disk or the old disk?

What is the actual df -h output?

Rich.

On Sat, May 24, 2014, at 10:39 AM, Colin Walters wrote:
> 
> > Dan Walsh helpfully pointed out to us that we've been doing it
wrong
> > all along :-)  A much better way to relabel is to run:
> > 
> >   setfiles /etc/selinux/targeted/contexts/files/file_contexts DIR
> 
> Yes, this is what I'm doing with OSTree.  However in the general cross
> labeling case it has a subtle issue with PCRE:
> http://comments.gmane.org/gmane.comp.security.selinux/20214
> 
> There is of course always the potential issue for incompatible future
> changes in the file_contexts format.
> 
> My current workaround is:
>
https://github.com/cgwalters/rpm-ostree/commit/0cb346b798aead0fd544e2c9ef45f1817ba19434
FWIW I don't have an immediate major need for the relabeling API because
we use Anaconda which does initial labeling there.  The remaining
use cases for libguestfs apps of debug/repair are still valid, but those
aren't
critical path.