* [PATCH userspace] fixfiles: do not cross mounts when traversing directories
@ 2022-09-19 11:29 Ondrej Mosnacek
2022-09-19 11:35 ` Christian Göttsche
0 siblings, 1 reply; 6+ messages in thread
From: Ondrej Mosnacek @ 2022-09-19 11:29 UTC (permalink / raw)
To: selinux
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(-)
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories
2022-09-19 11:29 [PATCH userspace] fixfiles: do not cross mounts when traversing directories Ondrej Mosnacek
@ 2022-09-19 11:35 ` Christian Göttsche
2022-09-19 15:44 ` Ondrej Mosnacek
0 siblings, 1 reply; 6+ messages in thread
From: Christian Göttsche @ 2022-09-19 11:35 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: selinux
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories
2022-09-19 11:35 ` Christian Göttsche
@ 2022-09-19 15:44 ` Ondrej Mosnacek
2022-09-19 15:58 ` Dominick Grift
0 siblings, 1 reply; 6+ messages in thread
From: Ondrej Mosnacek @ 2022-09-19 15:44 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories
2022-09-19 15:44 ` Ondrej Mosnacek
@ 2022-09-19 15:58 ` Dominick Grift
2022-09-20 13:06 ` Ondrej Mosnacek
0 siblings, 1 reply; 6+ messages in thread
From: Dominick Grift @ 2022-09-19 15:58 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: Christian Göttsche, SElinux list
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.
>
--
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6 E0FF DA7E 521F 10F6 4098
Dominick Grift
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories
2022-09-19 15:58 ` Dominick Grift
@ 2022-09-20 13:06 ` Ondrej Mosnacek
2022-09-20 13:45 ` Dominick Grift
0 siblings, 1 reply; 6+ messages in thread
From: Ondrej Mosnacek @ 2022-09-20 13:06 UTC (permalink / raw)
To: Dominick Grift; +Cc: Christian Göttsche, SElinux list
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories
2022-09-20 13:06 ` Ondrej Mosnacek
@ 2022-09-20 13:45 ` Dominick Grift
0 siblings, 0 replies; 6+ messages in thread
From: Dominick Grift @ 2022-09-20 13:45 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: Christian Göttsche, SElinux list
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.
>
--
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6 E0FF DA7E 521F 10F6 4098
Dominick Grift
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-20 13:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-19 11:29 [PATCH userspace] fixfiles: do not cross mounts when traversing directories Ondrej Mosnacek
2022-09-19 11:35 ` Christian Göttsche
2022-09-19 15:44 ` Ondrej Mosnacek
2022-09-19 15:58 ` Dominick Grift
2022-09-20 13:06 ` Ondrej Mosnacek
2022-09-20 13:45 ` Dominick Grift
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.