Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved

On 2018-06-28 15:41, Paul Moore wrote: > On Thu, Jun 14, 2018 at 4:23 PM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > > The failure to add an audit rule due to audit locked gives no clue > > what CONFIG_CHANGE operation failed. > > Similarly the set operation is the only other operation that doesn't > > give the "op=" field to indicate the action. > > All other CONFIG_CHANGE records include an op= field to give a clue as > > to what sort of configuration change is being executed. > > > > Since these are the only CONFIG_CHANGE records that that do not have an > > op= field, add them to bring them in line with the rest. > > Normally this would be an immediate reject because this patch inserts > a field into an existing record, but the CONFIG_CHANGE record is so > variable (supposedly bad in its own right) that I don't this really > matters. > > With that out of the way, I think this patch is fine, but I don't > think it is complete. At the very least there is another > CONFIG_CHANGE record in audit_watch_log_rule_change() that doesn't > appear to include an "op" field. If we want to make sure we have an > "op" field in every CONFIG_CHANGE record, let's actually add them all > :) The version I'm looking at already had it when it was added in 2009.

This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the audit_context() introduced in another patch.

> and one more in audit_receive_msg(). There may be more. I believe they're covered by other patches in the ghak59 set.

> > Old records: > > type=CONFIG_CHANGE msg=audit(1519812997.781:374): pid=610 uid=0 auid=0 ses=1 subj=... audit_enabled=2 res=0 > > type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes > > > > New records: > > type=CONFIG_CHANGE msg=audit(1520958477.855:100): pid=610 uid=0 auid=0 ses=1 subj=... op=add_rule audit_enabled=2 res=0 > > > > type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : op=set audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes > > > > See: https://github.com/linux-audit/audit-kernel/issues/59 > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > > --- > > kernel/audit.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index e7478cb..ad54339 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -403,7 +403,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old, > > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE); > > if (unlikely(!ab)) > > return rc; > > - audit_log_format(ab, "%s=%u old=%u", function_name, new, old); > > + audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old); > > audit_log_session_info(ab); > > rc = audit_log_task_context(ab); > > if (rc) > > @@ -1365,7 +1365,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) > > return -EINVAL; > > if (audit_enabled == AUDIT_LOCKED) { > > audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); > > - audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled); > > + audit_log_format(ab, " op=%s_rule audit_enabled=%d res=0", > > + msg_type == AUDIT_ADD_RULE ? "add" : "remove", > > + audit_enabled); > > audit_log_end(ab); > > return -EPERM; > > } > > -- > > 1.8.3.1 > > -- > paul moore > www.paul-moore.com - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635