linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] integrity: improve ima measurement accuracy
@ 2019-05-13 12:53 Janne Karhunen
  2019-05-13 12:53 ` [PATCH 1/5] integrity: keep the integrity state of open files up to date Janne Karhunen
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar; +Cc: Janne Karhunen

By default the linux integrity subsystem measures a file only
when a file is being closed. While this certainly provides
low overhead as the re-measurements are never done, it also
makes sure the system has zero means to recover from a crash
or a power outage when operating in 'appraise' mode. 

This patch series adds two new IMA api functions to retrigger
the measurements as the files change. Synchronous variant
should be invoked from less performance sensitive locations
such as sync|msync|truncate where the user is expecting some
latency, and the asynchronous variant can be called from
performance sensitive locations such as direct write or mmio.

Asynchronous variant is mostly 'out of the way' on write hot
paths, each file write is only checking that we have a cmwq
work entry pending to re-calculate the file measurement later
on. Re-measurement latencies are build time tunables and the
latencies are automatically raised for very large files.

While this does not provide absolutely perfect tolerance to
system resets, for most reasonable embedded system workloads
it can be tuned to achieve really high measurement accurancy
with the measurements being accurate 99.9%+ of the day.

Janne Karhunen (5):
  integrity: keep the integrity state of open files up to date
  integrity: update the file measurement on truncate
  integrity: update the file measurement on write
  integrity: measure the file on sync
  integrity: measure the file on msync

 fs/namei.c                            |   5 +-
 fs/open.c                             |   3 +
 fs/read_write.c                       |  11 ++-
 fs/sync.c                             |   3 +
 include/linux/ima.h                   |  12 +++
 mm/msync.c                            |   7 ++
 security/integrity/ima/Kconfig        |  20 +++++
 security/integrity/ima/ima_appraise.c |   6 +-
 security/integrity/ima/ima_main.c     | 103 +++++++++++++++++++++++++-
 security/integrity/integrity.h        |   6 ++
 10 files changed, 171 insertions(+), 5 deletions(-)

-- 
2.17.1


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

* [PATCH 1/5] integrity: keep the integrity state of open files up to date
  2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
@ 2019-05-13 12:53 ` Janne Karhunen
  2019-05-13 12:53 ` [PATCH 2/5] integrity: update the file measurement on truncate Janne Karhunen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Konsta Karsisto

When a file is open for writing, kernel crash or power outage
is guaranteed to corrupt the inode integrity state leading to
file appraisal failure on the subsequent boot. Add some basic
infrastructure to keep the integrity measurements up to date
as the files are written to.

Core file operations (open, close, sync, msync, truncate) are
now allowed to update the measurement immediately. In order
to maintain sufficient write performance for writes, add a
latency tunable delayed work workqueue for computing the
re-measurements.

Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 include/linux/ima.h                   |  12 +++
 security/integrity/ima/Kconfig        |  20 +++++
 security/integrity/ima/ima_appraise.c |   6 +-
 security/integrity/ima/ima_main.c     | 103 +++++++++++++++++++++++++-
 security/integrity/integrity.h        |   6 ++
 5 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index dc12fbcf484c..c4e83f5c450a 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -20,6 +20,8 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask);
 extern void ima_post_create_tmpfile(struct inode *inode);
 extern void ima_file_free(struct file *file);
+extern void ima_file_update(struct file *file);
+extern void ima_delayed_update(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_load_data(enum kernel_load_data_id id);
 extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
@@ -66,6 +68,16 @@ static inline void ima_file_free(struct file *file)
 	return;
 }
 
+static inline void ima_file_update(struct file *file)
+{
+	return;
+}
+
+static inline void ima_delayed_update(struct file *file)
+{
+	return;
+}
+
 static inline int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	return 0;
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index a18f8c6d13b5..a2588d72cbc1 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -295,3 +295,23 @@ config IMA_APPRAISE_SIGNED_INIT
 	default n
 	help
 	   This option requires user-space init to be signed.
+
+config IMA_HASH_LATENCY
+	int
+	depends on IMA_APPRAISE
+	range 0 60000
+        default 50
+	help
+	   This value defines the re-measurement interval when files are
+	   being written. Value is in milliseconds.
+
+config IMA_HASH_LATENCY_CEILING
+	int
+	depends on IMA_APPRAISE
+	range 100 60000
+	default 30000
+	help
+	   In order to maintain high write performance for large files,
+	   IMA increases the re-measurement rate as the file size grows.
+	   This value defines the ceiling for the re-measurement delay
+	   in milliseconds.
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 5fb7127bbe68..9558ae0cc462 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -234,10 +234,14 @@ int ima_appraise_measurement(enum ima_hooks func,
 		status = INTEGRITY_NOLABEL;
 		if (file->f_mode & FMODE_CREATED)
 			iint->flags |= IMA_NEW_FILE;
+
 		if ((iint->flags & IMA_NEW_FILE) &&
 		    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
-		     (inode->i_size == 0)))
+		     (inode->i_size == 0))) {
+			ima_fix_xattr(dentry, iint);
+			xattr_len = ima_read_xattr(dentry, &xattr_value);
 			status = INTEGRITY_PASS;
+		}
 		goto out;
 	}
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 357edd140c09..67d41005cd22 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -16,7 +16,7 @@
  *
  * File: ima_main.c
  *	implements the IMA hooks: ima_bprm_check, ima_file_mmap,
- *	and ima_file_check.
+ *	ima_delayed_update, ima_file_update and ima_file_check.
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -30,6 +30,8 @@
 #include <linux/xattr.h>
 #include <linux/ima.h>
 #include <linux/iversion.h>
+#include <linux/workqueue.h>
+#include <linux/sizes.h>
 #include <linux/fs.h>
 
 #include "ima.h"
@@ -40,8 +42,15 @@ int ima_appraise = IMA_APPRAISE_ENFORCE;
 int ima_appraise;
 #endif
 
+#if CONFIG_IMA_HASH_LATENCY == 0
+#define IMA_LATENCY_INCREMENT 100
+#else
+#define IMA_LATENCY_INCREMENT CONFIG_IMA_HASH_LATENCY
+#endif
+
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
+static struct workqueue_struct *ima_update_wq;
 
 static int __init hash_setup(char *str)
 {
@@ -127,6 +136,7 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
 {
 	fmode_t mode = file->f_mode;
 	bool update;
+	bool creq;
 
 	if (!(mode & FMODE_WRITE))
 		return;
@@ -322,6 +332,17 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	return 0;
 }
 
+static void ima_delayed_update_handler(struct work_struct *work)
+{
+	struct ima_work_entry *entry;
+
+	entry = container_of(work, typeof(*entry), work.work);
+
+	ima_file_update(entry->file);
+	fput(entry->file);
+	entry->file = NULL;
+}
+
 /**
  * ima_file_mmap - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured (May be NULL)
@@ -375,6 +396,78 @@ int ima_bprm_check(struct linux_binprm *bprm)
 				   MAY_EXEC, CREDS_CHECK);
 }
 
+/**
+ * ima_delayed_update - add a file to delayed update list
+ * @file: pointer to file structure being updated
+ */
+void ima_delayed_update(struct file *file)
+{
+	struct inode *inode = file_inode(file);
+	struct integrity_iint_cache *iint;
+	unsigned long blocks;
+	unsigned long msecs;
+	bool creq;
+
+	iint = integrity_iint_find(inode);
+	if (!iint)
+		return;
+
+	if (iint->ima_work.file)
+		return;
+
+	/* Slow down the samping rate per the file size */
+	blocks = inode->i_size / SZ_1M + 1;
+	msecs = blocks * IMA_LATENCY_INCREMENT;
+	if (msecs > CONFIG_IMA_HASH_LATENCY_CEILING)
+		msecs = CONFIG_IMA_HASH_LATENCY_CEILING;
+
+	get_file(file);
+	iint->ima_work.file = file;
+	INIT_DELAYED_WORK(&iint->ima_work.work, ima_delayed_update_handler);
+
+	creq = queue_delayed_work(ima_update_wq,
+				  &iint->ima_work.work,
+				  msecs_to_jiffies(msecs));
+	if (creq == false) {
+		iint->ima_work.file = NULL;
+		fput(file);
+	}
+}
+EXPORT_SYMBOL_GPL(ima_delayed_update);
+
+/**
+ * ima_file_update - update the file measurement
+ * @file: pointer to file structure being updated
+ */
+void ima_file_update(struct file *file)
+{
+	struct inode *inode = file_inode(file);
+	struct integrity_iint_cache *iint;
+	bool should_measure = true;
+	u64 i_version;
+
+	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
+		return;
+
+	iint = integrity_iint_find(inode);
+	if (!iint)
+		return;
+
+	mutex_lock(&iint->mutex);
+	clear_bit(IMA_UPDATE_XATTR, &iint->atomic_flags);
+	if (IS_I_VERSION(inode)) {
+		i_version = inode_query_iversion(inode);
+		if (i_version == iint->version)
+			should_measure = false;
+	}
+	if (should_measure) {
+		iint->flags &= ~IMA_COLLECTED;
+		ima_update_xattr(iint, file);
+	}
+	mutex_unlock(&iint->mutex);
+}
+EXPORT_SYMBOL_GPL(ima_file_update);
+
 /**
  * ima_path_check - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured
@@ -580,6 +673,12 @@ static int __init init_ima(void)
 {
 	int error;
 
+	ima_update_wq = alloc_workqueue("ima-update-wq",
+					WQ_MEM_RECLAIM | WQ_CPU_INTENSIVE,
+					0);
+	if (!ima_update_wq)
+		return -ENOMEM;
+
 	ima_init_template_list();
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
 	error = ima_init();
@@ -595,6 +694,8 @@ static int __init init_ima(void)
 
 	if (!error)
 		ima_update_policy_flag();
+	else
+		destroy_workqueue(ima_update_wq);
 
 	return error;
 }
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 7de59f44cba3..860c7a475a24 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -113,6 +113,11 @@ struct signature_v2_hdr {
 	uint8_t sig[0];		/* signature payload */
 } __packed;
 
+struct ima_work_entry {
+	struct delayed_work work;
+	struct file *file;
+};
+
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
@@ -129,6 +134,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_creds_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;
+	struct ima_work_entry ima_work;
 };
 
 /* rbtree tree calls to lookup, insert, delete
-- 
2.17.1


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

* [PATCH 2/5] integrity: update the file measurement on truncate
  2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
  2019-05-13 12:53 ` [PATCH 1/5] integrity: keep the integrity state of open files up to date Janne Karhunen
