From: Richard Guy Briggs <rgb@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Eric Paris <eparis@parisplace.org>,
Linux-Audit Mailing List <linux-audit@redhat.com>
Subject: Re: [PATCH v2] audit: log AUDIT_TIME_* records only from rules
Date: Tue, 25 Jan 2022 16:47:54 -0500 [thread overview]
Message-ID: <20220125214754.GA1708086@madcap2.tricolour.ca> (raw)
In-Reply-To: <CAHC9VhRDinLrUqiUaW7KcUOPsMTZC2UQ1a_c9U7PLWUExFn5NQ@mail.gmail.com>
On 2022-01-24 22:11, Paul Moore wrote:
> On Fri, Jan 21, 2022 at 11:17 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > AUDIT_TIME_* events are generated when there are syscall rules present that are
> > not related to time keeping. This will produce noisy log entries that could
> > flood the logs and hide events we really care about.
> >
> > Rather than immediately produce the AUDIT_TIME_* records, store the data in the
> > context and log it at syscall exit time respecting the filter rules.
> >
> > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919
> >
> > Fixes: 7e8eda734d30 ("ntp: Audit NTP parameters adjustment")
> > Fixes: 2d87a0674bd6 ("timekeeping: Audit clock adjustments")
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > Changelog:
> > v2:
> > - rename __audit_ntp_log_ to audit_log_ntp
> > - pre-check ntp before storing
> > - move tk out of the context union and move ntp logging to the bottom of audit_show_special()
> > - restructure logging of ntp to use ab and allocate more only if more
> > - add Fixes lines
> >
> > kernel/audit.h | 2 ++
> > kernel/auditsc.c | 77 +++++++++++++++++++++++++++++++++++-------------
> > 2 files changed, 59 insertions(+), 20 deletions(-)
> >
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index c4498090a5bd..11789249d838 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -201,8 +201,10 @@ struct audit_context {
> > struct {
> > char *name;
> > } module;
> > + struct audit_ntp_data ntp_data;
> > };
> > int fds[2];
> > + struct timespec64 tk_injoffset;
> > struct audit_proctitle proctitle;
> > };
>
> Why do we need to keep tk_injoffset outside of the audit_context
> union? I think we could do something like this, which would be an
> improvement IMO:
I did three implementations. The first put ntp_data outside which was
the quickest and simplest patch, but I really didn't like the wastage in
audit_context. The second did as you suggest below in the union, but
having the type overwritten by the other AUDIT_TIME_* didn't sit well
with me, but since we have to check all the values anyways, we don't
really lose anything. This means restructuring the loop a bit but that
should work. Both types will need to be written to the context->type.
I'll code that up...
> struct audit_context {
> /* ... */
> union {
> /* ... */
> struct {
> struct audit_ntp_data ntp_data;
> struct timespec64 tk_injoffset;
> } time;
> };
> /* ... */
> }
>
> void __audit_tk_injoffset(offset)
> {
> struct audit_context *ctx = audit_context();
> memcpy(&ctx->time->tk_injoffset, offset, sizeof(offset));
> }
>
> void audit_log_time(ctx)
> {
> /* ... */
>
> offset = ctx->time->tk_injoffset;
> if (offset->tv_sec != 0 || offset->tv_nsec != 0) {
> ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_TIME_INJOFFSET);
> /* ... */
> audit_log_end(ab);
> }
>
> ntp = ctx->time->ntp_data;
> for (i = 0; i < AUDIT_NTP_NVALS; i++) {
> if (ntp->vals[i].newval != ntp->vals[i].oldval ) {
> /* ... log the ntp/time param changes ... */
> }
> }
>
> /* ... */
> }
>
> void show_special(...)
> {
> /* ... */
> case AUDIT_TIME_INJOFFSET:
> case AUDIT_TIME_ADJNTPVAL:
> audit_log_time(context);
> break;
> /* ... */
> }
>
> --
> paul moore
> paul-moore.com
>
- RGB
--
Richard Guy Briggs <rgb@redhat.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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
prev parent reply other threads:[~2022-01-25 21:48 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-21 16:17 [PATCH v2] audit: log AUDIT_TIME_* records only from rules Richard Guy Briggs
2022-01-21 20:10 ` Paul Moore
2022-01-25 3:11 ` Paul Moore
2022-01-25 21:47 ` Richard Guy Briggs [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220125214754.GA1708086@madcap2.tricolour.ca \
--to=rgb@redhat.com \
--cc=eparis@parisplace.org \
--cc=linux-audit@redhat.com \
--cc=paul@paul-moore.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).