On Thu, Sep 24, 2020 at 12:12 PM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx> 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?
As a side note : since non-system directories do not need immediate
update, how about delegating the policy update of these ones to a
one-off service running in the background ?
> >
>
> 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
--
Cedric Buissart,
Product Security
