Re: [PATCH ghak10 v5 2/2] timekeeping/ntp: Audit clock/NTP params adjustments

From: John Stultz
Date: Fri Aug 24 2018 - 16:20:48 EST

Next message: Marcus Folkesson: "[PATCH] iio: dac: mcp4922: fix error handling in mcp4922_write_raw"
Previous message: Evan Green: "Re: [PATCH v2 3/4] drivers: edac: Add EDAC driver support for QCOM SoCs"
In reply to: Richard Guy Briggs: "Re: [PATCH ghak10 v5 2/2] timekeeping/ntp: Audit clock/NTP params adjustments"
Next in thread: Ondrej Mosnacek: "Re: [PATCH ghak10 v5 2/2] timekeeping/ntp: Audit clock/NTP params adjustments"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Aug 24, 2018 at 12:47 PM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> On 2018-08-24 14:00, Ondrej Mosnacek wrote:
>> 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).
>
> I thought I saw it suggested earlier in one of the replies to a previous
> revision of the patchset to separate the two types of records with their
> calling circumstances. The inj-offset bits could stand alone in their
> own patch leaving all the rest in its own patch. The record numbers and
> examples are easier to offer when given together, but they aren't as
> clear they are indepnendent records and callers. That way, each patch
> stands on its own. (more below)
>
>> 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): op=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=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): op=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): op=status old=8256 new=8257
>> type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): op=offset old=0 new=0
>> type=TIME_ADJNTPVAL msg=audit(1530616044.507:8): op=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): op=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): op=freq old=0 new=49180377088000
>> type=TIME_ADJNTPVAL msg=audit(1530616044.511:11): op=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): op=status old=64 new=64
>> 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): op=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): op=freq old=49180377088000 new=49180377088000
>> type=TIME_ADJNTPVAL msg=audit(1530616033.783:14): op=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.)
>>
>> The changes to the time_maxerror, time_esterror, and time_constant
>> variables are not logged, as these are not important for security.
>>
>> 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).
>>
>> An overview of changes that can be done via adjtimex(2) (based on
>> information from Miroslav Lichvar) and whether they are audited:
>> timekeeping_inject_offset() -- injects offset directly into system
>> time (AUDITED)
>> __timekeeping_set_tai_offset() -- sets the offset from the
>> International Atomic Time
>> (AUDITED)
>> NTP variables:
>> time_offset -- can adjust the clock by up to 0.5 seconds per call
>> and also speed it up or slow down by up to about
>> 0.05% (43 seconds per day) (AUDITED)
>> time_freq -- can speed up or slow down by up to about 0.05%
>> time_status -- can insert/delete leap seconds and it also enables/
>> disables synchronization of the hardware real-time
>> clock (AUDITED)
>> time_maxerror, time_esterror -- change error estimates used to
>> inform userspace applications
>> (NOT AUDITED)
>> time_constant -- controls the speed of the clock adjustments that
>> are made when time_offset is set (NOT AUDITED)
>> time_adjust -- can temporarily speed up or slow down the clock by up
>> to 0.05% (AUDITED)
>> tick_usec -- a more extreme version of time_freq; can speed up or
>> slow down the clock by up to 10% (AUDITED)
>>
>> Cc: Miroslav Lichvar <mlichvar@xxxxxxxxxx>
>> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
>> ---
>> kernel/time/ntp.c | 38 ++++++++++++++++++++++++++++++--------
>> kernel/time/timekeeping.c | 3 +++
>> 2 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
>> index a09ded765f6c..f96c6d326aae 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,21 +675,31 @@ 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)
>> @@ -700,14 +716,18 @@ static inline void process_adjtimex_modes(struct timex *txc,
>> time_constant = max(time_constant, 0l);
>> }
>>
>> - 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;
>> + }
>
> It appears this time_tai use of "constant" is different than
> time_constant, the former not mentioned by Miroslav Lichvar. What is it
> and is it important to log for security? It sounds like it is
> important.

>From the adjtimex man page:
ADJ_TAI (since Linux 2.6.26)
Set TAI (Atomic International Time) offset from buf->constant.

thanks
-john

Next message: Marcus Folkesson: "[PATCH] iio: dac: mcp4922: fix error handling in mcp4922_write_raw"
Previous message: Evan Green: "Re: [PATCH v2 3/4] drivers: edac: Add EDAC driver support for QCOM SoCs"
In reply to: Richard Guy Briggs: "Re: [PATCH ghak10 v5 2/2] timekeeping/ntp: Audit clock/NTP params adjustments"
Next in thread: Ondrej Mosnacek: "Re: [PATCH ghak10 v5 2/2] timekeeping/ntp: Audit clock/NTP params adjustments"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]