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

From: Olga Kornievskaia <olga.kornievskaia@gmail.com>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "omosnace@redhat.com" <omosnace@redhat.com>,
	"selinux@vger.kernel.org" <selinux@vger.kernel.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
	"paul@paul-moore.com" <paul@paul-moore.com>,
	"stephen.smalley.work@gmail.com" <stephen.smalley.work@gmail.com>
Subject: Re: [PATCH 2/2] NFSv4.2: condition READDIR's mask for security label based on LSM state
Date: Thu, 5 Nov 2020 16:43:01 -0500	[thread overview]
Message-ID: <CAN-5tyHc_fjDXwUngqVshB0Z7SzhAqjkXP7E=-k4sAPbfRwMmQ@mail.gmail.com> (raw)
In-Reply-To: <a96c14c0f86ec274d5bdc255050ae71238bb43fe.camel@hammerspace.com>

On Thu, Nov 5, 2020 at 4:18 PM Trond Myklebust <trondmy@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@redhat.com>
> > wrote:
> > >
> > > On Thu, Nov 5, 2020 at 6:33 PM Olga Kornievskaia
> > > <olga.kornievskaia@gmail.com> wrote:
> > > > From: Olga Kornievskaia <kolga@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.

>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
>
>