[RFC PATCH v4 0/2] ima,fuse: introduce new fs flag FS_IMA_NO_CACHE

[RFC PATCH v4 0/2] ima,fuse: introduce new fs flag FS_IMA_NO_CACHE

[Dongsu Park]

This patchset v4 introduces a new fs flag FS_IMA_NO_CACHE and uses it in
FUSE. This forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

There was a previous attempt (unmerged) with a IMA option named "force" and using
that option for FUSE filesystems. These patches use a different approach
so that the IMA subsystem does not need to know about FUSE.
- https://www.spinics.net/lists/linux-integrity/msg00948.html
- https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1584131.html

Changes since v1: https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1587390.html
- include linux-fsdevel mailing list in cc
- mark patch as RFC
- based on next-integrity, without other unmerged FUSE / IMA patches

Changes since v2: https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1587678.html
- rename flag to FS_IMA_NO_CACHE
- split patch into 2

Changes since v3: https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1592393.html
- make the code simpler by resetting IMA_DONE_MASK

The patchset is also available in our github repo:
  https://github.com/kinvolk/linux/tree/dongsu/fuse-flag-ima-nocache-v4


Alban Crequy (2):
  fuse: introduce new fs_type flag FS_IMA_NO_CACHE
  ima: force re-appraisal on filesystems with FS_IMA_NO_CACHE

 fs/fuse/inode.c                   |  2 +-
 include/linux/fs.h                |  1 +
 security/integrity/ima/ima_main.c | 15 +++++++++++++--
 3 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Dongsu Park]

From: Alban Crequy <alban@kinvolk.io>

This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
re-appraised and re-audited each time. Cached integrity results should
not be used.

It is useful in FUSE because the userspace FUSE process can change the
underlying files at any time without notifying the kernel.

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <jmorris@namei.org>
Cc: Christoph Hellwig <hch@infradead.org>
Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
Acked-by: Seth Forshee <seth.forshee@canonical.com>
Tested-by: Dongsu Park <dongsu@kinvolk.io>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
---
 fs/fuse/inode.c    | 2 +-
 include/linux/fs.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 624f18bb..0a9e5164 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
 static struct file_system_type fuse_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "fuse",
