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 v2 2/3] IMA: add policy support for restricting the accepted hash algorithms
Date: Tue, 20 Jul 2021 09:25:24 +0000	[thread overview]
Message-ID: <20210720092404.120172-3-simon.thoby@viveris.fr> (raw)
In-Reply-To: <20210720092404.120172-1-simon.thoby@viveris.fr>

This patch defines a new IMA policy rule option "appraise_hash=",
that restricts the hash algorithms accepted for the extended attribute
security.ima when appraising.

This patch is *not* self-contained, as it plugs in the support for
parsing the parameter and showing it to the user, but it doesn't enforce
the hashes yet, this will come in a subsequent patch.

Here is an example of a valid rule that enforces the use of sha256 or
sha512 when appraising iptables binaries:
  appraise func=BPRM_CHECK obj_type=iptables_exec_t appraise_type=imasig appraise_hash=sha256,sha512

This do not apply only to IMA in hash mode, it also works with digital
signatures, in which case it requires the hash (which was then signed
by a trusted private key) to have been generated with one of the
whitelisted algorithms.

Signed-off-by: Simon Thoby <simon.thoby@viveris.fr>
---
 Documentation/ABI/testing/ima_policy |  6 +-
 security/integrity/ima/ima.h         |  2 +-
 security/integrity/ima/ima_policy.c  | 82 +++++++++++++++++++++++++++-
 3 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 070779e8d836..365e4c91719e 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -27,7 +27,7 @@ Description:
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
-				[appraise_flag=] [keyrings=]
+				[appraise_flag=] [keyrings=] [appraise_hash=]
 		  base:
 			func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 			        [FIRMWARE_CHECK]
@@ -55,6 +55,10 @@ Description:
 			label:= [selinux]|[kernel_info]|[data_label]
 			data_label:= a unique string used for grouping and limiting critical data.
 			For example, "selinux" to measure critical data for SELinux.
+			appraise_hash:= comma-separated list of hash algorithms
+			For example, "sha256,sha512" to only accept to appraise
+			files where the security.ima xattr was hashed with one
+			of these two algorithms.
 
 		  default policy:
 			# PROC_SUPER_MAGIC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f0e448ed1f9f..049748e3fe9b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -47,7 +47,7 @@ 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 int ima_sha1_idx __ro_after_init;
 extern int ima_hash_algo_idx __ro_after_init;
 extern int ima_extra_slots __ro_after_init;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd5d46e511f1..1b6c00baa397 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -35,6 +35,7 @@
 #define IMA_FSNAME	0x0200
 #define IMA_KEYRINGS	0x0400
 #define IMA_LABEL	0x0800
