ima: Fail rule parsing when buffer hook functions have an invalid action

Buffer based hook functions, such as KEXEC_CMDLINE and KEY_CHECK, can
only measure. The process_buffer_measurement() function quietly ignores
all actions except measure so make this behavior clear at the time of
policy load.

The parsing of the keyrings conditional had a check to ensure that it
was only specified with measure actions but the check should be on the
hook function and not the keyrings conditional since
"appraise func=KEY_CHECK" is not a valid rule.

Fixes: b0935123a183 ("IMA: Define a new hook to measure the kexec boot command line arguments")
Fixes: 5808611cccb2 ("IMA: Add KEY_CHECK func to measure keys")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e458cd4..40c28f1 100644 (file)
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -973,6 +973,43 @@ static void check_template_modsig(const struct ima_template_desc *template)
 #undef MSG
 }
 
+static bool ima_validate_rule(struct ima_rule_entry *entry)
+{
+       /* Ensure that the action is set */
+       if (entry->action == UNKNOWN)
+               return false;
+
+       /*
+        * Ensure that the hook function is compatible with the other
+        * components of the rule
+        */
+       switch (entry->func) {
+       case NONE:
+       case FILE_CHECK:
+       case MMAP_CHECK:
+       case BPRM_CHECK:
+       case CREDS_CHECK:
+       case POST_SETATTR:
+       case MODULE_CHECK:
+       case FIRMWARE_CHECK:
+       case KEXEC_KERNEL_CHECK:
+       case KEXEC_INITRAMFS_CHECK:
+       case POLICY_CHECK:
+               /* Validation of these hook functions is in ima_parse_rule() */
+               break;
+       case KEXEC_CMDLINE:
+       case KEY_CHECK:
+               if (entry->action & ~(MEASURE | DONT_MEASURE))
+                       return false;
+
+               break;
+       default:
+               return false;
+       }
+
+       return true;
+}
+
 static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 {
        struct audit_buffer *ab;
@@ -1150,7 +1187,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
                        keyrings_len = strlen(args[0].from) + 1;
 
                        if ((entry->keyrings) ||
-                           (entry->action != MEASURE) ||
                            (entry->func != KEY_CHECK) ||
                            (keyrings_len < 2)) {
                                result = -EINVAL;
@@ -1356,7 +1392,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
                        break;
                }
        }
-       if (!result && (entry->action == UNKNOWN))
+       if (!result && !ima_validate_rule(entry))
                result = -EINVAL;
        else if (entry->action == APPRAISE)
                temp_ima_appraise |= ima_appraise_flag(entry->func);