From b815521384ef366954b94d0e71314978059654d9 Mon Sep 17 00:00:00 2001
From: James Elliott <james-d-elliott@users.noreply.github.com>
Date: Tue, 17 Jan 2023 22:54:17 +1100
Subject: [PATCH] fix(ntp): version 4 encoded incorrectly (#4773)

This fixes an issue where version 4 was actually serialized as version 5 due to some binary math issues. It also fixes the fact the leap value was incorrect, it should have been set to unknown.
---
 internal/ntp/const.go     | 26 +++++++++++++++++++-------
 internal/ntp/ntp.go       |  2 +-
 internal/ntp/util.go      | 14 ++++++--------
 internal/ntp/util_test.go | 12 ++++--------
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/internal/ntp/const.go b/internal/ntp/const.go
index ed9c35216..8f60fc22f 100644
--- a/internal/ntp/const.go
+++ b/internal/ntp/const.go
@@ -1,15 +1,27 @@
 package ntp
 
-const (
-	ntpClientModeValue  uint8 = 3  // 00000011.
-	ntpLeapEnabledValue uint8 = 64 // 01000000.
-	ntpVersion3Value    uint8 = 24 // 00011000.
-	ntpVersion4Value    uint8 = 40 // 00101000.
-)
-
 const ntpEpochOffset = 2208988800
 
 const (
 	ntpV3 ntpVersion = iota
 	ntpV4
 )
+
+const (
+	maskMode    = 0xf8
+	maskVersion = 0xc7
+	maskLeap    = 0x3f
+)
+
+const (
+	modeClient = 3
+)
+
+const (
+	version3 = 3
+	version4 = 4
+)
+
+const (
+	leapUnknown = 3
+)
diff --git a/internal/ntp/ntp.go b/internal/ntp/ntp.go
index 991eec9ce..fe8ba1964 100644
--- a/internal/ntp/ntp.go
+++ b/internal/ntp/ntp.go
@@ -40,7 +40,7 @@ func (p *Provider) StartupCheck() (err error) {
 		version = ntpV3
 	}
 
-	req := &ntpPacket{LeapVersionMode: ntpLeapVersionClientMode(false, version)}
+	req := &ntpPacket{LeapVersionMode: ntpLeapVersionClientMode(version)}
 
 	if err := binary.Write(conn, binary.BigEndian, req); err != nil {
 		p.log.Warnf("Could not write to the NTP server socket to validate the system time is properly synchronized: %+v", err)
diff --git a/internal/ntp/util.go b/internal/ntp/util.go
index fd6b6a835..4e590c6d1 100644
--- a/internal/ntp/util.go
+++ b/internal/ntp/util.go
@@ -3,20 +3,18 @@ package ntp
 import "time"
 
 // ntpLeapVersionClientMode does the mathematics to configure the leap/version/mode value of an NTP client packet.
-func ntpLeapVersionClientMode(leap bool, version ntpVersion) (lvm uint8) {
-	lvm = ntpClientModeValue
-
-	if leap {
-		lvm += ntpLeapEnabledValue
-	}
+func ntpLeapVersionClientMode(version ntpVersion) (lvm uint8) {
+	lvm = (lvm & maskMode) | uint8(modeClient)
 
 	switch version {
 	case ntpV3:
-		lvm += ntpVersion3Value
+		lvm = (lvm & maskVersion) | uint8(version3)<<3
 	case ntpV4:
-		lvm += ntpVersion4Value
+		lvm = (lvm & maskVersion) | uint8(version4)<<3
 	}
 
+	lvm = (lvm & maskLeap) | uint8(leapUnknown)<<6
+
 	return lvm
 }
 
diff --git a/internal/ntp/util_test.go b/internal/ntp/util_test.go
index caf07ca02..cff026679 100644
--- a/internal/ntp/util_test.go
+++ b/internal/ntp/util_test.go
@@ -29,13 +29,9 @@ func TestNtpPacketToTime(t *testing.T) {
 }
 
 func TestLeapVersionClientMode(t *testing.T) {
-	v3Noleap := uint8(27)
-	v4Noleap := uint8(43)
-	v3leap := uint8(91)
-	v4leap := uint8(107)
+	v3Noleap := uint8(0xdb)
+	v4Noleap := uint8(0xe3)
 
-	assert.Equal(t, v3Noleap, ntpLeapVersionClientMode(false, ntpV3))
-	assert.Equal(t, v4Noleap, ntpLeapVersionClientMode(false, ntpV4))
-	assert.Equal(t, v3leap, ntpLeapVersionClientMode(true, ntpV3))
-	assert.Equal(t, v4leap, ntpLeapVersionClientMode(true, ntpV4))
+	assert.Equal(t, v3Noleap, ntpLeapVersionClientMode(ntpV3))
+	assert.Equal(t, v4Noleap, ntpLeapVersionClientMode(ntpV4))
 }