-	.fs_flags	= FS_HAS_SUBTYPE,
+	.fs_flags	= FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
 	.mount		= fuse_mount,
 	.kill_sb	= fuse_kill_sb_anon,
 };
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 511fbaab..ced841ba 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2075,6 +2075,7 @@ struct file_system_type {
 #define FS_BINARY_MOUNTDATA	2
 #define FS_HAS_SUBTYPE		4
 #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
+#define FS_IMA_NO_CACHE		16	/* Force IMA to re-measure, re-appraise, re-audit files */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Mimi Zohar]

Hi Miklos,

On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
> From: Alban Crequy <alban@kinvolk.io>
> 
> This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
> re-appraised and re-audited each time. Cached integrity results should
> not be used.
> 
> It is useful in FUSE because the userspace FUSE process can change the
> underlying files at any time without notifying the kernel.

Both IMA-measurement and IMA-appraisal cache the integrity results and
are dependent on the kernel to detect when a file changes in order to
clear the cached info and force the file to be re-evaluated.??This
detection was dependent on i_version changing.??For filesystems that
do not support i_version, remote or fuse filesystems, where the kernel
does not detect the file change, the file was measured and the
signature evaluated just once.

With commit a2a2c3c8580a ("ima: Use i_version only when filesystem
supports it"), which is being upstreamed in this open window,
i_version is considered an optimization.??If i_version is not enabled,
either because the local filesystem does not support it or the
filesystem wasn't mounted with i_version, the file will now always be
re-evaluated.

That patch does not address FUSE or remote filesystems, as the kernel
does not detect the change.??Further, even if the kernel could detect
the change, FUSE filesystems by definition are untrusted.

The original patches addressed FUSE filesystems, by defining a new IMA
policy option, forcing the file to be re-evaluated based on the
filesystem magic number. ?All of the changes were in the IMA
subsystem. ?These patches are the result of Christoph's comment on the
original patches saying, "ima has no business looking at either the
name _or_ the magic number."

Your help in resolving this problem is much appreciated!

Mimi

> 
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-integrity at vger.kernel.org
> Cc: linux-security-module at vger.kernel.org
> Cc: linux-fsdevel at vger.kernel.org
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
> Acked-by: Seth Forshee <seth.forshee@canonical.com>
> Tested-by: Dongsu Park <dongsu@kinvolk.io>
> Signed-off-by: Alban Crequy <alban@kinvolk.io>
> ---
>  fs/fuse/inode.c    | 2 +-
>  include/linux/fs.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 624f18bb..0a9e5164 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
>  static struct file_system_type fuse_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "fuse",
> -	.fs_flags	= FS_HAS_SUBTYPE,
> +	.fs_flags	= FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
>  	.mount		= fuse_mount,
>  	.kill_sb	= fuse_kill_sb_anon,
>  };
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 511fbaab..ced841ba 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2075,6 +2075,7 @@ struct file_system_type {
>  #define FS_BINARY_MOUNTDATA	2
>  #define FS_HAS_SUBTYPE		4
>  #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
> +#define FS_IMA_NO_CACHE		16	/* Force IMA to re-measure, re-appraise, re-audit files */
>  #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
>  	struct dentry *(*mount) (struct file_system_type *, int,
>  		       const char *, void *);

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Mimi Zohar]

On Fri, 2018-02-02 at 10:20 -0500, Mimi Zohar wrote:
> Hi Miklos,
> 
> On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
> > From: Alban Crequy <alban@kinvolk.io>
> > 
> > This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
> > re-appraised and re-audited each time. Cached integrity results should
> > not be used.
> > 
> > It is useful in FUSE because the userspace FUSE process can change the
> > underlying files at any time without notifying the kernel.
> 
> Both IMA-measurement and IMA-appraisal cache the integrity results and
> are dependent on the kernel to detect when a file changes in order to
> clear the cached info and force the file to be re-evaluated.??This
> detection was dependent on i_version changing.??For filesystems that
> do not support i_version, remote or fuse filesystems, where the kernel
> does not detect the file change, the file was measured and the
> signature evaluated just once.
> 
> With commit a2a2c3c8580a ("ima: Use i_version only when filesystem
> supports it"), which is being upstreamed in this open window,
> i_version is considered an optimization.??If i_version is not enabled,
> either because the local filesystem does not support it or the
> filesystem wasn't mounted with i_version, the file will now always be
> re-evaluated.
> 
> That patch does not address FUSE or remote filesystems, as the kernel
> does not detect the change.??Further, even if the kernel could detect
> the change, FUSE filesystems by definition are untrusted.
> 
> The original patches addressed FUSE filesystems, by defining a new IMA
> policy option, forcing the file to be re-evaluated based on the
> filesystem magic number. ?All of the changes were in the IMA
> subsystem. ?These patches are the result of Christoph's comment on the
> original patches saying, "ima has no business looking at either the
> name _or_ the magic number."
> 
> Your help in resolving this problem is much appreciated!

Meaning, can you ack the fuse flag addition so we can take the series
through the IMA tree?

thanks,

Mimi

> 
> > 
> > Cc: linux-kernel at vger.kernel.org
> > Cc: linux-integrity at vger.kernel.org
> > Cc: linux-security-module at vger.kernel.org
> > Cc: linux-fsdevel at vger.kernel.org
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
> > Acked-by: Seth Forshee <seth.forshee@canonical.com>
> > Tested-by: Dongsu Park <dongsu@kinvolk.io>
> > Signed-off-by: Alban Crequy <alban@kinvolk.io>
> > ---
> >  fs/fuse/inode.c    | 2 +-
> >  include/linux/fs.h | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 624f18bb..0a9e5164 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
> >  static struct file_system_type fuse_fs_type = {
> >  	.owner		= THIS_MODULE,
> >  	.name		= "fuse",
> > -	.fs_flags	= FS_HAS_SUBTYPE,
> > +	.fs_flags	= FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
> >  	.mount		= fuse_mount,
> >  	.kill_sb	= fuse_kill_sb_anon,
> >  };
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 511fbaab..ced841ba 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2075,6 +2075,7 @@ struct file_system_type {
> >  #define FS_BINARY_MOUNTDATA	2
> >  #define FS_HAS_SUBTYPE		4
> >  #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
> > +#define FS_IMA_NO_CACHE		16	/* Force IMA to re-measure, re-appraise, re-audit files */
> >  #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
> >  	struct dentry *(*mount) (struct file_system_type *, int,
> >  		       const char *, void *);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Miklos Szeredi]

On Fri, Feb 2, 2018 at 4:33 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2018-02-02 at 10:20 -0500, Mimi Zohar wrote:
>> Hi Miklos,
>>
>> On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
>> > From: Alban Crequy <alban@kinvolk.io>
>> >
>> > This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
>> > re-appraised and re-audited each time. Cached integrity results should
>> > not be used.
>> >
>> > It is useful in FUSE because the userspace FUSE process can change the
>> > underlying files at any time without notifying the kernel.

I don't really have an understanding what IMA is doing, I think the
same thing applies to any network filesystem (i.e. ones with
d_revalidate).

Isn't that the case?

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Mimi Zohar]

On Fri, 2018-02-02 at 17:10 +0100, Miklos Szeredi wrote:
> On Fri, Feb 2, 2018 at 4:33 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Fri, 2018-02-02 at 10:20 -0500, Mimi Zohar wrote:
> >> Hi Miklos,
> >>
> >> On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
> >> > From: Alban Crequy <alban@kinvolk.io>
> >> >
> >> > This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
> >> > re-appraised and re-audited each time. Cached integrity results should
> >> > not be used.
> >> >
> >> > It is useful in FUSE because the userspace FUSE process can change the
> >> > underlying files at any time without notifying the kernel.
> 
> I don't really have an understanding what IMA is doing, I think the
> same thing applies to any network filesystem (i.e. ones with
> d_revalidate).
> 
> Isn't that the case?

IMA is calculating the file hash, for inclusion in the measurement
list, verifying the file signature stored in the xattr, or both. ?For
the remote filesystem case, re-calculating the file hash would be
limited to inclusion in the measurement list. ?For FUSE, the kernel
has access to the xattr, so re-calculating the file hash could also be
used to re-verify the file signature.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Alban Crequy]

