Thread (32 messages) 32 messages, 3 authors, 2020-06-25

Re: [PATCH 05/12] ima: Fail rule parsing when buffer hook functions have an invalid action

From: Mimi Zohar <zohar@linux.ibm.com>
Date: 2020-06-25 21:52:09
Also in: linux-integrity, lkml

On Mon, 2020-06-22 at 19:32 -0500, Tyler Hicks wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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 ee5152ecd3d9..ecc234b956a2 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -979,6 +979,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;
+}
+
Good idea.  There are a couple of other examples that could be cleaned
up as well.  For example, for performance reasons
"appraise_flag=check_blacklist" is limited to files with appended
signatures, like kernel modules and the kexec kernel image
(OpenPower).

Mimi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help