linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Allow FUSE filesystems to provide out-of-band hashes to IMA
@ 2018-10-04 20:30 Matthew Garrett
  2018-10-04 20:30 ` [PATCH 1/3] VFS: Add a call to obtain a file's hash Matthew Garrett
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Matthew Garrett @ 2018-10-04 20:30 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, miklos, linux-fsdevel, viro

As of d77ccdc644a59b412d8e101576134c90a0aa6797, IMA will trigger a
rehash of any file on a FUSE filesystem for every measurement. This has
a significant impact on performance. Individual FUSE filesystems may
have the ability to identify whether a file needs to be rehashed, and
some may even have the ability to obtain the file hash out of band
without needing the kernel to do it. Longer term, this may also be
usable for other scenarios where a filesystem can provide a trustworthy
hash (eg, fs-verity on ext4 could provide a hash and then later abort
any read()s that discover that the file doesn't match the measurement).

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-04 20:30 Allow FUSE filesystems to provide out-of-band hashes to IMA Matthew Garrett
@ 2018-10-04 20:30 ` Matthew Garrett
  2018-10-11 15:22   ` Mimi Zohar
  2018-10-04 20:30 ` [PATCH 2/3] IMA: Make use of filesystem-provided hashes Matthew Garrett
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-04 20:30 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, dmitry.kasatkin, miklos, linux-fsdevel, viro, Matthew Garrett

IMA wants to know what the hash of a file is, and currently does so by
reading the entire file and generating the hash. Some filesystems may
have the ability to store the hash in a secure manner resistant to
offline attacks (eg, filesystem-level file signing), and in that case
it's a performance win for IMA to be able to use that rather than having
to re-hash everything. This patch simply adds VFS-level support for
calling down to filesystems.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 fs/read_write.c    | 24 ++++++++++++++++++++++++
 include/linux/fs.h |  6 +++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 39b4a21dd933..9ba3ce4bb838 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -2081,3 +2081,27 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
 	return ret;
 }
 EXPORT_SYMBOL(vfs_dedupe_file_range);
+
+/**
+ * vfs_gethash - obtain a file's hash
+ * @file:	file structure in question
+ * @hash_algo:	the hash algorithm requested
+ * @buf:	buffer to return the hash in
+ * @size:	size allocated for the buffer by the caller
+ *
+ * This function allows filesystems that support securely storing the hash
+ * of a file to return it rather than forcing the kernel to recalculate it.
+ * Filesystems that cannot provide guarantees about the hash being resistant
+ * to offline attack should not implement this functionality.
+ *
+ * Returns 0 on success, -EOPNOTSUPP if the filesystem doesn't support it.
+ */
+int vfs_get_hash(struct file *file, enum hash_algo hash, uint8_t *buf,
+		 size_t size)
+{
+	if (!file->f_op->get_hash)
+		return -EOPNOTSUPP;
+
+	return file->f_op->get_hash(file, hash, buf, size);
+}
+EXPORT_SYMBOL(vfs_get_hash);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6c0b4a1c22ff..540316cfd461 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -40,6 +40,7 @@
 
 #include <asm/byteorder.h>
 #include <uapi/linux/fs.h>
+#include <uapi/linux/hash_info.h>
 
 struct backing_dev_info;
 struct bdi_writeback;
