10:44 a.m.
This shouldn't change the effect of this code.
---
mlcustomize/SELinux_relabel.ml | 121 ++++++++++++++++++---------------
1 file changed, 65 insertions(+), 56 deletions(-)
diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
index 44995df..5df1f08 100644
--- a/mlcustomize/SELinux_relabel.ml
+++ b/mlcustomize/SELinux_relabel.ml
@@ -28,65 +28,74 @@ module G = Guestfs
let array_find a l =
List.mem a (Array.to_list l)
-let relabel (g : G.guestfs) =
- (* Is the guest using SELinux? *)
- if g#is_file ~followsymlinks:true "/usr/sbin/load_policy" &&
- g#is_file ~followsymlinks:true "/etc/selinux/config" then (
- (* Is setfiles / SELinux relabelling functionality available? *)
- if g#feature_available [| "selinuxrelabel" |] then (
- (* Use Augeas to parse /etc/selinux/config. *)
- g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *);
- (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *)
- ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~
regexp('^') + glob(incl) + regexp('/.*')]");
- g#aug_load ();
- debug_augeas_errors g;
-
- (* Get the SELinux policy name, eg. "targeted", "minimum".
- * Use "targeted" if not specified, just like libselinux does.
+let rec relabel (g : G.guestfs) =
+ (* Is the guest using SELinux? (Otherwise this is a no-op). *)
+ if is_selinux_guest g then (
+ try
+ use_setfiles g;
+ (* That worked, so we don't need to autorelabel. *)
+ g#rm_f "/.autorelabel"
+ with Failure _ ->
+ (* This is the fallback in case something in the setfiles
+ * method didn't work. That includes the case where a non-SELinux
+ * host is processing an SELinux guest, and other things.
*)
- let policy =
- let config_path = "/files/etc/selinux/config" in
- let selinuxtype_path = config_path ^ "/SELINUXTYPE" in
- let keys = g#aug_ls config_path in
- if array_find selinuxtype_path keys then
- g#aug_get selinuxtype_path
- else
- "targeted" in
+ g#touch "/.autorelabel"
+ )
- g#aug_close ();
+and is_selinux_guest g =
+ g#is_file ~followsymlinks:true "/usr/sbin/load_policy" &&
+ g#is_file ~followsymlinks:true "/etc/selinux/config"
- (* Get the spec file name. *)
- let specfile =
- sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in
+and use_setfiles g =
+ (* Is setfiles / SELinux relabelling functionality available? *)
+ if not (g#feature_available [| "selinuxrelabel" |]) then
+ failwith "no selinux relabel feature";
- (* RHEL 6.2 - 6.5 had a malformed specfile that contained the
- * invalid regular expression "/var/run/spice-vdagentd.\pid"
- * (instead of "\.p"). This stops setfiles from working on
- * the guest.
- *
- * Because an SELinux relabel writes all over the filesystem,
- * it seems reasonable to fix this problem in the specfile
- * at the same time. (RHBZ#1374232)
- *)
- if g#grep ~fixed:true "vdagentd.\\pid" specfile <> [||] then (
- debug "fixing invalid regular expression in %s" specfile;
- let old_specfile = specfile ^ "~" in
- g#mv specfile old_specfile;
- let content = g#read_file old_specfile in
- let content =
- String.replace content "vdagentd.\\pid" "vdagentd\\.pid"
in
- g#write specfile content;
- g#copy_attributes ~all:true old_specfile specfile
- );
+ (* Use Augeas to parse /etc/selinux/config. *)
+ g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *);
+ (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *)
+ ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~
regexp('^') + glob(incl) + regexp('/.*')]");
+ g#aug_load ();
+ debug_augeas_errors g;
- (* Relabel everything. *)
- g#selinux_relabel ~force:true specfile "/";
+ (* Get the SELinux policy name, eg. "targeted", "minimum".
+ * Use "targeted" if not specified, just like libselinux does.
+ *)
+ let policy =
+ let config_path = "/files/etc/selinux/config" in
+ let selinuxtype_path = config_path ^ "/SELINUXTYPE" in
+ let keys = g#aug_ls config_path in
+ if array_find selinuxtype_path keys then
+ g#aug_get selinuxtype_path
+ else
+ "targeted" in
- (* If that worked, we don't need to autorelabel. *)
- g#rm_f "/.autorelabel"
- )
- else (
- (* SELinux guest, but not SELinux host. Fallback to this. *)
- g#touch "/.autorelabel"
- )
- )
+ g#aug_close ();
+
+ (* Get the spec file name. *)
+ let specfile =
+ sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in
+
+ (* RHEL 6.2 - 6.5 had a malformed specfile that contained the
+ * invalid regular expression "/var/run/spice-vdagentd.\pid"
+ * (instead of "\.p"). This stops setfiles from working on
+ * the guest.
+ *
+ * Because an SELinux relabel writes all over the filesystem,
+ * it seems reasonable to fix this problem in the specfile
+ * at the same time. (RHBZ#1374232)
+ *)
+ if g#grep ~fixed:true "vdagentd.\\pid" specfile <> [||] then (
+ debug "fixing invalid regular expression in %s" specfile;
+ let old_specfile = specfile ^ "~" in
+ g#mv specfile old_specfile;
+ let content = g#read_file old_specfile in
+ let content =
+ String.replace content "vdagentd.\\pid" "vdagentd\\.pid" in
+ g#write specfile content;
+ g#copy_attributes ~all:true old_specfile specfile
+ );
+
+ (* Relabel everything. *)
+ g#selinux_relabel ~force:true specfile "/"
--
2.24.1
10:44 a.m.
New subject: [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).
https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c2
If SELINUXTYPE is set to some value other than targeted then we look
for a directory /etc/selinux/<SELINUXTYPE> which does not exist.
However this should not cause a fatal error. Using setfiles to do the
relabelling immediately is a nice-to-have, but we can fallback to
using autorelabel if we're unable to achieve it.
---
mlcustomize/SELinux_relabel.ml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
index 5df1f08..5ecf7bd 100644
--- a/mlcustomize/SELinux_relabel.ml
+++ b/mlcustomize/SELinux_relabel.ml
@@ -77,6 +77,12 @@ and use_setfiles g =
let specfile =
sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in
+ (* If the spec file doesn't exist then fall back to using
+ * autorelabel (RHBZ#1828952).
+ *)
+ if not (g#is_file ~followsymlinks:true specfile) then
+ failwith "no spec file";
+
(* RHEL 6.2 - 6.5 had a malformed specfile that contained the
* invalid regular expression "/var/run/spice-vdagentd.\pid"
* (instead of "\.p"). This stops setfiles from working on
--
2.24.1
4:12 a.m.
New subject: [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).
On Tuesday, 5 May 2020 17:44:15 CEST Richard W.M. Jones wrote:
I think we need to do a different approach than this patch.
The biggest thing is that currently we check only SELINUXTYPE for the
actual policy, however we do not check SELINUX in case SELinux is in
enforcing mode at all.
IMHO we rather need to read /etc/selinux/<SELINUX> first:
- if enforcing, go ahead with the current relabeling: check SELINUXTYPE,
get the policy path, etc; if set like this, then most probably the
SELINUXTYPE points to a valid policy, otherwise the guest would not
even boot
- if permissive or disabled, do not perform any relabeling, including
touching /.autorelabel; this is because SELinux was disabled, so
attempting any relabeling might result in failures
--
Pino Toscano
3:58 a.m.
New subject: [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).
On Mon, May 18, 2020 at 11:12:29AM +0200, Pino Toscano wrote:
On Tuesday, 5 May 2020 17:44:15 CEST Richard W.M. Jones wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c2
I think we need to do a different approach than this patch.
The biggest thing is that currently we check only SELINUXTYPE for the
actual policy, however we do not check SELINUX in case SELinux is in
enforcing mode at all.
IMHO we rather need to read /etc/selinux/<SELINUX> first:
- if enforcing, go ahead with the current relabeling: check SELINUXTYPE,
get the policy path, etc; if set like this, then most probably the
SELINUXTYPE points to a valid policy, otherwise the guest would not
even boot
- if permissive or disabled, do not perform any relabeling, including
touching /.autorelabel; this is because SELinux was disabled, so
attempting any relabeling might result in failures
Permissive and disabled seem to be different cases. If it's
permissive then SELinux is still "running" (but not enforcing
decisions).
I think the critical thing is actually what SELinux itself does to
update filesystem labels when selinux is set to either permissive or
disabled. I wasn't able to find any definitive information on this,
but testing by creating a new file in my home directory showed:
SELINUX=enforcing:
$ rm -f test
$ touch test
$ ls -lZ test
-rw-rw-r--. 1 rjones rjones unconfined_u:object_r:user_home_t:s0 0 Jun 24 09:48 test
permissive:
-rw-rw-r--. 1 rjones rjones unconfined_u:object_r:user_home_t:s0 0 Jun 24 09:49 test
disabled:
-rw-rw-r-- 1 rjones rjones ? 0 Jun 24 09:54 test
Anyway I think we at least need to treat enforcing and permissive the
same way.
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
4:13 a.m.
New subject: [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).
On Wednesday, 24 June 2020 10:58:12 CEST Richard W.M. Jones wrote:
On Mon, May 18, 2020 at 11:12:29AM +0200, Pino Toscano wrote:
> On Tuesday, 5 May 2020 17:44:15 CEST Richard W.M. Jones wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c2
>
> I think we need to do a different approach than this patch.
>
> The biggest thing is that currently we check only SELINUXTYPE for the
> actual policy, however we do not check SELINUX in case SELinux is in
> enforcing mode at all.
>
> IMHO we rather need to read /etc/selinux/<SELINUX> first:
> - if enforcing, go ahead with the current relabeling: check SELINUXTYPE,
> get the policy path, etc; if set like this, then most probably the
> SELINUXTYPE points to a valid policy, otherwise the guest would not
> even boot
> - if permissive or disabled, do not perform any relabeling, including
> touching /.autorelabel; this is because SELinux was disabled, so
> attempting any relabeling might result in failures
Permissive and disabled seem to be different cases. If it's
permissive then SELinux is still "running" (but not enforcing
decisions).
This is true from the POV of the running guest, not so much from the
POV of libguestfs that inspects/manipulate the offline guest.
In this case, both the labels and the policy may be all wrong, and
SELinux will not error out in permissive mode (only "complain").
And at least in case of permissive mode, this is "fine": the
administrator configured the guest that way, as a mild "do not bother
me", so it is expected that labels may be not set/updated. In case they
want to switch SELinux to enforcing, they will need to
a) ensure the policy is correct (including file contexts)
b) relabel all the filesystems
before the actual switch.
Anyway I think we at least need to treat enforcing and permissive
the
same way.
Because of the above, I think instead that trying to relabel a
permissive guest is not a good idea: we may apply wrong
policies/labels.
--
Pino Toscano
3:54 a.m.
On Tuesday, 5 May 2020 17:44:14 CEST Richard W.M. Jones wrote:
This shouldn't change the effect of this code.
---
This is generally OK for me, however I'd create a custom exception to
signal the "cannot relabel using selinux_relabel" instead of reusing
Failure. This way we can catch this specific exception, and avoid
catching potential Failure issues in the future.
--
Pino Toscano
1650
days inactive
1700
days old
5 comments
2 participants
participants (2)
-
Pino Toscano -
Richard W.M. Jones