* Re: [PATCH] IMA: Add log statements for failure conditions [not found] ` <1591382782.5816.36.camel@linux.ibm.com> @ 2020-06-05 19:37 ` Paul Moore 2020-06-05 19:54 ` Lakshmi Ramasubramanian 0 siblings, 1 reply; 6+ messages in thread From: Paul Moore @ 2020-06-05 19:37 UTC (permalink / raw) To: Mimi Zohar Cc: tusharsu, Lakshmi Ramasubramanian, linux-integrity, linux-audit, linux-kernel On Fri, Jun 5, 2020 at 2:46 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > [Cc'ing Paul Moore] If it's audit related, it's generally best to CC the linux-audit list, not just me (fixed). It's not clear to me what this pr_err() is trying to indicate other than *something* failed. Can someone provide some more background on this message? > Hi Lakshmi, > > On Thu, 2020-06-04 at 09:32 -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 error level, > > and add eventname and ima_hooks enum to the message for better triaging > > failures in the function. > > > > ima_alloc_key_entry() does not log a message for failure condition. > > Add an error message for failure condition in this function. > > > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > > These messages should probably be turned into audit messages. Look at > integerity_audit_msg(). > > Mimi > > > --- > > security/integrity/ima/ima_main.c | 3 ++- > > security/integrity/ima/ima_queue_keys.c | 2 ++ > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > > index 9d0abedeae77..3b371f31597b 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -756,7 +756,8 @@ void process_buffer_measurement(const void *buf, int size, > > > > out: > > if (ret < 0) > > - pr_devel("%s: failed, result: %d\n", __func__, ret); > > + pr_err("%s failed. eventname: %s, func: %d, result: %d\n", > > + __func__, eventname, func, ret); > > > > return; > > } > > diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c > > index cb3e3f501593..e51d0eb08d8a 100644 > > --- a/security/integrity/ima/ima_queue_keys.c > > +++ b/security/integrity/ima/ima_queue_keys.c > > @@ -88,6 +88,8 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, > > > > out: > > if (rc) { > > + pr_err("%s failed. keyring: %s, result: %d\n", > > + __func__, keyring->description, rc); > > ima_free_key_entry(entry); > > entry = NULL; > > } > -- 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] 6+ messages in thread
* Re: [PATCH] IMA: Add log statements for failure conditions 2020-06-05 19:37 ` [PATCH] IMA: Add log statements for failure conditions Paul Moore @ 2020-06-05 19:54 ` Lakshmi Ramasubramanian 2020-06-05 20:49 ` Paul Moore 0 siblings, 1 reply; 6+ messages in thread From: Lakshmi Ramasubramanian @ 2020-06-05 19:54 UTC (permalink / raw) To: Paul Moore, Mimi Zohar Cc: tusharsu, linux-integrity, linux-audit, linux-kernel On 6/5/20 12:37 PM, Paul Moore wrote: > If it's audit related, it's generally best to CC the linux-audit list, > not just me (fixed). > > It's not clear to me what this pr_err() is trying to indicate other > than *something* failed. Can someone provide some more background on > this message? process_buffer_measurement() is currently used to measure "kexec command line", "keys", and "blacklist-hash". If there was any error in the measurement, this pr_err() will indicate which of the above measurement failed and the related error code. Please let me know if you need more info on this one. Since a pr_xyz() call was already present, I just wanted to change the log level to keep the code change to the minimum. But if audit log is the right approach for this case, I'll update. thanks, -lakshmi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IMA: Add log statements for failure conditions 2020-06-05 19:54 ` Lakshmi Ramasubramanian @ 2020-06-05 20:49 ` Paul Moore 2020-06-05 21:09 ` Lakshmi Ramasubramanian 0 siblings, 1 reply; 6+ messages in thread From: Paul Moore @ 2020-06-05 20:49 UTC (permalink / raw) To: Lakshmi Ramasubramanian Cc: tusharsu, linux-integrity, linux-audit, linux-kernel, Mimi Zohar On Fri, Jun 5, 2020 at 3:54 PM Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote: > On 6/5/20 12:37 PM, Paul Moore wrote: > > > If it's audit related, it's generally best to CC the linux-audit list, > > not just me (fixed). > > > > It's not clear to me what this pr_err() is trying to indicate other > > than *something* failed. Can someone provide some more background on > > this message? > > process_buffer_measurement() is currently used to measure > "kexec command line", "keys", and "blacklist-hash". If there was any > error in the measurement, this pr_err() will indicate which of the above > measurement failed and the related error code. > > Please let me know if you need more info on this one. That helps, thank you. > Since a pr_xyz() call was already present, I just wanted to change the > log level to keep the code change to the minimum. But if audit log is > the right approach for this case, I'll update. Generally we reserve audit for things that are required for various security certifications and/or "security relevant". From what you mentioned above, it seems like this would fall into the second category if not the first. Looking at your patch it doesn't look like you are trying to record anything special so you may be able to use the existing integrity_audit_msg(...) helper. Of course then the question comes down to the audit record type (the audit_msgno argument), the operation (op), and the comm/cause (cause). Do you feel that any of the existing audit record types are a good fit for this? -- 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] 6+ messages in thread
* Re: [PATCH] IMA: Add log statements for failure conditions 2020-06-05 20:49 ` Paul Moore @ 2020-06-05 21:09 ` Lakshmi Ramasubramanian 2020-06-05 21:34 ` Mimi Zohar 0 siblings, 1 reply; 6+ messages in thread From: Lakshmi Ramasubramanian @ 2020-06-05 21:09 UTC (permalink / raw) To: Paul Moore Cc: tusharsu, linux-integrity, linux-audit, linux-kernel, Mimi Zohar On 6/5/20 1:49 PM, Paul Moore wrote: > >> Since a pr_xyz() call was already present, I just wanted to change the >> log level to keep the code change to the minimum. But if audit log is >> the right approach for this case, I'll update. > > Generally we reserve audit for things that are required for various > security certifications and/or "security relevant". From what you > mentioned above, it seems like this would fall into the second > category if not the first. > > Looking at your patch it doesn't look like you are trying to record > anything special so you may be able to use the existing > integrity_audit_msg(...) helper. Of course then the question comes > down to the audit record type (the audit_msgno argument), the > operation (op), and the comm/cause (cause). > > Do you feel that any of the existing audit record types are a good fit for this? > Maybe I can use the audit_msgno "AUDIT_INTEGRITY_PCR" with appropriate strings for "op" and "cause". Mimi - please let me know if you think this audit_msgno would be ok to use. I see this code used, for instance, for boot aggregate measurement. integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, boot_aggregate_name, op, audit_cause, result, 0); thanks, -lakshmi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IMA: Add log statements for failure conditions 2020-06-05 21:09 ` Lakshmi Ramasubramanian @ 2020-06-05 21:34 ` Mimi Zohar 2020-06-05 21:36 ` Lakshmi Ramasubramanian 0 siblings, 1 reply; 6+ messages in thread From: Mimi Zohar @ 2020-06-05 21:34 UTC (permalink / raw) To: Lakshmi Ramasubramanian, Paul Moore Cc: tusharsu, linux-integrity, linux-audit, linux-kernel On Fri, 2020-06-05 at 14:09 -0700, Lakshmi Ramasubramanian wrote: > On 6/5/20 1:49 PM, Paul Moore wrote: > > > > >> Since a pr_xyz() call was already present, I just wanted to change the > >> log level to keep the code change to the minimum. But if audit log is > >> the right approach for this case, I'll update. > > > > Generally we reserve audit for things that are required for various > > security certifications and/or "security relevant". From what you > > mentioned above, it seems like this would fall into the second > > category if not the first. > > > > Looking at your patch it doesn't look like you are trying to record > > anything special so you may be able to use the existing > > integrity_audit_msg(...) helper. Of course then the question comes > > down to the audit record type (the audit_msgno argument), the > > operation (op), and the comm/cause (cause). > > > > Do you feel that any of the existing audit record types are a good fit for this? > > > > Maybe I can use the audit_msgno "AUDIT_INTEGRITY_PCR" with appropriate > strings for "op" and "cause". > > Mimi - please let me know if you think this audit_msgno would be ok to > use. I see this code used, for instance, for boot aggregate measurement. > > integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, boot_aggregate_name, op, > audit_cause, result, 0); Yes, AUDIT_INTEGRITY_PCR is also used for failures to add to the measurement list. thanks, Mimi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IMA: Add log statements for failure conditions 2020-06-05 21:34 ` Mimi Zohar @ 2020-06-05 21:36 ` Lakshmi Ramasubramanian 0 siblings, 0 replies; 6+ messages in thread From: Lakshmi Ramasubramanian @ 2020-06-05 21:36 UTC (permalink / raw) To: Mimi Zohar, Paul Moore Cc: tusharsu, linux-integrity, linux-audit, linux-kernel On 6/5/20 2:34 PM, Mimi Zohar wrote: >> >> Maybe I can use the audit_msgno "AUDIT_INTEGRITY_PCR" with appropriate >> strings for "op" and "cause". >> >> Mimi - please let me know if you think this audit_msgno would be ok to >> use. I see this code used, for instance, for boot aggregate measurement. >> >> integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, boot_aggregate_name, op, >> audit_cause, result, 0); > > Yes, AUDIT_INTEGRITY_PCR is also used for failures to add to the > measurement list. > thanks - i'll post an updated patch shortly. -lakshmi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-06 2:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200604163243.2575-1-nramas@linux.microsoft.com> [not found] ` <1591382782.5816.36.camel@linux.ibm.com> 2020-06-05 19:37 ` [PATCH] IMA: Add log statements for failure conditions Paul Moore 2020-06-05 19:54 ` Lakshmi Ramasubramanian 2020-06-05 20:49 ` Paul Moore 2020-06-05 21:09 ` Lakshmi Ramasubramanian 2020-06-05 21:34 ` Mimi Zohar 2020-06-05 21:36 ` Lakshmi Ramasubramanian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).