| Message ID | 20200211231414.6640-3-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IMA: improve log messages in IMA | expand |
Hi Tushar, Please remove the period at the end of the Subject line. On Tue, 2020-02-11 at 15:14 -0800, Tushar Sugandhi wrote: > process_buffer_measurement() does not have log messages for failure > conditions. > > This change adds a log statement in the above function. I agree some form of notification needs to be added. The question is whether the failure should be audited or a kernel message emitted. IMA emits audit messages (integrity_audit_msg) for a number of reasons - on failure to calculate a file hash, invalid policy rules, failure to communicate with the TPM, signature verification errors, etc. > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Suggested-by: Joe Perches <joe@perches.com> > --- > security/integrity/ima/ima_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 9fe949c6a530..6e1576d9eb48 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -757,6 +757,9 @@ void process_buffer_measurement(const void *buf, int size, > ima_free_template_entry(entry); > > out: > + if (ret < 0) > + pr_err("%s: failed, result: %d\n", __func__, ret); > + > return; > } > With 3/3 "IMA: Add module name and base name prefix to log", the resulting message will be "KBUILD_MODNAME: KBUILD_BASENAME: func:". Isn't that a bit much? Mimi
On 2020-02-12 6:47 a.m., Mimi Zohar wrote: > Hi Tushar, > > Please remove the period at the end of the Subject line. Thanks. I will fix it in the next iteration. > > On Tue, 2020-02-11 at 15:14 -0800, Tushar Sugandhi wrote: >> process_buffer_measurement() does not have log messages for failure >> conditions. >> >> This change adds a log statement in the above function. > > I agree some form of notification needs to be added. The question is > whether the failure should be audited or a kernel message emitted. > IMA emits audit messages (integrity_audit_msg) for a number of > reasons - on failure to calculate a file hash, invalid policy rules, > failure to communicate with the TPM, signature verification errors, > etc. I believe both IMA audit messages and kernel message should be emitted - for better discoverability and diagnosability. > >> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> >> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> >> Suggested-by: Joe Perches <joe@perches.com> >> --- >> security/integrity/ima/ima_main.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c >> index 9fe949c6a530..6e1576d9eb48 100644 >> --- a/security/integrity/ima/ima_main.c >> +++ b/security/integrity/ima/ima_main.c >> @@ -757,6 +757,9 @@ void process_buffer_measurement(const void *buf, int size, >> ima_free_template_entry(entry); >> >> out: >> + if (ret < 0) >> + pr_err("%s: failed, result: %d\n", __func__, ret); >> + >> return; >> } >> > > With 3/3 "IMA: Add module name and base name prefix to log", the > resulting message will be "KBUILD_MODNAME: KBUILD_BASENAME: func:". > Isn't that a bit much? > For this specific message, it will look like below. "ima: ima_main: process_buffer_measurement: failed, result: %d" In general, adding KBUILD_BASENAME seems helpful to pinpoint the location of the issue. > Mimi >
On Wed, 2020-02-12 at 14:30 -0800, Tushar Sugandhi wrote: > > On 2020-02-12 6:47 a.m., Mimi Zohar wrote: > > Hi Tushar, > > > > Please remove the period at the end of the Subject line. > Thanks. I will fix it in the next iteration. > > > > On Tue, 2020-02-11 at 15:14 -0800, Tushar Sugandhi wrote: > >> process_buffer_measurement() does not have log messages for failure > >> conditions. > >> > >> This change adds a log statement in the above function. > > > > I agree some form of notification needs to be added. The question is > > whether the failure should be audited or a kernel message emitted. > > IMA emits audit messages (integrity_audit_msg) for a number of > > reasons - on failure to calculate a file hash, invalid policy rules, > > failure to communicate with the TPM, signature verification errors, > > etc. > I believe both IMA audit messages and kernel message should be emitted - > for better discoverability and diagnosability. Like file measurement failures, failure to measure a key or the boot command line should be audited as well. For debugging purposes, you could make this message pr_devel. Mimi
On 2020-02-12 4:21 p.m., Mimi Zohar wrote: > On Wed, 2020-02-12 at 14:30 -0800, Tushar Sugandhi wrote: >> >> On 2020-02-12 6:47 a.m., Mimi Zohar wrote: >>> Hi Tushar, >>> >>> Please remove the period at the end of the Subject line. >> Thanks. I will fix it in the next iteration. >>> >>> On Tue, 2020-02-11 at 15:14 -0800, Tushar Sugandhi wrote: >>>> process_buffer_measurement() does not have log messages for failure >>>> conditions. >>>> >>>> This change adds a log statement in the above function. >>> >>> I agree some form of notification needs to be added. The question is >>> whether the failure should be audited or a kernel message emitted. >>> IMA emits audit messages (integrity_audit_msg) for a number of >>> reasons - on failure to calculate a file hash, invalid policy rules, >>> failure to communicate with the TPM, signature verification errors, >>> etc. >> I believe both IMA audit messages and kernel message should be emitted - >> for better discoverability and diagnosability. > > Like file measurement failures, failure to measure a key or the boot > command line should be audited as well. For debugging purposes, you > could make this message pr_devel. Ok. I will change this to pr_devel in next iteration. > > Mimi >
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 9fe949c6a530..6e1576d9eb48 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -757,6 +757,9 @@ void process_buffer_measurement(const void *buf, int size, ima_free_template_entry(entry); out: + if (ret < 0) + pr_err("%s: failed, result: %d\n", __func__, ret); + return; }