[RFC PATCH ghak10 v3 0/3] audit: Log modifying adjtimex(2) calls

This patch adds two functions to the audit interface:
- audit_tk_injoffset(), which will be called whenever a timekeeping
offset is injected by a syscall from userspace,
- audit_ntp_adjust(), which will be called whenever an NTP internal
variable is changed by a syscall from userspace.

Syntax of records produced by these messages:
AUDIT_TIME_INJOFFSET
sec - the 'seconds' part of the offset
nsec - the 'nanoseconds' part of the offset
AUDIT_TIME_ADJNTPVAL
type - which value was adjusted:
offset - corresponding to the time_offset variable
freq - corresponding to the time_freq variable
status - corresponding to the time_status variable
maxerr - corresponding to the time_maxerror variable
esterr - corresponding to the time_esterror variable
const - corresponding to the time_constant variable
adjust - corresponding to the time_adjust variable
tick - corresponding to the tick_usec variable
tai - corresponding to the timekeeping's TAI offset
old - the original value
new - the new value

Signed-off-by: Ondrej Mosnacek <***@redhat.com>
---
include/linux/audit.h | 21 +++++++++++++++++++++
kernel/auditsc.c | 15 +++++++++++++++
2 files changed, 36 insertions(+)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 9334fbef7bae..0d084d4b4042 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -26,6 +26,7 @@
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <uapi/linux/audit.h>
+#include <uapi/linux/timex.h>

#define AUDIT_INO_UNSET ((unsigned long)-1)
#define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -356,6 +357,8 @@ 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);
+extern void __audit_ntp_adjust(const char *type, s64 oldval, s64 newval);

static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -458,6 +461,18 @@ static inline void audit_fanotify(unsigned int response)
__audit_fanotify(response);
}

+static inline void audit_tk_injoffset(struct timespec64 offset)
+{
+ if (!audit_dummy_context())
+ __audit_tk_injoffset(offset);
+}
+
+static inline void audit_ntp_adjust(const char *type, s64 oldval, s64 newval)
+{
+ if (!audit_dummy_context())
+ __audit_ntp_adjust(type, oldval, newval);
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else /* CONFIG_AUDITSYSCALL */
@@ -584,6 +599,12 @@ 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_ntp_adjust(const char *type, s64 oldval, s64 newval)
+{ }
+
static inline void audit_ptrace(struct task_struct *t)
{ }
#define audit_n_rules 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d762e0b8160e..078249e7444d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2422,6 +2422,21 @@ void __audit_fanotify(unsigned int response)
AUDIT_FANOTIFY, "resp=%u", response);
}

