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>
Cc:     Bruce Fields <bfields@fieldses.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        linux-integrity@vger.kernel.org,
        "Serge E. Hallyn" <serge@hallyn.com>
Date:   Thu, 21 Mar 2019 07:44:47 -0400
In-Reply-To: <0E02D70A-A5E9-4B27-9922-521D5A0755A3@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>
         <1553027371.4899.116.camel@linux.ibm.com>
         <0E02D70A-A5E9-4B27-9922-521D5A0755A3@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Message-Id: <1553168687.4899.396.camel@linux.ibm.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Wed, 2019-03-20 at 08:40 -0500, Chuck Lever wrote:
> 
> > On Mar 19, 2019, at 3:29 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > 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)?
> 
> Hi Mimi, thanks for the review.
> 
> Architecturally, the server is not using the file's data, it is
> merely part of the filesystem that stores it. But that said, there
> are several concrete reasons why I feel an NFS server should not be
> involved in measurement/attestation, but only with storing file
> content and IMA metadata.

"Remote attestation" is the process of verifying the measurement list
against the TPM PCRs, based on a TPM quote.  I think you meant
"measurement/appraisal".

> 
> 1. The broadest attack surface for a remote filesystem is modification
> of data in flight. Attestation of the file on the server is not going
> to defend against that attack, only attestation on the client will do
> that. Is there a good reason to pay the cost of double attestation?

Doesn't the server have a responsibility to provide files that have
not been unintentionally or maliciously altered?

> 2. It is possible (perhaps even likely) that the NFS server and a
> client of that server will have different IMA policies and even
> different file signing authorities.

That doesn't negate the due diligence on the server's part of
preventing the spread of malware.
> 
> A third, perhaps related, reason is that NFS can run on non-Linux NFS
> servers which would not have any attestation at all. An NFS client
> should not have to rely on the server for attestation, but should
> trust only its own measurement of each file, which would be done as
> late as possible before use.

The ima_file_check() hook can also audit the file, providing
additional forensic information (eg. the file hash).

Mimi

> 
> Lastly, the NFS protocol does not enable an NFS client to tell a
> server how the file is to be used. For example, the server's policy
> might block execution of an unverifiable file, but the server won't
> have any way of knowing how the client is going to use that file.
> The client might be opening the file to copy it or update its IMA
> metadata.
> 
> Speaking of protocol, there's no special error code that reports an
> integrity verification failure. The client just sees that the UID
> does not have access to the file. There's no way the user or client
> can do anything to clear this condition via NFS without IMA support.
> 
> If these reasons make sense, should I add them to the patch description?
>