On Thu, Nov 5, 2020 at 10:43 PM Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote: > On Thu, Nov 5, 2020 at 4:18 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx> > > > wrote: > > > > > > > > On Thu, Nov 5, 2020 at 6:33 PM Olga Kornievskaia > > > > <olga.kornievskaia@xxxxxxxxx> wrote: > > > > > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > > > > > > > > > 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@xxxxxxxxxxxxxx/T/ -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.