+#define IMA_VALIDATE_HASH	0x1000
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -79,6 +80,7 @@ struct ima_rule_entry {
 	bool (*uid_op)(kuid_t, kuid_t);    /* Handlers for operators       */
 	bool (*fowner_op)(kuid_t, kuid_t); /* uid_eq(), uid_gt(), uid_lt() */
 	int pcr;
+	unsigned int allowed_hashes;
 	struct {
 		void *rule;	/* LSM file metadata specific */
 		char *args_p;	/* audit value */
@@ -90,6 +92,15 @@ struct ima_rule_entry {
 	struct ima_template_desc *template;
 };
 
+
+/**
+ * sanity check in case the kernels gains more hash algorithms that can
+ * fit in un unsigned int
+ */
+static_assert(
+	8 * sizeof(unsigned int) >= HASH_ALGO__LAST,
+	"The bitfields allowed_hashes in ruleset are too small to contain all the supported hash algorithms, consider changing its type");
+
 /*
  * Without LSM specific knowledge, the default policy can only be
  * written in terms of .action, .func, .mask, .fsmagic, .uid, and .fowner
@@ -948,7 +959,7 @@ enum {
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_appraise_flag,
 	Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
-	Opt_label, Opt_err
+	Opt_label, Opt_appraise_hash, Opt_err
 };
 
 static const match_table_t policy_tokens = {
@@ -986,6 +997,7 @@ static const match_table_t policy_tokens = {
 	{Opt_template, "template=%s"},
 	{Opt_keyrings, "keyrings=%s"},
 	{Opt_label, "label=%s"},
+	{Opt_appraise_hash, "appraise_hash=%s"},
 	{Opt_err, NULL}
 };
 
@@ -1111,7 +1123,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
 				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
-				     IMA_PERMIT_DIRECTIO))
+				     IMA_PERMIT_DIRECTIO | IMA_VALIDATE_HASH))
 			return false;
 
 		break;
@@ -1123,7 +1135,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
 				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
 				     IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
-				     IMA_CHECK_BLACKLIST))
+				     IMA_CHECK_BLACKLIST | IMA_VALIDATE_HASH))
 			return false;
 
 		break;
@@ -1173,6 +1185,27 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 	return true;
 }
 
+static unsigned int ima_parse_appraise_hash(char *arg)
+{
+	unsigned int res = 0;
+	char *token;
+
+	while ((token = strsep(&arg, ",")) != NULL) {
+		int idx = match_string(hash_algo_name, HASH_ALGO__LAST, token);
+
+		if (idx < 0) {
+			pr_err("unknown hash algorithm \"%s\", ignoring",
+			       token);
+			continue;
+		}
+
+		/* Add the hash algorithm to the 'allowed' bitfield */
+		res |= (1U << idx);
+	}
+
+	return res;
+}
+
 static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 {
 	struct audit_buffer *ab;
@@ -1190,6 +1223,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 	entry->uid_op = &uid_eq;
 	entry->fowner_op = &uid_eq;
 	entry->action = UNKNOWN;
+	entry->allowed_hashes = 0;
 	while ((p = strsep(&rule, " \t")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
 		int token;
@@ -1542,6 +1576,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 						 &(template_desc->fields),
 						 &(template_desc->num_fields));
 			entry->template = template_desc;
+			break;
+		case Opt_appraise_hash:
+			ima_log_string(ab, "appraise_hash", args[0].from);
+
+			if (entry->allowed_hashes) {
+				result = -EINVAL;
+				break;
+			}
+
+			entry->allowed_hashes = ima_parse_appraise_hash(args[0].from);
+			if (!entry->allowed_hashes) {
+				result = -EINVAL;
+				break;
+			}
+
+			entry->flags |= IMA_VALIDATE_HASH;
+
 			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
@@ -1700,6 +1751,25 @@ static void ima_show_rule_opt_list(struct seq_file *m,
 		seq_printf(m, "%s%s", i ? "|" : "", opt_list->items[i]);
 }
 
+static void ima_policy_show_appraise_hash(struct seq_file *m,
+					  unsigned int allowed_hashes)
+{
+	int idx;
+	bool first = true;
+
+	for (idx = 0; idx < HASH_ALGO__LAST; idx++) {
+		if (!(allowed_hashes & (1U << idx)))
+			continue;
+
+		if (!first)
+			seq_puts(m, ",");
+		first = false;
+
+		seq_puts(m, hash_algo_name[idx]);
+	}
+
+}
+
 int ima_policy_show(struct seq_file *m, void *v)
 {
 	struct ima_rule_entry *entry = v;
@@ -1811,6 +1881,12 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, " ");
 	}
 
+	if (entry->flags & IMA_VALIDATE_HASH) {
+		seq_puts(m, "appraise_hash=");
+		ima_policy_show_appraise_hash(m, entry->allowed_hashes);
+		seq_puts(m, " ");
+	}
+
 	for (i = 0; i < MAX_LSM_RULES; i++) {
 		if (entry->lsm[i].rule) {
 			switch (i) {
-- 
2.31.1

  parent reply	other threads:[~2021-07-20  9:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-20  9:25 [PATCH v2 0/3] IMA: restrict the accepted digest algorithms THOBY Simon
2021-07-20  9:25 ` [PATCH v2 1/3] IMA: block writes of the security.ima xattr with weak hash algorithms THOBY Simon
2021-07-23 11:46   ` Mimi Zohar
2021-07-26  9:49     ` THOBY Simon
2021-07-26 16:02       ` Mimi Zohar
2021-07-20  9:25 ` THOBY Simon [this message]
2021-07-23 11:36   ` [PATCH v2 2/3] IMA: add policy support for restricting the accepted " Mimi Zohar
2021-07-20  9:25 ` [PATCH v2 3/3] IMA: honor the appraise_hash policy option THOBY Simon

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=20210720092404.120172-3-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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).