linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: THOBY Simon <Simon.THOBY@viveris.fr>
To: "zohar@linux.ibm.com" <zohar@linux.ibm.com>,
	"dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	BARVAUX Didier <Didier.BARVAUX@viveris.fr>
Cc: THOBY Simon <Simon.THOBY@viveris.fr>
Subject: [PATCH v4 5/5] IMA: introduce a new policy option func=SETXATTR_CHECK
Date: Tue, 27 Jul 2021 16:33:36 +0000	[thread overview]
Message-ID: <20210727163330.790010-6-simon.thoby@viveris.fr> (raw)
In-Reply-To: <20210727163330.790010-1-simon.thoby@viveris.fr>

While users can restrict the accepted hash algorithms for the
security.ima xattr file signature when appraising said file, users
cannot restrict the algorithms that can be set on that attribute:
any algorithm built in the kernel is accepted on a write.

Define a new value for the ima policy option 'func' that restricts
globally the hash algorithms accepted when writing the security.ima
xattr.

When a policy contains a rule of the form
	appraise func=SETXATTR_CHECK appraise_hash=sha256,sha384,sha512
only values corresponding to one of these three digest algorithms
will be accepted for writing the security.ima xattr.
Attempting to write the attribute using another algorithm (or "free-form"
data) will be denied with an audit log message.
In the absence of such a policy rule, the default is still to only
accept hash algorithms built in the kernel (with all the limitations
that entails).

On policy update, the latest SETXATTR_CHECK rule is the only one
that apply, and other SETXATTR_CHECK rules are deleted.

Signed-off-by: Simon Thoby <simon.thoby@viveris.fr>
---
 Documentation/ABI/testing/ima_policy  |  9 +++-
 security/integrity/ima/ima.h          |  4 ++
 security/integrity/ima/ima_appraise.c | 33 ++++++++++--
 security/integrity/ima/ima_policy.c   | 75 +++++++++++++++++++++++++++
 4 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index aeb622698047..537be0e1720e 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -30,9 +30,10 @@ Description:
 				[appraise_flag=] [appraise_hash=] [keyrings=]
 		  base:
 			func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
-			        [FIRMWARE_CHECK]
+				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 				[KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA]
+				[SETXATTR_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
@@ -138,3 +139,9 @@ Description:
 		keys added to .builtin_trusted_keys or .ima keyring:
 
 			measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
+		Example of the special SETXATTR_CHECK appraise rule, that
+		restricts the hash algorithms allowed when writing to the
+		security.ima xattr of a file:
+
+			appraise func=SETXATTR_CHECK appraise_hash=sha256,sha384,sha512
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 7ef1b214d358..aeb3bf30c0f9 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -46,6 +46,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 /* current content of the policy */
 extern int ima_policy_flag;
 
+/* bitset of digests algorithms allowed in the setxattr hook */
+extern atomic_t ima_setxattr_allowed_hash_algorithms;
+
 /* set during initialization */
 extern int ima_hash_algo __ro_after_init;
 extern int ima_sha1_idx __ro_after_init;
@@ -198,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
 	hook(KEXEC_CMDLINE, kexec_cmdline)		\
 	hook(KEY_CHECK, key)				\
 	hook(CRITICAL_DATA, critical_data)		\
+	hook(SETXATTR_CHECK, setxattr_check)		\
 	hook(MAX_CHECK, none)
 
 #define __ima_hook_enumify(ENUM, str)	ENUM,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f751410930a5..b938a053366c 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -596,23 +596,46 @@ int ima_setxattr_validate_hash_alg(struct dentry *dentry,
 {
 	int res = -EACCES;
 	char *path = NULL, *pathbuf = NULL;
+	const char *errmsg = "unavailable-hash-algorithm";
 	enum hash_algo dentry_hash;
+	unsigned int allowed_hashes;
 
 	dentry_hash = ima_get_hash_algo((struct evm_ima_xattr_data *)xattr_value,
 				     xattr_value_len);
 
-	if (likely(dentry_hash == ima_hash_algo
-	    || crypto_has_alg(hash_algo_name[dentry_hash], 0, 0)))
-		return 0;
+	allowed_hashes = atomic_read(&ima_setxattr_allowed_hash_algorithms);
+
+	if (allowed_hashes) {
+		/* success if the algorithm is whitelisted in the ima policy */
+		if (allowed_hashes & (1U << dentry_hash))
+			return 0;
+
+		/*
+		 * We use a different audit message when the hash algorithm
+		 * is denied by a policy rule, instead of not being built
+		 * in the kernel image
+		 */
+		errmsg = "denied-hash-algorithm";
+	} else {
+		if (likely(dentry_hash == ima_hash_algo))
+			return 0;
+
+		/* allow any xattr using an algorithm built in the kernel */
+		if (crypto_has_alg(hash_algo_name[dentry_hash], 0, 0))
+			return 0;
+	}
 
 	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
 	/* no memory available ? no file path for you */
 	if (pathbuf)
 		path = dentry_path(dentry, pathbuf, PATH_MAX);
 
-	/* disallow xattr writes with algorithms not built in the kernel */
+	/*
+	 * disallow xattr writes with algorithms not built in the kernel or
+	 * denied by policy
+	 */
 	integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry),
-		path, "collect_data", "unavailable-hash-algorithm", res, 0);
+		path, "collect_data", errmsg, res, 0);
 
 	kfree(pathbuf);
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index aadd95753229..a74b78e1746e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -53,6 +53,8 @@ int ima_policy_flag;
 static int temp_ima_appraise;
 static int build_ima_appraise __ro_after_init;
 
