Linux-audit Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] IMA: Add audit log for failure conditions
@ 2020-06-07 22:14 Lakshmi Ramasubramanian
  2020-06-08 11:52 ` Mimi Zohar
  0 siblings, 1 reply; 3+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-06-07 22:14 UTC (permalink / raw)
  To: zohar, paul; +Cc: linux-integrity, linux-audit, linux-kernel

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.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima_main.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_queue_keys.c |  4 ++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 800fb3bba418..1225198fceb1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -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;
 }
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index cb3e3f501593..fa606ce68f87 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -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;
 	}
-- 
2.27.0


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] IMA: Add audit log for failure conditions
  2020-06-07 22:14 [PATCH v2] IMA: Add audit log for failure conditions Lakshmi Ramasubramanian
@ 2020-06-08 11:52 ` Mimi Zohar
  2020-06-08 21:45   ` Paul Moore
  0 siblings, 1 reply; 3+ messages in thread
From: Mimi Zohar @ 2020-06-08 11:52 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, paul; +Cc: linux-integrity, linux-audit, linux-kernel

Hi Lakshmi,

On Sun, 2020-06-07 at 15:14 -0700, Lakshmi Ramasubramanian wrote:
> 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.
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

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

> ---
>  security/integrity/ima/ima_main.c       | 17 ++++++++++++-----
>  security/integrity/ima/ima_queue_keys.c |  4 ++++
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 800fb3bba418..1225198fceb1 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -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;
>  }
> diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
> index cb3e3f501593..fa606ce68f87 100644
> --- a/security/integrity/ima/ima_queue_keys.c
> +++ b/security/integrity/ima/ima_queue_keys.c
> @@ -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;
>  	}


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] IMA: Add audit log for failure conditions
  2020-06-08 11:52 ` Mimi Zohar
@ 2020-06-08 21:45   ` Paul Moore
  0 siblings, 0 replies; 3+ messages in thread
From: Paul Moore @ 2020-06-08 21:45 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Lakshmi Ramasubramanian, linux-integrity, linux-audit, linux-kernel

On Mon, Jun 8, 2020 at 7:52 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> Hi Lakshmi,
>
> On Sun, 2020-06-07 at 15:14 -0700, Lakshmi Ramasubramanian wrote:
> > 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.
> >
> > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>
> 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.

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

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-07 22:14 [PATCH v2] IMA: Add audit log for failure conditions Lakshmi Ramasubramanian
2020-06-08 11:52 ` Mimi Zohar
2020-06-08 21:45   ` Paul Moore

Linux-audit Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-audit/0 linux-audit/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-audit linux-audit/ https://lore.kernel.org/linux-audit \
		linux-audit@redhat.com
	public-inbox-index linux-audit

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.redhat.linux-audit


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git