Re: Weird bug in NFS/SELinux

Re: Weird bug in NFS/SELinux

[Ondrej Mosnacek]

On Thu, Mar 18, 2021 at 2:57 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Thu, Mar 18, 2021 at 2:43 PM Olga Kornievskaia <aglo@umich.edu> wrote:
> > On Thu, Mar 18, 2021 at 5:59 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > Hello,
> > >
> > > While trying to figure out why the NFS tests in the selinux-testsuite
> > > [1] are failing, I ran into this strange bug: When I mount an NFS
> > > filesystem on some directory, and then immediately attempt to create
> > > exactly the same mount on the same directory (fails with -EBUSY as
> > > expected per mount(2)), then all the entries inside the mount (but not
> > > the root node) show up as unlabeled
> > > (system_u:object_r:unlabeled_t:s0). For some reason this doesn't
> > > happen if I list the directory contents between the two mounts.
> > >
> > > It happens at least with kernels 5.12-rc2 and 5.8.6, so it's likely an old bug.
> > >
> > > Minimal reproducer (assumes an SELinux-enabled system and that nothing
> > > is mounted at /etc):
> > > ```
> > > # set up a trivial NFS export
> > > systemctl start nfs-server
> > > exportfs -o rw,no_root_squash,security_label localhost:/
> > >
> > > #
> > > # reference scenario - single mount
> > > #
> > > mount -t nfs -o "nfsvers=4.2" localhost:/etc /mnt
> > >
> > > ls -lZ /mnt    # labels are correct
> > > ls -lZd /mnt   # label is correct
> > >
> > > #
> > > # double mount - BUG
> > > #
> > > mount -t nfs -o "nfsvers=4.2" localhost:/etc /mnt
> > > mount -t nfs -o "nfsvers=4.2" localhost:/etc /mnt
> > >
> > > ls -lZ /mnt    # all labels are system_u:object_r:unlabeled_t:s0
> > > ls -lZd /mnt   # label is correct
> > >
> > > #
> > > # double mount with ls in between - OK
> > > #
> > > mount -t nfs -o "nfsvers=4.2" localhost:/etc /mnt
> > > ls -lZ /mnt    # labels are correct
> > > ls -lZd /mnt   # label is correct
> > > mount -t nfs -o "nfsvers=4.2" localhost:/etc /mnt
> > >
> > > ls -lZ /mnt    # labels are correct
> > > ls -lZd /mnt   # label is correct
> >
> > Hi Ondrej, a couple of questions about the reproducer. (1) are you
> > saying that only "mount, mount, ls" sequence is problematic as you
> > write "mount, ls, mount, ls" is correct? (2) what is your selinux
> > configuration. I can't reproduce it on my setup. I get the same labels
> > regardless of how many times I mount.
>
> (1) Yes, exactly.
> (2) I reproduced it reliably on clean Fedora VM images (e.g. Fedora 33
> or Rawhide, both showed this bug).

(Adding also linux-security-module@, since this affects the LSM interface.)

After some off-list exchange trying to get the bug to reproduce on
Olga's side, we have made some progress, so let me summarize our
findings here.

First, the issue only appears when you export the root directory, not
just some path underneath. I suspect that it could be any directory
with a mount on it rather than just the root dir, but I haven't
verified that...

Second, as Olga found out, the issue stems from the call to
security_sb_set_mnt_opts() (from nfs_get_root()) on an already
initialized superblock (AFAIK it is needed so that the LSM can check
if the security mount options match (and error out the mount if they
don't), where NFS processes the SECURITY_LSM_NATIVE_LABELS flag the
same way as on the first mount, but SELinux ignores it on the repeated
mount. Thus NFS turns off the NFS_CAP_SECURITY_LABEL flag and stops
fetching labels from the server, so fresh inodes then show up as
unlabeled.

So I think there are two options how to fix it:
1) Require filesystems to always pass (0, NULL) as kern_flags when
calling it on already initialized superblock - turning the labeling
support on/off for an existing superblock wouldn't work with SELinux
anyway.
2) When selinux_set_mnt_opts() is called again on a superblock,
validate that the passed kern_flags match the expected value (i.e. the
FS isn't trying to set an incompatible SECURITY_LSM_NATIVE_LABELS
setting) and also return back the same flags as on the first call.

It seems doing 1) would make the code in nfs_get_root() a bit ugly
(and it might require some changes in VFS, too), so I think I like 2)
more... SELinux/LSM folks, any thoughts?

-- 
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.