| Message ID | 20190307152854.11306.84006.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: Linux IMA on NFS prototype | expand |
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. Can we really remove this call? --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
> 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. > > Can we really remove this call? Why wouldn't we be able to? > --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
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. > > > > 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--is it really OK just to skip this on opens by nfsd? --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 > >
> 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. >>> >>> 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. > 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
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 > > >
> 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. 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? 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. 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. 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? >>>>> 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 >> >> >> > -- Chuck Lever
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? >
> On Mar 21, 2019, at 6:44 AM, Mimi Zohar <zohar@linux.ibm.com> wrote: > > 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? It's a design goal of any filesystem to present unaltered file data to applications. But the responsibility is end-to-end. Adding extra checks in the middle introduce a cost. Measuring on the client is sufficient, and it is equivalent to what local filesystems do (and, it allows each client to apply its own security policy). I'm going to claim here without proof that there is little value in using IMA on an NFS server that serves NFS clients that are not IMA-aware. :-) >> 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. Commercial NFS servers (like NetApp filers) perform malware and integrity checking via a scrubbing agent rather than checking in a hot path. Filesystems are not only responsible for leaving data unchanged, they also have performance requirements. >> 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). IIUC, you are talking about troubleshooting, which should be rare. That can be done with tools on the server if needed, but IMO can be avoided in performance-sensitive paths. > 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? -- Chuck Lever
On Thu, 2019-03-21 at 09:04 -0500, Chuck Lever wrote: > > > On Mar 21, 2019, at 6:44 AM, Mimi Zohar <zohar@linux.ibm.com> wrote: > > 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 > >>>>> 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? > > It's a design goal of any filesystem to present unaltered file data > to applications. But the responsibility is end-to-end. Adding extra > checks in the middle introduce a cost. Files are measured/appraised/audited based on the IMA policy. Have you measured the performance cost of measuring and appraising the files being served? Unless a policy has been supplied, the performance impact, if any, would be limited to walking the IMA policy rules. > Measuring on the client is > sufficient, and it is equivalent to what local filesystems do (and, > it allows each client to apply its own security policy). I'm not arguing with you about an end-to-end file integrity solution. That is the goal, but one that assumes this proposed work, based on fs-verity signatures. > I'm going to claim here without proof that there is little value in > using IMA on an NFS server that serves NFS clients that are not > IMA-aware. :-) For systems that don't or haven't implemented the proposed end-to-end file integrity solution, verifying the file integrity on the server is all the more important. > > >> 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. > > Commercial NFS servers (like NetApp filers) perform malware and > integrity checking via a scrubbing agent rather than checking in a > hot path. Filesystems are not only responsible for leaving data > unchanged, they also have performance requirements. Any userspace application leaves a window of opportunity between the time the file has been created/modified and the time that the application verifies it. This is one of the main reason for IMA being in the kernel. > > >> 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). > > IIUC, you are talking about troubleshooting, which should be > rare. That can be done with tools on the server if needed, but > IMO can be avoided in performance-sensitive paths. No, this isn't about "troubleshooting", but about auditing the files served and using the file hashes for forensic investigations.[1][2] Mimi [1] Commit e7c568e0fd0c ("ima: audit log hashes") [2] https://www.fireeye.com/blog/threat-research/2016/11/extending_linux_exec.html
> On Mar 22, 2019, at 6:55 PM, Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2019-03-21 at 09:04 -0500, Chuck Lever wrote: >> >>> On Mar 21, 2019, at 6:44 AM, Mimi Zohar <zohar@linux.ibm.com> wrote: >>> 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 >>>>>>> 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? >> >> It's a design goal of any filesystem to present unaltered file data >> to applications. But the responsibility is end-to-end. Adding extra >> checks in the middle introduce a cost. > > Files are measured/appraised/audited based on the IMA policy. Have > you measured the performance cost of measuring and appraising the > files being served? Unless a policy has been supplied, the > performance impact, if any, would be limited to walking the IMA policy > rules. I have not measured the performance cost, but I'm simply observing that it could be substantial in some cases. nfsd_open is called in the NFSv3 READ and WRITE paths, in addition to whenever a file is locked (NFSv3) or opened (NFSv4). The impact is small when there is no IMA policy, but could be substantial when there is an IMA policy in effect. Note that NFSv3 does not have an OPEN operation. I'm not sure what would be an adequate substitute location for an IMA hash audit for NFSv3 file accesses. Measuring on the client gives us the most effective integrity coverage and an implementation that is closest in behavior to local filesystem semantics. I'm just saying I'd rather not measure twice, because it doesn't provide a great benefit (more below) and can be costly. I agree that we should figure out a way to assess its performance impact. >> Measuring on the client is >> sufficient, and it is equivalent to what local filesystems do (and, >> it allows each client to apply its own security policy). > > I'm not arguing with you about an end-to-end file integrity solution. > That is the goal, but one that assumes this proposed work, based on > fs-verity signatures. > >> I'm going to claim here without proof that there is little value in >> using IMA on an NFS server that serves NFS clients that are not >> IMA-aware. :-) > > For systems that don't or haven't implemented the proposed end-to-end > file integrity solution, verifying the file integrity on the server is > all the more important. As I said before, the largest integrity exposure is modification of file data as it is transported between server and client -- _after_ the server has measured it. IMHO that makes measuring on the server a bit of security theatre. In addition to that, ima_file_check returns -EACCES if it fails (and the IMA policy is in enforcing mode). That is a confusing situation for NFS, since that looks like the user does not have permission to access the file (ie, that it can be corrected by the user by changing an ACL or mode bits). Maybe NFS should have new error codes that can convey "blocked by administrative policy" or "failed integrity check". In the meantime, though, existing NFS systems without any IMA support do not have these codes. >>>> 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. >> >> Commercial NFS servers (like NetApp filers) perform malware and >> integrity checking via a scrubbing agent rather than checking in a >> hot path. Filesystems are not only responsible for leaving data >> unchanged, they also have performance requirements. > > Any userspace application leaves a window of opportunity between the > time the file has been created/modified and the time that the > application verifies it. This is one of the main reason for IMA being > in the kernel. > >> >>>> 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). >> >> IIUC, you are talking about troubleshooting, which should be >> rare. That can be done with tools on the server if needed, but >> IMO can be avoided in performance-sensitive paths. > > No, this isn't about "troubleshooting", but about auditing the files > served and using the file hashes for forensic investigations.[1][2] > > Mimi > > [1] Commit e7c568e0fd0c ("ima: audit log hashes") > [2] https://www.fireeye.com/blog/threat-research/2016/11/extending_linux_exec.html Auditing can be done by keeping the ima_file_check call site but ignoring its return code, for example. In any event, removing the ima_file_check call is not required for the prototype to be functional. I can drop this patch for now, but I encourage examination of how the NFS server measures and audits files when an actual IMA policy is in effect. -- Chuck Lever
On Mon, 2019-03-25 at 10:24 -0400, Chuck Lever wrote: > Auditing can be done by keeping the ima_file_check call site but > ignoring its return code, for example. Neither the "measure" or "audit" rules control the return code. Only an IMA "appraise" rule verifies a file's integrity, which could fail, resulting in an error return code. Different systems might have different requirements. Having the IMA hook here, allows IMA custom policies to be defined, based on the specific system requirements. In terms of performance, IMA calculates the file hash once, which can then be used for measuring, appraising, and auditing. Unless the file changes, calculating the file hash is only done once. > In any event, removing the ima_file_check call is not required for > the prototype to be functional. I can drop this patch for now, but > I encourage examination of how the NFS server measures and audits > files when an actual IMA policy is in effect. Thank you! Mimi
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
The NFS server needs to allow NFS clients to perform their own attestation and measurement. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfsd/vfs.c | 6 ------ 1 file changed, 6 deletions(-)