@@ -1764,6 +1765,8 @@ struct file_operations {
 	int (*dedupe_file_range)(struct file *, loff_t, struct file *, loff_t,
 			u64);
 	int (*fadvise)(struct file *, loff_t, loff_t, int);
+	int (*get_hash)(struct file *, enum hash_algo hash, uint8_t *buf,
+			size_t size);
 } __randomize_layout;
 
 struct inode_operations {
@@ -1838,7 +1841,8 @@ extern int vfs_dedupe_file_range(struct file *file,
 extern int vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
 				     struct file *dst_file, loff_t dst_pos,
 				     u64 len);
-
+extern int vfs_get_hash(struct file *file, enum hash_algo hash, uint8_t *buf,
+			size_t size);
 
 struct super_operations {
    	struct inode *(*alloc_inode)(struct super_block *sb);
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-04 20:30 Allow FUSE filesystems to provide out-of-band hashes to IMA Matthew Garrett
  2018-10-04 20:30 ` [PATCH 1/3] VFS: Add a call to obtain a file's hash Matthew Garrett
@ 2018-10-04 20:30 ` Matthew Garrett
  2018-10-11 15:23   ` Mimi Zohar
  2018-10-04 20:30 ` [PATCH 3/3] FUSE: Allow filesystems to provide gethash methods Matthew Garrett
  2018-10-05 10:49 ` Allow FUSE filesystems to provide out-of-band hashes to IMA Mimi Zohar
  3 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-04 20:30 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, dmitry.kasatkin, miklos, linux-fsdevel, viro, Matthew Garrett

Some filesystems may be able to provide hashes in an out of band manner,
and allowing them to do so is a performance win. This is especially true
of FUSE-based filesystems where otherwise we recalculate the hash on
every measurement. Make use of this by default, but provide a parameter
to force recalculation rather than trusting the filesystem.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 security/integrity/ima/ima_crypto.c             | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 92eb1f42240d..617ae0f83b14 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1612,6 +1612,11 @@
 			different crypto accelerators. This option can be used
 			to achieve best performance for particular HW.
 
+	ima.force_hash= [IMA] Force hash calculation in IMA
+			Format: <bool>
+			Always calculate hashes rather than trusting the
+			filesystem to provide them to us.
+
 	init=		[KNL]
 			Format: <full_path>
 			Run specified binary instead of /sbin/init as init
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 7e7e7e7c250a..f25259b2b6ec 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -32,6 +32,10 @@ static unsigned long ima_ahash_minsize;
 module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
 MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use");
 
+static bool ima_force_hash;
+module_param_named(force_hash, ima_force_hash, bool_enable_only, 0644);
+MODULE_PARM_DESC(force_hash, "Always calculate hashes");
+
 /* default is 0 - 1 page. */
 static int ima_maxorder;
 static unsigned int ima_bufsize = PAGE_SIZE;
@@ -431,6 +435,13 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 		return -EINVAL;
 	}
 
+	if (!ima_force_hash) {
+		hash->length = hash_digest_size[hash->algo];
+		rc = vfs_get_hash(file, hash->algo, hash->digest, hash->length);
+		if (!rc)
+			return 0;
+	}
+
 	i_size = i_size_read(file_inode(file));
 
 	if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] FUSE: Allow filesystems to provide gethash methods
  2018-10-04 20:30 Allow FUSE filesystems to provide out-of-band hashes to IMA Matthew Garrett
  2018-10-04 20:30 ` [PATCH 1/3] VFS: Add a call to obtain a file's hash Matthew Garrett
  2018-10-04 20:30 ` [PATCH 2/3] IMA: Make use of filesystem-provided hashes Matthew Garrett
@ 2018-10-04 20:30 ` Matthew Garrett
  2018-10-05 10:49 ` Allow FUSE filesystems to provide out-of-band hashes to IMA Mimi Zohar
  3 siblings, 0 replies; 30+ messages in thread
From: Matthew Garrett @ 2018-10-04 20:30 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, dmitry.kasatkin, miklos, linux-fsdevel, viro, Matthew Garrett

FUSE implementations may have a secure way to provide file hashes (eg,
they're a front-end to a remote store that ties files to their hashes).
Allow filesystems to expose this information, but require an option to
be provided before it can be used. This is to avoid malicious users
being able to mount an unprivileged FUSE filesystem that provides
incorrect hashes.

A sufficiently malicious FUSE filesystem may still simply swap out its
contents after the hash has been obtained - this patchset does nothing
to change that, and sysadmins should have appropriate policy in place to
protect against that.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 fs/fuse/file.c            | 35 +++++++++++++++++++++++++++++++++++
 fs/fuse/fuse_i.h          |  7 +++++++
 fs/fuse/inode.c           | 10 ++++++++++
 include/uapi/linux/fuse.h |  6 ++++++
 4 files changed, 58 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 32d0b883e74f..0696671ea070 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3011,6 +3011,39 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 	return err;
 }
 
+
+static int fuse_file_get_hash(struct file *file, enum hash_algo hash,
+			      uint8_t *buf, size_t size)
+{
+	struct fuse_file *ff = file->private_data;
+	struct fuse_conn *fc = ff->fc;
+	FUSE_ARGS(args);
+	struct fuse_gethash_in inarg;
+	int err = 0;
+
+	if (!fc->allow_gethash)
+		return -EOPNOTSUPP;
+
+	memset(&inarg, 0, sizeof(inarg));
+	inarg.size = size;
+	inarg.hash = hash;
+	args.in.h.opcode = FUSE_GETHASH;
+	args.in.h.nodeid = ff->nodeid;
+	args.in.numargs = 1;
+	args.in.args[0].size = sizeof(inarg);
+	args.in.args[0].value = &inarg;
+	args.out.numargs = 1;
+	args.out.args[0].size = size;
+	args.out.args[0].value = buf;
+
+	err = fuse_simple_request(fc, &args);
+
+	if (err == -ENOSYS)
+		err = -EOPNOTSUPP;
+
+	return err;
+}
+
 static const struct file_operations fuse_file_operations = {
 	.llseek		= fuse_file_llseek,
 	.read_iter	= fuse_file_read_iter,
@@ -3027,6 +3060,7 @@ static const struct file_operations fuse_file_operations = {
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
+	.get_hash	= fuse_file_get_hash,
 };
 
 static const struct file_operations fuse_direct_io_file_operations = {
@@ -3044,6 +3078,7 @@ static const struct file_operations fuse_direct_io_file_operations = {
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
+	.get_hash	= fuse_file_get_hash,
 	/* no splice_read */
 };
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f78e9614bb5f..b4d4736cbb38 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -637,6 +637,13 @@ struct fuse_conn {
 	/** Allow other than the mounter user to access the filesystem ? */
 	unsigned allow_other:1;
 
+	/*
+	 * Allow the underlying filesystem to the hash of a file. This is
+	 * used by IMA to avoid needing to calculate the hash on every
+	 * measurement
+	 */
+	unsigned allow_gethash:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index db9e60b7eb69..8051506d245f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -70,6 +70,7 @@ struct fuse_mount_data {
 	unsigned group_id_present:1;
 	unsigned default_permissions:1;
 	unsigned allow_other:1;
+	unsigned allow_gethash:1;
 	unsigned max_read;
 	unsigned blksize;
 };
@@ -454,6 +455,7 @@ enum {
 	OPT_ALLOW_OTHER,
 	OPT_MAX_READ,
 	OPT_BLKSIZE,
+	OPT_ALLOW_GETHASH,
 	OPT_ERR
 };
 
@@ -466,6 +468,7 @@ static const match_table_t tokens = {
 	{OPT_ALLOW_OTHER,		"allow_other"},
 	{OPT_MAX_READ,			"max_read=%u"},
 	{OPT_BLKSIZE,			"blksize=%u"},
+	{OPT_ALLOW_GETHASH,		"allow_gethash"},
 	{OPT_ERR,			NULL}
 };
 
@@ -552,6 +555,10 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev,
 			d->blksize = value;
 			break;
 
+		case OPT_ALLOW_GETHASH:
+			d->allow_gethash = 1;
+			break;
+
 		default:
 			return 0;
 		}
@@ -575,6 +582,8 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root)
 		seq_puts(m, ",default_permissions");
 	if (fc->allow_other)
 		seq_puts(m, ",allow_other");
+	if (fc->allow_gethash)
+		seq_puts(m, ",allow_gethash");
 	if (fc->max_read != ~0)
 		seq_printf(m, ",max_read=%u", fc->max_read);
 	if (sb->s_bdev && sb->s_blocksize != FUSE_DEFAULT_BLKSIZE)
@@ -1141,6 +1150,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 	fc->user_id = d.user_id;
 	fc->group_id = d.group_id;
 	fc->max_read = max_t(unsigned, 4096, d.max_read);
+	fc->allow_gethash = d.allow_gethash;
 
 	/* Used by get_root_inode() */
 	sb->s_fs_info = fc;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 92fa24c24c92..a43e80ea675d 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -381,6 +381,7 @@ enum fuse_opcode {
 	FUSE_READDIRPLUS   = 44,
 	FUSE_RENAME2       = 45,
 	FUSE_LSEEK         = 46,
+	FUSE_GETHASH       = 47,
 
 	/* CUSE specific operations */
 	CUSE_INIT          = 4096,
@@ -792,4 +793,9 @@ struct fuse_lseek_out {
 	uint64_t	offset;
 };
 
+struct fuse_gethash_in {
+	uint32_t	size;
+	uint32_t	hash;
+};
+
 #endif /* _LINUX_FUSE_H */
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-04 20:30 Allow FUSE filesystems to provide out-of-band hashes to IMA Matthew Garrett
                   ` (2 preceding siblings ...)
  2018-10-04 20:30 ` [PATCH 3/3] FUSE: Allow filesystems to provide gethash methods Matthew Garrett
@ 2018-10-05 10:49 ` Mimi Zohar
  2018-10-05 17:26   ` Matthew Garrett
  3 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-05 10:49 UTC (permalink / raw)
  To: Matthew Garrett, linux-integrity
  Cc: dmitry.kasatkin, miklos, linux-fsdevel, viro

Hi Matthew,

On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> As of d77ccdc644a59b412d8e101576134c90a0aa6797, IMA will trigger a
> rehash of any file on a FUSE filesystem for every measurement. This has
> a significant impact on performance. Individual FUSE filesystems may
> have the ability to identify whether a file needs to be rehashed, and
> some may even have the ability to obtain the file hash out of band
> without needing the kernel to do it. Longer term, this may also be
> usable for other scenarios where a filesystem can provide a trustworthy
> hash (eg, fs-verity on ext4 could provide a hash and then later abort
> any read()s that discover that the file doesn't match the measurement).

Really, a security vs. performance argument?!  I don't need to tell
you of all people, that one of the basic tenents of trusted boot is
calculating the actual file hash before use.  Limiting the file hash
re-calculation is one thing, but relying on some out of band method of
obtaining the file hash without the kernel ever calculating it is
totally different.  The only exception will be for fs-verity, which
will return not the file hash, but the file's Merkle tree root hash.

If you want to introduce support for identifying whether a FUSE file,
on a trusted mount, needs to be rehashed, that's fine.  It should not
be the default behavior.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-05 10:49 ` Allow FUSE filesystems to provide out-of-band hashes to IMA Mimi Zohar
@ 2018-10-05 17:26   ` Matthew Garrett
  2018-10-05 18:18     ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-05 17:26 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Fri, Oct 5, 2018 at 3:49 AM Mimi Zohar <zohar@linux.ibm.com> wrote:

> Really, a security vs. performance argument?!  I don't need to tell
> you of all people, that one of the basic tenents of trusted boot is
> calculating the actual file hash before use.  Limiting the file hash
> re-calculation is one thing, but relying on some out of band method of
> obtaining the file hash without the kernel ever calculating it is
> totally different.  The only exception will be for fs-verity, which
> will return not the file hash, but the file's Merkle tree root hash.

Using FUSE means you're inherently accepting the risk of TOCTOU.
Having the kernel read everything once and hash it is no guarantee
that the filesystem will return the same value on further reads, so if
you're going to use FUSE in an environment where you're using IMA then
you already need to assert that your filesystems are trustworthy.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-05 17:26   ` Matthew Garrett
@ 2018-10-05 18:18     ` Mimi Zohar
  2018-10-05 19:25       ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-05 18:18 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Fri, 2018-10-05 at 10:26 -0700, Matthew Garrett wrote:
> On Fri, Oct 5, 2018 at 3:49 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> > Really, a security vs. performance argument?!  I don't need to tell
> > you of all people, that one of the basic tenents of trusted boot is
> > calculating the actual file hash before use.  Limiting the file hash
> > re-calculation is one thing, but relying on some out of band method of
> > obtaining the file hash without the kernel ever calculating it is
> > totally different.  The only exception will be for fs-verity, which
> > will return not the file hash, but the file's Merkle tree root hash.
> 
> Using FUSE means you're inherently accepting the risk of TOCTOU.
> Having the kernel read everything once and hash it is no guarantee
> that the filesystem will return the same value on further reads, so if
> you're going to use FUSE in an environment where you're using IMA then
> you already need to assert that your filesystems are trustworthy.

Right, the correct behavior should be not to trust FUSE filesystems,
but since we don't break userspace there is the
"ima_policy=fail_securely" boot command line option.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-05 18:18     ` Mimi Zohar
@ 2018-10-05 19:25       ` Matthew Garrett
  2018-10-08 11:25         ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-05 19:25 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Fri, Oct 5, 2018 at 11:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> Right, the correct behavior should be not to trust FUSE filesystems,
> but since we don't break userspace there is the
> "ima_policy=fail_securely" boot command line option.

There seem to be two scenarios:

1) You trust FUSE mounts, perhaps because you have some other policy
in place to ensure that only trusted binaries can mount stuff. In this
scenario you already trust that the filesystem will give you
consistent results when you read data from it - it seems reasonable to
also trust it to give you back an accurate hash if you ask for one.
2) You don't trust FUSE mounts, in which case you pass
ima_policy=fail_securely. This patch doesn't change that behaviour.

I agree that using FUSE in general is incompatible with IMA's goals,
but it's possible to configure systems where you can ensure that only
trustworthy code is involved. In that scenario this patch improves
performance without compromising security.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-05 19:25       ` Matthew Garrett
@ 2018-10-08 11:25         ` Mimi Zohar
  2018-10-08 20:19           ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-08 11:25 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Fri, 2018-10-05 at 12:25 -0700, Matthew Garrett wrote:
> On Fri, Oct 5, 2018 at 11:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > Right, the correct behavior should be not to trust FUSE filesystems,
> > but since we don't break userspace there is the
> > "ima_policy=fail_securely" boot command line option.
> 
> There seem to be two scenarios:
> 
> 1) You trust FUSE mounts, perhaps because you have some other policy
> in place to ensure that only trusted binaries can mount stuff. In this
> scenario you already trust that the filesystem will give you
> consistent results when you read data from it - 

In the trusted mount scenario, we trust the data should not change
between calculating the file hash and reading the file data, making it
similar to other local filesystems.  Unlike other local filesystems,
however, we can't detect when the file changes.  For this reason we
need to re-calculate the file hash to measure/appraise the file each
time.

> it seems reasonable to
> also trust it to give you back an accurate hash if you ask for one.

Going from trusting the filesystem to behave properly, to trusting the
file hash that the filesystem provides is a major leap.  We don't do
this today for any local filesystem.

> 2) You don't trust FUSE mounts, in which case you pass
> ima_policy=fail_securely. This patch doesn't change that behaviour.
> 
> I agree that using FUSE in general is incompatible with IMA's goals,
> but it's possible to configure systems where you can ensure that only
> trustworthy code is involved. In that scenario this patch improves
> performance without compromising security.

If you trust a FUSE filesystem to not only behave properly, but also
to return file hashes, what is the value of measuring/appraising the
files?  Define a custom policy that doesn't measure/appraise files on
FUSE filesystems.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-08 11:25         ` Mimi Zohar
@ 2018-10-08 20:19           ` Matthew Garrett
  2018-10-08 22:40             ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-08 20:19 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Mon, Oct 8, 2018 at 4:25 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2018-10-05 at 12:25 -0700, Matthew Garrett wrote:
> > 1) You trust FUSE mounts, perhaps because you have some other policy
> > in place to ensure that only trusted binaries can mount stuff. In this
> > scenario you already trust that the filesystem will give you
> > consistent results when you read data from it -
>
> In the trusted mount scenario, we trust the data should not change
> between calculating the file hash and reading the file data, making it
> similar to other local filesystems.  Unlike other local filesystems,
> however, we can't detect when the file changes.  For this reason we
> need to re-calculate the file hash to measure/appraise the file each
> time.

But we don't re-measure for every read, so it's still possible for the
filesystem to give us back different results without affecting the
measurement.

> > it seems reasonable to
> > also trust it to give you back an accurate hash if you ask for one.
>
> Going from trusting the filesystem to behave properly, to trusting the
> file hash that the filesystem provides is a major leap.  We don't do
> this today for any local filesystem.

I don't think any local filesystems provide a mechanism for this - we
have FUSE filesystems that do.

> > I agree that using FUSE in general is incompatible with IMA's goals,
> > but it's possible to configure systems where you can ensure that only
> > trustworthy code is involved. In that scenario this patch improves
> > performance without compromising security.
>
> If you trust a FUSE filesystem to not only behave properly, but also
> to return file hashes, what is the value of measuring/appraising the
> files?  Define a custom policy that doesn't measure/appraise files on
> FUSE filesystems.

We trust that the filesystem will return us accurate binaries and
hashes, but we don't the binaries themselves may not be trustworthy -
we want the same level of audit trail associated with their execution
that we'd have for something run off local disk. We could certainly
rearchitect our filesystems to generate audit events themselves, but
we'd be duplicating functionality that already exists in the kernel.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-08 20:19           ` Matthew Garrett
@ 2018-10-08 22:40             ` Mimi Zohar
  2018-10-09 17:21               ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-08 22:40 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Mon, 2018-10-08 at 13:19 -0700, Matthew Garrett wrote:
> On Mon, Oct 8, 2018 at 4:25 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Fri, 2018-10-05 at 12:25 -0700, Matthew Garrett wrote:

> > > I agree that using FUSE in general is incompatible with IMA's goals,
> > > but it's possible to configure systems where you can ensure that only
> > > trustworthy code is involved. In that scenario this patch improves
> > > performance without compromising security.
> >
> > If you trust a FUSE filesystem to not only behave properly, but also
> > to return file hashes, what is the value of measuring/appraising the
> > files?  Define a custom policy that doesn't measure/appraise files on
> > FUSE filesystems.
> 
> We trust that the filesystem will return us accurate binaries and
> hashes, but we don't the binaries themselves may not be trustworthy -
> we want the same level of audit trail associated with their execution
> that we'd have for something run off local disk. We could certainly
> rearchitect our filesystems to generate audit events themselves, but
> we'd be duplicating functionality that already exists in the kernel.

I'm really not comfortable with the FUSE filesystem calculating the
file hash being used by IMA.  Adding FUSE i_version support would have
been better, instead of returning the actual file hash.  Based on a
mount option and the i_version, the kernel could then decide whether
or not to limit re-calculating the file hash.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-08 22:40             ` Mimi Zohar
@ 2018-10-09 17:21               ` Matthew Garrett
  2018-10-09 18:04                 ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-09 17:21 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Mon, Oct 8, 2018 at 3:40 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2018-10-08 at 13:19 -0700, Matthew Garrett wrote:
> > We trust that the filesystem will return us accurate binaries and
> > hashes, but we don't the binaries themselves may not be trustworthy -
> > we want the same level of audit trail associated with their execution
> > that we'd have for something run off local disk. We could certainly
> > rearchitect our filesystems to generate audit events themselves, but
> > we'd be duplicating functionality that already exists in the kernel.
>
> I'm really not comfortable with the FUSE filesystem calculating the
> file hash being used by IMA.  Adding FUSE i_version support would have
> been better, instead of returning the actual file hash.  Based on a
> mount option and the i_version, the kernel could then decide whether
> or not to limit re-calculating the file hash.

Well, there's a performance benefit as well - reading 500MB
executables over the network is time consuming and otherwise mostly
unnecessary. Given two solutions that have the same properties in
terms of which components we need to trust, why not pick the one
that's faster?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-09 17:21               ` Matthew Garrett
@ 2018-10-09 18:04                 ` Mimi Zohar
  2018-10-09 19:29                   ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-09 18:04 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Tue, 2018-10-09 at 10:21 -0700, Matthew Garrett wrote:
> On Mon, Oct 8, 2018 at 3:40 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Mon, 2018-10-08 at 13:19 -0700, Matthew Garrett wrote:
> > > We trust that the filesystem will return us accurate binaries and
> > > hashes, but we don't the binaries themselves may not be trustworthy -
> > > we want the same level of audit trail associated with their execution
> > > that we'd have for something run off local disk. We could certainly
> > > rearchitect our filesystems to generate audit events themselves, but
> > > we'd be duplicating functionality that already exists in the kernel.
> >
> > I'm really not comfortable with the FUSE filesystem calculating the
> > file hash being used by IMA.  Adding FUSE i_version support would have
> > been better, instead of returning the actual file hash.  Based on a
> > mount option and the i_version, the kernel could then decide whether
> > or not to limit re-calculating the file hash.
> 
> Well, there's a performance benefit as well - reading 500MB
> executables over the network is time consuming and otherwise mostly
> unnecessary. Given two solutions that have the same properties in
> terms of which components we need to trust, why not pick the one
> that's faster?

With the existing cover letter, the purpose of this patch set should
be to address the performance of calculating the file hash on trusted
local FUSE mounted filesystems, not remote filesystems or fs-verity
filesystems.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-09 18:04                 ` Mimi Zohar
@ 2018-10-09 19:29                   ` Matthew Garrett
  2018-10-09 20:52                     ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-09 19:29 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Tue, Oct 9, 2018 at 11:04 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2018-10-09 at 10:21 -0700, Matthew Garrett wrote:
> > Well, there's a performance benefit as well - reading 500MB
> > executables over the network is time consuming and otherwise mostly
> > unnecessary. Given two solutions that have the same properties in
> > terms of which components we need to trust, why not pick the one
> > that's faster?
>
> With the existing cover letter, the purpose of this patch set should
> be to address the performance of calculating the file hash on trusted
> local FUSE mounted filesystems, not remote filesystems or fs-verity
> filesystems.

The performance hit is more noticeable over remote filesystems, but we
have large binaries that take several seconds to hash even on local
filesystems. Would it be helpful to try to define the assumptions that
IMA makes in terms of whether or not it produces trustworthy results?
It feels like it's be easier to talk about this if we have a more
formal set of conditions to take into consideration.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-09 19:29                   ` Matthew Garrett
@ 2018-10-09 20:52                     ` Mimi Zohar
  2018-10-09 21:32                       ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-09 20:52 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel,
	Alexander Viro, Chuck Lever

On Tue, 2018-10-09 at 12:29 -0700, Matthew Garrett wrote:
> On Tue, Oct 9, 2018 at 11:04 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2018-10-09 at 10:21 -0700, Matthew Garrett wrote:
> > > Well, there's a performance benefit as well - reading 500MB
> > > executables over the network is time consuming and otherwise mostly
> > > unnecessary. Given two solutions that have the same properties in
> > > terms of which components we need to trust, why not pick the one
> > > that's faster?
> >
> > With the existing cover letter, the purpose of this patch set should
> > be to address the performance of calculating the file hash on trusted
> > local FUSE mounted filesystems, not remote filesystems or fs-verity
> > filesystems.
> 
> The performance hit is more noticeable over remote filesystems, but we
> have large binaries that take several seconds to hash even on local
> filesystems. Would it be helpful to try to define the assumptions that
> IMA makes in terms of whether or not it produces trustworthy results?
> It feels like it's be easier to talk about this if we have a more
> formal set of conditions to take into consideration.

[Cc'ing Chuck Lever]

Integrity of files on remote filesystems should probably be discussed
in the context of fs-verity, not FUSE filesystems.

Do you want to continue the discussion here or perhaps as an LSS-EU
BoF?

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-09 20:52                     ` Mimi Zohar
@ 2018-10-09 21:32                       ` Matthew Garrett
  2018-10-10 11:09                         ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-09 21:32 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel,
	Alexander Viro, chuck.lever

On Tue, Oct 9, 2018 at 1:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> > The performance hit is more noticeable over remote filesystems, but we
> > have large binaries that take several seconds to hash even on local
> > filesystems. Would it be helpful to try to define the assumptions that
> > IMA makes in terms of whether or not it produces trustworthy results?
> > It feels like it's be easier to talk about this if we have a more
> > formal set of conditions to take into consideration.
>
> [Cc'ing Chuck Lever]
>
> Integrity of files on remote filesystems should probably be discussed
> in the context of fs-verity, not FUSE filesystems.

Hm. We /could/ fake up fs-verity style behaviour, but we're not
talking about files that are expected to be immutable so it would seem
like there might be mismatched semantics there.

> Do you want to continue the discussion here or perhaps as an LSS-EU
> BoF?

This is something that's causing us pain at the moment, so if there's
any chance we can reach a resolution over the next couple of weeks
then I'd like to continue discussing it :)

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-09 21:32                       ` Matthew Garrett
@ 2018-10-10 11:09                         ` Mimi Zohar
  2018-10-10 16:19                           ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-10 11:09 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel,
	Alexander Viro, chuck.lever

On Tue, 2018-10-09 at 14:32 -0700, Matthew Garrett wrote:
> On Tue, Oct 9, 2018 at 1:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > > The performance hit is more noticeable over remote filesystems, but we
> > > have large binaries that take several seconds to hash even on local
> > > filesystems. Would it be helpful to try to define the assumptions that
> > > IMA makes in terms of whether or not it produces trustworthy results?
> > > It feels like it's be easier to talk about this if we have a more
> > > formal set of conditions to take into consideration.
> >
> > [Cc'ing Chuck Lever]
> >
> > Integrity of files on remote filesystems should probably be discussed
> > in the context of fs-verity, not FUSE filesystems.
> 
> Hm. We /could/ fake up fs-verity style behaviour, but we're not
> talking about files that are expected to be immutable so it would seem
> like there might be mismatched semantics there.

If these aren't immutable files, then the file hash needs to be
calculated and re-calculated on file change.  Trust between the kernel
and FUSE isn't bi-directional.  IMA already supports hardware crypto
acceleration.  (Refer to the "ima.ahash_minsize" and
"ima.ahash_bufsize" boot command line options.)  Why is it better that
FUSE calculates the file hash than the kernel?

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Allow FUSE filesystems to provide out-of-band hashes to IMA
  2018-10-10 11:09                         ` Mimi Zohar
@ 2018-10-10 16:19                           ` Matthew Garrett
  0 siblings, 0 replies; 30+ messages in thread
From: Matthew Garrett @ 2018-10-10 16:19 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel,
	Alexander Viro, chuck.lever

On Wed, Oct 10, 2018 at 4:09 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2018-10-09 at 14:32 -0700, Matthew Garrett wrote:
> > Hm. We /could/ fake up fs-verity style behaviour, but we're not
> > talking about files that are expected to be immutable so it would seem
> > like there might be mismatched semantics there.
>
> If these aren't immutable files, then the file hash needs to be
> calculated and re-calculated on file change.  Trust between the kernel
> and FUSE isn't bi-directional.  IMA already supports hardware crypto
> acceleration.  (Refer to the "ima.ahash_minsize" and
> "ima.ahash_bufsize" boot command line options.)  Why is it better that
> FUSE calculates the file hash than the kernel?

The trust /has/ to be bi-directional - if FUSE isn't trustworthy then
it can return false data after the measurement, since we're not
performing measurements at every read. FUSE can recalculate the hash
once a write occurs (it has all the rest of the data already), whereas
having the kernel redo it requires it to read the entire data back
from FUSE. This patch still leaves FUSE measurements as uncached, so
the filesystem will be queried for a new hash every time we want a new
measurement. For cases where there's no benefit in having the
filesystem do the hashing, the filesystem can simply not provide the
feature and the kernel will continue using the existing code.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-04 20:30 ` [PATCH 1/3] VFS: Add a call to obtain a file's hash Matthew Garrett
@ 2018-10-11 15:22   ` Mimi Zohar
  2018-10-11 18:21     ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-11 15:22 UTC (permalink / raw)
  To: Matthew Garrett, linux-integrity
  Cc: zohar, dmitry.kasatkin, miklos, linux-fsdevel, viro

On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> IMA wants to know what the hash of a file is, and currently does so by
> reading the entire file and generating the hash. Some filesystems may
> have the ability to store the hash in a secure manner resistant to
> offline attacks (eg, filesystem-level file signing), and in that case
> it's a performance win for IMA to be able to use that rather than having
> to re-hash everything. This patch simply adds VFS-level support for
> calling down to filesystems.

This patch description starts out saying that IMA needs the file hash
without explaining why.  Without that explanation, simply extracting
the file hash included in the file signature might sound plausible,
but kind of defeats the purpose of IMA.

Mimi


> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  fs/read_write.c    | 24 ++++++++++++++++++++++++
>  include/linux/fs.h |  6 +++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 39b4a21dd933..9ba3ce4bb838 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -2081,3 +2081,27 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
>  	return ret;
>  }
>  EXPORT_SYMBOL(vfs_dedupe_file_range);
> +
> +/**
> + * vfs_gethash - obtain a file's hash
> + * @file:	file structure in question
> + * @hash_algo:	the hash algorithm requested
> + * @buf:	buffer to return the hash in
> + * @size:	size allocated for the buffer by the caller
> + *
> + * This function allows filesystems that support securely storing the hash
> + * of a file to return it rather than forcing the kernel to recalculate it.
> + * Filesystems that cannot provide guarantees about the hash being resistant
> + * to offline attack should not implement this functionality.
> + *
> + * Returns 0 on success, -EOPNOTSUPP if the filesystem doesn't support it.
> + */
> +int vfs_get_hash(struct file *file, enum hash_algo hash, uint8_t *buf,
> +		 size_t size)
> +{
> +	if (!file->f_op->get_hash)
> +		return -EOPNOTSUPP;
> +
> +	return file->f_op->get_hash(file, hash, buf, size);
> +}
> +EXPORT_SYMBOL(vfs_get_hash);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6c0b4a1c22ff..540316cfd461 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -40,6 +40,7 @@
> 
>  #include <asm/byteorder.h>
>  #include <uapi/linux/fs.h>
> +#include <uapi/linux/hash_info.h>
> 
>  struct backing_dev_info;
>  struct bdi_writeback;
> @@ -1764,6 +1765,8 @@ struct file_operations {
>  	int (*dedupe_file_range)(struct file *, loff_t, struct file *, loff_t,
>  			u64);
>  	int (*fadvise)(struct file *, loff_t, loff_t, int);
> +	int (*get_hash)(struct file *, enum hash_algo hash, uint8_t *buf,
> +			size_t size);
>  } __randomize_layout;
> 
>  struct inode_operations {
> @@ -1838,7 +1841,8 @@ extern int vfs_dedupe_file_range(struct file *file,
>  extern int vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
>  				     struct file *dst_file, loff_t dst_pos,
>  				     u64 len);
> -
> +extern int vfs_get_hash(struct file *file, enum hash_algo hash, uint8_t *buf,
> +			size_t size);
> 
>  struct super_operations {
>     	struct inode *(*alloc_inode)(struct super_block *sb);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-04 20:30 ` [PATCH 2/3] IMA: Make use of filesystem-provided hashes Matthew Garrett
@ 2018-10-11 15:23   ` Mimi Zohar
  2018-10-11 20:30     ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-11 15:23 UTC (permalink / raw)
  To: Matthew Garrett, linux-integrity
  Cc: dmitry.kasatkin, miklos, linux-fsdevel, viro

On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> Some filesystems may be able to provide hashes in an out of band manner,
> and allowing them to do so is a performance win. This is especially true
> of FUSE-based filesystems where otherwise we recalculate the hash on
> every measurement. Make use of this by default, but provide a parameter
> to force recalculation rather than trusting the filesystem.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

Support for not calculating the file hash would need to be finer
grained than this, probably on a per mount basis.  The default should
be for IMA to always calculate the file hash, unless explicitly told
not to.

> ---
>  Documentation/admin-guide/kernel-parameters.txt |  5 +++++
>  security/integrity/ima/ima_crypto.c             | 11 +++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 92eb1f42240d..617ae0f83b14 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1612,6 +1612,11 @@
>  			different crypto accelerators. This option can be used
>  			to achieve best performance for particular HW.
> 
> +	ima.force_hash= [IMA] Force hash calculation in IMA
> +			Format: <bool>
> +			Always calculate hashes rather than trusting the
> +			filesystem to provide them to us.
> +
>  	init=		[KNL]
>  			Format: <full_path>
>  			Run specified binary instead of /sbin/init as init
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 7e7e7e7c250a..f25259b2b6ec 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -32,6 +32,10 @@ static unsigned long ima_ahash_minsize;
>  module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
>  MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use");
> 
> +static bool ima_force_hash;
> +module_param_named(force_hash, ima_force_hash, bool_enable_only, 0644);
> +MODULE_PARM_DESC(force_hash, "Always calculate hashes");
> +
>  /* default is 0 - 1 page. */
>  static int ima_maxorder;
>  static unsigned int ima_bufsize = PAGE_SIZE;
> @@ -431,6 +435,13 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>  		return -EINVAL;
>  	}
> 
> +	if (!ima_force_hash) {

IMA should never skip the file hash calculation if the filesystem is
an untrusted mount (eg. SB_I_UNTRUSTED_MOUNTER).

Mimi


> +		hash->length = hash_digest_size[hash->algo];
> +		rc = vfs_get_hash(file, hash->algo, hash->digest, hash->length);
> +		if (!rc)
> +			return 0;
> +	}
> +
>  	i_size = i_size_read(file_inode(file));
> 
>  	if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-11 15:22   ` Mimi Zohar
@ 2018-10-11 18:21     ` Matthew Garrett
  2018-10-11 18:24       ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-11 18:21 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin, miklos,
	linux-fsdevel, Alexander Viro

On Thu, Oct 11, 2018 at 8:22 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> > IMA wants to know what the hash of a file is, and currently does so by
> > reading the entire file and generating the hash. Some filesystems may
> > have the ability to store the hash in a secure manner resistant to
> > offline attacks (eg, filesystem-level file signing), and in that case
> > it's a performance win for IMA to be able to use that rather than having
> > to re-hash everything. This patch simply adds VFS-level support for
> > calling down to filesystems.
>
> This patch description starts out saying that IMA needs the file hash
> without explaining why.  Without that explanation, simply extracting
> the file hash included in the file signature might sound plausible,
> but kind of defeats the purpose of IMA.

I'm not sure how it defeats the purpose - IMA wants to know the hash
of a file so it can either log it or compare it against a signature,
and it currently obtains this hash by reading the entire file at
measurement time. If the filesystem later returns different data then
IMA won't notice, which allows a malicious filesystem to bypass the
measurements - there's no guarantee that we won't evict large parts of
the copy of an executable that IMA read, and the filesystem can give
us back a modified page when we page it back in. So IMA fundamentally
relies on the filesystem to be trustworthy, and if we rely on the
filesystem to be trustworthy then we should be able to rely on it to
accurately store and provide the hash of a file.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-11 18:21     ` Matthew Garrett
@ 2018-10-11 18:24       ` Matthew Garrett
  2018-10-11 18:37         ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-11 18:24 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin, miklos,
	linux-fsdevel, Alexander Viro

On Thu, Oct 11, 2018 at 11:21 AM Matthew Garrett <mjg59@google.com> wrote:
>
> On Thu, Oct 11, 2018 at 8:22 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > This patch description starts out saying that IMA needs the file hash
> > without explaining why.  Without that explanation, simply extracting
> > the file hash included in the file signature might sound plausible,
> > but kind of defeats the purpose of IMA.
>
> I'm not sure how it defeats the purpose - IMA wants to know the hash
> of a file so it can either log it or compare it against a signature,
> and it currently obtains this hash by reading the entire file at
> measurement time. If the filesystem later returns different data then
> IMA won't notice, which allows a malicious filesystem to bypass the
> measurements - there's no guarantee that we won't evict large parts of
> the copy of an executable that IMA read, and the filesystem can give
> us back a modified page when we page it back in. So IMA fundamentally
> relies on the filesystem to be trustworthy, and if we rely on the
> filesystem to be trustworthy then we should be able to rely on it to
> accurately store and provide the hash of a file.

Oh, to clarify on the signature part of things - it would obviously be
inappropriate to, say, just read the hash out of security.ima and hand
that back. But for a hypothetical case where the filesystem itself
verifies the signature, then the filesystem would abort the
transaction if the signature didn't match and it seems reasonable to
avoid doing the validation twice (once up front and then again on
every read)

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-11 18:24       ` Matthew Garrett
@ 2018-10-11 18:37         ` Mimi Zohar
  2018-10-11 18:43           ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-11 18:37 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin, miklos,
	linux-fsdevel, Alexander Viro

On Thu, 2018-10-11 at 11:24 -0700, Matthew Garrett wrote:
> On Thu, Oct 11, 2018 at 11:21 AM Matthew Garrett <mjg59@google.com> wrote:
> >
> > On Thu, Oct 11, 2018 at 8:22 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > This patch description starts out saying that IMA needs the file hash
> > > without explaining why.  Without that explanation, simply extracting
> > > the file hash included in the file signature might sound plausible,
> > > but kind of defeats the purpose of IMA.
> >
> > I'm not sure how it defeats the purpose - IMA wants to know the hash
> > of a file so it can either log it or compare it against a signature,
> > and it currently obtains this hash by reading the entire file at
> > measurement time. If the filesystem later returns different data then
> > IMA won't notice, which allows a malicious filesystem to bypass the
> > measurements - there's no guarantee that we won't evict large parts of
> > the copy of an executable that IMA read, and the filesystem can give
> > us back a modified page when we page it back in. So IMA fundamentally
> > relies on the filesystem to be trustworthy, and if we rely on the
> > filesystem to be trustworthy then we should be able to rely on it to
> > accurately store and provide the hash of a file.
> 
> Oh, to clarify on the signature part of things - it would obviously be
> inappropriate to, say, just read the hash out of security.ima and hand
> that back.

Right, reading it either directly or extracted from the file signature
stored in security.ima.

> But for a hypothetical case where the filesystem itself
> verifies the signature, then the filesystem would abort the
> transaction if the signature didn't match and it seems reasonable to
> avoid doing the validation twice (once up front and then again on
> every read)

Right, this is a hypothetical scenario as far as I'm aware, since none
of the filesystems are currently calculating and storing the file
hash.  The default should be for IMA to re-calculate the file hash.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] VFS: Add a call to obtain a file's hash
  2018-10-11 18:37         ` Mimi Zohar
@ 2018-10-11 18:43           ` Matthew Garrett
  0 siblings, 0 replies; 30+ messages in thread
From: Matthew Garrett @ 2018-10-11 18:43 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin, miklos,
	linux-fsdevel, Alexander Viro

On Thu, Oct 11, 2018 at 11:37 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Thu, 2018-10-11 at 11:24 -0700, Matthew Garrett wrote:
> > But for a hypothetical case where the filesystem itself
> > verifies the signature, then the filesystem would abort the
> > transaction if the signature didn't match and it seems reasonable to
> > avoid doing the validation twice (once up front and then again on
> > every read)
>
> Right, this is a hypothetical scenario as far as I'm aware, since none
> of the filesystems are currently calculating and storing the file
> hash.  The default should be for IMA to re-calculate the file hash.

There are FUSE filesystems that do.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-11 15:23   ` Mimi Zohar
@ 2018-10-11 20:30     ` Matthew Garrett
  2018-10-11 23:03       ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-11 20:30 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Thu, Oct 11, 2018 at 8:23 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> > Some filesystems may be able to provide hashes in an out of band manner,
> > and allowing them to do so is a performance win. This is especially true
> > of FUSE-based filesystems where otherwise we recalculate the hash on
> > every measurement. Make use of this by default, but provide a parameter
> > to force recalculation rather than trusting the filesystem.
> >
> > Signed-off-by: Matthew Garrett <mjg59@google.com>
>
> Support for not calculating the file hash would need to be finer
> grained than this, probably on a per mount basis.  The default should
> be for IMA to always calculate the file hash, unless explicitly told
> not to.

Ok, should this just be part of the IMA policy?

> IMA should never skip the file hash calculation if the filesystem is
> an untrusted mount (eg. SB_I_UNTRUSTED_MOUNTER).

Ok.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-11 20:30     ` Matthew Garrett
@ 2018-10-11 23:03       ` Mimi Zohar
  2018-10-12 18:31         ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-11 23:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Thu, 2018-10-11 at 13:30 -0700, Matthew Garrett wrote:
> On Thu, Oct 11, 2018 at 8:23 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2018-10-04 at 13:30 -0700, Matthew Garrett wrote:
> > > Some filesystems may be able to provide hashes in an out of band manner,
> > > and allowing them to do so is a performance win. This is especially true
> > > of FUSE-based filesystems where otherwise we recalculate the hash on
> > > every measurement. Make use of this by default, but provide a parameter
> > > to force recalculation rather than trusting the filesystem.
> > >
> > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> >
> > Support for not calculating the file hash would need to be finer
> > grained than this, probably on a per mount basis.  The default should
> > be for IMA to always calculate the file hash, unless explicitly told
> > not to.
> 
> Ok, should this just be part of the IMA policy?

How would you be able to differentiate between different FUSE
filesystems for example?

> 
> > IMA should never skip the file hash calculation if the filesystem is
> > an untrusted mount (eg. SB_I_UNTRUSTED_MOUNTER).
> 
> Ok.
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-11 23:03       ` Mimi Zohar
@ 2018-10-12 18:31         ` Matthew Garrett
  2018-10-15  1:38           ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-12 18:31 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Thu, Oct 11, 2018 at 4:03 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Thu, 2018-10-11 at 13:30 -0700, Matthew Garrett wrote:

> > Ok, should this just be part of the IMA policy?
>
> How would you be able to differentiate between different FUSE
> filesystems for example?

There's a couple of ways. We could extend the filesystem type matching
logic to also check the subtype - you'd then need to enforce that at
the LSM level in order to protect against untrusted filesystems
spoofing the filesystem type. Alternatively, we could add an
additional policy match type for mount point and iterate through
s_mounts on the superblock - if any match, we could define the policy
there?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-12 18:31         ` Matthew Garrett
@ 2018-10-15  1:38           ` Mimi Zohar
  2018-10-15 18:46             ` Matthew Garrett
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2018-10-15  1:38 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Fri, 2018-10-12 at 11:31 -0700, Matthew Garrett wrote:
> On Thu, Oct 11, 2018 at 4:03 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Thu, 2018-10-11 at 13:30 -0700, Matthew Garrett wrote:
> 
> > > Ok, should this just be part of the IMA policy?
> >
> > How would you be able to differentiate between different FUSE
> > filesystems for example?
> 
> There's a couple of ways. We could extend the filesystem type matching
> logic to also check the subtype - you'd then need to enforce that at
> the LSM level in order to protect against untrusted filesystems
> spoofing the filesystem type. Alternatively, we could add an
> additional policy match type for mount point and iterate through
> s_mounts on the superblock - if any match, we could define the policy
> there?

The first method differentiates between different subtypes of FUSE
filesystems, while the second method allows differentiating between
the same type and subtype on different mount points.  Both criteria
are needed, but instead of the second method based on a mount point,
perhaps based instead on a mount flag?

Trusted mount of permitted filesystem type and subtype, that is
mounted with the defined mount flag. 

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-15  1:38           ` Mimi Zohar
@ 2018-10-15 18:46             ` Matthew Garrett
  2018-10-16 13:16               ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Matthew Garrett @ 2018-10-15 18:46 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Sun, Oct 14, 2018 at 6:38 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2018-10-12 at 11:31 -0700, Matthew Garrett wrote:
> > There's a couple of ways. We could extend the filesystem type matching
> > logic to also check the subtype - you'd then need to enforce that at
> > the LSM level in order to protect against untrusted filesystems
> > spoofing the filesystem type. Alternatively, we could add an
> > additional policy match type for mount point and iterate through
> > s_mounts on the superblock - if any match, we could define the policy
> > there?
>
> The first method differentiates between different subtypes of FUSE
> filesystems, while the second method allows differentiating between
> the same type and subtype on different mount points.  Both criteria
> are needed, but instead of the second method based on a mount point,
> perhaps based instead on a mount flag?

Patch 3 already requires that the allow_gethash option be passed for
this to work - I can restrict that to CAP_SYS_ADMIN?

> Trusted mount of permitted filesystem type and subtype, that is
> mounted with the defined mount flag.

Ok, I'll write up a patch that allows policy matching of filesystem
subtype as well as type and try to get that posted this week so we can
discuss it in Edinburgh?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] IMA: Make use of filesystem-provided hashes
  2018-10-15 18:46             ` Matthew Garrett
@ 2018-10-16 13:16               ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2018-10-16 13:16 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Dmitry Kasatkin, miklos, linux-fsdevel, Alexander Viro

On Mon, 2018-10-15 at 11:46 -0700, Matthew Garrett wrote:
> On Sun, Oct 14, 2018 at 6:38 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Fri, 2018-10-12 at 11:31 -0700, Matthew Garrett wrote:
> > > There's a couple of ways. We could extend the filesystem type matching
> > > logic to also check the subtype - you'd then need to enforce that at
> > > the LSM level in order to protect against untrusted filesystems
> > > spoofing the filesystem type. Alternatively, we could add an
> > > additional policy match type for mount point and iterate through
> > > s_mounts on the superblock - if any match, we could define the policy
> > > there?
> >
> > The first method differentiates between different subtypes of FUSE
> > filesystems, while the second method allows differentiating between
> > the same type and subtype on different mount points.  Both criteria
> > are needed, but instead of the second method based on a mount point,
> > perhaps based instead on a mount flag?
> 
> Patch 3 already requires that the allow_gethash option be passed for
> this to work - I can restrict that to CAP_SYS_ADMIN?

In the case of FUSE filesystems, using "gethash" should be limited to
trusted mounts, not fileystems mounted with SB_I_UNTRUSTED_MOUNTER.
 So requiring CAP_SYS_ADMIN seems unnecessary.  The difference in the
approaches is that root has CAP_SYS_ADMIN, while providing a mount
flag requires intention.

> 
> > Trusted mount of permitted filesystem type and subtype, that is
> > mounted with the defined mount flag.
> 
> Ok, I'll write up a patch that allows policy matching of filesystem
> subtype as well as type and try to get that posted this week so we can
> discuss it in Edinburgh?

Sounds good.  Hopefully I'll have time to review it before Edinburgh.

Mimi

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-10-16 21:07 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 20:30 Allow FUSE filesystems to provide out-of-band hashes to IMA Matthew Garrett
2018-10-04 20:30 ` [PATCH 1/3] VFS: Add a call to obtain a file's hash Matthew Garrett
2018-10-11 15:22   ` Mimi Zohar
2018-10-11 18:21     ` Matthew Garrett
2018-10-11 18:24       ` Matthew Garrett
2018-10-11 18:37         ` Mimi Zohar
2018-10-11 18:43           ` Matthew Garrett
2018-10-04 20:30 ` [PATCH 2/3] IMA: Make use of filesystem-provided hashes Matthew Garrett
2018-10-11 15:23   ` Mimi Zohar
2018-10-11 20:30     ` Matthew Garrett
2018-10-11 23:03       ` Mimi Zohar
2018-10-12 18:31         ` Matthew Garrett
2018-10-15  1:38           ` Mimi Zohar
2018-10-15 18:46             ` Matthew Garrett
2018-10-16 13:16               ` Mimi Zohar
2018-10-04 20:30 ` [PATCH 3/3] FUSE: Allow filesystems to provide gethash methods Matthew Garrett
2018-10-05 10:49 ` Allow FUSE filesystems to provide out-of-band hashes to IMA Mimi Zohar
2018-10-05 17:26   ` Matthew Garrett
2018-10-05 18:18     ` Mimi Zohar
2018-10-05 19:25       ` Matthew Garrett
2018-10-08 11:25         ` Mimi Zohar
2018-10-08 20:19           ` Matthew Garrett
2018-10-08 22:40             ` Mimi Zohar
2018-10-09 17:21               ` Matthew Garrett
2018-10-09 18:04                 ` Mimi Zohar
2018-10-09 19:29                   ` Matthew Garrett
2018-10-09 20:52                     ` Mimi Zohar
2018-10-09 21:32                       ` Matthew Garrett
2018-10-10 11:09                         ` Mimi Zohar
2018-10-10 16:19                           ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).