[Git][NTPsec/ntpsec][master] Back off the representation change of refids from uint32_t to char[4].

Eric S. Raymond gitlab at mg.gitlab.com
Wed Jan 30 06:45:57 UTC 2019


Eric S. Raymond pushed to branch master at NTPsec / ntpsec


Commits:
fae565d4 by Eric S. Raymond at 2019-01-30T06:42:28Z
Back off the representation change of refids from uint32_t to char[4].

It broke things on big-endian machines. Needs to be redone more carefully.

- - - - -


12 changed files:

- include/ntp.h
- include/ntp_endian.h
- include/ntpd.h
- libntp/ntp_endian.c
- ntpd/ntp_control.c
- ntpd/ntp_io.c
- ntpd/ntp_proto.c
- ntpd/ntp_refclock.c
- ntpd/ntp_timer.c
- ntpd/ntp_util.c
- ntpd/refclock_generic.c
- ntpd/refclock_modem.c


Changes:

=====================================
include/ntp.h
=====================================
@@ -127,7 +127,7 @@ extern uint64_t ntp_random64 (void);
  * is not yet supplying time.
  */
 #define REFIDLEN	sizeof(uint32_t)	/* size of IPv4 network addr */
-typedef unsigned char refid_t[REFIDLEN];
+typedef uint32_t	refid_t;
 
 /*
  * The netendpt structure is used to hold the addresses and socket
@@ -396,13 +396,6 @@ struct peer {
 #define	FLAG_DNS	0x0800u	/* needs DNS lookup */
 #define FLAG_TSTAMP_PPS	0x4cd000u	/* PPS source provides absolute timestamp */
 
-/* The MAC follows any extension fields. */
-/* Its length includes 1 word of keyID. */
-/* MD5 length is 16 bytes => 4+1 */
-/* SHA length is 20 bytes => 5+1 */
-#define MIN_MAC_LEN	(1 * sizeof(uint32_t))	/* crypto_NAK */
-#define	MAX_MAC_LEN	(6 * sizeof(uint32_t))	/* maximum MAC length */
-
 /* This is the new, sane way of representing packets. All fields are
    in host byte order, and the fixed-point time fields are just integers,
    with uints of 2^-16 or 2^-32 seconds as appropriate. */