+/* We need to allocate with GFP_ATOMIC here, since these two functions will be
+ * called while holding the timekeeping lock: */
+void __audit_tk_injoffset(struct timespec64 offset)
+{
+ audit_log(audit_context(), GFP_ATOMIC, AUDIT_TIME_INJOFFSET,
+ "sec=%lli nsec=%li", (long long)offset.tv_sec, offset.tv_nsec);
+}
+
+void __audit_ntp_adjust(const char *type, s64 oldval, s64 newval)
+{
+ audit_log(audit_context(), GFP_ATOMIC, AUDIT_TIME_ADJNTPVAL,
+ "type=%s old=%lli new=%lli", type,
+ (long long)oldval, (long long)newval);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

--
2.17.1

Ondrej Mosnacek

2018-07-03 12:44:35 UTC

This patch adds two auxiliary record types that will be used to annotate
the adjtimex SYSCALL records with the NTP/timekeeping values that have
been changed (if any).

Signed-off-by: Ondrej Mosnacek <***@redhat.com>
---
include/uapi/linux/audit.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e3eaba84175..242ce562b41a 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -114,6 +114,8 @@
#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_TIME_ADJNTPVAL 1333 /* NTP value adjustment */

#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */

--
2.17.1

Ondrej Mosnacek

2018-07-03 12:44:37 UTC

TODO ??split patch into two

This patch adds logging of all attempts to either inject an offset into
the clock (producing an AUDIT_TIME_INJOFFSET record) or adjust an NTP
parameter (producing an AUDIT_TIME_ADJNTPVAL record).

For reference, running the following commands:

auditctl -D
auditctl -a exit,always -F arch=b64 -S adjtimex
chronyd -q

produces audit records like this:

type=TIME_ADJNTPVAL msg=audit(1530616044.507:5): type=adjust old=0 new=0
type=SYSCALL msg=audit(1530616044.507:5): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=0 a2=4 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:5): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:6): type=maxerr old=16000000 new=0
type=SYSCALL msg=audit(1530616044.507:6): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=1 a2=1 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:6): proctitle=6368726F6E7964002D71
type=TIME_INJOFFSET msg=audit(1530616044.507:7): sec=0 nsec=0
type=TIME_ADJNTPVAL msg=audit(1530616044.507:7): type=status old=64 new=8256
type=SYSCALL msg=audit(1530616044.507:7): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=1 a2=1 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:7): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=status old=8256 new=8257
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=offset old=0 new=0
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=freq old=0 new=0
type=SYSCALL msg=audit(1530616044.507:8): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ab0 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:8): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:9): type=status old=8257 new=64
type=SYSCALL msg=audit(1530616044.507:9): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ab0 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:9): proctitle=6368726F6E7964002D71
type=SYSCALL msg=audit(1530616044.507:10): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78a70 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:10): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.511:11): type=freq old=0 new=49180377088000
type=TIME_ADJNTPVAL msg=audit(1530616044.511:11): type=tick old=10000 new=10000
type=SYSCALL msg=audit(1530616044.511:11): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ad0 a1=0 a2=2710 a3=f42f82a800000 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.511:11): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=status old=64 new=64
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=maxerr old=16000000 new=16000000
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=esterr old=16000000 new=16000000
type=SYSCALL msg=audit(1530616044.521:12): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78b40 a1=1 a2=40 a3=f91f6ef84fbab items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.521:12): proctitle=6368726F6E7964002D71
type=TIME_INJOFFSET msg=audit(1530616049.652:13): sec=-16 nsec=124887145
type=TIME_ADJNTPVAL msg=audit(1530616049.652:13): type=status old=64 new=8256
type=SYSCALL msg=audit(1530616049.652:13): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78270 a1=1 a2=fffffffffffffff0 a3=137b828205ca12 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616049.652:13): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616033.783:14): type=freq old=49180377088000 new=49180377088000
type=TIME_ADJNTPVAL msg=audit(1530616033.783:14): type=tick old=10000 new=10000
type=SYSCALL msg=audit(1530616033.783:14): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78bc0 a1=0 a2=2710 a3=0 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616033.783:14): proctitle=6368726F6E7964002D71

The chronyd command that produced the above records executed the
following adjtimex(2) syscalls (as per strace output):

adjtimex({modes=ADJ_OFFSET|0x8000, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507215}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_MAXERROR, offset=0, freq=0, maxerror=0, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507438}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_SETOFFSET|ADJ_NANO, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507604737}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_OFFSET|ADJ_STATUS, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_PLL|STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507698330}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_STATUS, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507792}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=0, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=508000}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_FREQUENCY|ADJ_TICK, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=512146}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_MAXERROR|ADJ_ESTERROR|ADJ_STATUS, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=522506}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_SETOFFSET|ADJ_NANO, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616033, tv_usec=778717675}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_FREQUENCY|ADJ_TICK, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616033, tv_usec=784644657}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)

(The struct timex fields above are from *after* the syscall was
executed, so they contain the current (new) values as set from the
kernel, except of the 'modes' field, which contains the original value
sent by the caller.)

Note that the records are emitted even when the actual value does not
change (i.e. when there is an explicit attempt to change a value, but
the new value equals the old one).

(Intentionally not sending to the timekeeping maintainers just yet,
let's settle on the record contents/format first.)

Signed-off-by: Ondrej Mosnacek <***@redhat.com>
---
kernel/time/ntp.c | 50 +++++++++++++++++++++++++++++++--------
kernel/time/timekeeping.c | 3 +++
2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index a09ded765f6c..cb2073a69ada 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/rtc.h>
#include <linux/math64.h>
+#include <linux/audit.h>

#include "ntp_internal.h"
#include "timekeeping_internal.h"
@@ -294,6 +295,8 @@ static inline s64 ntp_update_offset_fll(s64 offset64, long secs)

static void ntp_update_offset(long offset)
{
+ s64 old_offset = time_offset;
+ s64 old_freq = time_freq;
s64 freq_adj;
s64 offset64;
long secs;
@@ -342,6 +345,9 @@ static void ntp_update_offset(long offset)
time_freq = max(freq_adj, -MAXFREQ_SCALED);

time_offset = div_s64(offset64 << NTP_SCALE_SHIFT, NTP_INTERVAL_FREQ);
+
+ audit_ntp_adjust("offset", old_offset, time_offset);
+ audit_ntp_adjust("freq", old_freq, time_freq);
}

/**
@@ -669,45 +675,67 @@ static inline void process_adjtimex_modes(struct timex *txc,
struct timespec64 *ts,
s32 *time_tai)
{
- if (txc->modes & ADJ_STATUS)
- process_adj_status(txc, ts);
+ if (txc->modes & (ADJ_STATUS | ADJ_NANO | ADJ_MICRO)) {
+ int old_status = time_status;
+
+ if (txc->modes & ADJ_STATUS)
+ process_adj_status(txc, ts);

- if (txc->modes & ADJ_NANO)
- time_status |= STA_NANO;
+ if (txc->modes & ADJ_NANO)
+ time_status |= STA_NANO;

- if (txc->modes & ADJ_MICRO)
- time_status &= ~STA_NANO;
+ if (txc->modes & ADJ_MICRO)
+ time_status &= ~STA_NANO;
+
+ audit_ntp_adjust("status", old_status, time_status);
+ }

if (txc->modes & ADJ_FREQUENCY) {
+ s64 old_freq = time_freq;
+
time_freq = txc->freq * PPM_SCALE;
time_freq = min(time_freq, MAXFREQ_SCALED);
time_freq = max(time_freq, -MAXFREQ_SCALED);
/* update pps_freq */
pps_set_freq(time_freq);
+
+ audit_ntp_adjust("freq", old_freq, time_freq);
}

- if (txc->modes & ADJ_MAXERROR)
+ if (txc->modes & ADJ_MAXERROR) {
+ audit_ntp_adjust("maxerr", time_maxerror, txc->maxerror);
time_maxerror = txc->maxerror;
+ }

- if (txc->modes & ADJ_ESTERROR)
+ if (txc->modes & ADJ_ESTERROR) {
+ audit_ntp_adjust("esterr", time_esterror, txc->esterror);
time_esterror = txc->esterror;
+ }

if (txc->modes & ADJ_TIMECONST) {
+ long old_constant = time_constant;
+
time_constant = txc->constant;
if (!(time_status & STA_NANO))
time_constant += 4;
time_constant = min(time_constant, (long)MAXTC);
time_constant = max(time_constant, 0l);
+
+ audit_ntp_adjust("const", old_constant, time_constant);
}

- if (txc->modes & ADJ_TAI && txc->constant > 0)
+ if (txc->modes & ADJ_TAI && txc->constant > 0) {
+ audit_ntp_adjust("tai", *time_tai, txc->constant);
*time_tai = txc->constant;
+ }

if (txc->modes & ADJ_OFFSET)
ntp_update_offset(txc->offset);

- if (txc->modes & ADJ_TICK)
+ if (txc->modes & ADJ_TICK) {
+ audit_ntp_adjust("tick", tick_usec, txc->tick);
tick_usec = txc->tick;
+ }

