[PATCH] IMA: add an option to restrict the accepted hash algorithms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry it took me some time before I could write a patch (in addition it's my
first kernel contribution so I'm still learning how things should be done, like
fighting Outlook on the web, giving up on it and moving to Mozilla Thunderbird).

This patch is a follow up on the discussion at
https://lore.kernel.org/linux-integrity/10dde047d76b447f32ca91356599be679b8a76e5.camel@xxxxxxxxxxxxx/t/#m0f8127c6982ef94aa42f5cc13ea83b9f9000917e.


This patch was built on top of linux-5.14.0-rc1, and I successfully tested the patch
in the following configurations:
- disabled: no visible impact (as expected)
- with ima_allowed_hashes=sha256,sha512:
	- alone: blocks both execution and xattr writes
	  (tested with `emvctl ima_hash -a md5 <my binary>`)
	- with ima_appraise=log: audit logs but still permits access
	- with ima_appraise=fix: audit logs but still permits access, however it
	  doesn't fix the hash (so maybe I should do something about it, because
	  right now it's basically the same as ima_appraise=log w.r.t. hash
	  algorithms ?)
	- with ima_accept_any_hash: permits access, no warnings whatsoever
Do you want ideas of other configurations that I could test?

I would also like to point out that I'm more than open to suggestions for
changing the names of the parameters (`ima_allowed_hashes` and
`ima_accept_any_hash`) and the "cause" in the audit log (currently 
forbidden-hash-algorithm"), because as you know, "naming things is hard".


IMA: add an option to restrict the accepted hash algorithms

Adds two command-line parameters to limit the hash algorithms
allowed for the security.ima xattr. This gives users the
ability to ensure their systems will not accept weak hashes,
and potentially increase users trust in their IMA configuration,
because they can ensure only strong collision-resistant hashes
are employed and files generated otherwise will not be accepted.

The main point is to safeguard users from mislabelling their files
when using userland utilities to update their files, e.g. evmctl
(`evmctl ima_hash` defaults to sha1). Another possible target
would be people that have deployed IMA years ago, possibly using
algorithms that was then deemed sufficiently collision-resistant,
but that proved to be weak with the passage of time (note that this
could also happen in the future with algorithms considered safe today).
This patch also provides a migration path for users.

The first parameter supplies a list of allowed algorithms to
the kernel, restricting appraisal to files hashed with "strong"
hash algorithms (by default IMA will keep accepting any hash
algorithm, as enabling such a feature by default would both be
backward-incompatible and very probably break real systems).
The second parameter is an escape hatch that adds a weaker form
of backward-compatibility: when activated, IMA apparaisal will
keep working on files hashed with an algorithm not present in
the list, but updates to these files or new writes to the security.ima
xattr will enforce the selected hash algorithms. This may be useful
to perform an online migration from one algorithm to another.


Signed-off-by: Simon Thoby <simon.thoby@xxxxxxxxxx>
---
 .../admin-guide/kernel-parameters.txt         | 42 +++++++++
 security/integrity/ima/ima.h                  |  6 +-
 security/integrity/ima/ima_appraise.c         | 22 +++++
 security/integrity/ima/ima_main.c             | 88 ++++++++++++++++++-
 4 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..af7567b3a44e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1817,6 +1817,48 @@
 			The list of supported hash algorithms is defined
 			in crypto/hash_info.h.
 
+	ima_allowed_hashes= [IMA]
+			Format: comma-separated list of { md5 | sha1 | rmd160
+				| sha256 | sha384 | sha512 | ... }
+
+			When using IMA in hashing mode, this attribute specifies the
+			list of hash algorithms that are allowed in the security.ima
+			xattr. Files hashed with an algorithm not present in that
+			list will not pass appraisal, and writes of invalid xattrs
+			are disallowed too.
+
+			Use this option to ensure only strong and collision-resistant
+			hashes are used and accepted by the kernel, instead of the
+			default that will happily appraise files hashed with any of
+			the numerous hash algorithms the kenrel supports.
+
+			See also "ima_accept_any_hash" to only restrict writes.
+			The list of supported hash algorithms is defined
+			in crypto/hash_info.h.
+
+	ima_accept_any_hash [IMA]
+
+			Accept for appraisal files with an IMA hash not allowed
+			in "ima_allowed_hashes", to allow iterative migrations
+			of your files to stronger algorithms.
+
+			When using IMA in hashing mode with "ima_allowed_hashes"
+			selected, the kernel restricts the hash algorithms
+			considered valid, and prevent both writes of files with
+			invalid xattrs and execution/read/... (depending on
+			the IMA policy) of such files.
+
+			This option changes that behavior to be backward compatible:
+			the kernel accept for appraisal files hashed with algorithms
+			not present in the list, but still enforces the algorithms
+			on write.
+			This may be useful for migrating to a new hash algorithm:
+			first changing ima_allowed_hashed and adding the ima_accept_any_hash
+			parameter to keep accepting the old files, rebooting and
+			relabelling all the files on the system, and finally
+			rebooting again without the ima_accept_any_hash parameter
+			to enforce the new policy.
+
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
 			Format: "tcb | appraise_tcb | secure_boot |
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f0e448ed1f9f..ef173fcb7112 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -21,6 +21,7 @@
 #include <linux/tpm.h>
 #include <linux/audit.h>
 #include <crypto/hash_info.h>
+#include <stdbool.h>
 
 #include "../integrity.h"
 
@@ -47,7 +48,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 extern int ima_policy_flag;
 
 /* set during initialization */
-extern int ima_hash_algo;
+extern int ima_hash_algo __ro_after_init;
+extern unsigned int ima_allowed_hashes __ro_after_init;
+extern bool ima_accept_any_hash __ro_after_init;
 extern int ima_sha1_idx __ro_after_init;
 extern int ima_hash_algo_idx __ro_after_init;
 extern int ima_extra_slots __ro_after_init;
@@ -164,6 +167,7 @@ void ima_init_template_list(void);
 int __init ima_init_digests(void);
 int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
 			  void *lsm_data);
