linux-kernel - Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180713004122.qlxdpkae4ihkxatg@madcap2.tricolour.ca>
Date:   Thu, 12 Jul 2018 20:41:22 -0400
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     linux-audit@...hat.com, linux-kernel@...r.kernel.org,
        Eric Paris <eparis@...isplace.org>, sgrubb@...hat.com,
        aviro@...hat.com
Subject: 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@...hat.com> 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.

> There appears to be another one in audit_mark_log_rule_change() ...

Same, 2015.

> 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@...hat.com>
> > ---
> >  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@...hat.com>
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