Skip to content

Commit

Permalink
fix(ntp): version 4 encoded incorrectly (#4773)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
james-d-elliott committed Jan 17, 2023
1 parent 091f871 commit b815521
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 25 deletions.
26 changes: 19 additions & 7 deletions 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
)
2 changes: 1 addition & 1 deletion internal/ntp/ntp.go
Expand Up @@ -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)
Expand Down
14 changes: 6 additions & 8 deletions internal/ntp/util.go
Expand Up @@ -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
}

Expand Down
14 changes: 5 additions & 9 deletions internal/ntp/util_test.go
Expand Up @@ -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)

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))
v3Noleap := uint8(0xdb)
v4Noleap := uint8(0xe3)

assert.Equal(t, v3Noleap, ntpLeapVersionClientMode(ntpV3))
assert.Equal(t, v4Noleap, ntpLeapVersionClientMode(ntpV4))
}

0 comments on commit b815521

Please sign in to comment.