+int ima_validate_hash_algorithm(enum hash_algo hash_alg, bool xattr_update);
 
 /*
  * used to protect h_table and sha_table
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index ef9dcfce45d4..ecbfd644dca8 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -592,6 +592,28 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
 		digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
 	}
 	if (result == 1 || evm_revalidate_status(xattr_name)) {
+		enum hash_algo hash_alg =
+			ima_get_hash_algo((struct evm_ima_xattr_data *)xattr_value,
+					  xattr_value_len);
+
+		/* Ensure the user-supplied xattr value uses a whitelisted hash algorithm */
+		int rc = ima_validate_hash_algorithm(hash_alg, true);
+		if (rc != 0) {
+			char *path = NULL;
+			char *pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
+			/* no memory available ? no file path for you */
+			if (pathbuf)
+				path = dentry_path(dentry, pathbuf, PATH_MAX);
+
+			integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry),
+				path, "collect_data", "forbidden-hash-algorithm", rc, 0);
+
+			if (pathbuf)
+				kfree(pathbuf);
+
+			return rc;
+		}
+
 		ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
 		if (result == 1)
 			result = 0;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 287b90509006..4206f2459bc1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -38,6 +38,17 @@ int ima_appraise;
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
 
+unsigned int ima_allowed_hashes;
+static_assert(
+	8 * sizeof(ima_allowed_hashes) >= HASH_ALGO__LAST,
+	"The bitfield ima_allowed_hashes is too small to contain all the supported hash algorithms");
+static char *ima_allowed_hashes_cmdline __initdata;
+core_param(ima_allowed_hashes, ima_allowed_hashes_cmdline, charp, 0);
+
+bool ima_accept_any_hash;
+core_param(ima_accept_any_hash, ima_accept_any_hash, bool, 0);
+
+
 static struct notifier_block ima_lsm_policy_notifier = {
 	.notifier_call = ima_lsm_policy_change,
 };
@@ -76,6 +87,46 @@ static int __init hash_setup(char *str)
 }
 __setup("ima_hash=", hash_setup);
 
+static void __init hash_restrictions_setup(void)
+{
+	char *str_copy, *iter;
+
+	if (!ima_allowed_hashes_cmdline)
+		return;
+
+	pr_info("restricting allowed hash algorithms to %s", ima_allowed_hashes_cmdline);
+
+	/* copy the string to perform strsep() on it */
+	str_copy = kmalloc(strlen(ima_allowed_hashes_cmdline) + 1, GFP_KERNEL);
+	if (!str_copy) {
+		pr_err("couldn't allocate memory for parsing the list of allowed hashes");
+		return;
+	}
+
+	memcpy(str_copy, ima_allowed_hashes_cmdline, strlen(ima_allowed_hashes_cmdline) + 1);
+
+	iter = str_copy;
+	do {
+		int idx;
+		char *token = strsep(&iter, ",");
+
+		if (token == NULL)
+			break;
+
+		idx = match_string(hash_algo_name, HASH_ALGO__LAST, token);
+		if (idx < 0) {
+			pr_err("invalid hash algorithm \"%s\", ignoring",
+			       token);
+			continue;
+		}
+
+		/* Add the hash algorithm to the 'allowed' bitfield */
+		ima_allowed_hashes |= (1U << idx);
+	} while (iter != NULL);
+
+	kfree(str_copy);
+}
+
 /* Prevent mmap'ing a file execute that is already mmap'ed write */
 static int mmap_violation_check(enum ima_hooks func, struct file *file,
 				char **pathbuf, const char **pathname,
@@ -172,6 +223,29 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
 	mutex_unlock(&iint->mutex);
 }
 
+/**
+ * ima_validate_hash_algorithm
+ * @hash_alg: the requested hash algorithm
+ * @attr_update: true if the user is trying to write the security.ima
+ * xattr with 'hash_alg' format. False means we are performing appraisal
+ * against a file that possesses a xattr hashed with 'hash_alg'.
+ *
+ * Ensure that the digest was generated using an allowed algorithm
+ */
+int ima_validate_hash_algorithm(enum hash_algo hash_alg, bool xattr_update)
+{
+	/* when reading/executing a file in "permissive" mode, accept any hash */
+	if (ima_accept_any_hash && !xattr_update)
+		return 0;
+
+	if (ima_allowed_hashes != 0 &&
+	    unlikely(!(ima_allowed_hashes & (1U << hash_alg)))) {
+		return -EACCES;
+	}
+
+	return 0;
+}
+
 /**
  * ima_file_free - called on __fput()
  * @file: pointer to file structure being freed
@@ -327,11 +401,22 @@ static int process_measurement(struct file *file, const struct cred *cred,
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
+	rc = ima_validate_hash_algorithm(hash_algo, false);
+	if (rc != 0) {
+		if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
+			pathname = ima_d_path(&file->f_path, &pathbuf, filename);
+
+		integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file),
+			pathname, "collect_data", "forbidden-hash-algorithm", rc, 0);
+
+		goto out_locked;
+	}
+
 	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
-	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
+	if (!pathbuf)	/* ima_rdwr_violation or hash_algs checks above possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
 	if (action & IMA_MEASURE)
@@ -993,6 +1078,7 @@ static int __init init_ima(void)
 	ima_appraise_parse_cmdline();
 	ima_init_template_list();
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
+	hash_restrictions_setup();
 	error = ima_init();
 
 	if (error && strcmp(hash_algo_name[ima_hash_algo],
-- 
2.31.1




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux