Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180824120001.20771-1-omosnace@redhat.com> <20180824120001.20771-2-omosnace@redhat.com>
In-Reply-To: <20180824120001.20771-2-omosnace@redhat.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Thu, 13 Sep 2018 23:18:56 -0400
Message-ID: <CAHC9VhTZkrmgt7rxNYWTgYvm+TVGgRD6iFu9dDtZ7eYNXBKqHQ@mail.gmail.com>
Subject: Re: [PATCH ghak10 v5 1/2] audit: Add functions to log time adjustments
To:     omosnace@redhat.com
Cc:     linux-audit@redhat.com, rgb@redhat.com, sgrubb@redhat.com,
        mlichvar@redhat.com, john.stultz@linaro.org, tglx@linutronix.de,
        sboyd@kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 24, 2018 at 8:00 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> 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.
>
> Next, it 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.
>
> Quick reference for the fields of the new records:
>     AUDIT_TIME_INJOFFSET
>         sec - the 'seconds' part of the offset
>         nsec - the 'nanoseconds' part of the offset
>     AUDIT_TIME_ADJNTPVAL
>         op - 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
>             adjust - corresponding to the time_adjust variable
>             tick   - corresponding to the tick_usec variable
>             tai    - corresponding to the timekeeping's TAI offset

I understand that reusing "op" is tempting, but the above aren't
really operations, they are state variables which are being changed.
Using the CONFIG_CHANGE record as a basis, I wonder if we are better
off with something like the following:

 type=TIME_CHANGE <var>=<value_new> old=<value_old>

... you might need to preface the variable names with something like
"ntp_" or "offset_".  You'll notice I'm also suggesting we use a
single record type here; is there any reason why two records types are
required?

>         old - the old value
>         new - the new value
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  include/linux/audit.h      | 21 +++++++++++++++++++++
>  include/uapi/linux/audit.h |  2 ++
>  kernel/auditsc.c           | 15 +++++++++++++++
>  3 files changed, 38 insertions(+)

A reminder that we need tests for these new records and a RFE page on the wiki:

* https://github.com/linux-audit/audit-testsuite
* https://github.com/linux-audit/audit-kernel/wiki

-- 
paul moore
www.paul-moore.com