[GIT PULL] Integrity: IMA FUSE fixes

[GIT PULL] Integrity: IMA FUSE fixes

[James Morris]

These patches ensure that IMA works correctly on FUSE filesystems, so that 
cached integrity data is not used.  FUSE filesystems can change this data 
at any time without notifying the kernel and we now verify it for each 
use.

This work is late in the kernel cycle, but they have had good review, 
testing, and acks.  They only impact FUSE and IMA.

Please consider pulling for v4.16.

---

The following changes since commit 9a61df9e5f7471fe5be3e02bd0bed726b2761a54:

  Merge tag 'kbuild-v4.16-2' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild (2018-02-09 19:32:41 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-v4.16

for you to fetch changes up to 8f7765f67642424650e22c1077b149c2820e7d18:

  fuse: introduce new fs_type flag FS_IMA_NO_CACHE (2018-02-10 15:10:09 +1100)

----------------------------------------------------------------
Alban Crequy (2):
      ima: force re-appraisal on filesystems with FS_IMA_NO_CACHE
      fuse: introduce new fs_type flag 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(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 624f18b..0a9e516 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 2a81556..8d3f97d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2069,6 +2069,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 *);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2cfb0c7..55d9149 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -25,6 +25,7 @@
 #include <linux/xattr.h>
 #include <linux/ima.h>
 #include <linux/iversion.h>
+#include <linux/fs.h>
 
 #include "ima.h"
 
@@ -229,9 +230,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,
--
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

[GIT PULL] Integrity: IMA FUSE fixes

[Linus Torvalds]

On Fri, Feb 9, 2018 at 10:26 PM, James Morris <jmorris@namei.org> wrote:
> These patches ensure that IMA works correctly on FUSE filesystems, so that
> cached integrity data is not used.  FUSE filesystems can change this data
> at any time without notifying the kernel and we now verify it for each
> use.
>
> This work is late in the kernel cycle, but they have had good review,
> testing, and acks.  They only impact FUSE and IMA.

This seems entirely insane.

You simply cannot use IMA on a fuse filesystem, because the data can
change dynamically any time.

But that doesn't mean that you can't cache the measurements - it means
that the measurements are pointless. Those are two completely
different things.

This patch seems to disable caching, but still _use_ the measurement.

Which seems *worse* than what we do now, in that it wastes time and
effort on re-creating those pointless measurements because it disables
the caching of them.

So honestly, the only sane thing seems to be to disable IMA on fuse,
not to force it to do even _more_ pointless work.

What am I missing?

                   Linus
--
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

[GIT PULL] Integrity: IMA FUSE fixes

[Mimi Zohar]

On Sat, 2018-02-10 at 12:44 -0800, Linus Torvalds wrote:
> On Fri, Feb 9, 2018 at 10:26 PM, James Morris <jmorris@namei.org> wrote:
> > These patches ensure that IMA works correctly on FUSE filesystems, so that
> > cached integrity data is not used.  FUSE filesystems can change this data
> > at any time without notifying the kernel and we now verify it for each
> > use.
> >
> > This work is late in the kernel cycle, but they have had good review,
> > testing, and acks.  They only impact FUSE and IMA.
> 
> This seems entirely insane.
> 
> You simply cannot use IMA on a fuse filesystem, because the data can
> change dynamically any time.
> 
> But that doesn't mean that you can't cache the measurements - it means
> that the measurements are pointless. Those are two completely
> different things.
> 
> This patch seems to disable caching, but still _use_ the measurement.
> 
> Which seems *worse* than what we do now, in that it wastes time and
> effort on re-creating those pointless measurements because it disables
> the caching of them.
> 
> So honestly, the only sane thing seems to be to disable IMA on fuse,
> not to force it to do even _more_ pointless work.
> 
> What am I missing?

No, you're right. ?The file could change at any time, making the
measurement(s) and by extension signature verification meaningless.?
Custom policy rules could be defined to disable measurement,
appraisal, and audit for files on fuse. ?However, I don't think we
want to automatically disable measurement, even meaningless
measurements. ?Some indication needs to be included for remote
attestation, security analytics, or forensics. ?For systems with
policies that require file signatures even on fuse, the safest thing
would seem to be to fail the signature verification.

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

[GIT PULL] Integrity: IMA FUSE fixes

[Linus Torvalds]

On Sat, Feb 10, 2018 at 8:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>
>> What am I missing?
>
> No, you're right.  The file could change at any time, making the
> measurement(s) and by extension signature verification meaningless.
> Custom policy rules could be defined to disable measurement,
> appraisal, and audit for files on fuse.  However, I don't think we
> want to automatically disable measurement, even meaningless
> measurements.  Some indication needs to be included for remote
> attestation, security analytics, or forensics.  For systems with
> policies that require file signatures even on fuse, the safest thing
> would seem to be to fail the signature verification.

Failing seems like a sane model, although I also suspect it would just
break a lot of cases that currently work fine because *in*practice*
fuse works fine as a normal filesystem (think fuse "exfat" module
etc).

So yes, the failing behavior is sane, but I agree with you that it
should be something that requires a specific policy ("fail on
untrusted filesystems like fuse").

But regardless, disabling caching just seems broken in all situations
and never right, so I really don't want to pull that tree unless
somebody can point out where it makes sense.

             Linus
--
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

[GIT PULL] Integrity: IMA FUSE fixes

[Mimi Zohar]

On Sat, 2018-02-10 at 20:50 -0800, Linus Torvalds wrote:
> On Sat, Feb 10, 2018 at 8:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >>
> >> What am I missing?
> >
> > No, you're right.  The file could change at any time, making the
> > measurement(s) and by extension signature verification meaningless.
> > Custom policy rules could be defined to disable measurement,
> > appraisal, and audit for files on fuse.  However, I don't think we
> > want to automatically disable measurement, even meaningless
> > measurements.  Some indication needs to be included for remote
> > attestation, security analytics, or forensics.  For systems with
> > policies that require file signatures even on fuse, the safest thing
> > would seem to be to fail the signature verification.
> 
> Failing seems like a sane model, although I also suspect it would just
> break a lot of cases that currently work fine because *in*practice*
> fuse works fine as a normal filesystem (think fuse "exfat" module
> etc).
> 
> So yes, the failing behavior is sane, but I agree with you that it
> should be something that requires a specific policy ("fail on
> untrusted filesystems like fuse").

Could we differentiate between untrusted from unprivileged and
untrusted fuse? ?The existing fuse would continue to work, but on
systems with IMA-appraisal enabled the new, unprivileged fuse would
fail.

> But regardless, disabling caching just seems broken in all situations
> and never right, so I really don't want to pull that tree unless
> somebody can point out where it makes sense.

Agreed. ?Re-measuring/appraising the file would only detect well
behaved malicious fuse.

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