Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201105173328.2539-1-olga.kornievskaia@gmail.com>
 <20201105173328.2539-2-olga.kornievskaia@gmail.com> <CAFqZXNtjMEF0LO4vtEmcgwydbWfUS36d8g24J6C-NDXORYbEJg@mail.gmail.com>
 <CAN-5tyF+cLpmT=rwAYvQQ445tjFKZtGq+Qzf6rDGg8COPpFRwA@mail.gmail.com> <a96c14c0f86ec274d5bdc255050ae71238bb43fe.camel@hammerspace.com>
In-Reply-To: <a96c14c0f86ec274d5bdc255050ae71238bb43fe.camel@hammerspace.com>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Thu, 5 Nov 2020 16:43:01 -0500
Message-ID: <CAN-5tyHc_fjDXwUngqVshB0Z7SzhAqjkXP7E=-k4sAPbfRwMmQ@mail.gmail.com>
Subject: Re: [PATCH 2/2] NFSv4.2: condition READDIR's mask for security label
 based on LSM state
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>
>