Messages in this thread | | | Date | Fri, 14 Sep 2018 12:24:03 -0400 | From | Richard Guy Briggs <> | Subject | Re: [PATCH ghak10 v5 1/2] audit: Add functions to log time adjustments |
| |
On 2018-09-14 11:34, Steve Grubb wrote: > On Friday, September 14, 2018 11:16:43 AM EDT Richard Guy Briggs wrote: > > On 2018-09-13 23:18, Paul Moore wrote: > > > 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? > > > > Why not do something like: > > > > type=TIME_CHANGE var=<var> new=<value_new> old=<value_old> > > > > So that we don't pollute the field namespace *and* create 8 variants on > > the same record format? This shouldn't be much of a concern with binary > > record formats, but we're stuck with the current parsing scheme for now. > > Something like this or the other format is fine. Neither hurt parsing because > these are not searchable fields. We only have issues when it involves a > searchable field name.
Ok, fair enough. Thanks Steve.
> HTH... > > -Steve > > > > > 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 > > > > - RGB
- RGB
-- Richard Guy Briggs <rgb@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
| |