From 9fbf8ac3d9527181c5279dc6c6e9140f09e5b315 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Wed, 22 May 2024 15:05:59 +0400 Subject: [PATCH] fix: offset calculation for NTP responses The problem here is that `time.Duration` and `time.Time` types don't exactly match the NTP 64-bit time value, so the expected calculations for big time jumps (including across NTP eras) don't work properly. See: * https://www.eecis.udel.edu/~mills/y2k.html * https://www.eecis.udel.edu/~mills/time.html Ref: https://github.com/siderolabs/talos/issues/8771 Signed-off-by: Andrey Smirnov --- ntp.go | 15 ++++++++++++--- ntp_test.go | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/ntp.go b/ntp.go index 0d25544..009528b 100644 --- a/ntp.go +++ b/ntp.go @@ -749,9 +749,18 @@ func rtt(org, rec, xmt, dst ntpTime) time.Duration { func offset(org, rec, xmt, dst ntpTime) time.Duration { // local clock offset // offset = ((rec-org) + (xmt-dst)) / 2 - a := rec.Time().Sub(org.Time()) - b := xmt.Time().Sub(dst.Time()) - return (a + b) / time.Duration(2) + // + // see https://www.eecis.udel.edu/~mills/time.html for more details + // the input is 64 unsigned numbers, output is 63 bits signed (precision) + a := int64(rec) - int64(org) + b := int64(xmt) - int64(dst) + d := (a + b) / 2 + + if d > 0 { + return ntpTime(d).Duration() + } else { + return -ntpTime(-d).Duration() + } } func minError(org, rec, xmt, dst ntpTime) time.Duration { diff --git a/ntp_test.go b/ntp_test.go index badf383..bcb928d 100644 --- a/ntp_test.go +++ b/ntp_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // The NTP server to use for online unit tests. May be overridden by the @@ -388,6 +389,26 @@ func TestOfflineOffsetCalculationNegative(t *testing.T) { assert.Equal(t, expectedOffset, offset) } +func TestOfflineOffsetCalculationNegativeBig(t *testing.T) { + // these timestamps are in different NTP epochs, + // see: + // * https://www.eecis.udel.edu/~mills/y2k.html + serverNow, err := time.Parse("2006-01-02 15:04:05", "2024-05-22 11:30:15") + require.NoError(t, err) + + clientNow, err := time.Parse("2006-01-02 15:04:05", "2047-01-01 00:00:00") + require.NoError(t, err) + + org := toNtpTime(clientNow) + rec := toNtpTime(serverNow) + xmt := toNtpTime(serverNow.Add(1 * time.Second)) + dst := toNtpTime(clientNow.Add(1 * time.Second)) + + expectedOffset := -clientNow.Sub(serverNow) + offset := offset(org, rec, xmt, dst) + assert.Equal(t, expectedOffset, offset) +} + func TestOfflineReferenceString(t *testing.T) { cases := []struct { Stratum byte