Re: [PATCH userspace] fixfiles: do not cross mounts when traversing directories

[Dominick Grift <dominick.grift@defensec.nl>]

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