Improve comments and minor refactors

Author: beevik

auth.go

@@ -115,15 +115,15 @@ func calcCMAC_AES(payload, key []byte) []byte {
 		xor(cmac, payload)
 		xor(cmac, k1)
 	} else {
-		xor(cmac, pad(payload))
+		xor(cmac, padblock(payload))
 		xor(cmac, k2)
 	}
 	c.Encrypt(cmac, cmac)
 
 	return cmac
 }
 
-func pad(block []byte) []byte {
+func padblock(block []byte) []byte {
 	pad := make([]byte, 16-len(block))
 	pad[0] = 0x80
 	return append(block, pad...)

ntp.go

@@ -177,13 +177,12 @@ type QueryOptions struct {
 	// of the response. Used only in NTPv5.
 	RequestReferenceID ReferenceIDRequest
 
-	// RequestSupportedVersions indicates whether to request which versions of
-	// the NTP protocol are supported by the server. When used with NTPv5, the
+	// RequestSupportedVersions indicates whether to request the versions of
+	// the NTP protocol supported by the server. When used with NTPv5, the
 	// response will list all supported versions. When used with NTPv3 or
-	// NTPv4, the response's supported versions will include the version used
-	// in the query as well as 5 if the server supports it. The response's
-	// ReferenceTime field will be invalid in NTPv3 and NTPv4 if this option
-	// is set.
+	// NTPv4, the response's ReferenceTime value will be invalid, and the
+	// supported version list will include the version used in the query as
+	// well as 5 if the server supports it.
 	RequestSupportedVersions bool
 
 	// RequestCorrection indicates whether to request delay corrections from
@@ -216,13 +215,9 @@ type QueryOptions struct {
 	// is guaranteed to include a port number.
 	Dialer func(localAddress, remoteAddress string) (net.Conn, error)
 
-	// Dial is a callback used to override the default UDP network dialer.
-	//
 	// DEPRECATED. Use Dialer instead.
 	Dial func(laddr string, lport int, raddr string, rport int) (net.Conn, error)
 
-	// Port indicates the port used to reach the remote NTP server.
-	//
 	// DEPRECATED. Embed the port number in the query address string instead.
 	Port int
 }
@@ -370,10 +365,6 @@ type Response struct {
 	// Used only in NTPv5.
 	ServerCookie uint64
 
-	// Time is the time the server transmitted this response, measured using
-	// its own clock. You should not use this value for time synchronization
-	// purposes. Add ClockOffset to your system clock instead.
-	//
 	// DEPRECATED. Use Timestamps.ServerXmit instead.
 	Time time.Time
 

ntp4.go

@@ -198,12 +198,14 @@ func queryV4(conn net.Conn, opt *QueryOptions) (*Response, error) {
 		Time:           m.TransmitTime.TimeV4(),
 	}
 
-	// If NTPv5 support was requested, check for it.
+	// If supported versions were requested, check the response for an answer.
 	if opt.RequestSupportedVersions {
+		// Always include the version used in the query.
 		r.SupportedVersions = []int{opt.Version}
 
 		// If the server responded to the NTPv5 support request in its
-		// ReferenceTime field, add version 5 and clear the ReferenceTime.
+		// ReferenceTime field, add version 5 to the list and invalidate the
+		// ReferenceTime.
 		if m.ReferenceTime == v5sentinel {
 			r.SupportedVersions = append(r.SupportedVersions, 5)
 			r.ReferenceTime = ntpEra0

ntp5.go

@@ -18,6 +18,7 @@ import (
 	"time"
 )
 
+// NTPv5 errors. Will move to ntp.go once NTPv5 is finalized.
 var (
 	ErrInvalidDraftID            = errors.New("invalid draft ID value in response")
 	ErrInvalidExtensionField     = errors.New("invalid extension field in response")
@@ -186,7 +187,7 @@ func (m *messageV5) getLeap() LeapIndicator {
 
 // queryV5 performs an NTPv5 time query using the provided connection.
 func queryV5(conn net.Conn, opt *QueryOptions) (*Response, error) {
-	// Generate a random client cookie if not set by the caller.
+	// Generate a random client cookie. The response cookie needs to match.
 	clientCookie, err := randUint64()
 	if err != nil {
 		return nil, err
@@ -258,7 +259,7 @@ func queryV5(conn net.Conn, opt *QueryOptions) (*Response, error) {
 		return nil, err
 	}
 
-	// Compare cookies.
+	// Validate the client cookie.
 	if m.ClientCookie != clientCookie {
 		return nil, ErrServerResponseMismatch
 	}
@@ -339,7 +340,7 @@ func queryV5(conn net.Conn, opt *QueryOptions) (*Response, error) {
 
 		case extServerInfo:
 			bits := binary.BigEndian.Uint16(body[0:2])
-			for v := 3; v <= 5; v++ {
+			for v := 1; v <= 5; v++ {
 				if bits&(1<<uint16(v-1)) != 0 {
 					r.SupportedVersions = append(r.SupportedVersions, v)
 				}
@@ -397,7 +398,7 @@ func queryV5(conn net.Conn, opt *QueryOptions) (*Response, error) {
 			}
 		}
 
-		offset += paddedLen(xlen)
+		offset += padlen(xlen)
 		curr = recvBuf[offset:]
 	}
 
@@ -535,13 +536,10 @@ func parseV5Response(data []byte) (*messageV5, error) {
 }
 
 func writeExtDraftID(buf *bytes.Buffer) {
-	valueLenPadded := paddedLen(len(draftID))
-	totalLen := 4 + len(draftID) // Confirm that length shouldn't include pad!
-
 	binary.Write(buf, binary.BigEndian, extDraftID)
-	binary.Write(buf, binary.BigEndian, uint16(totalLen))
+	binary.Write(buf, binary.BigEndian, uint16(4+len(draftID)))
 	buf.Write([]byte(draftID))
-	buf.Write(padBytes[:valueLenPadded-len(draftID)])
+	buf.Write(pad(len(draftID)))
 }
 
 func writeExtRefIDRequest(buf *bytes.Buffer, req ReferenceIDRequest) {
@@ -592,6 +590,10 @@ func writeExtSecondaryTimestamp(buf *bytes.Buffer, timescale Timescale) {
 
 var padBytes = make([]byte, 4)
 
-func paddedLen(len int) int {
+func padlen(len int) int {
 	return (len + 3) & ^3
 }
+
+func pad(len int) []byte {
+	return padBytes[:padlen(len)-len]
+}

ntp5_test.go

@@ -264,7 +264,7 @@ func TestOfflineV5DraftIDExtension(t *testing.T) {
 	xlen := binary.BigEndian.Uint16(data[2:4])
 
 	assert.Equal(t, extDraftID, xtype)
-	assert.Equal(t, buf.Len(), paddedLen(int(xlen)), "Length should include header")
+	assert.Equal(t, buf.Len(), padlen(int(xlen)), "Length should include header")
 
 	assert.True(t, bytes.Contains(data[4:], []byte(draftID)), "Extension should contain draft ID string")
 }