| Message ID | 20220919112901.1127409-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [userspace] fixfiles: do not cross mounts when traversing directories | expand |
On Mon, 19 Sept 2022 at 13:29, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > Always run find with -xdev to avoid unintended deleting/relabeling. > While this may sometimes skip subdirectories that should be relabeled, > the danger of crossing into random mounts is greater than leaving behind > some unlabeled files. The find commands are just best-effort attempts to > fix the labels anyway. The xdev option does not work for bind mounts (they are still followed). > > In case of /run (renamed from the deprecated /var/run), traverse > /run/user/* directories separately, as there is commonly an additional > layer of tmpfs mounted on them. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > policycoreutils/scripts/fixfiles | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles > index c72ca0eb..c9ab2a93 100755 > --- a/policycoreutils/scripts/fixfiles > +++ b/policycoreutils/scripts/fixfiles > @@ -153,7 +153,7 @@ newer() { > shift > LogReadOnly > for m in `echo $FILESYSTEMSRW`; do > - find $m -mount -newermt $DATE -print0 2>/dev/null | ${RESTORECON} ${FORCEFLAG} ${VERBOSE} ${THREADS} $* -i -0 -f - > + find $m -xdev -newermt $DATE -print0 2>/dev/null | ${RESTORECON} ${FORCEFLAG} ${VERBOSE} ${THREADS} $* -i -0 -f - > done; > } > > @@ -273,18 +273,22 @@ case "$RESTORE_MODE" in > > UNDEFINED=`get_undefined_type` || exit $? > UNLABELED=`get_unlabeled_type` || exit $? > - find /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) \( -type s -o -type p \) -delete > - find /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /tmp {} \; > - find /var/tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/tmp {} \; > - find /var/run \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/run {} \; > - [ ! -e /var/lib/debug ] || find /var/lib/debug \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /lib {} \; > + find -xdev /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) \( -type s -o -type p \) -delete > + find -xdev /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /tmp {} \; > + find -xdev /var/tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/tmp {} \; > + find -xdev /run \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /run {} \; > + # /run/user/<pid> may have an additional tmpfs mounted on it > + for userdir in /run/user/*; do > + find -xdev "$userdir" \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference "$userdir" {} \; > + done > + [ ! -e /var/lib/debug ] || find -xdev /var/lib/debug \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /lib {} \; > ;; > esac > } > > fullrelabel() { > echo "Cleaning out /tmp" > - find /tmp/ -mindepth 1 -delete > + find -xdev /tmp/ -mindepth 1 -delete > restore Relabel > } > > -- > 2.37.3 >
On Mon, Sep 19, 2022 at 1:35 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > On Mon, 19 Sept 2022 at 13:29, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > Always run find with -xdev to avoid unintended deleting/relabeling. > > While this may sometimes skip subdirectories that should be relabeled, > > the danger of crossing into random mounts is greater than leaving behind > > some unlabeled files. The find commands are just best-effort attempts to > > fix the labels anyway. > > The xdev option does not work for bind mounts (they are still followed). Hm... it does not if the bind mounted dir is on the same filesystem (superblock), so in the case where /tmp is a plain directory on the root filesystem it will allow traversing to other directories directly on the root filesystem. I guess that's still better than nothing, though... An alternative (at least for the more dangerous -delete part) could be to change the prompt to suggest switching to do the equivalent of `fixfiles -F onboot` + reboot. The current prompt instructs the user to reboot the machine anyway, so it wouldn't really make things more complicated for the user. Maybe I'll draft a patch for this... -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
Ondrej Mosnacek <omosnace@redhat.com> writes: > On Mon, Sep 19, 2022 at 1:35 PM Christian Göttsche > <cgzones@googlemail.com> wrote: >> >> On Mon, 19 Sept 2022 at 13:29, Ondrej Mosnacek <omosnace@redhat.com> wrote: >> > >> > Always run find with -xdev to avoid unintended deleting/relabeling. >> > While this may sometimes skip subdirectories that should be relabeled, >> > the danger of crossing into random mounts is greater than leaving behind >> > some unlabeled files. The find commands are just best-effort attempts to >> > fix the labels anyway. >> >> The xdev option does not work for bind mounts (they are still followed). > > Hm... it does not if the bind mounted dir is on the same filesystem > (superblock), so in the case where /tmp is a plain directory on the > root filesystem it will allow traversing to other directories directly > on the root filesystem. I guess that's still better than nothing, > though... > > An alternative (at least for the more dangerous -delete part) could be > to change the prompt to suggest switching to do the equivalent of > `fixfiles -F onboot` + reboot. The current prompt instructs the user > to reboot the machine anyway, so it wouldn't really make things more > complicated for the user. Maybe I'll draft a patch for this... The reason why one is presented with an option to "clear" /tmp is because /tmp is a shared location. That property makes it so that file context specifications usually do not work for these locations in general and /tmp in particular. Relabeling does not apply there -because setfiles is told to ignore these locations- also not with fixfiles (-F) onboot. So telling people to run fixfiles onboot && reboot instead of clearing /tmp does not address the challenge. What I find strange is that one is not also presented with an option to clear /var/tmp, because the same applies there. In that sense, I believe, this opportunity to clear /tmp is half baked. It does not solve the underlying issue of addressing locations that have no file context specifications associated with them for one reason or another. > > -- > Ondrej Mosnacek > Senior Software Engineer, Linux Security - SELinux kernel > Red Hat, Inc. >
On Mon, Sep 19, 2022 at 5:58 PM Dominick Grift <dominick.grift@defensec.nl> wrote: > Ondrej Mosnacek <omosnace@redhat.com> writes: > > > On Mon, Sep 19, 2022 at 1:35 PM Christian Göttsche > > <cgzones@googlemail.com> wrote: > >> > >> On Mon, 19 Sept 2022 at 13:29, Ondrej Mosnacek <omosnace@redhat.com> wrote: > >> > > >> > Always run find with -xdev to avoid unintended deleting/relabeling. > >> > While this may sometimes skip subdirectories that should be relabeled, > >> > the danger of crossing into random mounts is greater than leaving behind > >> > some unlabeled files. The find commands are just best-effort attempts to > >> > fix the labels anyway. > >> > >> The xdev option does not work for bind mounts (they are still followed). > > > > Hm... it does not if the bind mounted dir is on the same filesystem > > (superblock), so in the case where /tmp is a plain directory on the > > root filesystem it will allow traversing to other directories directly > > on the root filesystem. I guess that's still better than nothing, > > though... > > > > An alternative (at least for the more dangerous -delete part) could be > > to change the prompt to suggest switching to do the equivalent of > > `fixfiles -F onboot` + reboot. The current prompt instructs the user > > to reboot the machine anyway, so it wouldn't really make things more > > complicated for the user. Maybe I'll draft a patch for this... > > The reason why one is presented with an option to "clear" /tmp is because > /tmp is a shared location. That property makes it so that file context > specifications usually do not work for these locations in general and > /tmp in particular. Relabeling does not apply there -because setfiles is > told to ignore these locations- also not with > fixfiles (-F) onboot. So telling people to run fixfiles onboot && reboot > instead of clearing /tmp does not address the challenge. Oh, sorry, I meant -f, not -F. That is, the user would be given two choices: a) Don't touch /tmp and do the general relabeling immediately. In this case the user will be warned that /tmp contents may remain incorrectly labeled. b) Schedule /tmp resetting + relabeling for next early boot (where the recursive delete should be safe). In this case the user will be warned that they need to reboot for anything to happen. > What I find strange is that one is not also presented with an option to > clear /var/tmp, because the same applies there. In that sense, I believe, > this opportunity to clear /tmp is half baked. It does not solve the > underlying issue of addressing locations that have no file context > specifications associated with them for one reason or another. To be fair, the whole fixfiles script is a mess... Feel free to propose a patch, but my goal right now is just to make it less prone to unexpected consequences. -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
Ondrej Mosnacek <omosnace@redhat.com> writes: > On Mon, Sep 19, 2022 at 5:58 PM Dominick Grift > <dominick.grift@defensec.nl> wrote: >> Ondrej Mosnacek <omosnace@redhat.com> writes: >> >> > On Mon, Sep 19, 2022 at 1:35 PM Christian Göttsche >> > <cgzones@googlemail.com> wrote: >> >> >> >> On Mon, 19 Sept 2022 at 13:29, Ondrej Mosnacek <omosnace@redhat.com> wrote: >> >> > >> >> > Always run find with -xdev to avoid unintended deleting/relabeling. >> >> > While this may sometimes skip subdirectories that should be relabeled, >> >> > the danger of crossing into random mounts is greater than leaving behind >> >> > some unlabeled files. The find commands are just best-effort attempts to >> >> > fix the labels anyway. >> >> >> >> The xdev option does not work for bind mounts (they are still followed). >> > >> > Hm... it does not if the bind mounted dir is on the same filesystem >> > (superblock), so in the case where /tmp is a plain directory on the >> > root filesystem it will allow traversing to other directories directly >> > on the root filesystem. I guess that's still better than nothing, >> > though... >> > >> > An alternative (at least for the more dangerous -delete part) could be >> > to change the prompt to suggest switching to do the equivalent of >> > `fixfiles -F onboot` + reboot. The current prompt instructs the user >> > to reboot the machine anyway, so it wouldn't really make things more >> > complicated for the user. Maybe I'll draft a patch for this... >> >> The reason why one is presented with an option to "clear" /tmp is because >> /tmp is a shared location. That property makes it so that file context >> specifications usually do not work for these locations in general and >> /tmp in particular. Relabeling does not apply there -because setfiles is >> told to ignore these locations- also not with >> fixfiles (-F) onboot. So telling people to run fixfiles onboot && reboot >> instead of clearing /tmp does not address the challenge. > > Oh, sorry, I meant -f, not -F. That is, the user would be given two choices: > a) Don't touch /tmp and do the general relabeling immediately. In this > case the user will be warned that /tmp contents may remain incorrectly > labeled. > b) Schedule /tmp resetting + relabeling for next early boot (where the > recursive delete should be safe). In this case the user will be warned > that they need to reboot for anything to happen. Yes, something like that. Here are a few aspects to consider: - We should be careful with clearing /tmp in general as it could interfere with the running system. (for example /tmp/.X11-unix/X0) - The issue, i think, were trying to solve is not limited to /tmp. The up-side is that we can determine what is affected by just interpretting the <<none>> specs from the file_contexts were using to relabel the file system. Anything with a <<none>> spec is affected and not just /tmp/.* - One alternative would be to, instead of giving the caller to option to clear /tmp, explain the issue and how to solve it manually. - If we leave /tmp uncleared then that might affect the next boot. For example youre enforcing a new policy, youre relabeling the filesystem according to new specifications. the context currently associated with /tmp/.X11-unix/X0 is invalidated by the to be enforced policy. Now Xserver might no longer be allowed to operate on the sock file with invalid label, and break. (of course I suppose any currently running Xserver might "break" as well if you clear /tmp/.X11-unix/X0 at run-time). > >> What I find strange is that one is not also presented with an option to >> clear /var/tmp, because the same applies there. In that sense, I believe, >> this opportunity to clear /tmp is half baked. It does not solve the >> underlying issue of addressing locations that have no file context >> specifications associated with them for one reason or another. > > To be fair, the whole fixfiles script is a mess... Feel free to > propose a patch, but my goal right now is just to make it less prone > to unexpected consequences. True. Just stating that something is currently half-baked does not necessarily mean I can do a better job. > > > -- > Ondrej Mosnacek > Senior Software Engineer, Linux Security - SELinux kernel > Red Hat, Inc. >
diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles index c72ca0eb..c9ab2a93 100755 --- a/policycoreutils/scripts/fixfiles +++ b/policycoreutils/scripts/fixfiles @@ -153,7 +153,7 @@ newer() { shift LogReadOnly for m in `echo $FILESYSTEMSRW`; do - find $m -mount -newermt $DATE -print0 2>/dev/null | ${RESTORECON} ${FORCEFLAG} ${VERBOSE} ${THREADS} $* -i -0 -f - + find $m -xdev -newermt $DATE -print0 2>/dev/null | ${RESTORECON} ${FORCEFLAG} ${VERBOSE} ${THREADS} $* -i -0 -f - done; } @@ -273,18 +273,22 @@ case "$RESTORE_MODE" in UNDEFINED=`get_undefined_type` || exit $? UNLABELED=`get_unlabeled_type` || exit $? - find /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) \( -type s -o -type p \) -delete - find /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /tmp {} \; - find /var/tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/tmp {} \; - find /var/run \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/run {} \; - [ ! -e /var/lib/debug ] || find /var/lib/debug \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /lib {} \; + find -xdev /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) \( -type s -o -type p \) -delete + find -xdev /tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /tmp {} \; + find -xdev /var/tmp \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /var/tmp {} \; + find -xdev /run \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /run {} \; + # /run/user/<pid> may have an additional tmpfs mounted on it + for userdir in /run/user/*; do + find -xdev "$userdir" \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference "$userdir" {} \; + done + [ ! -e /var/lib/debug ] || find -xdev /var/lib/debug \( -context "*:${UNLABELED}*" -o -context "*:${UNDEFINED}*" \) -exec chcon --no-dereference --reference /lib {} \; ;; esac } fullrelabel() { echo "Cleaning out /tmp" - find /tmp/ -mindepth 1 -delete + find -xdev /tmp/ -mindepth 1 -delete restore Relabel }
Always run find with -xdev to avoid unintended deleting/relabeling. While this may sometimes skip subdirectories that should be relabeled, the danger of crossing into random mounts is greater than leaving behind some unlabeled files. The find commands are just best-effort attempts to fix the labels anyway. In case of /run (renamed from the deprecated /var/run), traverse /run/user/* directories separately, as there is commonly an additional layer of tmpfs mounted on them. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- policycoreutils/scripts/fixfiles | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)