| Message ID | 20200617204436.2226-2-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] IMA: pass error code in result parameter to integrity_audit_msg() | expand |
On Wed, Jun 17, 2020 at 4:44 PM Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote: > > Error code is not included in the audit messages logged by > the integrity subsystem. Add "errno" field in the audit messages > logged by the integrity subsystem and set the value to the error code > passed to integrity_audit_msg() in the "result" parameter. > > Sample audit messages: > > [ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 errno=-12 > > [ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 errno=0 > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Suggested-by: Steve Grubb <sgrubb@redhat.com> > --- > security/integrity/integrity_audit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Paul Moore <paul@paul-moore.com> > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c > index 5109173839cc..a265024f82f3 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > - audit_log_format(ab, " res=%d", !result); > + audit_log_format(ab, " res=%d errno=%d", !result, result); > audit_log_end(ab); > } > -- > 2.27.0
On Wednesday, June 17, 2020 4:44:36 PM EDT Lakshmi Ramasubramanian wrote: > Error code is not included in the audit messages logged by > the integrity subsystem. Add "errno" field in the audit messages > logged by the integrity subsystem and set the value to the error code > passed to integrity_audit_msg() in the "result" parameter. > > Sample audit messages: > > [ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 > auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate > cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 errno=-12 > > [ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 > auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 > op=policy_update cause=completed comm="systemd" res=1 errno=0 > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Suggested-by: Steve Grubb <sgrubb@redhat.com> Acked-by: Steve Grubb <sgrubb@redhat.com> > --- > security/integrity/integrity_audit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/integrity/integrity_audit.c > b/security/integrity/integrity_audit.c index 5109173839cc..a265024f82f3 > 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode > *inode, audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > - audit_log_format(ab, " res=%d", !result); > + audit_log_format(ab, " res=%d errno=%d", !result, result); > audit_log_end(ab); > }
On Wed, 2020-06-17 at 13:44 -0700, Lakshmi Ramasubramanian wrote: > Error code is not included in the audit messages logged by > the integrity subsystem. Add "errno" field in the audit messages > logged by the integrity subsystem and set the value to the error code > passed to integrity_audit_msg() in the "result" parameter. > > Sample audit messages: > > [ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 errno=-12 > > [ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 errno=0 > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Suggested-by: Steve Grubb <sgrubb@redhat.com> > --- > security/integrity/integrity_audit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c > index 5109173839cc..a265024f82f3 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > - audit_log_format(ab, " res=%d", !result); > + audit_log_format(ab, " res=%d errno=%d", !result, result); > audit_log_end(ab); > } For the reasons that I mentioned previously, unless others are willing to add their Reviewed-by tag not for the audit aspect in particular, but IMA itself, I'm not comfortable making this change all at once. Previously I suggested making the existing integrity_audit_msg() a wrapper for a new function with errno. Steve said, "We normally do not like to have fields that swing in and out ...", but said setting errno to 0 is fine. The original integrity_audit_msg() function would call the new function with errno set to 0. Mimi
On 6/18/20 10:41 AM, Mimi Zohar wrote: > > For the reasons that I mentioned previously, unless others are willing > to add their Reviewed-by tag not for the audit aspect in particular, > but IMA itself, I'm not comfortable making this change all at once. > > Previously I suggested making the existing integrity_audit_msg() a > wrapper for a new function with errno. Steve said, "We normally do > not like to have fields that swing in and out ...", but said setting > errno to 0 is fine. The original integrity_audit_msg() function would > call the new function with errno set to 0. If the original integrity_audit_msg() always calls the new function with errno set to 0, there would be audit messages where "res" field is set to "0" (fail) because "result" was non-zero, but errno set to "0" (success). Wouldn't this be confusing? In PATCH 1/2 I've made changes to make the "result" parameter to integrity_audit_msg() consistent - i.e., it is always an error code (0 for success and a negative value for error). Would that address your concerns? thanks, -lakshmi
On Thu, 2020-06-18 at 11:05 -0700, Lakshmi Ramasubramanian wrote: > On 6/18/20 10:41 AM, Mimi Zohar wrote: > > > > > For the reasons that I mentioned previously, unless others are willing > > to add their Reviewed-by tag not for the audit aspect in particular, > > but IMA itself, I'm not comfortable making this change all at once. > > > > Previously I suggested making the existing integrity_audit_msg() a > > wrapper for a new function with errno. Steve said, "We normally do > > not like to have fields that swing in and out ...", but said setting > > errno to 0 is fine. The original integrity_audit_msg() function would > > call the new function with errno set to 0. > > If the original integrity_audit_msg() always calls the new function with > errno set to 0, there would be audit messages where "res" field is set > to "0" (fail) because "result" was non-zero, but errno set to "0" > (success). Wouldn't this be confusing? > > In PATCH 1/2 I've made changes to make the "result" parameter to > integrity_audit_msg() consistent - i.e., it is always an error code (0 > for success and a negative value for error). Would that address your > concerns? You're overloading "res" to imply errno. Define a new parameter specifically for errno. Mimi
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index 5109173839cc..a265024f82f3 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, audit_log_untrustedstring(ab, inode->i_sb->s_id); audit_log_format(ab, " ino=%lu", inode->i_ino); } - audit_log_format(ab, " res=%d", !result); + audit_log_format(ab, " res=%d errno=%d", !result, result); audit_log_end(ab); }
Error code is not included in the audit messages logged by the integrity subsystem. Add "errno" field in the audit messages logged by the integrity subsystem and set the value to the error code passed to integrity_audit_msg() in the "result" parameter. Sample audit messages: [ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 errno=-12 [ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 errno=0 Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Suggested-by: Steve Grubb <sgrubb@redhat.com> --- security/integrity/integrity_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)