Libguestfs - May 2022 - [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors

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

In the "selinux-relabel" API, utilize the "-C" option of
setfiles(8), if
it is available. (It's going to be part of the policycoreutils-3.4
release.) See patch#2 for a bit longer explanation.

Thanks
Laszlo

Laszlo Ersek (2):
  daemon/selinux-relabel: generalize setfiles_has_m_option()
  daemon/selinux-relabel: tolerate relabeling errors

 daemon/selinux-relabel.c | 36 +++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)


base-commit: 08c4ac90f5a3c08b48444e2faf3d0f58d6ddc206
-- 
2.19.1.3.g30247aa5d201

Allow the caller to pass in the option to check for, and to store the
result in a (usually static) variable of their choice.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 daemon/selinux-relabel.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/daemon/selinux-relabel.c b/daemon/selinux-relabel.c
index 5679a29cf141..a34287fe27cb 100644
--- a/daemon/selinux-relabel.c
+++ b/daemon/selinux-relabel.c
@@ -38,17 +38,19 @@ optgroup_selinuxrelabel_available (void)
 }
 
 static int
-setfiles_has_m_option (void)
+setfiles_has_option (int *flag, char opt_char)
 {
-  static int flag = -1;
   CLEANUP_FREE char *err = NULL;
 
-  if (flag == -1) {
-    ignore_value (command (NULL, &err, "setfiles",
"-m", NULL));
-    flag = err && strstr (err, /* "invalid option -- " */
"'m'") == NULL;
+  if (*flag == -1) {
+    char option[] = { '-', opt_char, '\0' };       /*
"-X" */
+    char err_opt[] = { '\'', opt_char, '\'',
'\0'}; /* "'X'" */
+
+    ignore_value (command (NULL, &err, "setfiles", option,
NULL));
+    *flag = err && strstr (err, /* "invalid option -- " */
err_opt) == NULL;
   }
 
-  return flag;
+  return *flag;
 }
 
 /* Takes optional arguments, consult optargs_bitmask. */
@@ -56,6 +58,7 @@ int
 do_selinux_relabel (const char *specfile, const char *path,
                     int force)
 {
+  static int flag_m = -1;
   const char *argv[MAX_ARGS];
   CLEANUP_FREE char *s_dev = NULL, *s_proc = NULL, *s_selinux = NULL,
     *s_sys = NULL, *s_specfile = NULL, *s_path = NULL;
@@ -101,7 +104,7 @@ do_selinux_relabel (const char *specfile, const char *path,
    * setfiles puts all the mountpoints on the excludes list for no
    * useful reason (RHBZ#1433577).
    */
-  if (setfiles_has_m_option ())
+  if (setfiles_has_option (&flag_m, 'm'))
     ADD_ARG (argv, i, "-m");
 
   /* Relabelling in a chroot. */
-- 
2.19.1.3.g30247aa5d201

Option "-C" of setfiles(8) causes setfiles(8) to exit with status 1
rather
than status 255 if it encounters relabeling errors, but no other (fatal)
error. Pass "-C" to setfiles(8) in "selinux-relabel",
because we don't
want the "selinux-relabel" API to fail if setfiles(8) only encounters
relabeling errors.

(NB even without "-C", setfiles(8) continues traversing the directory
tree(s) and relabeling files across relabeling errors, so this change is
specifically about the exit status.)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 daemon/selinux-relabel.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/daemon/selinux-relabel.c b/daemon/selinux-relabel.c
index a34287fe27cb..976cffe37261 100644
--- a/daemon/selinux-relabel.c
+++ b/daemon/selinux-relabel.c
@@ -59,11 +59,13 @@ do_selinux_relabel (const char *specfile, const char *path,
                     int force)
 {
   static int flag_m = -1;
+  static int flag_C = -1;
   const char *argv[MAX_ARGS];
   CLEANUP_FREE char *s_dev = NULL, *s_proc = NULL, *s_selinux = NULL,
     *s_sys = NULL, *s_specfile = NULL, *s_path = NULL;
   CLEANUP_FREE char *err = NULL;
   size_t i = 0;
+  int setfiles_status;
 
   s_dev = sysroot_path ("/dev");
   if (!s_dev) {
@@ -107,6 +109,13 @@ do_selinux_relabel (const char *specfile, const char *path,
   if (setfiles_has_option (&flag_m, 'm'))
     ADD_ARG (argv, i, "-m");
 
+  /* Not only do we want setfiles to trudge through individual relabeling
+   * errors, we also want the setfiles exit status to differentiate a fatal
+   * error from "relabeling errors only". See RHBZ#1794518.
+   */
+  if (setfiles_has_option (&flag_C, 'C'))
+    ADD_ARG (argv, i, "-C");
+
   /* Relabelling in a chroot. */
   if (STRNEQ (sysroot, "/")) {
     ADD_ARG (argv, i, "-r");
@@ -124,10 +133,10 @@ do_selinux_relabel (const char *specfile, const char
*path,
   ADD_ARG (argv, i, s_path);
   ADD_ARG (argv, i, NULL);
 
-  if (commandv (NULL, &err, argv) == -1) {
-    reply_with_error ("%s", err);
-    return -1;
-  }
+  setfiles_status = commandrv (NULL, &err, argv);
+  if ((setfiles_status == 0) || (setfiles_status == 1 && flag_C))
+    return 0;
 
-  return 0;
+  reply_with_error ("%s", err);
+  return -1;
 }
-- 
2.19.1.3.g30247aa5d201