+atomic_t ima_setxattr_allowed_hash_algorithms;
+
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
 	LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE
@@ -87,6 +89,7 @@ struct ima_rule_entry {
 		int type;	/* audit type */
 	} lsm[MAX_LSM_RULES];
 	char *fsname;
+	struct rcu_head rcu;
 	struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
 	struct ima_rule_opt_list *label; /* Measure data grouped under this label */
 	struct ima_template_desc *template;
@@ -368,6 +371,13 @@ static void ima_free_rule(struct ima_rule_entry *entry)
 	kfree(entry);
 }
 
+static void ima_free_rule_rcu(struct rcu_head *rcu)
+{
+	struct ima_rule_entry *entry = container_of(rcu, struct ima_rule_entry, rcu);
+
+	ima_free_rule(entry);
+}
+
 static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 {
 	struct ima_rule_entry *nentry;
@@ -903,6 +913,8 @@ void __init ima_init_policy(void)
 			  ARRAY_SIZE(critical_data_rules),
 			  IMA_DEFAULT_POLICY);
 
+	atomic_xchg(&ima_setxattr_allowed_hash_algorithms, 0);
+
 	ima_update_policy_flag();
 }
 
@@ -914,6 +926,52 @@ int ima_check_policy(void)
 	return 0;
 }
 
