7:23 a.m.
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
7:23 a.m.
New subject: [libguestfs PATCH 1/2] daemon/selinux-relabel: generalize setfiles_has_m_option()
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(a)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
7:23 a.m.
New subject: [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors
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(a)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
7:31 a.m.
New subject: [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors
On Wed, May 11, 2022 at 02:23:45PM +0200, Laszlo Ersek wrote:
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(a)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;
}
I've been following this one for a while, thanks for all the work
especially in dark corners of the SELinux code. For the series:
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
10:05 a.m.
New subject: [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors
On 05/11/22 14:31, Richard W.M. Jones wrote:
On Wed, May 11, 2022 at 02:23:45PM +0200, Laszlo Ersek wrote:
> 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(a)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;
> }
I've been following this one for a while, thanks for all the work
especially in dark corners of the SELinux code. For the series:
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Commit range 08c4ac90f5a3..a39b79f6079c.
Thank you!
Laszlo
926
days inactive
926
days old
4 comments
2 participants
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones