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 <[email protected]>
---

* v2
  - No change

 security/integrity/ima/ima_policy.c | 36 +++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index e458cd47c099..166124d67774 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -973,6 +973,39 @@ static void check_template_modsig(const struct 
ima_template_desc *template)
 #undef MSG
 }
 
+static bool ima_validate_rule(struct ima_rule_entry *entry)
+{
+       if (entry->action == UNKNOWN)
+               return false;
+
+       if (entry->flags & IMA_FUNC) {
+               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:
+                       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 +1183,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 +1388,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);
-- 
2.25.1

Reply via email to