+/** ima_update_setxattr_allowed_hash_algorithms - cleanup SETXATTR_CHECK rules
+ * in the new ruleset
+ * @policy: the list of ima_rules_entry to clean
+ *
+ * Context: called when updating the IMA policy. Delete non-applicable
+ * rules with 'func' set to SETXATTR_CHECK and update the atomic variable
+ * to hold the list of allowed hash algorithms for the security.ima xattr.
+ *
+ * SETXATTR_CHECK rules do not implement a full policy check because of
+ * the performance impact performing rules checking on setxattr() would
+ * have. The consequence is that only one SETXATTR_CHECK can be active at
+ * a time. To prevent confusion, on policy updates, if a new SETXATTR_CHECK
+ * is defined, other SETXATTR_CHECK rules are remove from the ruleset.
+ */
+void ima_update_setxattr_allowed_hash_algorithms(struct list_head *policy)
+{
+	struct ima_rule_entry *entry, *tmp;
+	bool setxattr_check_already_defined = false;
+
+	rcu_read_lock();
+	list_for_each_entry_safe_reverse(entry, tmp, policy, list) {
+		if (entry->func != SETXATTR_CHECK)
+			continue;
+
+		if (setxattr_check_already_defined) {
+			/*
+			 * delete old SETXATTR_CHECK entries when a newer
+			 * one already exists
+			 */
+			list_del_rcu(&entry->list);
+			call_rcu(&entry->rcu, ima_free_rule_rcu);
+		} else {
+			/*
+			 * only the last entry with the SETXATTR_CHECK func
+			 * apply: this allows runtime upgrades of the
+			 * digest algorithm policy, unlike the other IMA
+			 * rules
+			 */
+			atomic_xchg(&ima_setxattr_allowed_hash_algorithms,
+				    entry->allowed_hashes);
+			setxattr_check_already_defined = true;
+		}
+	}
+	rcu_read_unlock();
+}
+
 /**
  * ima_update_policy - update default_rules with new measure rules
  *
@@ -931,6 +989,8 @@ void ima_update_policy(void)
 
 	list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu);
 
+	ima_update_setxattr_allowed_hash_algorithms(policy);
+
 	if (ima_rules != policy) {
 		ima_policy_flag = 0;
 		ima_rules = policy;
@@ -1176,6 +1236,19 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 		if (ima_rule_contains_lsm_cond(entry))
 			return false;
 
+		break;
+	case SETXATTR_CHECK:
+		/* any action other than APPRAISE is unsupported */
+		if (entry->action != APPRAISE)
+			return false;
+
+		/*
+		 * full policies are not supported, they would have too
+		 * much of a performance impact
+		 */
+		if (entry->flags & ~(IMA_FUNC | IMA_VALIDATE_HASH))
+			return false;
+
 		break;
 	default:
 		return false;
@@ -1333,6 +1406,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = KEY_CHECK;
 			else if (strcmp(args[0].from, "CRITICAL_DATA") == 0)
 				entry->func = CRITICAL_DATA;
+			else if (strcmp(args[0].from, "SETXATTR_CHECK") == 0)
+				entry->func = SETXATTR_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)
-- 
2.31.1

  parent reply	other threads:[~2021-07-27 16:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 16:33 [PATCH v4 0/5] IMA: restrict the accepted digest algorithms for THOBY Simon
2021-07-27 16:33 ` [PATCH v4 1/5] IMA: remove the dependency on CRYPTO_MD5 THOBY Simon
2021-07-27 17:57   ` Mimi Zohar
2021-07-27 16:33 ` [PATCH v4 2/5] IMA: block writes of the security.ima xattr with unsupported algorithms THOBY Simon
2021-07-27 20:32   ` Mimi Zohar
2021-07-28  7:00     ` THOBY Simon
2021-07-28 12:43       ` Mimi Zohar
2021-07-28 12:53         ` THOBY Simon
2021-07-28 13:09           ` Mimi Zohar
2021-07-27 16:33 ` [PATCH v4 3/5] IMA: add support to restrict the hash algorithms used for file appraisal THOBY Simon
2021-07-27 20:38   ` Mimi Zohar
2021-07-27 16:33 ` [PATCH v4 4/5] IMA: add a policy option to restrict xattr hash algorithms on appraisal THOBY Simon
2021-07-27 21:07   ` Mimi Zohar
2021-07-27 16:33 ` THOBY Simon [this message]
2021-07-27 17:25   ` [PATCH v4 5/5] IMA: introduce a new policy option func=SETXATTR_CHECK Mimi Zohar
2021-07-27 17:58     ` THOBY Simon
2021-07-27 17:47 ` [PATCH v4 0/5] IMA: restrict the accepted digest algorithms for Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210727163330.790010-6-simon.thoby@viveris.fr \
    --to=simon.thoby@viveris.fr \
    --cc=Didier.BARVAUX@viveris.fr \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=zohar@linux.ibm.com \
    --subject='Re: [PATCH v4 5/5] IMA: introduce a new policy option func=SETXATTR_CHECK' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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