From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ondrej Mosnacek Subject: Re: [RFC PATCH ghak10 3/3] ntp: Audit valid attempts to adjust the clock Date: Mon, 18 Jun 2018 09:35:53 +0200 Message-ID: References: <20180615124523.5474-1-omosnace@redhat.com> <20180615124523.5474-3-omosnace@redhat.com> <20180616164421.og47ljos5f4d6er5@madcap2.tricolour.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (ext-mx19.extmail.prod.ext.phx2.redhat.com [10.5.110.48]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8349C8BB27 for ; Mon, 18 Jun 2018 07:36:09 +0000 (UTC) Received: from mail-ot0-f197.google.com (mail-ot0-f197.google.com [74.125.82.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8E3BE307D85F for ; Mon, 18 Jun 2018 07:36:09 +0000 (UTC) Received: by mail-ot0-f197.google.com with SMTP id j24-v6so9493893otk.11 for ; Mon, 18 Jun 2018 00:36:09 -0700 (PDT) In-Reply-To: <20180616164421.og47ljos5f4d6er5@madcap2.tricolour.ca> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Richard Guy Briggs Cc: Linux-Audit Mailing List List-Id: linux-audit@redhat.com 2018-06-16 18:44 GMT+02:00 Richard Guy Briggs : > On 2018-06-15 14:45, Ondrej Mosnacek wrote: >> (Intentionally not sending to the timekeeping/ntp maintainers just yet, >> let's settle on the record contents/format first.) >> >> Signed-off-by: Ondrej Mosnacek >> --- >> kernel/time/ntp.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c >> index a09ded765f6c..78a01df0dbdb 100644 >> --- a/kernel/time/ntp.c >> +++ b/kernel/time/ntp.c >> @@ -18,6 +18,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "ntp_internal.h" >> #include "timekeeping_internal.h" >> @@ -722,6 +723,16 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32 *time_tai) >> { >> int result; >> >> + /* Only log audit event if the clock was changed/attempted to be changed. >> + * Based on the logic inside timekeeping_validate_timex(). >> + * NOTE: We need to log the event before any of the fields get >> + * overwritten by the output values (the function will not fail, so it >> + * is OK). */ >> + if ( ( (txc->modes & ADJ_ADJTIME) && !(txc->modes & ADJ_OFFSET_READONLY)) >> + || (!(txc->modes & ADJ_ADJTIME) && txc->modes) >> + || (txc->modes & ADJ_SETOFFSET)) > > Wouldn't the third condition be covered by the second? Not if txc->modes has ADJ_ADJTIME set, ADJ_OFFSET_READONLY unset, and ADJ_SETOFFSET set. Then it fails the first two conditions and only matches on the third one. I don't know if such combination can actually occur in practice, but timekeeping_validate_timex() doesn't reject it, so it's better to be on the safe side here. BTW, I just realized that the logging function can be called directly from inside the do_adjtimex() function in timekeeping.c (which is a wrapper around __do_adjtimex()), where it probably suits better (since this function contains the ADJ_SETOFFSET handling code and timekeeping.c also contains the timekeeping_validate_timex() function referenced in the comment). I will move it in v2. > >> + audit_adjtime(txc); >> + >> if (txc->modes & ADJ_ADJTIME) { >> long save_adjust = time_adjust; > > - RGB > > -- > Richard Guy Briggs > 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 Associate Software Engineer, Security Technologies Red Hat, Inc.