linux-kernel - Re: [RFC PATCH ghak10 v6 0/2] audit: Log changes that can affect the system clock

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: <CAHC9VhQavjf5M2wDU2jfFe3NpsxCd_mFjtrUC-0jujZFj2tTdg@mail.gmail.com>
Date:   Mon, 25 Mar 2019 10:50:36 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Ondrej Mosnacek <omosnace@...hat.com>,
        Miroslav Lichvar <mlichvar@...hat.com>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-audit@...hat.com, Richard Guy Briggs <rgb@...hat.com>,
        Steve Grubb <sgrubb@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH ghak10 v6 0/2] audit: Log changes that can affect the
 system clock

On Thu, Mar 7, 2019 at 7:33 AM Ondrej Mosnacek <omosnace@...hat.com> wrote:
> This patchset implements auditing of (syscall-triggered) changes that
> can modify or indirectly affect the system clock. Some of these
> changes can already be detected by simply logging relevant syscalls,
> but this has some disadvantages:
>   a) It is usually not possible to find out from the syscall records
>      the amount by which the time was shifted.
>   b) Syscalls like adjtimex(2) or clock_adjtime(2) can be used also
>      for read-only operations, which might flood the audit log with
>      false positives. (Note that these patches don't solve this
>      problem yet due to the limitations of current record filtering
>      capabilities.)
>
> The main motivation is to provide better reliability of timestamps
> on the system as mandated by the FPT_STM.1 security functional
> requirement from Common Criteria. This requirement apparently demands
> that it is possible to reconstruct from audit trail the old and new
> values of the time when it is adjusted (see [1]).
>
> The current version of the patchset logs the following changes:
>   - direct setting of system time to a given value
>   - direct injection of timekeeping offset
>   - adjustment of timekeeping's TAI offset
>   - NTP value adjustments:
>     - time_offset
>     - time_freq
>     - time_status
>     - time_adjust
>     - tick_usec
>
> Changes to the following NTP values are not logged, as they are not
> important for security:
>   - time_maxerror
>   - time_esterror
>   - time_constant
>
> Audit kernel GitHub issue: https://github.com/linux-audit/audit-kernel/issues/10
> Audit kernel RFE page: https://github.com/linux-audit/audit-kernel/wiki/RFE-More-detailed-auditing-of-changes-to-system-clock
>
> Testing: Passed audit-testuite; functional tests TBD
>
> Changes in v6:
>   - Reorganized the patches to group changes by record type, not
>     kernel subsytem, as suggested in earlier discussions
>   - Added checks to ignore no-change events (new value == old value)
>   - Added TIME_INJOFFSET logging also to do_settimeofday64() to cover
>     syscalls such as settimeofday(2), stime(2), clock_settime(2)
>   - Created an RFE page on audit-kernel GitHub
> TODO:
>   - tests for audit-testsuite
>
> v5: https://www.redhat.com/archives/linux-audit/2018-August/msg00039.html
> Changes in v5:
>   - Dropped logging of some less important changes and update commit messages
>   - No longer mark the patchset as RFC
>
> v4: https://www.redhat.com/archives/linux-audit/2018-August/msg00023.html
> Changes in v4:
>   - Squashed first two patches into one
>   - Renamed ADJNTPVAL's "type" field to "op" to align with audit record
>     conventions
>   - Minor commit message editing
>   - Cc timekeeping/NTP people for feedback
>
> v3: https://www.redhat.com/archives/linux-audit/2018-July/msg00001.html
> Changes in v3:
>   - 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)
>
> v2: https://www.redhat.com/archives/linux-audit/2018-June/msg00114.html
> Changes in v2:
>   - 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.
>
> v1: https://www.redhat.com/archives/linux-audit/2018-June/msg00095.html
>
> [1] https://www.niap-ccevs.org/MMO/PP/pp_ca_v2.1.pdf -- section 5.1,
>     table 4
>
> Ondrej Mosnacek (2):
>   timekeeping: Audit clock adjustments
>   ntp: Audit NTP parameters adjustment
>
>  include/linux/audit.h      | 29 +++++++++++++++++++++++++++++
>  include/uapi/linux/audit.h |  2 ++
>  kernel/auditsc.c           | 15 +++++++++++++++
>  kernel/time/ntp.c          | 38 ++++++++++++++++++++++++++++++--------
>  kernel/time/timekeeping.c  |  6 ++++++
>  5 files changed, 82 insertions(+), 8 deletions(-)

These patches look fine to me, but it would be really nice to get an
ACK from the time folks before I merge this into audit/next.  Time
folks, I know you've looked at previous versions of this patchset, can
you give this a quick look to make sure everything is still okay from
your perspective?

-- 
paul moore
www.paul-moore.com