On Fri, Feb 2, 2018 at 5:10 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Feb 2, 2018 at 4:33 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> On Fri, 2018-02-02 at 10:20 -0500, Mimi Zohar wrote:
>>> Hi Miklos,
>>>
>>> On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
>>> > From: Alban Crequy <alban@kinvolk.io>
>>> >
>>> > This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
>>> > re-appraised and re-audited each time. Cached integrity results should
>>> > not be used.
>>> >
>>> > It is useful in FUSE because the userspace FUSE process can change the
>>> > underlying files at any time without notifying the kernel.
>
> I don't really have an understanding what IMA is doing, I think the
> same thing applies to any network filesystem (i.e. ones with
> d_revalidate).
>
> Isn't that the case?

Hi Miklos,

>From my limited understanding, network filesystems might need that
too, yes. I don't know if there are people interested in using both
IMA and network filesystems. If so, they would have to write that
patch and test it.

It is not a new issue, for neither network filesystems or FUSE. But I
am more interested in the FUSE use case because FUSE can be mounted by
unprivileged users either today with fusermount installed with setuid,
or soon with the coming patches to allow FUSE mounts in a non-init
user namespace. That makes the issue more visible than for network
filesystems where unprivileged users cannot mount.

Cheers,
Alban
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Miklos Szeredi]

