Re: [RFC PATCH ghak10 v6 2/2] ntp: Audit NTP parameters adjustment

[Ondrej Mosnacek]

On Tue, Apr 2, 2019 at 11:33 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Mon, 1 Apr 2019, Ondrej Mosnacek wrote:
> > On Thu, Mar 28, 2019 at 1:02 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > > On Thu, 7 Mar 2019, Ondrej Mosnacek wrote:
> > > >                       /* adjtime() is independent from ntp_adjtime() */
> > > >                       time_adjust = txc->offset;
> > > >                       ntp_update_frequency();
> > > > +
> > > > +                     audit_ntp_adjust("adjust", save_adjust, txc->offset);
> > > >               }
> > > >               txc->offset = save_adjust;
> > > >       } else {
> > >
> > > Not going to happen. We are not reshuffling all that code just to
> > > accomodate random audit log invocations in a critical section plus having a
> > > gazillion of GFP_ATOMIC allocation in the critical section just because.
> >
> > OK, seems I underestimated the consequences of putting the logging
> > calls directly in there. While I was offline over the weekend I
> > already came up with a cleaner version that collects the changes in a
> > structure and does the logging outside of the critical section. I
> > currently does a few unnecessary writes into memory under
> > CONFIG_AUDIT=n, but if that is an issue I can boost the abstraction or
> > just add some #ifdefs to avoid that.
>
> No ifdefs please.

Yep, no worries, I already found a clean way to make all the audit
operations a no-op under CONFIG_AUDITSYSCALL=n that keeps all the
#ifdefs contained within <linux/audit.h>.

> Aside of that, why do you need all those details of the
> ntp internals in the first place? The changelog does not give me an answer
> to that.

Yeah, the changelog should be more explanatory, I'll try to improve
that in the next revision. I actually stated the justification in the
RFE page [1] (paragraphs 3-4) linked from the cover letter (I don't
blame you for not finding it...). The idea is that it is (or at least
seems) possible to do some evil changes to the clock indirectly by
modifying these parameters as well. Maybe it is a bit overkill, but
it's better to be safe than sorry... If anyone doesn't want the extra
records, they can be easily filtered out by adding a simple audit
rule.

[1] https://github.com/linux-audit/audit-kernel/wiki/RFE-More-detailed-auditing-of-changes-to-system-clock#feature-design

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.