Skip to contentSkip to conversation

IMA: Add audit log for failure conditions

PATCH Superseded Lakshmi Ramasubramanian 3 participants 3 messages Jun 7, 2020 – Jun 8, 2020

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 IMA: Add audit log for failure conditions

The final log statement in process_buffer_measurement() for failure condition is at debug level. This does not log the message unless the system log level is raised which would significantly increase the messages in the system log. Change this log message to an audit message for better triaging failures in the function.

ima_alloc_key_entry() does not log a message for failure condition. Add an audit message for failure condition in this function.

---
 security/integrity/ima/ima_main.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_queue_keys.c |  4 ++++
 2 files changed, 16 insertions(+), 5 deletions(-)
security/integrity/ima/ima_main.c +12 -5
index 800fb3bba418..1225198fceb1 100644
@@ -739,6 +739,7 @@ void process_buffer_measurement(const void *buf, int size,
 				int pcr, const char *keyring)
 {
 	int ret = 0;
+	const char *audit_cause = "ENOMEM";
 	struct ima_template_entry *entry = NULL;
 	struct integrity_iint_cache iint = {};
 	struct ima_event_data event_data = {.iint = &iint,
@@ -793,21 +794,27 @@ void process_buffer_measurement(const void *buf, int size,
 	iint.ima_hash->length = hash_digest_size[ima_hash_algo];
 
 	ret = ima_calc_buffer_hash(buf, size, iint.ima_hash);
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "calc_buffer_hash";
 		goto out;
+	}
 
 	ret = ima_alloc_init_template(&event_data, &entry, template);
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "alloc_init_template";
 		goto out;
+	}
 
 	ret = ima_store_template(entry, violation, NULL, buf, pcr);
-
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "store_template";
 		ima_free_template_entry(entry);
+	}
 
 out:
 	if (ret < 0)
-		pr_devel("%s: failed, result: %d\n", __func__, ret);
+		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, eventname,
+				    __func__, audit_cause, ret, 0);
 
 	return;
 }
security/integrity/ima/ima_queue_keys.c +4 -0
index cb3e3f501593..fa606ce68f87 100644
@@ -68,6 +68,7 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 						 size_t payload_len)
 {
 	int rc = 0;
+	const char *audit_cause = "ENOMEM";
 	struct ima_key_entry *entry;
 
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
@@ -88,6 +89,9 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 
 out:
 	if (rc) {
+		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL,
+				    keyring->description, __func__,
+				    audit_cause, rc, 0);
 		ima_free_key_entry(entry);
 		entry = NULL;
 	}
Signature · 1 line
2.27.0
maintainer
#1 lore raw

Hi Lakshmi,

Quoted 10 lines · Lakshmi Ramasubramanian

The final log statement in process_buffer_measurement() for failure condition is at debug level. This does not log the message unless the system log level is raised which would significantly increase the messages in the system log. Change this log message to an audit message for better triaging failures in the function.

ima_alloc_key_entry() does not log a message for failure condition. Add an audit message for failure condition in this function.

Audit messages should be at a higher level.  For example, "hashing_error", "collect_data", "invalid_pcr".  In the "invalid_pcr" case, the audit log contains the reason - "ToMToU" or "open_writers" - as to why the measurement list doesn't match the PCR.

Here you would want "measuring_keys", "measuring_boot_cmdline" with the reason it failed, not the function name "process_buffer_measurement".

Userspace needs to be aware of the new audit messages.  Maybe include samples of them in the cover letter.

thanks,

Mimi

Quoted 72 lines
---
 security/integrity/ima/ima_main.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_queue_keys.c |  4 ++++
 2 files changed, 16 insertions(+), 5 deletions(-)
security/integrity/ima/ima_main.c +12 -5
index 800fb3bba418..1225198fceb1 100644
@@ -739,6 +739,7 @@ void process_buffer_measurement(const void *buf, int size,
 				int pcr, const char *keyring)
 {
 	int ret = 0;
+	const char *audit_cause = "ENOMEM";
 	struct ima_template_entry *entry = NULL;
 	struct integrity_iint_cache iint = {};
 	struct ima_event_data event_data = {.iint = &iint,
@@ -793,21 +794,27 @@ void process_buffer_measurement(const void *buf, int size,
 	iint.ima_hash->length = hash_digest_size[ima_hash_algo];
 
 	ret = ima_calc_buffer_hash(buf, size, iint.ima_hash);
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "calc_buffer_hash";
 		goto out;
+	}
 
 	ret = ima_alloc_init_template(&event_data, &entry, template);
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "alloc_init_template";
 		goto out;
+	}
 
 	ret = ima_store_template(entry, violation, NULL, buf, pcr);
-
-	if (ret < 0)
+	if (ret < 0) {
+		audit_cause = "store_template";
 		ima_free_template_entry(entry);
+	}
 
 out:
 	if (ret < 0)
-		pr_devel("%s: failed, result: %d\n", __func__, ret);
+		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, eventname,
+				    __func__, audit_cause, ret, 0);
 
 	return;
 }
security/integrity/ima/ima_queue_keys.c +4 -0
index cb3e3f501593..fa606ce68f87 100644
@@ -68,6 +68,7 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 						 size_t payload_len)
 {
 	int rc = 0;
+	const char *audit_cause = "ENOMEM";
 	struct ima_key_entry *entry;
 
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
@@ -88,6 +89,9 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 
 out:
 	if (rc) {
+		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL,
+				    keyring->description, __func__,
+				    audit_cause, rc, 0);
 		ima_free_key_entry(entry);
 		entry = NULL;
 	}
maintainer
#2 lore raw
Quoted 19 lines · Mimi Zohar

Hi Lakshmi,

Quoted 10 lines · Lakshmi Ramasubramanian

The final log statement in process_buffer_measurement() for failure condition is at debug level. This does not log the message unless the system log level is raised which would significantly increase the messages in the system log. Change this log message to an audit message for better triaging failures in the function.

ima_alloc_key_entry() does not log a message for failure condition. Add an audit message for failure condition in this function.

Audit messages should be at a higher level. For example, "hashing_error", "collect_data", "invalid_pcr". In the "invalid_pcr" case, the audit log contains the reason - "ToMToU" or "open_writers" - as to why the measurement list doesn't match the PCR.

Quoted 6 lines

Here you would want "measuring_keys", "measuring_boot_cmdline" with the reason it failed, not the function name "process_buffer_measurement".

Userspace needs to be aware of the new audit messages. Maybe include samples of them in the cover letter.

Yes, examples of the audit record in the commit description (the cover letter isn't recorded in the git log), are encouraged.

Signature · 2 lines
paul moore
www.paul-moore.com