From: Juergen Perlinger Date: Thu, 14 May 2015 15:08:20 +0000 (+0200) Subject: [Bug 2830] ntpd doesn't always transfer the correct TAI offset via autokey X-Git-Tag: NTP_4_3_35~3^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=335e135a935a26ac395877597bc63f7641c9603b;p=thirdparty%2Fntp.git [Bug 2830] ntpd doesn't always transfer the correct TAI offset via autokey NTP does *not* transfer the TAI offset, but a leap second announcement. Handling of these announcements and stepping over leap seconds needed improvement. bk: 5554ba6458aE3Pz9dd9VyOz8zyO9fg --- diff --git a/ChangeLog b/ChangeLog index 6dd8aba23..f849c79b7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -39,6 +39,10 @@ * [Bug 2821] Add a missing NTP_PRINTF and a missing const. * [Bug 2822] New leap column in sntp broke NTP::Util.pm. * [Bug 2825] Quiet file installation in html/ . +* [Bug 2830] ntpd doesn't always transfer the correct TAI offset via autokey + NTP does *not* transfer the TAI offset, but a leap second announcement. + Handling of these announcements and stepping over leap seconds + needed improvement. * Add an assert to the ntpq ifstats code. * Clean up the RLIMIT_STACK code. * Improve the ntpq documentation around the controlkey keyid. diff --git a/ntpd/ntp_leapsec.c b/ntpd/ntp_leapsec.c index 2cf7fab82..099d7bfc2 100644 --- a/ntpd/ntp_leapsec.c +++ b/ntpd/ntp_leapsec.c @@ -305,18 +305,32 @@ leapsec_query( * Some operations below are actually NOPs in electric * mode, but having only one code path that works for * both modes is easier to maintain. + * + * There's another quirk we must keep looking out for: + * If we just stepped the clock, the step might have + * crossed a leap boundary. As with backward steps, we + * do not want to raise the 'fired' event in that case. + * So we raise the 'fired' event only if we're close to + * the transition and just reload the limits otherwise. */ - last = pt->head.ttime; - qr->warped = (int16_t)(last.D_s.lo - - pt->head.dtime.D_s.lo); - next = addv64i32(&ts64, qr->warped); - reload_limits(pt, &next); - fired = ucmpv64(&pt->head.ebase, &last) == 0; - if (fired) { - ts64 = next; - ts32 = next.D_s.lo; + last = addv64i32(&pt->head.dtime, 3); /* get boundary */ + if (ucmpv64(&ts64, &last) >= 0) { + /* that was likely a query after a step */ + reload_limits(pt, &ts64); } else { - qr->warped = 0; + /* close enough for deeper examination */ + last = pt->head.ttime; + qr->warped = (int16_t)(last.D_s.lo - + pt->head.dtime.D_s.lo); + next = addv64i32(&ts64, qr->warped); + reload_limits(pt, &next); + fired = ucmpv64(&pt->head.ebase, &last) == 0; + if (fired) { + ts64 = next; + ts32 = next.D_s.lo; + } else { + qr->warped = 0; + } } } @@ -326,7 +340,7 @@ leapsec_query( if (ucmpv64(&ts64, &pt->head.stime) < 0) return fired; - /* now start to collect the remaing data */ + /* now start to collect the remaining data */ due32 = pt->head.dtime.D_s.lo; qr->tai_diff = pt->head.next_tai - pt->head.this_tai; @@ -639,9 +653,15 @@ add_range( const leap_info_t * pi) { /* If the table is full, make room by throwing out the oldest - * entry. But remember the accumulated leap seconds! + * entry. But remember the accumulated leap seconds! Likewise, + * assume a positive leap insertion if this is the first entry + * in the table. This is not necessarily the best of all ideas, + * but it helps a great deal if a system does not have a leap + * table and gets updated from an upstream server. */ - if (pt->head.size >= MAX_HIST) { + if (pt->head.size == 0) { + pt->head.base_tai = pi->taiof - 1; + } else if (pt->head.size >= MAX_HIST) { pt->head.size = MAX_HIST - 1; pt->head.base_tai = pt->info[pt->head.size].taiof; } @@ -707,7 +727,7 @@ skipws( return (char*)noconst(ptr); } -/* [internal] check if a strtoXYZ ended at EOL or whistespace and +/* [internal] check if a strtoXYZ ended at EOL or whitespace and * converted something at all. Return TRUE if something went wrong. */ static int/*BOOL*/ @@ -800,7 +820,7 @@ leapsec_add( struct calendar fts; leap_info_t li; - /* Check against the table expiration and the lates available + /* Check against the table expiration and the latest available * leap entry. Do not permit inserts, only appends, and only if * the extend the table beyond the expiration! */ @@ -852,10 +872,22 @@ leapsec_raw( struct calendar fts; leap_info_t li; - /* Check that we only extend the table. Paranoia rulez! */ - if (pt->head.size && ucmpv64(ttime, &pt->info[0].ttime) <= 0) { - errno = ERANGE; - return FALSE; + /* Check that we either extend the table or get a duplicate of + * the latest entry. The latter is a benevolent overwrite with + * identical data and could happen if we get an autokey message + * that extends the lifetime of the current leapsecond table. + * Otherwise paranoia rulez! + */ + if (pt->head.size) { + int cmp = ucmpv64(ttime, &pt->info[0].ttime); + if (cmp == 0) + cmp -= (taiof != pt->info[0].taiof); + if (cmp < 0) { + errno = ERANGE; + return FALSE; + } + if (cmp == 0) + return TRUE; } ntpcal_ntp64_to_date(&fts, ttime); @@ -865,7 +897,7 @@ leapsec_raw( return FALSE; } fts.month--; /* was in range 1..12, no overflow here! */ - starttime = ntpcal_date_to_ntp64(&fts); + starttime = ntpcal_date_to_ntp64(&fts); li.ttime = *ttime; li.stime = ttime->D_s.lo - starttime.D_s.lo; li.taiof = (int16_t)taiof; @@ -1006,11 +1038,13 @@ static char * lstostr( char * buf; struct calendar tm; - LIB_GETBUF(buf); - ntpcal_ntp64_to_date(&tm, ts); - snprintf(buf, LIB_BUFLENGTH, "%04d-%02d-%02dT%02d:%02dZ", - tm.year, tm.month, tm.monthday, - tm.hour, tm.minute); + if ( ! (ts->d_s.hi >= 0 && ntpcal_ntp64_to_date(&tm, ts) >= 0)) + return "9999-12-31T23:59:59Z"; + + LIB_GETBUF(buf); + snprintf(buf, LIB_BUFLENGTH, "%04d-%02d-%02dT%02d:%02d:%02dZ", + tm.year, tm.month, tm.monthday, + tm.hour, tm.minute, tm.second); return buf; } diff --git a/tests/ntpd/leapsec.cpp b/tests/ntpd/leapsec.cpp index 011595154..d9a68de7b 100644 --- a/tests/ntpd/leapsec.cpp +++ b/tests/ntpd/leapsec.cpp @@ -222,8 +222,9 @@ static const char leap_gthash [] = { "#h 1151a8f e85a5069 9000fcdb 3d5e5365 1d505b37" }; -static uint32_t lsec2009 = 3439756800u; // 1 Jan 2009, 00:00:00 utc -static uint32_t lsec2012 = 3550089600u; // 1 Jul 2012, 00:00:00 utc +static const uint32_t lsec2006 = 3345062400u; // 1 Jan 2006, 00:00:00 utc +static const uint32_t lsec2009 = 3439756800u; // 1 Jan 2009, 00:00:00 utc +static const uint32_t lsec2012 = 3550089600u; // 1 Jul 2012, 00:00:00 utc int stringreader(void* farg) { @@ -429,6 +430,10 @@ TEST_F(leapsecTest, loadFileTTL) { EXPECT_EQ(-1, rc); } +// ===================================================================== +// RANDOM QUERY TESTS +// ===================================================================== + // ---------------------------------------------------------------------- // test query in pristine state (bug#2745 misbehaviour) TEST_F(leapsecTest, lsQueryPristineState) { @@ -552,7 +557,7 @@ TEST_F(leapsecTest, ls2009limdata) { rc = setup_load_table(leap1, TRUE); EXPECT_EQ(1, rc); - // test on-spot with limted table - does not work if build before 2013! + // test on-spot with limited table - does not work if build before 2013! rc = leapsec_query(&qr, lsec2009, NULL); EXPECT_EQ(FALSE, rc); EXPECT_EQ(35, qr.tai_offs); @@ -560,6 +565,73 @@ TEST_F(leapsecTest, ls2009limdata) { EXPECT_EQ(LSPROX_NOWARN, qr.proximity); } +// ---------------------------------------------------------------------- +// Far-distance forward jump into a transiton window. +TEST_F(leapsecTest, qryJumpFarAhead) { + int rc; + leap_result_t qr; + int last, idx; + + for (int mode=0; mode < 2; ++mode) { + leapsec_ut_pristine(); + rc = setup_load_table(leap1, FALSE); + EXPECT_EQ(1, rc); + leapsec_electric(mode); + + rc = leapsec_query(&qr, lsec2006, NULL); + EXPECT_EQ(FALSE, rc); + + rc = leapsec_query(&qr, lsec2012, NULL); + EXPECT_EQ(FALSE, rc); + } +} + +// ---------------------------------------------------------------------- +// Forward jump into the next transition window +TEST_F(leapsecTest, qryJumpAheadToTransition) { + int rc; + leap_result_t qr; + int last, idx; + + for (int mode=0; mode < 2; ++mode) { + leapsec_ut_pristine(); + rc = setup_load_table(leap1, FALSE); + EXPECT_EQ(1, rc); + leapsec_electric(mode); + + rc = leapsec_query(&qr, lsec2009-SECSPERDAY, NULL); + EXPECT_EQ(FALSE, rc); + + rc = leapsec_query(&qr, lsec2009+1, NULL); + EXPECT_EQ(TRUE, rc); + } +} + +// ---------------------------------------------------------------------- +// Forward jump over the next transition window +TEST_F(leapsecTest, qryJumpAheadOverTransition) { + int rc; + leap_result_t qr; + int last, idx; + + for (int mode=0; mode < 2; ++mode) { + leapsec_ut_pristine(); + rc = setup_load_table(leap1, FALSE); + EXPECT_EQ(1, rc); + leapsec_electric(mode); + + rc = leapsec_query(&qr, lsec2009-SECSPERDAY, NULL); + EXPECT_EQ(FALSE, rc); + + rc = leapsec_query(&qr, lsec2009+5, NULL); + EXPECT_EQ(FALSE, rc); + } +} + +// ===================================================================== +// TABLE MODIFICATION AT RUNTIME +// ===================================================================== + // ---------------------------------------------------------------------- // add dynamic leap second (like from peer/clock) TEST_F(leapsecTest, addDynamic) { @@ -621,7 +693,7 @@ TEST_F(leapsecTest, addFixed) { NULL); EXPECT_EQ(FALSE, rc); } - // no do it right + // now do it right for (int idx=0; insns[idx].tt; ++idx) { rc = leapsec_add_fix( insns[idx].of, @@ -640,6 +712,92 @@ TEST_F(leapsecTest, addFixed) { //leapsec_dump(pt, (leapsec_dumper)fprintf, stdout); } +// ---------------------------------------------------------------------- +// add fixed leap seconds (like from network packet) +TEST_F(leapsecTest, addFixedExtend) { + int rc; + leap_result_t qr; + int last, idx; + + static const struct { uint32_t tt; int of; } insns[] = { + {2982009600, 29},// # 1 Jul 1994 + {3029443200, 30},// # 1 Jan 1996 + {0,0} // sentinel + }; + + rc = setup_load_table(leap2, FALSE); + EXPECT_EQ(1, rc); + + leap_table_t * pt = leapsec_get_table(0); + for (last=idx=0; insns[idx].tt; ++idx) { + last = idx; + rc = leapsec_add_fix( + insns[idx].of, + insns[idx].tt, + insns[idx].tt + SECSPERDAY, + NULL); + EXPECT_EQ(TRUE, rc); + } + + // try to extend the expiration of the last entry + rc = leapsec_add_fix( + insns[last].of, + insns[last].tt, + insns[last].tt + 128*SECSPERDAY, + NULL); + EXPECT_EQ(TRUE, rc); + + // try to extend the expiration of the last entry with wrong offset + rc = leapsec_add_fix( + insns[last].of+1, + insns[last].tt, + insns[last].tt + 129*SECSPERDAY, + NULL); + EXPECT_EQ(FALSE, rc); + //leapsec_dump(pt, (leapsec_dumper)fprintf, stdout); +} + +// ---------------------------------------------------------------------- +// add fixed leap seconds (like from network packet) in an otherwise +// empty table and test queries before / between /after the tabulated +// values. +TEST_F(leapsecTest, setFixedExtend) { + int rc; + leap_result_t qr; + int last, idx; + + static const struct { uint32_t tt; int of; } insns[] = { + {2982009600, 29},// # 1 Jul 1994 + {3029443200, 30},// # 1 Jan 1996 + {0,0} // sentinel + }; + + leap_table_t * pt = leapsec_get_table(0); + for (last=idx=0; insns[idx].tt; ++idx) { + last = idx; + rc = leapsec_add_fix( + insns[idx].of, + insns[idx].tt, + insns[idx].tt + 128*SECSPERDAY, + NULL); + EXPECT_EQ(TRUE, rc); + } + + rc = leapsec_query(&qr, insns[0].tt - 86400, NULL); + EXPECT_EQ(28, qr.tai_offs); + + rc = leapsec_query(&qr, insns[0].tt + 86400, NULL); + EXPECT_EQ(29, qr.tai_offs); + + rc = leapsec_query(&qr, insns[1].tt - 86400, NULL); + EXPECT_EQ(29, qr.tai_offs); + + rc = leapsec_query(&qr, insns[1].tt + 86400, NULL); + EXPECT_EQ(30, qr.tai_offs); + + //leapsec_dump(pt, (leapsec_dumper)fprintf, stdout); +} + // ===================================================================== // SEQUENCE TESTS // =====================================================================