if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
ntp_update_frequency();
@@ -729,6 +757,8 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32 *time_tai)
/* 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 {
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..9089ac329e69 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -25,6 +25,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"
@@ -2308,6 +2309,8 @@ int do_adjtimex(struct timex *txc)
ret = timekeeping_inject_offset(&delta);
if (ret)
return ret;
+
+ audit_tk_injoffset(delta);
}

getnstimeofday64(&ts);

--
2.17.1

Richard Guy Briggs

2018-07-13 19:21:28 UTC

Post by Ondrej Mosnacek
TODO ??split patch into two
This patch adds logging of all attempts to either inject an offset into
the clock (producing an AUDIT_TIME_INJOFFSET record) or adjust an NTP
parameter (producing an AUDIT_TIME_ADJNTPVAL record).
auditctl -D
auditctl -a exit,always -F arch=b64 -S adjtimex
chronyd -q

I like the format of the output below. That is very clear and should be
much easier to drop certain types if they are determined to be
unimportant to log, or drop unchanged values, but that should be done in
consultation with the time folks. Steve may have issue with
the field type "type", and I might suggest "op=" might be more
appropriate, but check with Steve.

I'd probably combine the header file definition of the record type with
the code that actually adds that record output. I agree splitting the
patch along the lines of the two different record types.

Very nice! (more below...)

Post by Ondrej Mosnacek
type=TIME_ADJNTPVAL msg=audit(1530616044.507:5): type=adjust old=0 new=0
type=SYSCALL msg=audit(1530616044.507:5): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=0 a2=4 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:5): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:6): type=maxerr old=16000000 new=0
type=SYSCALL msg=audit(1530616044.507:6): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=1 a2=1 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:6): proctitle=6368726F6E7964002D71
type=TIME_INJOFFSET msg=audit(1530616044.507:7): sec=0 nsec=0
type=TIME_ADJNTPVAL msg=audit(1530616044.507:7): type=status old=64 new=8256
type=SYSCALL msg=audit(1530616044.507:7): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=1 a2=1 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:7): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=status old=8256 new=8257
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=offset old=0 new=0
type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): type=freq old=0 new=0
type=SYSCALL msg=audit(1530616044.507:8): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ab0 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:8): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.507:9): type=status old=8257 new=64
type=SYSCALL msg=audit(1530616044.507:9): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ab0 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:9): proctitle=6368726F6E7964002D71
type=SYSCALL msg=audit(1530616044.507:10): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78a70 a1=0 a2=55e129c850c0 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.507:10): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.511:11): type=freq old=0 new=49180377088000
type=TIME_ADJNTPVAL msg=audit(1530616044.511:11): type=tick old=10000 new=10000
type=SYSCALL msg=audit(1530616044.511:11): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78ad0 a1=0 a2=2710 a3=f42f82a800000 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.511:11): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=status old=64 new=64
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=maxerr old=16000000 new=16000000
type=TIME_ADJNTPVAL msg=audit(1530616044.521:12): type=esterr old=16000000 new=16000000
type=SYSCALL msg=audit(1530616044.521:12): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78b40 a1=1 a2=40 a3=f91f6ef84fbab items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616044.521:12): proctitle=6368726F6E7964002D71
type=TIME_INJOFFSET msg=audit(1530616049.652:13): sec=-16 nsec=124887145
type=TIME_ADJNTPVAL msg=audit(1530616049.652:13): type=status old=64 new=8256
type=SYSCALL msg=audit(1530616049.652:13): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78270 a1=1 a2=fffffffffffffff0 a3=137b828205ca12 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616049.652:13): proctitle=6368726F6E7964002D71
type=TIME_ADJNTPVAL msg=audit(1530616033.783:14): type=freq old=49180377088000 new=49180377088000
type=TIME_ADJNTPVAL msg=audit(1530616033.783:14): type=tick old=10000 new=10000
type=SYSCALL msg=audit(1530616033.783:14): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78bc0 a1=0 a2=2710 a3=0 items=0 ppid=626 pid=629 auid=0 uid=385 gid=382 euid=385 suid=385 fsuid=385 egid=382 sgid=382 fsgid=382 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
type=PROCTITLE msg=audit(1530616033.783:14): proctitle=6368726F6E7964002D71
The chronyd command that produced the above records executed the
adjtimex({modes=ADJ_OFFSET|0x8000, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507215}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_MAXERROR, offset=0, freq=0, maxerror=0, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507438}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_SETOFFSET|ADJ_NANO, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507604737}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_OFFSET|ADJ_STATUS, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_PLL|STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507698330}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_STATUS, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=507792}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=0, offset=0, freq=0, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=508000}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_FREQUENCY|ADJ_TICK, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=512146}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_MAXERROR|ADJ_ESTERROR|ADJ_STATUS, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616044, tv_usec=522506}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_SETOFFSET|ADJ_NANO, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616033, tv_usec=778717675}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
adjtimex({modes=ADJ_FREQUENCY|ADJ_TICK, offset=0, freq=750433, maxerror=16000000, esterror=16000000, status=STA_UNSYNC|STA_NANO, constant=2, precision=1, tolerance=32768000, time={tv_sec=1530616033, tv_usec=784644657}, tick=10000, ppsfreq=0, jitter=0, shift=0, stabil=0, jitcnt=0, calcnt=0, errcnt=0, stbcnt=0, tai=0}) = 5 (TIME_ERROR)
(The struct timex fields above are from *after* the syscall was
executed, so they contain the current (new) values as set from the
kernel, except of the 'modes' field, which contains the original value
sent by the caller.)
Note that the records are emitted even when the actual value does not
change (i.e. when there is an explicit attempt to change a value, but
the new value equals the old one).

Great. This should be reflected in the syscall's "success=" value.

Post by Ondrej Mosnacek
(Intentionally not sending to the timekeeping maintainers just yet,
let's settle on the record contents/format first.)
---
kernel/time/ntp.c | 50 +++++++++++++++++++++++++++++++--------
kernel/time/timekeeping.c | 3 +++
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index a09ded765f6c..cb2073a69ada 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/rtc.h>
#include <linux/math64.h>
+#include <linux/audit.h>
#include "ntp_internal.h"
#include "timekeeping_internal.h"
@@ -294,6 +295,8 @@ static inline s64 ntp_update_offset_fll(s64 offset64, long secs)
static void ntp_update_offset(long offset)
{
+ s64 old_offset = time_offset;
+ s64 old_freq = time_freq;
s64 freq_adj;
s64 offset64;
long secs;
@@ -342,6 +345,9 @@ static void ntp_update_offset(long offset)
time_freq = max(freq_adj, -MAXFREQ_SCALED);
time_offset = div_s64(offset64 << NTP_SCALE_SHIFT, NTP_INTERVAL_FREQ);
+
+ audit_ntp_adjust("offset", old_offset, time_offset);
+ audit_ntp_adjust("freq", old_freq, time_freq);
}
/**
@@ -669,45 +675,67 @@ static inline void process_adjtimex_modes(struct timex *txc,
struct timespec64 *ts,
s32 *time_tai)
{
- if (txc->modes & ADJ_STATUS)
- process_adj_status(txc, ts);
+ if (txc->modes & (ADJ_STATUS | ADJ_NANO | ADJ_MICRO)) {
+ int old_status = time_status;
+
+ if (txc->modes & ADJ_STATUS)
+ process_adj_status(txc, ts);
- if (txc->modes & ADJ_NANO)
- time_status |= STA_NANO;
+ if (txc->modes & ADJ_NANO)
+ time_status |= STA_NANO;
- if (txc->modes & ADJ_MICRO)
- time_status &= ~STA_NANO;
+ if (txc->modes & ADJ_MICRO)
+ time_status &= ~STA_NANO;
+
+ audit_ntp_adjust("status", old_status, time_status);
+ }
if (txc->modes & ADJ_FREQUENCY) {
+ s64 old_freq = time_freq;
+
time_freq = txc->freq * PPM_SCALE;
time_freq = min(time_freq, MAXFREQ_SCALED);
time_freq = max(time_freq, -MAXFREQ_SCALED);
/* update pps_freq */
pps_set_freq(time_freq);
+
+ audit_ntp_adjust("freq", old_freq, time_freq);
}
- if (txc->modes & ADJ_MAXERROR)
+ if (txc->modes & ADJ_MAXERROR) {
+ audit_ntp_adjust("maxerr", time_maxerror, txc->maxerror);
time_maxerror = txc->maxerror;
+ }
- if (txc->modes & ADJ_ESTERROR)
+ if (txc->modes & ADJ_ESTERROR) {
+ audit_ntp_adjust("esterr", time_esterror, txc->esterror);
time_esterror = txc->esterror;
+ }
if (txc->modes & ADJ_TIMECONST) {
+ long old_constant = time_constant;
+
time_constant = txc->constant;
if (!(time_status & STA_NANO))
time_constant += 4;
time_constant = min(time_constant, (long)MAXTC);
time_constant = max(time_constant, 0l);
+
+ audit_ntp_adjust("const", old_constant, time_constant);
}
- if (txc->modes & ADJ_TAI && txc->constant > 0)
+ if (txc->modes & ADJ_TAI && txc->constant > 0) {
+ audit_ntp_adjust("tai", *time_tai, txc->constant);
*time_tai = txc->constant;
+ }
if (txc->modes & ADJ_OFFSET)
ntp_update_offset(txc->offset);
- if (txc->modes & ADJ_TICK)
+ if (txc->modes & ADJ_TICK) {
+ audit_ntp_adjust("tick", tick_usec, txc->tick);
tick_usec = txc->tick;
+ }
if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
ntp_update_frequency();
@@ -729,6 +757,8 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32 *time_tai)
/* 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 {
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4786df904c22..9089ac329e69 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -25,6 +25,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"
@@ -2308,6 +2309,8 @@ int do_adjtimex(struct timex *txc)
ret = timekeeping_inject_offset(&delta);
if (ret)
return ret;
+
+ audit_tk_injoffset(delta);
}
getnstimeofday64(&ts);
--
2.17.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <***@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

Ondrej Mosnacek

2018-07-16 08:15:36 UTC

To me "op" sounds like it should name a verb, not a noun. But if we
want "op" for consistency with other record types and for easier
searching/filtering, I'm OK with that. Steve?

Post by Richard Guy Briggs
I'd probably combine the header file definition of the record type with
the code that actually adds that record output. I agree splitting the
patch along the lines of the two different record types.

OK, now that I think of it, I don't know why I chose to always
separate the record type definition... Indeed if I combine the two
patches I will make git blame-ing much easier. I will also split the
last patch in the final patchset.

Post by Richard Guy Briggs
Very nice! (more below...)

Great. This should be reflected in the syscall's "success=" value.

I don't quite understand what you mean by this sentence. I should
clarify that I was talking about the case where the (modifying)
syscall was successful, but it happened to request a change to the
value that is already set (or to inject an offset of 0). IOW, it logs
also successful no-op changes. My reasoning behind that is that if you
have a policy that says, for example, "Log every attempt to change
the owner of this file.", you would most likely just log all
modification attempts, even if the change is a no-op (i.e. an attempt
to change the file's owner is itself suspicious, even if less serious
than an actual modification).

- RGB
--
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>
Associate Software Engineer, Security Technologies
Red Hat, Inc.

Richard Guy Briggs

2018-07-16 17:36:33 UTC

Post by Ondrej Mosnacek

To me "op" sounds like it should name a verb, not a noun. But if we
want "op" for consistency with other record types and for easier
searching/filtering, I'm OK with that. Steve?

Post by Richard Guy Briggs
Very nice! (more below...)

Great. This should be reflected in the syscall's "success=" value.

I would err on the side of logging *all* attempts to *set* a value,
whether the new value is the same as the existing value or not, and
whether the syscall is successful or not. So I think we are in violent
agreement.

Post by Ondrej Mosnacek

- RGB

Ondrej Mosnacek <omosnace at redhat dot com>

- RGB

--
Richard Guy Briggs <***@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

Paul Moore

2018-07-18 18:36:11 UTC

Post by Ondrej Mosnacek
I tried to implement separate records for each variable as suggested by
Richard and it turned out to be quite straightforward and results in
more compact and readable records (even though there is now a bit more
of them).
- Switched to separate records for each variable
- Both old and new value is now reported for each change
- Injecting offset is reported via a separate record (since this
offset consists of two values and is added directly to the clock,
i.e. it doesn't make sense to log old and new value)
- Added example records produced by chronyd -q (see the commit message
of the last patch)
- The audit_adjtime() function has been modified to only log those
fields that contain values that are actually used, resulting in more
compact records.
- The audit_adjtime() call has been moved to do_adjtimex() in
timekeeping.c
- Added an additional patch (for review) that simplifies the detection
if the syscall is read-only.

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).

While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

--
paul moore
www.paul-moore.com

Steve Grubb

2018-07-18 19:36:25 UTC

Post by Ondrej Mosnacek
- The audit_adjtime() function has been modified to only log those
fields that contain values that are actually used, resulting in more
compact records.
- The audit_adjtime() call has been moved to do_adjtimex() in
timekeeping.c
- Added an additional patch (for review) that simplifies the detection
if the syscall is read-only.

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).
While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

That we record any attempts to change the system time. The problem is that
adjtimex passes a data structure that is opaque to user space. So, we can't
tell if someone is setting time, adjusting a tolerance, or simply retrieving
status.

With stime, we can clearly see the time that was sent into the kernel and it
unconditionally sets time. With settimeofday, it uses a data structure that
we cannot see, but whatever the contents are we are definitely setting time.
Same goes for clock_settime. Only in 1 case do we actually see what the time
is. So, that is not really needed. So, I think what we need to know is did
the syscall do anything that adjusted the system's notion of time? And that's
all.

-Steve

Paul Moore

2018-07-18 19:59:24 UTC

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).
While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

That we record any attempts to change the system time. The problem is that
adjtimex passes a data structure that is opaque to user space. So, we can't
tell if someone is setting time, adjusting a tolerance, or simply retrieving
status.
With stime, we can clearly see the time that was sent into the kernel and it
unconditionally sets time. With settimeofday, it uses a data structure that
we cannot see, but whatever the contents are we are definitely setting time.
Same goes for clock_settime. Only in 1 case do we actually see what the time
is. So, that is not really needed. So, I think what we need to know is did
the syscall do anything that adjusted the system's notion of time? And that's
all.

So presumably my above suggestion of simply recording the system time
both before and after the change would be sufficient, yes?

--
paul moore
www.paul-moore.com

Steve Grubb

2018-07-18 22:34:37 UTC

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).
While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

That we record any attempts to change the system time. The problem is that
adjtimex passes a data structure that is opaque to user space. So, we
can't tell if someone is setting time, adjusting a tolerance, or simply
retrieving status.
With stime, we can clearly see the time that was sent into the kernel and
it unconditionally sets time. With settimeofday, it uses a data
structure that we cannot see, but whatever the contents are we are
definitely setting time. Same goes for clock_settime. Only in 1 case do
we actually see what the time is. So, that is not really needed. So, I
think what we need to know is did the syscall do anything that adjusted
the system's notion of time? And that's all.

So presumably my above suggestion of simply recording the system time
both before and after the change would be sufficient, yes?

Well, I think that we need to make it obvious that a time setting has changed
as in a y/n answer. Doing a comparison of the time to get a y/n answer is
less obvious if you are doing a grep. Also, what time will the event have? I
presume that the event will have the new time since we are on the exit
filter. So, do we really need to collect that?

-Steve

Paul Moore

2018-07-18 23:58:10 UTC

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).
While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

That we record any attempts to change the system time. The problem is that
adjtimex passes a data structure that is opaque to user space. So, we
can't tell if someone is setting time, adjusting a tolerance, or simply
retrieving status.
With stime, we can clearly see the time that was sent into the kernel and
it unconditionally sets time. With settimeofday, it uses a data
structure that we cannot see, but whatever the contents are we are
definitely setting time. Same goes for clock_settime. Only in 1 case do
we actually see what the time is. So, that is not really needed. So, I
think what we need to know is did the syscall do anything that adjusted
the system's notion of time? And that's all.

So presumably my above suggestion of simply recording the system time
both before and after the change would be sufficient, yes?

Well, I think that we need to make it obvious that a time setting has changed
as in a y/n answer. Doing a comparison of the time to get a y/n answer is
less obvious if you are doing a grep.

Presumably we would only be emitting this new record when the system
time changes as the result of a time setting change so the simple
presence of this record should be sufficient to indicate a time
change.

Post by Steve Grubb
Also, what time will the event have? I
presume that the event will have the new time since we are on the exit
filter. So, do we really need to collect that?

The audit event timestamp is not guaranteed to be the same as the user
supplied value so we should explicitly log the old/new system time.

--
paul moore
www.paul-moore.com

Ondrej Mosnacek

2018-07-19 07:36:35 UTC

Looking at these new records, and trying to guess a bit at the
original intent of the feature request, I think we may be going a bit
overboard with the information we are logging. I'm thinking all we
really need to capture in the audit log is the system time both before
and after the change (for the sake of simplicity I suggest using a
data format similar to the audit record timestamp).
While I created the GH issue for this, I believe the original request
came from a Red Hat BZ that Steve created; Steve, what sort of
certification requirements (if any?) are there for logging system time
changes?

That we record any attempts to change the system time. The problem is that
adjtimex passes a data structure that is opaque to user space. So, we can't
tell if someone is setting time, adjusting a tolerance, or simply retrieving
status.
With stime, we can clearly see the time that was sent into the kernel and it
unconditionally sets time. With settimeofday, it uses a data structure that
we cannot see, but whatever the contents are we are definitely setting time.
Same goes for clock_settime. Only in 1 case do we actually see what the time
is. So, that is not really needed. So, I think what we need to know is did
the syscall do anything that adjusted the system's notion of time? And that's
all.

So presumably my above suggestion of simply recording the system time
both before and after the change would be sufficient, yes?

I wouldn't say this is the right approach here, for two reasons that
I'll try to explain below.

adjtimex(2) can be basically used for two major types of adjustment:
1. for (immediately) injecting offset into system time,
2. for changing NTP status variables.

(1.) is the more obvious and simple adjustment, which *directly*
alters the system time. For this adjustment the current proposed
solution simply logs the delta, by which the time is adjusted (it can
be retrieved easily from the supplied timex struct). In my opinion,
logging of this delta is more explicit than logging the timestamps
(which might not give the precise offset anyway, since time might flow
between the two timestamps) and is also more easily grepped in the
logs, as Steve pointed out in his reply. For example, you can quickly
filter out adjustments of less than +/-1 second by simply comparing
the offset in the records.

(2.) is a lot more tricky... These adjustments modify the variables of
the NTP algorithm, which is quite complex and it is not obvious how it
influences the system time. From my understanding, the algorithm is
used for adjustments that require smooth transition over time - such
as (or maybe this is the only use case) inserting a leap second. Note
that in this case, just logging timestamp before and after the syscall
is not sufficient, since the change doesn't happen immediately, but in
small increments that are gradually added *after* the syscall already
finished. I don't know if (or which of) these adjustments can be used
for a real attack, but at least the leap second insertion sounds like
something that could be used to shift the time without being easily
detected. This is why I am wary of throwing things out from the
logging until we understand what is truly important and what is not.

So, to sum up, my two arguments against logging before/after timestamps are:
1. For the case of direct offset injection we can much more easily log
the actual offset and this is also more informative.
2. For the case of NTP adjustments it is just not sufficient, we need
to log some or all of the variable adjustments and let the people
analyzing the logs figure out how that influenced the clock later (NTP
algorithm is just too complex).

I hope this explanation makes the situation a bit more clear. Time is
hard [1] and this particular time issue just refuses to be any
different :)

[1] http://sflanders.net/2015/02/09/time-hard-proper-ntp-configuration/

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

Paul Moore

2018-07-19 23:01:50 UTC

Post by Ondrej Mosnacek