| Message ID | 1520540650-7451-5-git-send-email-zohar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > Files on FUSE can change at any point in time without IMA being able > to detect it. The file data read for the file signature verification > could be totally different from what is subsequently read, making the > signature verification useless. > > FUSE can be mounted by unprivileged users either today with fusermount > installed with setuid, or soon with the upcoming patches to allow FUSE > mounts in a non-init user namespace. > > This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when > appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. > > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > Cc: Miklos Szeredi <miklos@szeredi.hu> > Cc: Seth Forshee <seth.forshee@canonical.com> > Cc: Eric W. Biederman <ebiederm@xmission.com> > Cc: Dongsu Park <dongsu@kinvolk.io> > Cc: Alban Crequy <alban@kinvolk.io> > Cc: "Serge E. Hallyn" <serge@hallyn.com> Acked-by: Serge Hallyn <serge@hallyn.com> Of course when IMA namespacing hits, you'll want to compare the sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns right? > --- > fs/fuse/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index 624f18bbfd2b..ef309958e060 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) > sb->s_maxbytes = MAX_LFS_FILESIZE; > sb->s_time_gran = 1; > sb->s_export_op = &fuse_export_operations; > + sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE; > + if (sb->s_user_ns != &init_user_ns) > + sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER; > > file = fget(d.fd); > err = -EINVAL; > -- > 2.7.5
On 03/12/2018 03:29 PM, Serge E. Hallyn wrote: > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): >> Files on FUSE can change at any point in time without IMA being able >> to detect it. The file data read for the file signature verification >> could be totally different from what is subsequently read, making the >> signature verification useless. >> >> FUSE can be mounted by unprivileged users either today with fusermount >> installed with setuid, or soon with the upcoming patches to allow FUSE >> mounts in a non-init user namespace. >> >> This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when >> appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. >> >> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> >> Cc: Miklos Szeredi <miklos@szeredi.hu> >> Cc: Seth Forshee <seth.forshee@canonical.com> >> Cc: Eric W. Biederman <ebiederm@xmission.com> >> Cc: Dongsu Park <dongsu@kinvolk.io> >> Cc: Alban Crequy <alban@kinvolk.io> >> Cc: "Serge E. Hallyn" <serge@hallyn.com> > Acked-by: Serge Hallyn <serge@hallyn.com> > > Of course when IMA namespacing hits, you'll want to compare the > sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns > right? I suppose this would be the only way to enable 'trusted mounters' within IMA namespaces. Maybe there could be an additional capability gate that would allow one to be a 'trusted mounter' then? > >> --- >> fs/fuse/inode.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c >> index 624f18bbfd2b..ef309958e060 100644 >> --- a/fs/fuse/inode.c >> +++ b/fs/fuse/inode.c >> @@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) >> sb->s_maxbytes = MAX_LFS_FILESIZE; >> sb->s_time_gran = 1; >> sb->s_export_op = &fuse_export_operations; >> + sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE; >> + if (sb->s_user_ns != &init_user_ns) >> + sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER; >> >> file = fget(d.fd); >> err = -EINVAL; >> -- >> 2.7.5
Mimi Zohar <zohar@linux.vnet.ibm.com> writes: > Files on FUSE can change at any point in time without IMA being able > to detect it. The file data read for the file signature verification > could be totally different from what is subsequently read, making the > signature verification useless. > > FUSE can be mounted by unprivileged users either today with fusermount > installed with setuid, or soon with the upcoming patches to allow FUSE > mounts in a non-init user namespace. > > This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when > appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > Cc: Miklos Szeredi <miklos@szeredi.hu> > Cc: Seth Forshee <seth.forshee@canonical.com> > Cc: Eric W. Biederman <ebiederm@xmission.com> > Cc: Dongsu Park <dongsu@kinvolk.io> > Cc: Alban Crequy <alban@kinvolk.io> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > --- > fs/fuse/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index 624f18bbfd2b..ef309958e060 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) > sb->s_maxbytes = MAX_LFS_FILESIZE; > sb->s_time_gran = 1; > sb->s_export_op = &fuse_export_operations; > + sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE; > + if (sb->s_user_ns != &init_user_ns) > + sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER; > > file = fget(d.fd); > err = -EINVAL;
I do not have any comments about the patches but a question. I completely agree that the files can change without the VFS knowing about it, but isn't that in general the case with filesystems with a backend shared with others (network fs's?). Stef
On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: > I do not have any comments about the patches but a question. > I completely agree that the files can change without the VFS knowing > about it, but isn't that in general the case with filesystems with a > backend shared with others (network fs's?). Right, the problem is not limited to fuse, but needs to be addressed before unprivileged fuse mounts are upstreamed. Alban's response to this question: https://marc.info/?l=linux-kernel&m=151784020321045&w=2 Mimi
Quoting Stefan Berger (stefanb@linux.vnet.ibm.com): > On 03/12/2018 03:29 PM, Serge E. Hallyn wrote: > >Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > >>Files on FUSE can change at any point in time without IMA being able > >>to detect it. The file data read for the file signature verification > >>could be totally different from what is subsequently read, making the > >>signature verification useless. > >> > >>FUSE can be mounted by unprivileged users either today with fusermount > >>installed with setuid, or soon with the upcoming patches to allow FUSE > >>mounts in a non-init user namespace. > >> > >>This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when > >>appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. > >> > >>Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > >>Cc: Miklos Szeredi <miklos@szeredi.hu> > >>Cc: Seth Forshee <seth.forshee@canonical.com> > >>Cc: Eric W. Biederman <ebiederm@xmission.com> > >>Cc: Dongsu Park <dongsu@kinvolk.io> > >>Cc: Alban Crequy <alban@kinvolk.io> > >>Cc: "Serge E. Hallyn" <serge@hallyn.com> > >Acked-by: Serge Hallyn <serge@hallyn.com> > > > >Of course when IMA namespacing hits, you'll want to compare the > >sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns > >right? > > I suppose this would be the only way to enable 'trusted mounters' > within IMA namespaces. Maybe there could be an additional capability > gate that would allow one to be a 'trusted mounter' then? Wouldn't CAP_SYS_ADMIN to the ima_ns->user_ns suffice? I personally think CAP_INTEGRITY would make sense, but right now CAP_SYS_ADMIN seems to suffice so it wouldn't make sense to raise the bar there unless we raise it for all of IMA configuration. -serge
On 03/14/2018 10:27 AM, Serge E. Hallyn wrote: > Quoting Stefan Berger (stefanb@linux.vnet.ibm.com): >> On 03/12/2018 03:29 PM, Serge E. Hallyn wrote: >>> Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): >>>> Files on FUSE can change at any point in time without IMA being able >>>> to detect it. The file data read for the file signature verification >>>> could be totally different from what is subsequently read, making the >>>> signature verification useless. >>>> >>>> FUSE can be mounted by unprivileged users either today with fusermount >>>> installed with setuid, or soon with the upcoming patches to allow FUSE >>>> mounts in a non-init user namespace. >>>> >>>> This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when >>>> appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. >>>> >>>> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> >>>> Cc: Miklos Szeredi <miklos@szeredi.hu> >>>> Cc: Seth Forshee <seth.forshee@canonical.com> >>>> Cc: Eric W. Biederman <ebiederm@xmission.com> >>>> Cc: Dongsu Park <dongsu@kinvolk.io> >>>> Cc: Alban Crequy <alban@kinvolk.io> >>>> Cc: "Serge E. Hallyn" <serge@hallyn.com> >>> Acked-by: Serge Hallyn <serge@hallyn.com> >>> >>> Of course when IMA namespacing hits, you'll want to compare the >>> sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns >>> right? >> I suppose this would be the only way to enable 'trusted mounters' >> within IMA namespaces. Maybe there could be an additional capability >> gate that would allow one to be a 'trusted mounter' then? > Wouldn't CAP_SYS_ADMIN to the ima_ns->user_ns suffice? > > I personally think CAP_INTEGRITY would make sense, but right > now CAP_SYS_ADMIN seems to suffice so it wouldn't make sense to > raise the bar there unless we raise it for all of IMA configuration. So for IMA namespacing we may want to avoid CAP_SYS_ADMIN and introduce one or more capabilities to: - set security xattrs from inside the container (when building the container for example, maybe also during runtime) - access IMA's securityfs entries (https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_fs.c#L391) from inside the container for reading/writing the policy (during run-time) - then possibly mounting a trusted filesystem via fuse Stefan > > -serge >
Mimi Zohar <zohar@linux.vnet.ibm.com> writes: > On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: >> I do not have any comments about the patches but a question. >> I completely agree that the files can change without the VFS knowing >> about it, but isn't that in general the case with filesystems with a >> backend shared with others (network fs's?). > > Right, the problem is not limited to fuse, but needs to be addressed > before unprivileged fuse mounts are upstreamed. > > Alban's response to this question: > https://marc.info/?l=linux-kernel&m=151784020321045&w=2 Which goes to why it is a flag that get's set. All of this just needs a follow-up patch to update every filesystem that does not meet ima's requirements. Mimi I believe you said that the requirement is that all file changes can be detected through the final __fput of a file that calls ima_file_free. Eric
On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: > > > On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: > >> I do not have any comments about the patches but a question. > >> I completely agree that the files can change without the VFS knowing > >> about it, but isn't that in general the case with filesystems with a > >> backend shared with others (network fs's?). > > > > Right, the problem is not limited to fuse, but needs to be addressed > > before unprivileged fuse mounts are upstreamed. > > > > Alban's response to this question: > > https://marc.info/?l=linux-kernel&m=151784020321045&w=2 > > Which goes to why it is a flag that get's set. > > All of this just needs a follow-up patch to update every filesystem > that does not meet ima's requirements. Currently files on remote filesystems are measured/appraised/audited once. With the new flags, our options would be to either fail the signature verification or constantly re-measure/re-appraise files on remote file systems. Neither option seems like the right solution. There's some very initial discussions on how to support file integrity on remote filesystems. Chuck Lever has some thoughts on piggy-backing on the fs-verity work being done. From a very, very high level, IMA- appraisal would verify the file signature, but leave the integrity enforcement to the vfs/fs layer. By integrating fs-verity or similar proposal with IMA, measurements would be included in the measurement list and keys used for file signature verification would use the same existing IMA-appraisal infrastructure. > Mimi I believe you said that the requirement is that all file changes > can be detected through the final __fput of a file that calls > ima_file_free. Right, like for fuse, I don't believe this existing hook works for remote filesystems. Mimi
> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: >> Mimi Zohar <zohar@linux.vnet.ibm.com> writes: >> >>> On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: >>>> I do not have any comments about the patches but a question. >>>> I completely agree that the files can change without the VFS knowing >>>> about it, but isn't that in general the case with filesystems with a >>>> backend shared with others (network fs's?). >>> >>> Right, the problem is not limited to fuse, but needs to be addressed >>> before unprivileged fuse mounts are upstreamed. >>> >>> Alban's response to this question: >>> https://marc.info/?l=linux-kernel&m=151784020321045&w=2 >> >> Which goes to why it is a flag that get's set. >> >> All of this just needs a follow-up patch to update every filesystem >> that does not meet ima's requirements. > > Currently files on remote filesystems are measured/appraised/audited > once. With the new flags, our options would be to either fail the > signature verification or constantly re-measure/re-appraise files on > remote file systems. Neither option seems like the right solution. Being new to this game, I may be making a bad assumption, but I thought that the (NFSv4) change attribute can be used to detect remote mutations to a file's content or metadata, so that the appraisal could be cached as long as that attribute does not change. At least for NFSv4, clients assume their data cache is valid while the change attribute remains the same. NFSv4 (and SMB) also has a mechanism where a server guarantees it will report any other clients that want to update a file. This is an NFSv4 read delegation or an SMB oplock. NFSv4 clients use this mechanism to cache file data quite aggressively, and it could also be used to preserve or cache audit state on remote files. > There's some very initial discussions on how to support file integrity > on remote filesystems. Chuck Lever has some thoughts on piggy-backing > on the fs-verity work being done. From a very, very high level, IMA- > appraisal would verify the file signature, but leave the integrity > enforcement to the vfs/fs layer. By integrating fs-verity or similar > proposal with IMA, measurements would be included in the measurement > list and keys used for file signature verification would use the same > existing IMA-appraisal infrastructure. >> Mimi I believe you said that the requirement is that all file changes >> can be detected through the final __fput of a file that calls >> ima_file_free. > > Right, like for fuse, I don't believe this existing hook works for > remote filesystems. > > Mimi -- Chuck Lever
Chuck Lever <chuck.lever@oracle.com> writes: >> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >> >> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: >>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes: >>> >>>> On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: >>>>> I do not have any comments about the patches but a question. >>>>> I completely agree that the files can change without the VFS knowing >>>>> about it, but isn't that in general the case with filesystems with a >>>>> backend shared with others (network fs's?). >>>> >>>> Right, the problem is not limited to fuse, but needs to be addressed >>>> before unprivileged fuse mounts are upstreamed. >>>> >>>> Alban's response to this question: >>>> https://marc.info/?l=linux-kernel&m=151784020321045&w=2 >>> >>> Which goes to why it is a flag that get's set. >>> >>> All of this just needs a follow-up patch to update every filesystem >>> that does not meet ima's requirements. >> >> Currently files on remote filesystems are measured/appraised/audited >> once. With the new flags, our options would be to either fail the >> signature verification or constantly re-measure/re-appraise files on >> remote file systems. Neither option seems like the right solution. They are measured/appraised/audited once, and you can not trust that at all because you don't know when the files are going to be different. So either keeping the filesystem out of the ima game or failing sounds like the right answer to me. At least until you can get better information from the filesystem. > Being new to this game, I may be making a bad assumption, but I thought > that the (NFSv4) change attribute can be used to detect remote mutations > to a file's content or metadata, so that the appraisal could be cached > as long as that attribute does not change. At least for NFSv4, clients > assume their data cache is valid while the change attribute remains the > same. > > NFSv4 (and SMB) also has a mechanism where a server guarantees it will > report any other clients that want to update a file. This is an NFSv4 > read delegation or an SMB oplock. NFSv4 clients use this mechanism to > cache file data quite aggressively, and it could also be used to > preserve or cache audit state on remote files. The file data invalid message, plus trusting the server, is what would be needed for reliably caching the validity of the file. >> There's some very initial discussions on how to support file integrity >> on remote filesystems. Chuck Lever has some thoughts on piggy-backing >> on the fs-verity work being done. From a very, very high level, IMA- >> appraisal would verify the file signature, but leave the integrity >> enforcement to the vfs/fs layer. By integrating fs-verity or similar >> proposal with IMA, measurements would be included in the measurement >> list and keys used for file signature verification would use the same >> existing IMA-appraisal infrastructure. > >>> Mimi I believe you said that the requirement is that all file changes >>> can be detected through the final __fput of a file that calls >>> ima_file_free. >> >> Right, like for fuse, I don't believe this existing hook works for >> remote filesystems. I am trying to understand things. - I believe the beginning of any file write should invalidate the validity of the files cache. IMA does something like that by looking at i_writecount. - As I read the code ima_file_free triggers an update of the ima xattr and that update needs to wait until the file is quiescent. AKA no more writers. I am not certain how you get that in a remote filesystem. If the write is not coming from the local kernel I don't see how it makes any sense for IMA to actually update the xattr on write. Eric
> On Mar 14, 2018, at 3:46 PM, ebiederm@xmission.com wrote: > > Chuck Lever <chuck.lever@oracle.com> writes: > >>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >>> >>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: >>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes: >>>> >>>>> On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: >>>>>> I do not have any comments about the patches but a question. >>>>>> I completely agree that the files can change without the VFS knowing >>>>>> about it, but isn't that in general the case with filesystems with a >>>>>> backend shared with others (network fs's?). >>>>> >>>>> Right, the problem is not limited to fuse, but needs to be addressed >>>>> before unprivileged fuse mounts are upstreamed. >>>>> >>>>> Alban's response to this question: >>>>> https://marc.info/?l=linux-kernel&m=151784020321045&w=2 >>>> >>>> Which goes to why it is a flag that get's set. >>>> >>>> All of this just needs a follow-up patch to update every filesystem >>>> that does not meet ima's requirements. >>> >>> Currently files on remote filesystems are measured/appraised/audited >>> once. With the new flags, our options would be to either fail the >>> signature verification or constantly re-measure/re-appraise files on >>> remote file systems. Neither option seems like the right solution. > > They are measured/appraised/audited once, and you can not trust that at > all because you don't know when the files are going to be different. > > So either keeping the filesystem out of the ima game or failing sounds > like the right answer to me. At least until you can get better > information from the filesystem. > >> Being new to this game, I may be making a bad assumption, but I thought >> that the (NFSv4) change attribute can be used to detect remote mutations >> to a file's content or metadata, so that the appraisal could be cached >> as long as that attribute does not change. At least for NFSv4, clients >> assume their data cache is valid while the change attribute remains the >> same. >> >> NFSv4 (and SMB) also has a mechanism where a server guarantees it will >> report any other clients that want to update a file. This is an NFSv4 >> read delegation or an SMB oplock. NFSv4 clients use this mechanism to >> cache file data quite aggressively, and it could also be used to >> preserve or cache audit state on remote files. > > The file data invalid message, plus trusting the server, is what > would be needed for reliably caching the validity of the file. What establishes client trust in the server? I'm probably missing something. The NFS protocol can convey the contents of the file, it's attributes, and the contents of the security.ima and security.evm xattrs. The xattrs contain cryptographically signed integrity metadata, which I presume cannot be altered undetectably either at rest or in transit. The client has everything it needs to measure that file, doesn't it, as long as it has the correct set of keys to verify the signatures? Likely I am naive, but it seems to me a file server does not have to participate in this process, other than to store and return IMA xattrs along with the file content. All participating clients would need to carry the same set of keys, however. Please tell me if I'm hijacking the thread. >>> There's some very initial discussions on how to support file integrity >>> on remote filesystems. Chuck Lever has some thoughts on piggy-backing >>> on the fs-verity work being done. From a very, very high level, IMA- >>> appraisal would verify the file signature, but leave the integrity >>> enforcement to the vfs/fs layer. By integrating fs-verity or similar >>> proposal with IMA, measurements would be included in the measurement >>> list and keys used for file signature verification would use the same >>> existing IMA-appraisal infrastructure. >> >>>> Mimi I believe you said that the requirement is that all file changes >>>> can be detected through the final __fput of a file that calls >>>> ima_file_free. >>> >>> Right, like for fuse, I don't believe this existing hook works for >>> remote filesystems. > > I am trying to understand things. > > - I believe the beginning of any file write should invalidate the > validity of the files cache. IMA does something like that by looking > at i_writecount. > > - As I read the code ima_file_free triggers an update of the ima xattr > and that update needs to wait until the file is quiescent. AKA no > more writers. I am not certain how you get that in a remote > filesystem. With NFSv4, a read delegation is sufficient to guarantee that the client is the only writer. The mechanism varies (or can be absent) for other remote filesystem protocols. And, an NFSv4 server is not obligated to always provide a delegation. An NFSv4 client can also OPEN a file with share deny modes. That would prevent other clients from accessing the file while the IMA metadata was recomputed. Again, I believe something similar would work for SMB3, but might not be applicable to other remote file- system protocols (eg NFSv3 does not have all this magic). However, computing a fresh IMA xattr would require access to the whole file. For a large file, a client would have to read it from the file server in its entirety, unless the file server offloads this computation from the client somehow. The server would need to wait until that client had CLOSEd the file to ensure that the client had no more cached dirty data, and at that point the server can itself guarantee there are no other remote accessors. > If the write is not coming from the local kernel I don't see how it > makes any sense for IMA to actually update the xattr on write. > > Eric -- Chuck Lever
Chuck Lever <chuck.lever@oracle.com> writes: >> On Mar 14, 2018, at 3:46 PM, ebiederm@xmission.com wrote: >> >> Chuck Lever <chuck.lever@oracle.com> writes: >> >>>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >>>> >>>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: >>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes: >>>>> >>>>>> On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: >>>>>>> I do not have any comments about the patches but a question. >>>>>>> I completely agree that the files can change without the VFS knowing >>>>>>> about it, but isn't that in general the case with filesystems with a >>>>>>> backend shared with others (network fs's?). >>>>>> >>>>>> Right, the problem is not limited to fuse, but needs to be addressed >>>>>> before unprivileged fuse mounts are upstreamed. >>>>>> >>>>>> Alban's response to this question: >>>>>> https://marc.info/?l=linux-kernel&m=151784020321045&w=2 >>>>> >>>>> Which goes to why it is a flag that get's set. >>>>> >>>>> All of this just needs a follow-up patch to update every filesystem >>>>> that does not meet ima's requirements. >>>> >>>> Currently files on remote filesystems are measured/appraised/audited >>>> once. With the new flags, our options would be to either fail the >>>> signature verification or constantly re-measure/re-appraise files on >>>> remote file systems. Neither option seems like the right solution. >> >> They are measured/appraised/audited once, and you can not trust that at >> all because you don't know when the files are going to be different. >> >> So either keeping the filesystem out of the ima game or failing sounds >> like the right answer to me. At least until you can get better >> information from the filesystem. >> >>> Being new to this game, I may be making a bad assumption, but I thought >>> that the (NFSv4) change attribute can be used to detect remote mutations >>> to a file's content or metadata, so that the appraisal could be cached >>> as long as that attribute does not change. At least for NFSv4, clients >>> assume their data cache is valid while the change attribute remains the >>> same. >>> >>> NFSv4 (and SMB) also has a mechanism where a server guarantees it will >>> report any other clients that want to update a file. This is an NFSv4 >>> read delegation or an SMB oplock. NFSv4 clients use this mechanism to >>> cache file data quite aggressively, and it could also be used to >>> preserve or cache audit state on remote files. >> >> The file data invalid message, plus trusting the server, is what >> would be needed for reliably caching the validity of the file. > > What establishes client trust in the server? I'm probably missing > something. In this case I mean trust as in the believe that the server is not actively trying to subvert the guarantees that IMA is depending upon. One such guarantee is that if data is dropped from the page cache and reread it will be the same data (unless the server let's you know). AKA IMA needs to trust that the cache coherency protocol is implemented honestly. Except for the case of something like fusermount that is the standard assumption today. That the filesystem is not actively trying to trip up the kernel. > The NFS protocol can convey the contents of the file, it's attributes, > and the contents of the security.ima and security.evm xattrs. The xattrs > contain cryptographically signed integrity metadata, which I presume > cannot be altered undetectably either at rest or in transit. The client > has everything it needs to measure that file, doesn't it, as long as it > has the correct set of keys to verify the signatures? > > Likely I am naive, but it seems to me a file server does not have to > participate in this process, other than to store and return IMA xattrs > along with the file content. All participating clients would need to > carry the same set of keys, however. > > Please tell me if I'm hijacking the thread. Unless something brings us to non-consensus about the patches to merge we are good. I think this is an area that need some discussion. The big big thing right now, as I understand it, is these mechanisms that nfs uses to keep the cache in sync are not clearly reflected in the vfs in a way that ima can take advantage of them. Please note I am stretching what we can do with the vfs in the kernel, but working on unprivileged fuse mounts. This has me asking all are our kernel mechanisms ok if the server is actively hostile. What happens if the server on the first read returns an innocuous file that matches it's ima xattr, but on the next read of the file returns an evil trojan horse. Or what if the server implements a cache coherency protocol but lies and does not report all of the changes to a file. Which is what got this conversation started in the first place discovering that unprivileged fuse+xattrs is not something people have looked at closely. A side effect of the conversation is realizing that remote filesystems have many of the same issues, but we can trust that the remote filesystems are not actively hostile. They just happen to not maintain all of the same invariants as local filesystems (like all modifications go through the local kernel). Which leads to the implication that we need some of these mechanisms on filesystems like nfs as well. >>>> There's some very initial discussions on how to support file integrity >>>> on remote filesystems. Chuck Lever has some thoughts on piggy-backing >>>> on the fs-verity work being done. From a very, very high level, IMA- >>>> appraisal would verify the file signature, but leave the integrity >>>> enforcement to the vfs/fs layer. By integrating fs-verity or similar >>>> proposal with IMA, measurements would be included in the measurement >>>> list and keys used for file signature verification would use the same >>>> existing IMA-appraisal infrastructure. >>> >>>>> Mimi I believe you said that the requirement is that all file changes >>>>> can be detected through the final __fput of a file that calls >>>>> ima_file_free. >>>> >>>> Right, like for fuse, I don't believe this existing hook works for >>>> remote filesystems. >> >> I am trying to understand things. >> >> - I believe the beginning of any file write should invalidate the >> validity of the files cache. IMA does something like that by looking >> at i_writecount. >> >> - As I read the code ima_file_free triggers an update of the ima xattr >> and that update needs to wait until the file is quiescent. AKA no >> more writers. I am not certain how you get that in a remote >> filesystem. > > With NFSv4, a read delegation is sufficient to guarantee that the > client is the only writer. The mechanism varies (or can be absent) > for other remote filesystem protocols. And, an NFSv4 server is not > obligated to always provide a delegation. > > An NFSv4 client can also OPEN a file with share deny modes. That > would prevent other clients from accessing the file while the IMA > metadata was recomputed. Again, I believe something similar would > work for SMB3, but might not be applicable to other remote file- > system protocols (eg NFSv3 does not have all this magic). > > However, computing a fresh IMA xattr would require access to the > whole file. For a large file, a client would have to read it from > the file server in its entirety, unless the file server offloads > this computation from the client somehow. The server would need to > wait until that client had CLOSEd the file to ensure that the > client had no more cached dirty data, and at that point the server > can itself guarantee there are no other remote accessors. Which sounds to me like all of this is implementable if desired, but that ima is not currently tied into these mechanisms. Which I expect is the next step. Eric
On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote: > In this case I mean trust as in the believe that the server is not > actively trying to subvert the guarantees that IMA is depending > upon. The fs-verity adversarial model we're targeting includes a malicious layer backing the local file system inode's page cache. For example, the malicious layer can be a drive controller with trojaned firmware or a compromised remote server. > What happens if the server on the first read returns an innocuous > file that matches it's ima xattr, but on the next read of the file > returns an evil trojan horse. Or what if the server implements a > cache coherency protocol but lies and does not report all of the > changes to a file. fs-verity validates each page in the inode's page cache against the inode's root-of-trust every time the page is read into the cache. The signature mechanism on the root-of-trust must bind the inode identity. The initial version of fs-verity supports Secure Boot, and the TCB validates each inode identity in the mount during the boot process. fs-verity does this via an ioctl that provisions the fs-verity measurement, causing the inode (along with its immutable measurement) to be pinned by the superblock for the life of the mount. This meets requirements for the Android platform's use case, which only needs to cover about 15 APKs during boot. This doesn't scale. Future work will include integration with PKCS7 and/or IMA. Signatures will be dynamically verifiable against a key or keys in the TCB, and I'd like to establish a safe way to avoid having to pin inodes and their measurements.
2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>: >> >> Please tell me if I'm hijacking the thread. > > Unless something brings us to non-consensus about the patches to merge > we are good. I think this is an area that need some discussion. > > The big big thing right now, as I understand it, is these mechanisms that > nfs uses to keep the cache in sync are not clearly reflected in the vfs > in a way that ima can take advantage of them. > > Chuck you mean fschange notifications methods like NT_TRANSACT_NOTIFY_CHANGE for cifs. I believe that NFS4 has something simular. These mechanism will inform the client when a file in a watched directory is changed. This is not yet supported in Linux (these methods are not triggered any way when setting a watch using inotify for exmple). There was support with dnotify (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393). But these methods are triggered by the user and not the VFS/kernel and therefore cannot garantee that all files on the client are the same as on the server. This also counts for a read delegation with nfs and methods like leases in a client server environment. Stef
Hi Stef- > On Mar 15, 2018, at 6:07 AM, Stef Bon <stefbon@gmail.com> wrote: > > 2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>: >>> >>> Please tell me if I'm hijacking the thread. >> >> Unless something brings us to non-consensus about the patches to merge >> we are good. I think this is an area that need some discussion. >> >> The big big thing right now, as I understand it, is these mechanisms that >> nfs uses to keep the cache in sync are not clearly reflected in the vfs >> in a way that ima can take advantage of them. >> >> > > Chuck you mean fschange notifications methods like > NT_TRANSACT_NOTIFY_CHANGE for cifs. No, I don't mean notification. That's a different mechanism entirely. NFSv4 delegations are similar to SMB oplocks. It's a protocol guarantee that the server will tell the client that holds a delegation when another client wants conflicting access to a file. The client then has an opportunity to update the file with anything it has cached and then the client releases the delegation. Servers have the option of granting a delegation for a file when it is OPENed. NFSv4 OPEN with share deny is similar to the way that SMB does OPENs. When a user OPENs a file this way, no other user or client is allowed to access it until the user CLOSEs the file. > I believe that NFS4 has something simular. > These mechanism will inform the client when a file in a watched > directory is changed. > This is not yet supported in Linux (these methods are not triggered > any way when setting a watch using inotify for exmple). > There was support with dnotify > (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393). > > But these methods are triggered by the user and not the VFS/kernel and > therefore cannot garantee that all files on the client > are the same as on the server. > This also counts for a read delegation with nfs and methods like > leases in a client server environment. > > Stef -- Chuck Lever
On Wed, 2018-03-14 at 15:53 -0700, Michael Halcrow wrote: > On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote: > > In this case I mean trust as in the believe that the server is not > > actively trying to subvert the guarantees that IMA is depending > > upon. > > The fs-verity adversarial model we're targeting includes a malicious > layer backing the local file system inode's page cache. > > For example, the malicious layer can be a drive controller with > trojaned firmware or a compromised remote server. > > > What happens if the server on the first read returns an innocuous > > file that matches it's ima xattr, but on the next read of the file > > returns an evil trojan horse. Or what if the server implements a > > cache coherency protocol but lies and does not report all of the > > changes to a file. > > fs-verity validates each page in the inode's page cache against the > inode's root-of-trust every time the page is read into the cache. > > The signature mechanism on the root-of-trust must bind the inode > identity. > > The initial version of fs-verity supports Secure Boot, and the TCB > validates each inode identity in the mount during the boot process. > fs-verity does this via an ioctl that provisions the fs-verity > measurement, causing the inode (along with its immutable measurement) > to be pinned by the superblock for the life of the mount. This meets > requirements for the Android platform's use case, which only needs to > cover about 15 APKs during boot. > > This doesn't scale. Future work will include integration with PKCS7 > and/or IMA. Signatures will be dynamically verifiable against a key > or keys in the TCB, and I'd like to establish a safe way to avoid > having to pin inodes and their measurements. Next steps should also include identifying methods for storing and transporting the Merkle tree signature without having to read the entire file for remote filesystems. With this fs support, signed files on file systems mounted with the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag would then be verifiable. Mimi
On Wed, 2018-03-14 at 14:46 -0500, Eric W. Biederman wrote: > Chuck Lever <chuck.lever@oracle.com> writes: > > >> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > >> > >> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote: > >>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes: > >>> > >>>> On Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote: > >>>>> I do not have any comments about the patches but a question. > >>>>> I completely agree that the files can change without the VFS knowing > >>>>> about it, but isn't that in general the case with filesystems with a > >>>>> backend shared with others (network fs's?). > >>>> > >>>> Right, the problem is not limited to fuse, but needs to be addressed > >>>> before unprivileged fuse mounts are upstreamed. > >>>> > >>>> Alban's response to this question: > >>>> https://marc.info/?l=linux-kernel&m=151784020321045&w=2 > >>> > >>> Which goes to why it is a flag that get's set. > >>> > >>> All of this just needs a follow-up patch to update every filesystem > >>> that does not meet ima's requirements. > >> > >> Currently files on remote filesystems are measured/appraised/audited > >> once. With the new flags, our options would be to either fail the > >> signature verification or constantly re-measure/re-appraise files on > >> remote file systems. Neither option seems like the right solution. > > They are measured/appraised/audited once, and you can not trust that at > all because you don't know when the files are going to be different. > > So either keeping the filesystem out of the ima game or failing sounds > like the right answer to me. At least until you can get better > information from the filesystem. Agreed > > > Being new to this game, I may be making a bad assumption, but I thought > > that the (NFSv4) change attribute can be used to detect remote mutations > > to a file's content or metadata, so that the appraisal could be cached > > as long as that attribute does not change. At least for NFSv4, clients > > assume their data cache is valid while the change attribute remains the > > same. > > > > NFSv4 (and SMB) also has a mechanism where a server guarantees it will > > report any other clients that want to update a file. This is an NFSv4 > > read delegation or an SMB oplock. NFSv4 clients use this mechanism to > > cache file data quite aggressively, and it could also be used to > > preserve or cache audit state on remote files. > > The file data invalid message, plus trusting the server, is what > would be needed for reliably caching the validity of the file. > > >> There's some very initial discussions on how to support file integrity > >> on remote filesystems. Chuck Lever has some thoughts on piggy-backing > >> on the fs-verity work being done. From a very, very high level, IMA- > >> appraisal would verify the file signature, but leave the integrity > >> enforcement to the vfs/fs layer. By integrating fs-verity or similar > >> proposal with IMA, measurements would be included in the measurement > >> list and keys used for file signature verification would use the same > >> existing IMA-appraisal infrastructure. > > > >>> Mimi I believe you said that the requirement is that all file changes > >>> can be detected through the final __fput of a file that calls > >>> ima_file_free. > >> > >> Right, like for fuse, I don't believe this existing hook works for > >> remote filesystems. > > I am trying to understand things. > > - I believe the beginning of any file write should invalidate the > validity of the files cache. IMA does something like that by looking > at i_writecount. Unless we're willing to recalculate the file hash and update the security xattr each time the file changes, invalidating the IMA cache on the first write would result in any subsequent file opens failing. > - As I read the code ima_file_free triggers an update of the ima xattr > and that update needs to wait until the file is quiescent. AKA no > more writers. I am not certain how you get that in a remote > filesystem. > > If the write is not coming from the local kernel I don't see how it > makes any sense for IMA to actually update the xattr on write. Agreed, it doesn't. Mimi
Hi Miklos, On Tue, 2018-03-13 at 14:32 -0500, Eric W. Biederman wrote: > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: > > > Files on FUSE can change at any point in time without IMA being able > > to detect it. The file data read for the file signature verification > > could be totally different from what is subsequently read, making the > > signature verification useless. > > > > FUSE can be mounted by unprivileged users either today with fusermount > > installed with setuid, or soon with the upcoming patches to allow FUSE > > mounts in a non-init user namespace. > > > > This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when > > appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. > > Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> There's been a number of changes since the original version of this patch set. I would appreciate your Ack for this version? Thanks, Mimi > > > > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > > Cc: Miklos Szeredi <miklos@szeredi.hu> > > Cc: Seth Forshee <seth.forshee@canonical.com> > > Cc: Eric W. Biederman <ebiederm@xmission.com> > > Cc: Dongsu Park <dongsu@kinvolk.io> > > Cc: Alban Crequy <alban@kinvolk.io> > > Cc: "Serge E. Hallyn" <serge@hallyn.com> > > --- > > fs/fuse/inode.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > > index 624f18bbfd2b..ef309958e060 100644 > > --- a/fs/fuse/inode.c > > +++ b/fs/fuse/inode.c > > @@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) > > sb->s_maxbytes = MAX_LFS_FILESIZE; > > sb->s_time_gran = 1; > > sb->s_export_op = &fuse_export_operations; > > + sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE; > > + if (sb->s_user_ns != &init_user_ns) > > + sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER; > > > > file = fget(d.fd); > > err = -EINVAL; >
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 624f18bbfd2b..ef309958e060 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) sb->s_maxbytes = MAX_LFS_FILESIZE; sb->s_time_gran = 1; sb->s_export_op = &fuse_export_operations; + sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE; + if (sb->s_user_ns != &init_user_ns) + sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER; file = fget(d.fd); err = -EINVAL;
Files on FUSE can change at any point in time without IMA being able to detect it. The file data read for the file signature verification could be totally different from what is subsequently read, making the signature verification useless. FUSE can be mounted by unprivileged users either today with fusermount installed with setuid, or soon with the upcoming patches to allow FUSE mounts in a non-init user namespace. This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when appropriate sets the SB_I_UNTRUSTED_MOUNTER flag. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Miklos Szeredi <miklos@szeredi.hu> Cc: Seth Forshee <seth.forshee@canonical.com> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: Dongsu Park <dongsu@kinvolk.io> Cc: Alban Crequy <alban@kinvolk.io> Cc: "Serge E. Hallyn" <serge@hallyn.com> --- fs/fuse/inode.c | 3 +++ 1 file changed, 3 insertions(+)