diff mbox series

[v2,2/3] IMA: add policy support for restricting the accepted hash algorithms

Message ID 20210720092404.120172-3-simon.thoby@viveris.fr (mailing list archive)
State New, archived
Headers show
Series IMA: restrict the accepted digest algorithms | expand

Commit Message

THOBY Simon July 20, 2021, 9:25 a.m. UTC
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(-)

Comments

Mimi Zohar July 23, 2021, 11:36 a.m. UTC | #1
Hi Simon,

On Tue, 2021-07-20 at 09:25 +0000, THOBY Simon wrote:
> 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.

Right, in order for the patch set to be bisect safe, the order of
patches 2 & 3 should be reversed.  First implement the support, then
add the policy rule support.  Otherwise the policy rules could be
processed, but not enforced.

thanks,

Mimi

> 
> 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>
diff mbox series

Patch

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