[PATCH 2/2] NFSv4.2: condition READDIR's mask for security label based on LSM state

[Ondrej Mosnacek]

On Thu, Nov 5, 2020 at 10:43 PM Olga Kornievskaia
<olga.kornievskaia at gmail.com> wrote:
> On Thu, Nov 5, 2020 at 4:18 PM Trond Myklebust <trondmy at hammerspace.com> wrote:
> >
> > On Thu, 2020-11-05 at 14:51 -0500, Olga Kornievskaia wrote:
> > > On Thu, Nov 5, 2020 at 1:55 PM Ondrej Mosnacek <omosnace at redhat.com>
> > > wrote:
> > > >
> > > > On Thu, Nov 5, 2020 at 6:33 PM Olga Kornievskaia
> > > > <olga.kornievskaia at gmail.com> wrote:
> > > > > From: Olga Kornievskaia <kolga at netapp.com>
> > > > >
> > > > > Currently, the client will always ask for security_labels if the
> > > > > server
> > > > > returns that it supports that feature regardless of any LSM
> > > > > modules
> > > > > (such as Selinux) enforcing security policy. This adds
> > > > > performance
> > > > > penalty to the READDIR operation.
> > > > >
> > > > > Instead, query the LSM module to find if anything is enabled and
> > > > > if not, then remove FATTR4_WORD2_SECURITY_LABEL from the bitmask.
> > > >
> > > > Having spent some time staring at some of the NFS code very
> > > > recently,
> > > > I can't help but suggest: Would it perhaps be enough to decide
> > > > whether
> > > > to ask for labels based on (NFS_SB(dentry->d_sb)->caps &
> > > > NFS_CAP_SECURITY_LABEL)? It is set when mounting the FS iff some
> > > > LSM
> > > > confirms via the security_sb_*_mnt_opts() hook that it wants the
> > > > filesystem to give it labels (or at least that's how I interpret
> > > > the
> > > > cryptic name) [1]. It's just a shot in the dark, but it seems to
> > > > fit
> > > > this use case.
> > > >
> > > > [1]
> > > > https://elixir.bootlin.com/linux/v5.10-rc2/source/fs/nfs/getroot.c#L148
> > >
> > > Very interesting. I was not aware of something like that nor was it
> > > mentioned when I asked on the selinux mailing list. I wonder if this
> > > is a supported feature that will always stay? In that case, NFS
> > > wouldn't need the extra hook that was added for this series. I will
> > > try this out and report back.
> >
> > NFS_CAP_SECURITY_LABEL is just the NFS server capability flag. It tells
> > you whether or not the client believes that the server might support
> > NFSv4.2 requests for labelled NFS metadata.
> >
> > My understanding of Olga's requirement is that she needs to be able to
> > ignore that flag and simply not query for labelled NFS metadata if the
> > NFS client is not configured to enforce the LSM policy. The objective
> > is to avoid unnecessary RPC traffic to the server to query for data
> > that is unused.
>
> Actually that seems to be working. I think it's because while the
> server returns that it supports sec_labels, after calling
> security_sb_set_mnt_opts() the kflags_out don't have this
> SECURITY_LSM_NATIVE_LABELS set (assuming this happens because selinux
> isn't enabled) then we turned server's sec_labl cap off.
>
> I'm about to send v2 without relying on modifications to selinux.

Just to keep the LSM and SELinux lists in the loop: a v2 has been
posted to linux-nfs that uses NFS_CAP_SECURITY_LABEL instead of adding
a new hook:
https://lore.kernel.org/linux-nfs/CAFqZXNtJ2fkae7Xt-+nDR79kVs=yY=9vSoZaS-V-5UKSk22s4A@mail.gmail.com/T/

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