Subject: Re: [PATCH v2 3/5] NFSD: Remove ima_file_check call
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Chuck Lever <chuck.lever@oracle.com>,
        Bruce Fields <bfields@fieldses.org>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        linux-integrity@vger.kernel.org,
        "Serge E. Hallyn" <serge@hallyn.com>
Date:   Tue, 19 Mar 2019 16:29:31 -0400
In-Reply-To: <872F3DFD-E1A7-443E-B666-25C5931F0748@oracle.com>
References: <20190307151838.11306.94183.stgit@manet.1015granger.net>
         <20190307152854.11306.84006.stgit@manet.1015granger.net>
         <20190308211016.GB27011@fieldses.org>
         <ECE4FD20-840C-4047-9454-27BDF0A905EA@oracle.com>
         <20190308212310.GB28002@fieldses.org>
         <872F3DFD-E1A7-443E-B666-25C5931F0748@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Message-Id: <1553027371.4899.116.camel@linux.ibm.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Fri, 2019-03-08 at 16:29 -0500, Chuck Lever wrote:

Thanks Serge for bringing this thread to my attention.  Sorry for the
delay in responding ...

> > On Mar 8, 2019, at 4:23 PM, Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > On Fri, Mar 08, 2019 at 04:11:06PM -0500, Chuck Lever wrote:
> >> 
> >> 
> >>> On Mar 8, 2019, at 4:10 PM, bfields@fieldses.org wrote:
> >>> 
> >>> On Thu, Mar 07, 2019 at 10:28:54AM -0500, Chuck Lever wrote:
> >>>> The NFS server needs to allow NFS clients to perform their own
> >>>> attestation and measurement.

Measurement and attestation is only one aspect.  The other aspect is
verifying the integrity of files.  Shouldn't the NFS server verify the
integrity of a file before allowing it to be served (eg. malware)?

> >>> 
> >>> Can we really remove this call?
> >> 
> >> Why wouldn't we be able to?
> > 
> > I don't know the first thing about IMA, but surely it's there for some
> > reason--
> 
> It was originally added because the number of opens and closes of @file
> were counted, and not having that call was triggering a warning. Since
> commit 8eb988c70e770 ("fix ima breakage") the counters are maintained
> separately.

If that was the only reason, then the call itself would have been
removed with the counter code.

Mimi

> 
> 
> > is it really OK just to skip this on opens by nfsd?
> 
> That's why I split this out into a separate patch. I'm hoping to get
> some commentary from the linux-integrity community.
> 
> 
> > --b.
> > 
> >>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >>>> ---
> >>>> fs/nfsd/vfs.c |    6 ------
> >>>> 1 file changed, 6 deletions(-)
> >>>> 
> >>>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> >>>> index 3c00072..524c6e5 100644
> >>>> --- a/fs/nfsd/vfs.c
> >>>> +++ b/fs/nfsd/vfs.c
> >>>> @@ -802,12 +802,6 @@ static int nfsd_open_break_lease(struct inode *inode, int access)
> >>>> 		goto out_nfserr;
> >>>> 	}
> >>>> 
> >>>> -	host_err = ima_file_check(file, may_flags);
> >>>> -	if (host_err) {
> >>>> -		fput(file);
> >>>> -		goto out_nfserr;
> >>>> -	}
> >>>> -
> >>>> 	if (may_flags & NFSD_MAY_64BIT_COOKIE)
> >>>> 		file->f_mode |= FMODE_64BITHASH;
> >>>> 	else
> >> 
> >> --
> >> Chuck Lever
> >> 
> >> 
> 
> --
> Chuck Lever
> 
> 
>