linux-kernel - Re: [PATCH ghak120] audit: trigger accompanying records when no rules present

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 9 Mar 2020 16:31:07 -0400
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>, sgrubb@...hat.com,
        omosnace@...hat.com, Eric Paris <eparis@...isplace.org>
Subject: Re: [PATCH ghak120] audit: trigger accompanying records when no
 rules present

On 2020-02-27 20:02, Paul Moore wrote:
> On Tue, Feb 18, 2020 at 4:01 PM Richard Guy Briggs <rgb@...hat.com> wrote:
> >
> > When there are no audit rules registered, mandatory records (config,
> > etc.) are missing their accompanying records (syscall, proctitle, etc.).
> >
> > This is due to audit context dummy set on syscall entry based on absence
> > of rules that signals that no other records are to be printed.
> >
> > Clear the dummy bit in auditsc_set_stamp() when the first record of an
> > event is generated.
> >
> > Please see upstream github issue
> > https://github.com/linux-audit/audit-kernel/issues/120
> >
> > Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> > ---
> >  kernel/auditsc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 4effe01ebbe2..31195d122344 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2176,6 +2176,8 @@ int auditsc_get_stamp(struct audit_context *ctx,
> >         t->tv_sec  = ctx->ctime.tv_sec;
> >         t->tv_nsec = ctx->ctime.tv_nsec;
> >         *serial    = ctx->serial;
> > +       if (ctx->dummy)
> > +               ctx->dummy = 0;
> 
> Two comments:
> 
> * Why even bother checking to see if ctx->dummy is true?  If it is
> true you set it to false/0; if it is already false you leave it alone.
> Either way ctx->dummy is going to be set to false when you are past
> these two lines, might as well just always set ctx->dummy to false/0.

Ok, no problem.

> * Why are you setting ->dummy to false in auditsc_get_stamp() and not
> someplace a bit more obvious like audit_log_start()?  Is it because
> auditsc_get_stamp() only gets called once per event?  I'm willing to
> take the "hit" of one extra assignment in audit_log_start() to keep
> this in a more obvious place and not buried in auditsc_get_stamp().

It is because the context is only available when syscall logging is
enabled (which is on most platforms and hopefully eventually all) and
makes for cleaner code and lack of need to check existance of the
context.

> paul moore

- 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