@@ -414,7 +407,7 @@ struct parsed_pkt {
         int8_t precision;
         uint32_t rootdelay;
         uint32_t rootdisp;
-        refid_t refid;
+        char refid[REFIDLEN];
         uint64_t reftime;
         uint64_t org;
         uint64_t rec;
@@ -429,6 +422,36 @@ struct exten {
         uint8_t *body;
 };
 
+/* This is the old, insane way of representing packets. It'll gradually
+   be phased out and removed. Packets are simply pulled off the wire and
+   then type-punned into this structure, so all fields are in network
+   byte order. Note that there is no pack pragma. The only reason this
+   ever worked at all is that all the fields are self-aligned, so no ABI
+   has been evil enough to insert padding between fields. */
+struct pkt {
+	uint8_t	li_vn_mode;	/* peer leap indicator */
+	uint8_t	stratum;	/* peer stratum */
+	uint8_t	ppoll;		/* peer poll interval */
+	int8_t	precision;	/* peer clock precision */
+	u_fp	rootdelay;	/* roundtrip delay to primary source */
+	u_fp	rootdisp;	/* dispersion to primary source*/
+	refid_t	refid;		/* reference id */
+	l_fp_w	reftime;	/* last update time */
+	l_fp_w	org;		/* originate time stamp */
+	l_fp_w	rec;		/* receive time stamp */
+	l_fp_w	xmt;		/* transmit time stamp */
+
+/* Old style authentication was just appended
+ * without the type/length of an extension header. */
+/* Length includes 1 word of keyID */
+/* MD5 length is 16 bytes => 4+1 */
+/* SHA length is 20 bytes => 5+1 */
+#define MIN_MAC_LEN	(1 * sizeof(uint32_t))	/* crypto_NAK */
+#define	MAX_MAC_LEN	(6 * sizeof(uint32_t))	/* MAX of old style */
+
+	uint32_t	exten[(MAX_MAC_LEN + MAX_EXT_LEN) / sizeof(uint32_t)];
+} __attribute__ ((aligned));
+
 /* pythonize-header: stop ignoring */
 
 /*


=====================================
include/ntp_endian.h
=====================================
@@ -13,7 +13,6 @@
 
 uint16_t ntp_be16dec(const void *buf) __attribute__((pure));
 uint32_t ntp_be32dec(const void *buf) __attribute__((pure));
-void ntp_be32enc(void *buf, uint32_t x);
 uint64_t ntp_be64dec(const void *buf) __attribute__((pure));
 
 #endif


=====================================
include/ntpd.h
=====================================
@@ -189,7 +189,7 @@ extern	int	mprintf_clock_stats(struct peer *, const char *, ...)
 extern	void	record_raw_stats (struct peer *,
 				  int leap, int version, int mode, int stratum,
 				  int ppoll, int precision, double root_delay,
-				  double root_dispersion, uint32_t refid,
+				  double root_dispersion, refid_t refid,
 				  unsigned int outcount);
 extern	void	check_leap_file	(bool is_daily_check, time_t systime);
 
@@ -428,7 +428,4 @@ int nts_daily(void);
 int nts_validate(struct parsed_pkt *, struct ntspeer_t *);
 int nts_decorate(uint32_t *, size_t, struct ntspeer_t *);
 
-/* ntp_util.c */
-extern char * refid_dump(const refid_t, int);
-
 #endif	/* GUARD_NTPD_H */


=====================================
libntp/ntp_endian.c
=====================================
@@ -24,14 +24,6 @@ uint32_t ntp_be32dec(const void *buf) {
 	    (uint32_t)(b[3]);
 }
 
-void ntp_be32enc(void *buf, uint32_t x) {
-	uint8_t *b = (uint8_t*)buf;
-	b[0] = (x >> 24) & 0xff;
-	b[1] = (x >> 16) & 0xff;
-	b[2] = (x >> 8) & 0xff;
-	b[3] = x & 0xff;
-}
-
 uint64_t ntp_be64dec(const void *buf) {
 	const uint8_t *b = (const uint8_t*)buf;
 	return ((uint64_t)(b[0]) << 56) +


=====================================
ntpd/ntp_control.c
=====================================
@@ -26,7 +26,6 @@
 #include "lib_strbuf.h"
 #include "ntp_syscall.h"
 #include "ntp_auth.h"
-#include "ntp_endian.h"
 #include "timespecops.h"
 
 /* undefine to suppress random tags and get fixed emission order */
@@ -67,7 +66,7 @@ static	void	ctl_putuint	(const char *, uint64_t);
 static	void	ctl_puthex	(const char *, uint64_t);
 static	void	ctl_putint	(const char *, long);
 static	void	ctl_putts	(const char *, l_fp *);
-static	void	ctl_putadr	(const char *, refid_t *, sockaddr_u *);
+static	void	ctl_putadr	(const char *, refid_t, sockaddr_u *);
 static	void	ctl_putrefid	(const char *, refid_t);
 static	void	ctl_putarray	(const char *, double *, int);
 static	void	ctl_putsys	(int);
@@ -701,7 +700,7 @@ init_control(void)
 }
 
 /*
- * unmarshall_ntp_control - unmarshall data stream into a ntp_control struct
+ * unmarshall_ntp_control - unmarshall data stream into a ntp_sontrol struct
  */
 void
 unmarshall_ntp_control(struct ntp_control *pkt, struct recvbuf *rbufp)
@@ -1368,25 +1367,31 @@ ctl_putts(
 static void
 ctl_putadr(
 	const char *tag,
-	refid_t *refid,
+	refid_t addr32,
 	sockaddr_u *addr
 	)
 {
+	char *cp;
 	const char *cq;
 	char buffer[200];
 
-	strlcpy(buffer, tag, sizeof(buffer));
-	strlcat(buffer, "=", sizeof(buffer));
-	if (NULL != addr) {
-		cq = socktoa(addr);
-        } else if (NULL != refid) {
-		cq = refid_dump(*refid, 1);
-	} else {
-		cq = "0.0.0.0";
-        }
+	cp = buffer;
+	cq = tag;
+	while (*cq != '\0' && cp < buffer + sizeof(buffer) - 1)
+		*cp++ = *cq++;
 
-	strlcat(buffer, cq, sizeof(buffer));
-	ctl_putdata(buffer, strlen(buffer), false);
+	*cp++ = '=';
+	if (NULL == addr) {
+		struct in_addr in4;
+		in4.s_addr = addr32;
+		cq = inet_ntoa(in4);
+	}
+	else
+		cq = socktoa(addr);
+	INSIST((cp - buffer) < (int)sizeof(buffer));
+	snprintf(cp, sizeof(buffer) - (size_t)(cp - buffer), "%s", cq);
+	cp += strlen(cp);
+	ctl_putdata(buffer, (unsigned)(cp - buffer), false);
 }
 
 
@@ -1509,7 +1514,7 @@ ctl_putsys(
 
 	case CS_REFID:
 		if (sys_vars.sys_stratum > 1 && sys_vars.sys_stratum < STRATUM_UNSPEC)
-			ctl_putadr(sys_var[varid].text, &sys_vars.sys_refid, NULL);
+			ctl_putadr(sys_var[varid].text, sys_vars.sys_refid, NULL);
 		else
 			ctl_putrefid(sys_var[varid].text, sys_vars.sys_refid);
 		break;
@@ -2104,7 +2109,7 @@ ctl_putpeer(
 		break;
 
 	case CP_SRCADR:
-		ctl_putadr(peer_var[id].text, NULL, &p->srcadr);
+		ctl_putadr(peer_var[id].text, 0, &p->srcadr);
 		break;
 
 	case CP_SRCPORT:
@@ -2125,7 +2130,7 @@ ctl_putpeer(
 		break;
 
 	case CP_DSTADR:
-		ctl_putadr(peer_var[id].text, NULL,
+		ctl_putadr(peer_var[id].text, 0,
 			   (p->dstadr != NULL)
 				? &p->dstadr->sin
 				: NULL);
@@ -2182,7 +2187,7 @@ ctl_putpeer(
 		}
 #endif
 		if (p->stratum > 1 && p->stratum < STRATUM_UNSPEC)
-			ctl_putadr(peer_var[id].text, &p->refid,
+			ctl_putadr(peer_var[id].text, p->refid,
 				   NULL);
 		else
 			ctl_putrefid(peer_var[id].text, p->refid);


=====================================
ntpd/ntp_io.c
=====================================
@@ -19,7 +19,6 @@
 #include "ntp_refclock.h"
 #include "ntp_stdlib.h"
 #include "ntp_assert.h"
-#include "ntp_endian.h"
 #include "ntp_dns.h"
 #include "timespecops.h"
 
@@ -394,7 +393,7 @@ interface_dump(const endpt *itf)
 	sockaddr_dump(&itf->mask);
 	printf("name = %s\n", itf->name);
 	printf("flags = 0x%08x\n", itf->flags);
-	printf("addr_refid = %s\n", refid_dump(itf->addr_refid, 2));
+	printf("addr_refid = %08x\n", itf->addr_refid);
 	printf("received = %ld\n", itf->received);
 	printf("sent = %ld\n", itf->sent);
 	printf("notsent = %ld\n", itf->notsent);
@@ -748,7 +747,7 @@ add_interface(
 	)
 {
 	/* Calculate the refid */
-	ntp_be32enc(ep->addr_refid, addr2refid(&ep->sin));
+	ep->addr_refid = addr2refid(&ep->sin);
 	/* link at tail so ntpq -c ifstats index increases each row */
 	LINK_TAIL_SLIST(ep_list, ep, elink, endpt);
 	ninterfaces++;


=====================================
ntpd/ntp_proto.c
=====================================
@@ -19,30 +19,6 @@
 #endif
 #include <unistd.h>
 
-/* This is the old, insane way of representing packets,noqw only used here fot
-   parsing data off the wire. It'll gradually be phased out and removed. The 
-   publuc view of packets is struct parsed_pkt.
-
-   Packets are simply pulled off the wire and
-   then type-punned into this structure, so all fields are in network
-   byte order. Note that there is no pack pragma. The only reason this
-   ever worked at all is that all the fields are self-aligned, so no ABI
-   has been evil enough to insert padding between fields. */
-struct pkt {
-	uint8_t	li_vn_mode;	/* peer leap indicator */
-	uint8_t	stratum;	/* peer stratum */
-	uint8_t	ppoll;		/* peer poll interval */
-	int8_t	precision;	/* peer clock precision */
-	u_fp	rootdelay;	/* roundtrip delay to primary source */
-	u_fp	rootdisp;	/* dispersion to primary source*/
-	refid_t	refid;		/* reference id */
-	l_fp_w	reftime;	/* last update time */
-	l_fp_w	org;		/* originate time stamp */
-	l_fp_w	rec;		/* receive time stamp */
-	l_fp_w	xmt;		/* transmit time stamp */
-	uint32_t	exten[(MAX_MAC_LEN + MAX_EXT_LEN) / sizeof(uint32_t)];
-} __attribute__ ((aligned));
-
 /*
  * Byte order conversion
  */
@@ -991,10 +967,10 @@ clock_update(
 	poll_update(peer, sys_poll);
 	sys_vars.sys_stratum = min(peer->stratum + 1, STRATUM_UNSPEC);
 	if (peer->stratum == STRATUM_REFCLOCK ||
-		peer->stratum == STRATUM_UNSPEC)
-		memcpy(&sys_vars.sys_refid, &peer->refid, REFIDLEN);
+	    peer->stratum == STRATUM_UNSPEC)
+		sys_vars.sys_refid = peer->refid;
 	else
-	    ntp_be32enc(sys_vars.sys_refid, addr2refid(&peer->srcadr));
+		sys_vars.sys_refid = addr2refid(&peer->srcadr);
 	/*
 	 * Root Dispersion (E) is defined (in RFC 5905) as:
 	 *
@@ -1574,16 +1550,14 @@ clock_select(void)
 		 * orphan mode in timer().
 		 */
 		if (peer->stratum == sys_orphan) {
-			char addrhash[REFIDLEN];
 			uint32_t	localmet;
 			uint32_t peermet;
 
 			if (peer->dstadr != NULL)
-				localmet = ntp_be32dec(peer->dstadr->addr_refid);
+				localmet = ntohl(peer->dstadr->addr_refid);
 			else
 				localmet = UINT32_MAX;
-			ntp_be32enc(addrhash, addr2refid(&peer->srcadr));
-			peermet = ntp_be32dec(addrhash);
+			peermet = ntohl(addr2refid(&peer->srcadr));
 			if (peermet < localmet && peermet < orphmet) {
 				typeorphan = peer;
 				orphmet = peermet;
@@ -2109,7 +2083,7 @@ peer_xmit(
 		xpkt.stratum = 0;
 		xpkt.ppoll = 0;
 		xpkt.precision = 0x20;
-		memset(&xpkt.refid, '\0', REFIDLEN);
+		xpkt.refid = 0;
 		xpkt.rootdelay = 0;
 		xpkt.rootdisp =	0;
 		xpkt.reftime = htonl_fp(0);
@@ -2123,7 +2097,7 @@ peer_xmit(
 		xpkt.stratum = STRATUM_TO_PKT(sys_vars.sys_stratum);
 		xpkt.ppoll = peer->hpoll;
 		xpkt.precision = sys_vars.sys_precision;
-		memcpy(&xpkt.refid, &sys_vars.sys_refid, REFIDLEN);
+		xpkt.refid = sys_vars.sys_refid;
 		xpkt.rootdelay = HTONS_FP(DTOUFP(sys_vars.sys_rootdelay));
 		xpkt.rootdisp =	 HTONS_FP(DTOUFP(sys_vars.sys_rootdisp));
 		xpkt.reftime = htonl_fp(sys_vars.sys_reftime);
@@ -2256,7 +2230,7 @@ fast_xmit(
 		xpkt.stratum = STRATUM_TO_PKT(sys_vars.sys_stratum);
 		xpkt.ppoll = max(rbufp->pkt.ppoll, ntp_minpoll);
 		xpkt.precision = sys_vars.sys_precision;
-		memcpy(&xpkt.refid, &sys_vars.sys_refid, REFIDLEN);
+		xpkt.refid = sys_vars.sys_refid;
 		xpkt.rootdelay = HTONS_FP(DTOUFP(sys_vars.sys_rootdelay));
 		xpkt.rootdisp = HTONS_FP(DTOUFP(sys_vars.sys_rootdisp));
 


=====================================
ntpd/ntp_refclock.c
=====================================
@@ -12,7 +12,6 @@
 #include "lib_strbuf.h"
 #include "ntp_calendar.h"
 #include "timespecops.h"
-#include "ntp_endian.h"
 
 #include <stdio.h>
 
@@ -218,7 +217,7 @@ refclock_newpeer(
 		refclock_unpeer(peer);
 		return false;
 	}
-	memcpy(&peer->refid, &pp->refid, REFIDLEN);
+	peer->refid = pp->refid;
 	return true;
 }
 
@@ -859,10 +858,8 @@ refclock_control(
 			pp->fudgetime2 = in->fudgetime2;
 		if (in->haveflags & CLK_HAVEVAL1)
 			peer->stratum = pp->stratum = (uint8_t)in->fudgeval1;
-		if (in->haveflags & CLK_HAVEVAL2) {
-			ntp_be32enc(peer->refid,in->fudgeval2);
-			ntp_be32enc(pp->refid,in->fudgeval2);
-		}
+		if (in->haveflags & CLK_HAVEVAL2)
+			peer->refid = pp->refid = in->fudgeval2;
 		if (in->haveflags & CLK_HAVEFLAG1) {
 			pp->sloppyclockflag &= ~CLK_FLAG1;
 			pp->sloppyclockflag |= in->flags & CLK_FLAG1;
@@ -886,7 +883,7 @@ refclock_control(
 	 */
 	if (out != NULL) {
 		out->fudgeval1 = pp->stratum;
-		out->fudgeval2 = ntp_be16dec(pp->refid);
+		out->fudgeval2 = pp->refid;
 		out->haveflags = CLK_HAVEVAL1 | CLK_HAVEVAL2;
 		out->fudgetime1 = pp->fudgetime1;
 		if (!D_ISZERO_NS(out->fudgetime1))


=====================================
ntpd/ntp_timer.c
=====================================
@@ -234,13 +234,9 @@ timer(void)
 			sys_vars.sys_leap = LEAP_NOWARNING;
 		}
 		sys_vars.sys_stratum = (uint8_t)sys_orphan;
-		if (sys_vars.sys_stratum > 1) {
-			/* set LOOPBACKADR */
-			sys_vars.sys_refid[0] = 127;
-			sys_vars.sys_refid[1] = 0;
-			sys_vars.sys_refid[2] = 0;
-			sys_vars.sys_refid[3] = 1;
-		} else
+		if (sys_vars.sys_stratum > 1)
+			sys_vars.sys_refid = htonl(LOOPBACKADR);
+		else
 			memcpy(&sys_vars.sys_refid, "LOOP", REFIDLEN);
 		sys_offset = 0;
 		sys_vars.sys_rootdelay = 0;


=====================================
ntpd/ntp_util.c
=====================================
@@ -787,17 +787,3 @@ ntpd_time_stepped(void)
 		mon_start((int)saved_mon_enabled);
 	}
 }
-
-char *
-refid_dump(const refid_t refid, int mode)
-{
-    static char outbuf[16];
-    char const *fmt;
-    switch (mode) {
-    case 0: fmt = "%s";	break;
-    case 1: fmt = "%u.%u.%u.%u"; break;
-    default: fmt = "%02x%02x%02x%02x"; break;
-    }
-    snprintf(outbuf, sizeof(outbuf), fmt, refid[0],refid[1],refid[2],refid[3]);
-    return outbuf;
-}


=====================================
ntpd/refclock_generic.c
=====================================
@@ -24,7 +24,6 @@
 
 #include "config.h"
 #include "ntp_types.h"
-#include "ntp_endian.h"
 #include "timespecops.h"
 
 /*
@@ -2617,7 +2616,7 @@ parse_start(
 	if (peer->stratum <= 1)
 	    memmove((char *)&parse->generic->refid, parse->parse_type->cl_id, REFIDLEN);
 	else
-	    ntp_be32enc(parse->generic->refid, PARSEHSREFID);
+	    parse->generic->refid = htonl(PARSEHSREFID);
 
 	parse->generic->io.fd = fd232;
 


=====================================
ntpd/refclock_modem.c
=====================================
@@ -909,7 +909,7 @@ modem_timecode(
 	 * telephone networks the propatation time can be different for
 	 * each call and can reach 200 ms for some calls.
 	 */
-	memcpy(&peer->refid, &pp->refid, REFIDLEN);
+	peer->refid = pp->refid;
 	pp->lastrec = up->tstamp;
 	if (up->msgcnt == 0)
 		return;



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/fae565d45aae5eb78f79a80b99e3538991748a3d

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/fae565d45aae5eb78f79a80b99e3538991748a3d
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20190130/3a1affaf/attachment-0001.html>


More information about the vc mailing list