2020-06-13 02:31:39

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v2 1/2] integrity: Add result field in audit message

Result code is not included in the audit messages logged by
the integrity subsystem. Add "result" field in the audit messages
logged by the integrity subsystem and set the value to the result code
passed to integrity_audit_msg() in the "result" parameter.

Sample audit message:

[ 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 result=-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 result=0

Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Suggested-by: Steve Grubb <[email protected]>
---
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..84002d3d5a95 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 result=%d", !result, result);
audit_log_end(ab);
}
--
2.27.0


2020-06-15 22:53:48

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] integrity: Add result field in audit message

On Fri, Jun 12, 2020 at 10:26 PM Lakshmi Ramasubramanian
<[email protected]> wrote:
> Result code is not included in the audit messages logged by
> the integrity subsystem. Add "result" field in the audit messages
> logged by the integrity subsystem and set the value to the result code
> passed to integrity_audit_msg() in the "result" parameter.
>
> Sample audit message:
>
> [ 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 result=-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 result=0
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> Suggested-by: Steve Grubb <[email protected]>
> ---
> security/integrity/integrity_audit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

If we can't use "res=" to carry more than 0/1 then this seems reasonable.

Acked-by: Paul Moore <[email protected]>

> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
> index 5109173839cc..84002d3d5a95 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 result=%d", !result, result);
> audit_log_end(ab);
> }
> --
> 2.27.0

--
paul moore
http://www.paul-moore.com

2020-06-16 16:15:02

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] integrity: Add result field in audit message

On Monday, June 15, 2020 6:51:22 PM EDT Paul Moore wrote:
> On Fri, Jun 12, 2020 at 10:26 PM Lakshmi Ramasubramanian
>
> <[email protected]> wrote:
> > Result code is not included in the audit messages logged by
> > the integrity subsystem. Add "result" field in the audit messages
> > logged by the integrity subsystem and set the value to the result code
> > passed to integrity_audit_msg() in the "result" parameter.
> >
> > Sample audit message:
> >
> > [ 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
> > result=-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 result=0
> >
> > Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> > Suggested-by: Steve Grubb <[email protected]>
> > ---
> >
> > security/integrity/integrity_audit.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> If we can't use "res=" to carry more than 0/1 then this seems reasonable.

Paul,

But we can't do this. The field name dictionary says this is used to convey
success/fail. It is hard coded in the field interpretation table to look for
0/1 and interpret that. Interpeting this field will now produce an error
message. And "result" is a searchable field.

As I suggested a few emails back, let's just use errno or something not
already taken in the dictionary. NACK.

-Steve

> Acked-by: Paul Moore <[email protected]>
>
> > diff --git a/security/integrity/integrity_audit.c
> > b/security/integrity/integrity_audit.c index 5109173839cc..84002d3d5a95
> > 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 result=%d", !result, result);
> >
> > audit_log_end(ab);
> >
> > }
> >
> > --
> > 2.27.0