@ 2019-05-13 12:53 ` Janne Karhunen
  2019-05-13 12:53 ` [PATCH 3/5] integrity: update the file measurement on write Janne Karhunen
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Konsta Karsisto

Let IMA know when a file is being opened with truncate
or truncated directly.

Depends on commit c8213962517e ("integrity: keep the integrity state of open files up to date")'

Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 fs/namei.c | 5 ++++-
 fs/open.c  | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index dede0147b3f6..31303063143b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3418,8 +3418,11 @@ static int do_last(struct nameidata *nd,
 		goto out;
 opened:
 	error = ima_file_check(file, op->acc_mode);
-	if (!error && will_truncate)
+	if (!error && will_truncate) {
 		error = handle_truncate(file);
+		if (!error)
+			ima_file_update(file);
+	}
 out:
 	if (unlikely(error > 0)) {
 		WARN_ON(1);
diff --git a/fs/open.c b/fs/open.c
index 0285ce7dbd51..a2771b787383 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -62,6 +62,9 @@ int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
 	/* Note any delegations or leases have already been broken: */
 	ret = notify_change(dentry, &newattrs, NULL);
 	inode_unlock(dentry->d_inode);
+
+	if (filp)
+		ima_file_update(filp);
 	return ret;
 }
 
-- 
2.17.1


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

* [PATCH 3/5] integrity: update the file measurement on write
  2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
  2019-05-13 12:53 ` [PATCH 1/5] integrity: keep the integrity state of open files up to date Janne Karhunen
  2019-05-13 12:53 ` [PATCH 2/5] integrity: update the file measurement on truncate Janne Karhunen
@ 2019-05-13 12:53 ` Janne Karhunen
  2019-05-13 12:53 ` [PATCH 4/5] integrity: measure the file on sync Janne Karhunen
  2019-05-13 12:53 ` [PATCH 5/5] integrity: measure the file on msync Janne Karhunen
  4 siblings, 0 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Konsta Karsisto

When a file is being written, mark the file for IMA for delayed
re-measurement.

Depends on commit c8213962517e ("integrity: keep the integrity state of open files up to date")'

Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 fs/read_write.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 177ccc3d405a..bfe10d6dc135 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -20,6 +20,7 @@
 #include <linux/compat.h>
 #include <linux/mount.h>
 #include <linux/fs.h>
+#include <linux/ima.h>
 #include "internal.h"
 
 #include <linux/uaccess.h>
@@ -481,12 +482,18 @@ static ssize_t new_sync_write(struct file *filp, const char __user *buf, size_t
 static ssize_t __vfs_write(struct file *file, const char __user *p,
 			   size_t count, loff_t *pos)
 {
+	ssize_t sz;
+
 	if (file->f_op->write)
-		return file->f_op->write(file, p, count, pos);
+		sz = file->f_op->write(file, p, count, pos);
 	else if (file->f_op->write_iter)
-		return new_sync_write(file, p, count, pos);
+		sz = new_sync_write(file, p, count, pos);
 	else
 		return -EINVAL;
+
+	if (sz >= 1)
+		ima_delayed_update(file);
+	return sz;
 }
 
 ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t *pos)
-- 
2.17.1


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

* [PATCH 4/5] integrity: measure the file on sync
  2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
                   ` (2 preceding siblings ...)
  2019-05-13 12:53 ` [PATCH 3/5] integrity: update the file measurement on write Janne Karhunen
@ 2019-05-13 12:53 ` Janne Karhunen
  2019-05-13 12:53 ` [PATCH 5/5] integrity: measure the file on msync Janne Karhunen
  4 siblings, 0 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Konsta Karsisto

When user requests a file sync, make sure that the IMA
measurement reflects the state of the data being sync'd.

Depends on commit c8213962517e ("integrity: keep the integrity state of open files up to date")'

Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 fs/sync.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/sync.c b/fs/sync.c
index b54e0541ad89..90353f9ed931 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -16,6 +16,7 @@
 #include <linux/pagemap.h>
 #include <linux/quotaops.h>
 #include <linux/backing-dev.h>
+#include <linux/ima.h>
 #include "internal.h"
 
 #define VALID_FLAGS (SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE| \
@@ -194,6 +195,8 @@ int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync)
 		return -EINVAL;
 	if (!datasync && (inode->i_state & I_DIRTY_TIME))
 		mark_inode_dirty_sync(inode);
+	ima_file_update(file);
+
 	return file->f_op->fsync(file, start, end, datasync);
 }
 EXPORT_SYMBOL(vfs_fsync_range);
-- 
2.17.1


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

* [PATCH 5/5] integrity: measure the file on msync
  2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
                   ` (3 preceding siblings ...)
  2019-05-13 12:53 ` [PATCH 4/5] integrity: measure the file on sync Janne Karhunen
@ 2019-05-13 12:53 ` Janne Karhunen
  4 siblings, 0 replies; 6+ messages in thread
From: Janne Karhunen @ 2019-05-13 12:53 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Konsta Karsisto

When user requests a file msync, make sure that the IMA
measurement reflects the state of the data being sync'd.

Depends on commit c8213962517e ("integrity: keep the integrity state of open files up to date")'

Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 mm/msync.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/msync.c b/mm/msync.c
index ef30a429623a..5a256e08b49d 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -14,6 +14,7 @@
 #include <linux/file.h>
 #include <linux/syscalls.h>
 #include <linux/sched.h>
+#include <linux/ima.h>
 
 /*
  * MS_SYNC syncs the entire file - including mappings.
@@ -88,12 +89,18 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 			get_file(file);
 			up_read(&mm->mmap_sem);
 			error = vfs_fsync_range(file, fstart, fend, 1);
+			if (!error)
+				ima_file_update(file);
+
 			fput(file);
 			if (error || start >= end)
 				goto out;
 			down_read(&mm->mmap_sem);
 			vma = find_vma(mm, start);
 		} else {
+			if (file)
+				ima_delayed_update(file);
+
 			if (start >= end) {
 				error = 0;
 				goto out_unlock;
-- 
2.17.1


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

end of thread, other threads:[~2019-05-13 12:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-13 12:53 [PATCH 0/5] integrity: improve ima measurement accuracy Janne Karhunen
2019-05-13 12:53 ` [PATCH 1/5] integrity: keep the integrity state of open files up to date Janne Karhunen
2019-05-13 12:53 ` [PATCH 2/5] integrity: update the file measurement on truncate Janne Karhunen
2019-05-13 12:53 ` [PATCH 3/5] integrity: update the file measurement on write Janne Karhunen
2019-05-13 12:53 ` [PATCH 4/5] integrity: measure the file on sync Janne Karhunen
2019-05-13 12:53 ` [PATCH 5/5] integrity: measure the file on msync Janne Karhunen

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).