Re: `fixfiles -C` does not apply to all paths

[Petr Lautrbach <plautrba@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3682 bytes --]

On Thu, Sep 17, 2020 at 02:53:06PM -0400, Stephen Smalley wrote:
> On Thu, Sep 17, 2020 at 10:44 AM Cedric Buissart <cbuissar@redhat.com> wrote:
> >
> > Hello all,
> >
> > I would like to discuss the possible removal of the static path list
> > in fixfiles' differential update mode (`fixfiles -C`).
> >
> > Here is how it works :
> >
> > 160 # Compare PREVious File Context to currently installed File Context and
> > 161 # run restorecon on all files affected by the differences.
> > 162 #
> > 163 diff_filecontext() {
> > 164 EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
> > 165 for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC
> > /home /tmp /dev; do
> > 166     [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
> > 167 done
> > 168 LogExcluded
> > 169
> > 170 if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
> > 171     TEMPFILE=`mktemp ${FC}.XXXXXXXXXX`
> > 172     test -z "$TEMPFILE" && exit
> > 173     PREFCTEMPFILE=`mktemp ${PREFC}.XXXXXXXXXX`
> > 174     sed -r -e 's,:s0, ,g' $PREFC | sort -u > ${PREFCTEMPFILE}
> > 175     sed -r -e 's,:s0, ,g' $FC | sort -u | \
> > 176     /usr/bin/diff -b ${PREFCTEMPFILE} - | \
> > 177         grep '^[<>]'|cut -c3-| grep ^/ | \
> > 178         egrep -v '(^/home|^/root|^/tmp|^/dev)' |\
> > 179     sed -r -e 's,[[:blank:]].*,,g' \
> > [...]
> > 199     ${RESTORECON} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -i -R -f -; \
> >
> >
> > lines 165-167 and 178 statically prevent some paths to be updated with
> > the new policy. I suspect this was done for efficiency and historical
> > reasons.
> >
> > I would propose the removal of these path because :
> >
> > - restorecon is (by default) automatically ignoring paths that are not
> > mounted with `seclabel`. There shouldn't be a need to statically treat
> > paths differently
> > - Some paths currently in this list (e.g. `/home`) may require
> > updating. During a policy update, packages (at least RHEL and Fedora)
> > are using `fixfiles -C` to make the policy more efficient, resulting
> > in a possibly incomplete policy update.
> > - The admin may not be aware of the manual steps required to fully
> > apply the new policy after an update.
> >
> >
> > How about removing these lines ?
> 
> Looking at the list, I note that several of them have seclabel set in
> /proc/mounts so they would no longer be excluded after such a change.
> The biggest concern is probably /home due to making fixfiles very
> slow.  I think the whole idea of fixfiles -C was to try to minimize
> time spent on a policy update.  Maybe we need to re-think the whole
> approach.  Android has taken a different approach to allowing
> efficient relabeling on Android upgrades.  They save a hash of the
> matching file_contexts entries as an extended attribute of
> directories, and only descend into the directory during relabeling if
> the hash no longer matches.  Upstream, this is only enabled if the -D
> option is passed to setfiles/restorecon since it requires
> CAP_SYS_ADMIN to set the additional xattr.  Perhaps fixfiles should be
> extended with this option and we should be using it instead of -C?
> 

I'd like to say that I'm aware about this problem but I don't have answers yet.

It seems to be related to the way how `fixfiles -C` translates regexps to
glob's, e.g. '/home/[^/]+/\.yubico(/.*)?' would be translated to '/home/*' and
relabeling whole /home could be long and delicate? action as it would touch users
data.

As a short term workaround, I'd suggest that policy package maintainers enforce relabeling
of particular directories inside /home when they know it's really needed.

Petr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]