On Tue, Jan 30, 2018 at 7:06 PM, Dongsu Park <dongsu@kinvolk.io> wrote:
> From: Alban Crequy <alban@kinvolk.io>
>
> This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
> re-appraised and re-audited each time. Cached integrity results should
> not be used.
>
> It is useful in FUSE because the userspace FUSE process can change the
> underlying files at any time without notifying the kernel.
>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-integrity at vger.kernel.org
> Cc: linux-security-module at vger.kernel.org
> Cc: linux-fsdevel at vger.kernel.org
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
> Acked-by: Seth Forshee <seth.forshee@canonical.com>
> Tested-by: Dongsu Park <dongsu@kinvolk.io>
> Signed-off-by: Alban Crequy <alban@kinvolk.io>
> ---
>  fs/fuse/inode.c    | 2 +-
>  include/linux/fs.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 624f18bb..0a9e5164 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
>  static struct file_system_type fuse_fs_type = {
>         .owner          = THIS_MODULE,
>         .name           = "fuse",
> -       .fs_flags       = FS_HAS_SUBTYPE,
> +       .fs_flags       = FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
>         .mount          = fuse_mount,
>         .kill_sb        = fuse_kill_sb_anon,
>  };
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 511fbaab..ced841ba 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2075,6 +2075,7 @@ struct file_system_type {
>  #define FS_BINARY_MOUNTDATA    2
>  #define FS_HAS_SUBTYPE         4
>  #define FS_USERNS_MOUNT                8       /* Can be mounted by userns root */
> +#define FS_IMA_NO_CACHE                16      /* Force IMA to re-measure, re-appraise, re-audit files */

I think it would be more logical to change the order of the patches
(i.e. first patch adds this constant and the code handling it, and
second patch just adds it to fuse's .fs_flags).

Otherwise

Acked-by:   Miklos Szeredi <mszeredi@redhat.com>

Thanks,
Miklos


>  #define FS_RENAME_DOES_D_MOVE  32768   /* FS will handle d_move() during rename() internally. */
>         struct dentry *(*mount) (struct file_system_type *, int,
>                        const char *, void *);
> --
> 2.13.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 1/2] fuse: introduce new fs_type flag FS_IMA_NO_CACHE

[Mimi Zohar]

On Wed, 2018-02-07 at 10:21 +0100, Miklos Szeredi wrote:
> On Tue, Jan 30, 2018 at 7:06 PM, Dongsu Park <dongsu@kinvolk.io> wrote:
> > From: Alban Crequy <alban@kinvolk.io>
> >
> > This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
> > re-appraised and re-audited each time. Cached integrity results should
> > not be used.
> >
> > It is useful in FUSE because the userspace FUSE process can change the
> > underlying files at any time without notifying the kernel.
> >
> > Cc: linux-kernel at vger.kernel.org
> > Cc: linux-integrity at vger.kernel.org
> > Cc: linux-security-module at vger.kernel.org
> > Cc: linux-fsdevel at vger.kernel.org
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
> > Acked-by: Seth Forshee <seth.forshee@canonical.com>
> > Tested-by: Dongsu Park <dongsu@kinvolk.io>
> > Signed-off-by: Alban Crequy <alban@kinvolk.io>
> > ---
> >  fs/fuse/inode.c    | 2 +-
> >  include/linux/fs.h | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 624f18bb..0a9e5164 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
> >  static struct file_system_type fuse_fs_type = {
> >         .owner          = THIS_MODULE,
> >         .name           = "fuse",
> > -       .fs_flags       = FS_HAS_SUBTYPE,
> > +       .fs_flags       = FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
> >         .mount          = fuse_mount,
> >         .kill_sb        = fuse_kill_sb_anon,
> >  };
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 511fbaab..ced841ba 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2075,6 +2075,7 @@ struct file_system_type {
> >  #define FS_BINARY_MOUNTDATA    2
> >  #define FS_HAS_SUBTYPE         4
> >  #define FS_USERNS_MOUNT                8       /* Can be mounted by userns root */
> > +#define FS_IMA_NO_CACHE                16      /* Force IMA to re-measure, re-appraise, re-audit files */
> 
> I think it would be more logical to change the order of the patches
> (i.e. first patch adds this constant and the code handling it, and
> second patch just adds it to fuse's .fs_flags).
> 
> Otherwise
> 
> Acked-by:   Miklos Szeredi <mszeredi@redhat.com>

Sure, thank you!

Mimi 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 2/2] ima: force re-appraisal on filesystems with FS_IMA_NO_CACHE

[Dongsu Park]

From: Alban Crequy <alban@kinvolk.io>

This patch forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

How to test this:

The test I did was using a patched version of the memfs FUSE driver
[1][2] and two very simple "hello-world" programs [4] (prog1 prints
"hello world: 1" and prog2 prints "hello world: 2").

I copy prog1 and prog2 in the fuse-memfs mount point, execute them and
check the sha1 hash in
"/sys/kernel/security/ima/ascii_runtime_measurements".

My patch on the memfs FUSE driver added a backdoor command to serve
prog1 when the kernel asks for prog2 or vice-versa. In this way, I can
exec prog1 and get it to print "hello world: 2" without ever replacing
the file via the VFS, so the kernel is not aware of the change.

The test was done using the branch "dongsu/fuse-flag-ima-nocache-v4" [3].

Step by step test procedure:

1. Mount the memfs FUSE using [2]:
rm -f  /tmp/memfs-switch* ; memfs -L DEBUG  /mnt/memfs

2. Copy prog1 and prog2 using [4]
cp prog1 /mnt/memfs/prog1
cp prog2 /mnt/memfs/prog2

3. Lookup the files and let the FUSE driver to keep the handles open:
dd if=/mnt/memfs/prog1 bs=1 | (read -n 1 x ; sleep 3600 ) &
dd if=/mnt/memfs/prog2 bs=1 | (read -n 1 x ; sleep 3600 ) &

4. Check the 2 programs work correctly:
$ /mnt/memfs/prog1
hello world: 1
$ /mnt/memfs/prog2
hello world: 2

5. Check the measurements for prog1 and prog2:
$ sudo cat /sys/kernel/security/ima/ascii_runtime_measurements \
                | grep /mnt/memfs/prog
10 [...] ima-ng sha1:ac14c9268cd2[...] /mnt/memfs/prog1
10 [...] ima-ng sha1:799cb5d1e06d[...] /mnt/memfs/prog2

6. Use the backdoor command in my patched memfs to redirect file
operations on file handle 3 to file handle 2:
rm -f  /tmp/memfs-switch* ; touch /tmp/memfs-switch-3-2

7. Check how the FUSE driver serves different content for the files:
$ /mnt/memfs/prog1
hello world: 2
$ /mnt/memfs/prog2
hello world: 2

8. Check the measurements:
sudo cat /sys/kernel/security/ima/ascii_runtime_measurements \
                | grep /mnt/memfs/prog

Without the patch, there are no new measurements, despite the FUSE
driver having served different executables.

With the patch, I can see additional measurements for prog1 and prog2
with the hashes reversed when the FUSE driver served the alternative
content.

[1] https://github.com/bbengfort/memfs
[2] https://github.com/kinvolk/memfs/commits/alban/switch-files
[3] https://github.com/kinvolk/linux/commits/dongsu/fuse-flag-ima-nocache-v4
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <jmorris@namei.org>
Cc: Christoph Hellwig <hch@infradead.org>
Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
Acked-by: Seth Forshee <seth.forshee@canonical.com>
Tested-by: Dongsu Park <dongsu@kinvolk.io>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
[dongsu: optimized code to address review comments by Mimi]
Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
---
 security/integrity/ima/ima_main.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 6d78cb26..83edbad8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/ima.h>
+#include <linux/fs.h>
 
 #include "ima.h"
 
@@ -228,9 +229,19 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
 				 IMA_ACTION_FLAGS);
 
-	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags))
-		/* reset all flags if ima_inode_setxattr was called */
+	/*
+	 * Reset the measure, appraise and audit cached flags either if:
+	 * - ima_inode_setxattr was called, or
+	 * - based on filesystem feature flag
+	 * forcing the file to be re-evaluated.
+	 */
+	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags)) {
 		iint->flags &= ~IMA_DONE_MASK;
+	} else if (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
+		iint->flags &= ~IMA_DONE_MASK;
+		if (action & IMA_MEASURE)
+			iint->measured_pcrs = 0;
+	}
 
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v4 0/2] ima,fuse: introduce new fs flag FS_IMA_NO_CACHE

[Mimi Zohar]

On Tue, 2018-01-30 at 19:06 +0100, Dongsu Park wrote:
> This patchset v4 introduces a new fs flag FS_IMA_NO_CACHE and uses it in
> FUSE. This forces files to be re-measured, re-appraised and re-audited
> on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
> cached integrity results won't be used.

Thanks!

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html