Libguestfs - Jan 2014 - [PATCH 0/2] Implement virt-builder --selinux-relabel option.

Do SELinux relabelling properly.

Even though we are already bind-mounting /sys, it is necessary to also
bind-mount /sys/fs/selinux in order for SELinux commands (in
particular, 'load_policy') to work.

This fixes/reverts commit 7367729ec7a5d016878dd00b32cce45cec372931.
---
 daemon/command.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/daemon/command.c b/daemon/command.c
index 066c773..1aa1a52 100644
--- a/daemon/command.c
+++ b/daemon/command.c
@@ -48,7 +48,8 @@ struct bind_state {
   char *sysroot_dev_pts;
   char *sysroot_proc;
   char *sysroot_sys;
-  bool dev_ok, dev_pts_ok, proc_ok, sys_ok;
+  char *sysroot_sys_fs_selinux;
+  bool dev_ok, dev_pts_ok, proc_ok, sys_ok, sys_fs_selinux_ok;
 };
 
 struct resolver_state {
@@ -76,14 +77,17 @@ bind_mount (struct bind_state *bs)
   bs->sysroot_dev_pts = sysroot_path ("/dev/pts");
   bs->sysroot_proc = sysroot_path ("/proc");
   bs->sysroot_sys = sysroot_path ("/sys");
+  bs->sysroot_sys_fs_selinux = sysroot_path ("/sys/fs/selinux");
 
   if (bs->sysroot_dev == NULL || bs->sysroot_dev_pts == NULL ||
-      bs->sysroot_proc == NULL || bs->sysroot_sys == NULL) {
+      bs->sysroot_proc == NULL || bs->sysroot_sys == NULL ||
+      bs->sysroot_sys_fs_selinux == NULL) {
     reply_with_perror ("malloc");
     free (bs->sysroot_dev);
     free (bs->sysroot_dev_pts);
     free (bs->sysroot_proc);
     free (bs->sysroot_sys);
+    free (bs->sysroot_sys_fs_selinux);
     return -1;
   }
 
@@ -95,6 +99,8 @@ bind_mount (struct bind_state *bs)
   bs->proc_ok = r != -1;
   r = command (NULL, NULL, str_mount, "--bind", "/sys",
bs->sysroot_sys, NULL);
   bs->sys_ok = r != -1;
+  r = command (NULL, NULL, str_mount, "--bind",
"/sys/fs/selinux", bs->sysroot_sys_fs_selinux, NULL);
+  bs->sys_fs_selinux_ok = r != -1;
 
   bs->mounted = true;
 
@@ -111,6 +117,8 @@ static void
 free_bind_state (struct bind_state *bs)
 {
   if (bs->mounted) {
+    if (bs->sys_fs_selinux_ok) umount_ignore_fail
(bs->sysroot_sys_fs_selinux);
+    free (bs->sysroot_sys_fs_selinux);
     if (bs->sys_ok) umount_ignore_fail (bs->sysroot_sys);
     free (bs->sysroot_sys);
     if (bs->proc_ok) umount_ignore_fail (bs->sysroot_proc);
-- 
1.8.4.2

This adds the --selinux-relabel option which enables selinux in the
appliance and runs:

  load_policy && fixfiles restore && rm -f /.autorelabel

at the end of installation.

This fixes SELinux labels in the guest and makes the autorelabel step
unnecessary.

Some notes:

 - The previous commit is required so that load_policy works.

 - During the build, SELinux is enabled but no policy is loaded.  This
   works because SELinux is in permissive mode.

 - It's possible this flag will not work if the appliance kernel and
   the guest have greatly differing versions, eg. a Fedora 20 guest
   and a RHEL 6 appliance.  I have not tested it.

 - It's not clear if loading guest policy is safe.  All the more
   reason to trust the virt-builder templates and to use libguestfs
   confinement for additional protection.
---
 builder/builder.ml       | 12 ++++++++++--
 builder/cmdline.ml       |  8 ++++++--
 builder/virt-builder.pod | 46 +++++++++++++++++++++++++---------------------
 3 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index f3ada95..4b5dcb8 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -40,8 +40,8 @@ let main ()      edit, firstboot, run, format, gpg, hostname,
install, list_format, links,
     memsize, mkdirs,
     network, output, password_crypto, quiet, root_password, scrub,
-    scrub_logfile, size, smp, sources, sync, timezone, update, upload,
-    writes +    scrub_logfile, selinux_relabel, size, smp, sources, sync,
timezone,
+    update, upload, writes      parse_cmdline () in
 
   (* Timestamped messages in ordinary, non-debug non-quiet mode. *)
@@ -578,6 +578,8 @@ let main ()      (match smp with None -> () | Some smp
-> g#set_smp smp);
     g#set_network network;
 
+    g#set_selinux selinux_relabel;
+
     (* The output disk is being created, so use cache=unsafe here. *)
     g#add_drive_opts ~format:output_format ~cachemode:"unsafe"
output_filename;
 
@@ -890,6 +892,12 @@ exec >>%s 2>&1
       do_run ~display:cmd cmd
   ) run;
 
+  if selinux_relabel then (
+    msg (f_"SELinux relabelling");
+    let cmd = "load_policy && fixfiles restore && rm -f
/.autorelabel" in
+    do_run ~display:"load_policy && fixfiles restore" cmd
+  );
+
   (* Clean up the log file:
    *
    * If debugging, dump out the log file.
diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index e3b1484..a6cb6c5 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -180,6 +180,7 @@ let parse_cmdline ()    let add_scrub s = scrub := s ::
!scrub in
 
   let scrub_logfile = ref false in
+  let selinux_relabel = ref false in
 
   let size = ref None in
   let set_size arg = size := Some (parse_size ~prog arg) in
@@ -287,6 +288,8 @@ let parse_cmdline ()      "--run",     Arg.String
add_run,        "script" ^ " " ^ s_"Run script in disk
image";
     "--run-command", Arg.String add_run_cmd, "cmd+args" ^
" " ^ s_"Run command in disk image";
     "--scrub",   Arg.String add_scrub,      "name" ^ "
" ^ s_"Scrub a file";
+    "--selinux-relabel", Arg.Set selinux_relabel,
+                                            " " ^ s_"Relabel
files with correct SELinux labels";
     "--size",    Arg.String set_size,       "size" ^ "
" ^ s_"Set output disk size";
     "--smp",     Arg.Int set_smp,           "vcpus" ^
" " ^ s_"Set number of vCPUs";
     "--source",  Arg.String add_source,     "URL" ^ "
" ^ s_"Set source URL";
@@ -351,6 +354,7 @@ read the man page virt-builder(1).
   let root_password = !root_password in
   let scrub = List.rev !scrub in
   let scrub_logfile = !scrub_logfile in
+  let selinux_relabel = !selinux_relabel in
   let size = !size in
   let smp = !smp in
   let sources = List.rev !sources in
@@ -457,5 +461,5 @@ read the man page virt-builder(1).
   edit, firstboot, run, format, gpg, hostname, install, list_format, links,
   memsize, mkdirs,
   network, output, password_crypto, quiet, root_password, scrub,
-  scrub_logfile, size, smp, sources, sync, timezone, update, upload,
-  writes
+  scrub_logfile, selinux_relabel, size, smp, sources, sync, timezone,
+  update, upload, writes
diff --git a/builder/virt-builder.pod b/builder/virt-builder.pod
index f72ab30..227b767 100644
--- a/builder/virt-builder.pod
+++ b/builder/virt-builder.pod
@@ -27,6 +27,7 @@ virt-builder - Build virtual machine images quickly
     [--link TARGET:LINK[:LINK]]
     [--edit FILE:EXPR]
     [--delete FILE] [--scrub FILE]
+    [--selinux-relabel]
     [--run SCRIPT] [--run-command 'CMD ARGS ...']
     [--firstboot SCRIPT] [--firstboot-command 'CMD ARGS ...']
     [--firstboot-install PKG,[PKG...]]
@@ -593,6 +594,12 @@ It cannot delete directories, only regular files.
 
 =back
 
+=item B<--selinux-relabel>
+
+Relabel files in the guest so that they have the correct SELinux label.
+
+You should only use this option for guests which support SELinux.
+
 =item B<--size> SIZE
 
 Select the size of the output disk, where the size can be specified
@@ -1029,6 +1036,10 @@ Scripts are run (I<--run>, I<--run-command>).
 
 Scripts run in the order they appear on the command line.
 
+=item *
+
+SELinux relabelling is done (I<--selinux-relabel>).
+
 =back
 
 =head2 IMPORTING THE DISK IMAGE
@@ -1714,30 +1725,23 @@ raw-format guests.
 Guests which use SELinux (such as Fedora and Red Hat Enterprise Linux)
 require that each file has a correct SELinux label.
 
-Since virt-builder does not know how to give new files a correct
-label, the guest templates have an empty file C</.autorelabel> and
-this causes the guest to relabel itself at first boot.
+Virt-builder does not know how to give new files a correct label, so
+there are two possible strategies to give newly created files a
+correct label:
 
-This usually means that these guests will reboot themselves once the
-first time you use them.  B<This is normal, and harmless.>  However if
-you want to perform the relabelling at build time instead of delaying
-it to the first boot, you can boot the guest with the qemu
-I<-no-reboot> option (which means it will shut down after the relabel
-is complete without booting "for real").  Only do this if you are
sure
-it is an SELinux guest:
+=over 4
 
- qemu-system-x86_64 \
-   -no-reboot \
-   -nographic \
-   -machine accel=kvm:tcg \
-   -cpu host \
-   -m 2048 \
-   -drive file=disk.img,format=raw,if=virtio \
-   -serial stdio \
-   -monitor none
+=item Using I<--selinux-relabel>
 
-(For further information on the topic of SELinux labelling, see:
-L<https://www.redhat.com/archives/libguestfs/2014-January/msg00183.html>)
+This runs L<fixfiles(8)> just before finalizing the guest, which
+sets SELinux labels correctly.
+
+=item Using C</.autorelabel>
+
+This runs fixfiles at first boot.  Guests will reboot themselves once
+the first time you use them, which is normal and harmless.
+
+=back
 
 =head1 ENVIRONMENT VARIABLES
 
-- 
1.8.4.2