[Libguestfs] [common PATCH 3/3] mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

[Pino Toscano]

On Thursday, 24 September 2020 12:15:29 CEST Richard W.M. Jones wrote:
> On Wed, Sep 23, 2020 at 05:57:50PM +0200, Pino Toscano wrote:
> > Do not attempt to relabel a guest in case its SELinux enforcing mode is
> > not "enforcing", as it is either pointless, or it may fail because of an
> > invalid policy configured.
> > ---
> >  mlcustomize/SELinux_relabel.ml | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
> > index 647aeda..db00e59 100644
> > --- a/mlcustomize/SELinux_relabel.ml
> > +++ b/mlcustomize/SELinux_relabel.ml
> > @@ -24,6 +24,9 @@ open Printf
> >  
> >  module G = Guestfs
> >  
> > +exception SELinux_not_enforcing
> > +(* Interal exception to signal a non-enforcing SELinux. *)
> > +
> >  (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *)
> >  let array_find a l =
> >    List.mem a (Array.to_list l)
> > @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) =
> >        use_setfiles g;
> >        (* That worked, so we don't need to autorelabel. *)
> >        g#rm_f "/.autorelabel"
> > -    with Failure _ ->
> > +    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.
> >         *)
> >        g#touch "/.autorelabel"
> > +    | SELinux_not_enforcing ->
> > +      (* This means that SELinux was not configured to be in enforcing mode,
> > +       * so silently accept this.
> > +       *)
> > +      ()
> >    )
> >  
> >  and is_selinux_guest g =
> > @@ -59,6 +68,21 @@ and use_setfiles g =
> >    g#aug_load ();
> >    debug_augeas_errors g;
> >  
> > +  (* Get the SELinux enforcing mode, eg "enforcing", "permissive",
> > +   * "disabled".
> > +   * Use "disabled" if not specified, just like libselinux seems to do.
> > +   *)
> > +  let typ = read_selinux_config_key g "SELINUX" "disabled" in
> > +  (* Do not attempt any relabelling if the SELinux is not "enforcing":
> > +   * - in "permissive" mode SELinux is still running, however nothing is
> > +   *   enforced: this means labels can be wrong, and "it is fine"
> 
> I don't think it's fine.  As I showed here:
> 
> https://www.redhat.com/archives/libguestfs/2020-June/msg00115.html
> 
> in permissive mode labels are still being updated on disk.

This is true for default labels, yes.

> TBH I don't understand what you said here:
> 
> https://www.redhat.com/archives/libguestfs/2020-June/msg00117.html
> 
> about "both the labels and the policy may be all wrong".  If the
> administrator set the policy to permissive then labels ought still to
> be updated when the guest is running, and we ought to try to keep them
> updated if we can in v2v.

There are various cases when, even of an enforcing system, labels are
not kept up-to-date:

$ getenforce 
Enforcing
$ touch /tmp/test
$ ls -lZ /tmp/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /tmp/test
$ mv /tmp/test ~/var/
$ ls -lZ ~/var/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test
$ restorecon -v ~/var/test 
Relabeled /home/ptoscano/var/test from unconfined_u:object_r:user_tmp_t:s0 to unconfined_u:object_r:user_home_t:s0
$ ls -lZ ~/var/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_home_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test

Considering that /tmp is a general location for temporary files, it's
common that files may end with a tmp_t-alike label when moved back to
the destination place (e.g. after a rename()). That is not the only
situation like this that I saw in the past.

In permissive mode, all these situation are logged in the audit log,
yes, but they cause no blocks nor errors.

> It's also fine for an administrator to
> switch a system to permissive and then back to enforcing without
> relabelling or rebooting.

A mislabelled /etc/passwd is still read and used fine in permissive
mode. Switch back from permissive to enforcing without a relabelling
is generally not a good idea, especially after the system ran for a
lot of time after the switch to permissive.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200924/c0fb4d8e/attachment.sig>