Re: [PATCH v6 4/5] IMA: add a policy option to restrict xattr hash algorithms on appraisal
From: Lakshmi Ramasubramanian <hidden>
Date: 2021-08-04 20:51:03
Hi Simon, On 8/4/2021 2:20 AM, THOBY Simon wrote:
Signed-off-by: Simon Thoby <redacted> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> --- Documentation/ABI/testing/ima_policy | 6 ++- security/integrity/ima/ima_policy.c | 75 ++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 5 deletions(-)
+static unsigned int ima_parse_appraise_hash(char *arg)
+{
+ unsigned int res = 0;
+ int idx;
+ char *token;
+
+ while ((token = strsep(&arg, ",")) != NULL) {
+ idx = match_string(hash_algo_name, HASH_ALGO__LAST, token);
+
+ if (idx < 0) {
+ pr_err("unknown hash algorithm \"%s\", ignoring",
+ token);
+ continue;Is it right to ignore an invalid digest algorithm given in the IMA policy rule? Should "invalid ima policy" error be reported instead? Other changes look good. Reviewed-by: Lakshmi Ramasubramanian <redacted> -lakshmi
quoted hunk ↗ jump to hunk
+ } + + /* 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;@@ -1522,6 +1546,26 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else result = -EINVAL; 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); + + /* invalid or empty list of algorithms */ + if (!entry->allowed_hashes) { + result = -EINVAL; + break; + } + + entry->flags |= IMA_VALIDATE_HASH; + + break; case Opt_permit_directio: entry->flags |= IMA_PERMIT_DIRECTIO; break;@@ -1714,6 +1758,23 @@ 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, list_size = 0; + + for (idx = 0; idx < HASH_ALGO__LAST; idx++) { + if (!(allowed_hashes & (1U << idx))) + continue; + + /* only add commas if the list contains multiple entries */ + if (list_size++) + seq_puts(m, ","); + + seq_puts(m, hash_algo_name[idx]); + } +} + int ima_policy_show(struct seq_file *m, void *v) { struct ima_rule_entry *entry = v;@@ -1825,6 +1886,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) {