linux-kernel - Re: [PATCH ghak10 v7 1/2] timekeeping: Audit clock adjustments

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: <CAFqZXNuxbzKifFm-dcgaUMojqNZtLipfsztrbR1RLfMG0RJwaA@mail.gmail.com>
Date:   Tue, 9 Apr 2019 17:05:39 +0200
From:   Ondrej Mosnacek <omosnace@...hat.com>
To:     Richard Guy Briggs <rgb@...hat.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        Paul Moore <paul@...l-moore.com>,
        Steve Grubb <sgrubb@...hat.com>,
        Miroslav Lichvar <mlichvar@...hat.com>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH ghak10 v7 1/2] timekeeping: Audit clock adjustments

On Tue, Apr 9, 2019 at 4:26 PM Richard Guy Briggs <rgb@...hat.com> wrote:
> On 2019-04-09 14:31, Ondrej Mosnacek wrote:
> > Emit an audit record whenever the system clock is changed (i.e. shifted
> > by a non-zero offset) by a syscall from userspace. The syscalls than can
> > (at the time of writing) trigger such record are:
> >   - settimeofday(2), stime(2), clock_settime(2) -- via
> >     do_settimeofday64()
> >   - adjtimex(2), clock_adjtime(2) -- via do_adjtimex()
> >
> > The new records have type AUDIT_TIME_INJOFFSET and contain the following
> > fields:
> >   - sec -- the 'seconds' part of the offset
> >   - nsec -- the 'nanoseconds' part of the offset
> >
> > Example record (time was shifted backwards by ~16.125 seconds):
> >
> > type=TIME_INJOFFSET msg=audit(1530616049.652:13): sec=-16 nsec=124887145
>
> I have a minor nit about the interpretation of these values.  I realize
> the values printed are the ones that are supplied.  When the sec
> value is negative, does that imply that the nsec value has the same
> sign?  IOW, are the two values added with a multiplier to the nsec
> value, or are they concatenated with a decimal in the middle?  I expect
> the latter.  In the current userspace support, there is no explicit type
> support included yet for interpreted fields.  This is just a minor
> documentation issue to me.

Good point! I wrote the sentence too quickly without thinking about
it... I actually had to find this out for the tests (and looking at it
again, I got it wrong there as well, although in a different way...) -
it is actually the former variant (multiplier + addition). So the
"~16.125" should actually be "~15.875" (and I need to fix the
testsuite pull request...).

> > The records of this type will be associated with the corresponding
> > syscall records.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@...hat.com>
>
> Reviewed-by: Richard Guy Briggs <rgb@...hat.com>
>
> > ---
> >  include/linux/audit.h      | 14 ++++++++++++++
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/auditsc.c           |  6 ++++++
> >  kernel/time/timekeeping.c  |  6 ++++++
> >  4 files changed, 27 insertions(+)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 1e69d9fe16da..2c62c0468888 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -365,6 +365,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> >  extern void __audit_mmap_fd(int fd, int flags);
> >  extern void __audit_log_kern_module(char *name);
> >  extern void __audit_fanotify(unsigned int response);
> > +extern void __audit_tk_injoffset(struct timespec64 offset);
> >
> >  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> >  {
> > @@ -467,6 +468,16 @@ static inline void audit_fanotify(unsigned int response)
> >               __audit_fanotify(response);
> >  }
> >
> > +static inline void audit_tk_injoffset(struct timespec64 offset)
> > +{
> > +     /* ignore no-op events */
> > +     if (offset.tv_sec == 0 && offset.tv_nsec == 0)
> > +             return;
> > +
> > +     if (!audit_dummy_context())
> > +             __audit_tk_injoffset(offset);
> > +}
> > +
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -580,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
> >  static inline void audit_fanotify(unsigned int response)
> >  { }
> >
> > +static inline void audit_tk_injoffset(struct timespec64 offset)
> > +{ }
> > +
> >  static inline void audit_ptrace(struct task_struct *t)
> >  { }
> >  #define audit_n_rules 0
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 3901c51c0b93..ab58d67baf4d 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -114,6 +114,7 @@
> >  #define AUDIT_REPLACE                1329    /* Replace auditd if this packet unanswerd */
> >  #define AUDIT_KERN_MODULE    1330    /* Kernel Module events */
> >  #define AUDIT_FANOTIFY               1331    /* Fanotify access decision */
> > +#define AUDIT_TIME_INJOFFSET 1332    /* Timekeeping offset injected */
> >
> >  #define AUDIT_AVC            1400    /* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR    1401    /* Internal SE Linux Errors */
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 51a2ceb3a1ca..80dfe0cdc636 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2512,6 +2512,12 @@ void __audit_fanotify(unsigned int response)
> >               AUDIT_FANOTIFY, "resp=%u", response);
> >  }
> >
> > +void __audit_tk_injoffset(struct timespec64 offset)
> > +{
> > +     audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET,
> > +               "sec=%lli nsec=%li", (long long)offset.tv_sec, offset.tv_nsec);
> > +}
> > +
> >  static void audit_log_task(struct audit_buffer *ab)
> >  {
> >       kuid_t auid, uid;
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index f986e1918d12..3d24be4cd607 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/stop_machine.h>
> >  #include <linux/pvclock_gtod.h>
> >  #include <linux/compiler.h>
> > +#include <linux/audit.h>
> >
> >  #include "tick-internal.h"
> >  #include "ntp_internal.h"
> > @@ -1250,6 +1251,9 @@ out:
> >       /* signal hrtimers about time change */
> >       clock_was_set();
> >
> > +     if (!ret)
> > +             audit_tk_injoffset(ts_delta);
> > +
> >       return ret;
> >  }
> >  EXPORT_SYMBOL(do_settimeofday64);
> > @@ -2322,6 +2326,8 @@ int do_adjtimex(struct __kernel_timex *txc)
> >               ret = timekeeping_inject_offset(&delta);
> >               if (ret)
> >                       return ret;
> > +
> > +             audit_tk_injoffset(delta);
> >       }
> >
> >       ktime_get_real_ts64(&ts);
> > --
> > 2.20.1
> >
>
> - 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



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