[Bug 3864] ntpd IPv6 refid different for big-endian and little-endian.

bk: 64e5e663hRLWRqC-38IAdckA7hZt3A

diff --git a/ChangeLog b/ChangeLog
index aa7444bda81a54e8b9063c2ba0f1f87c4bb8f3d3..8509fa55046b3e45cde79c396f4c053b65179be9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 ---
 
+* [Bug 3864] ntpd IPv6 refid different for big-endian and little-endian.
+             <hart@ntp.org>
 * [Bug 3856] Enable Edit & Continue debugging with Visual Studio.
              <hart@ntp.org>
 * [Bug 3853] Clean up warnings with modern compilers. <hart@ntp.org>

diff --git a/include/ntp.h b/include/ntp.h
index a066d6ee782a5ddca5cd0c198ba12ec24f7d7cbd..441f5889d4abfd5c0d63e0790ab9a6dba920c071 100644 (file)
--- a/include/ntp.h
+++ b/include/ntp.h
@@ -188,6 +188,9 @@ struct interface {
        u_int32         flags;          /* interface flags */
        int             last_ttl;       /* last TTL specified */
        u_int32         addr_refid;     /* IPv4 addr or IPv6 hash */
+#    ifdef WORDS_BIGENDIAN
+       u_int32         old_refid;      /* byte-swapped IPv6 refid */
+#    endif
        int             num_mcast;      /* mcast addrs enabled */
        u_long          starttime;      /* current_time at creation */
        volatile long   received;       /* number of incoming packets */
@@ -948,4 +951,10 @@ struct endpoint {
 #define MRU_ROW_LIMIT  256
 /* similar datagrams per response limit for ntpd */
 #define MRU_FRAGS_LIMIT        128
+
+#define BYTESWAP32(u32)                                                        \
+                       (((u_int32)(u32) & 0xff000000) >> 24 |          \
+                        ((u_int32)(u32) &   0xff0000) >>  8 |          \
+                        ((u_int32)(u32) &     0xff00) <<  8 |          \
+                        ((u_int32)(u32) &       0xff) << 24)
 #endif /* NTP_H */

diff --git a/libntp/a_md5encrypt.c b/libntp/a_md5encrypt.c
index 7a372969123fd584ed3d1ecf2a8969dd198a35a2..ad6c28c17b1fd7f7893ec0e2138239cf8832f614 100644 (file)
--- a/libntp/a_md5encrypt.c
+++ b/libntp/a_md5encrypt.c
@@ -254,7 +254,16 @@ MD5authdecrypt(
  * Calculate the reference id from the address. If it is an IPv4
  * address, use it as is. If it is an IPv6 address, do a md5 on
  * it and use the bottom 4 bytes.
- * The result is in network byte order.
+ * The result is in network byte order for IPv4 addreseses.  For
+ * IPv6, ntpd long differed in the hash calculated on big-endian
+ * vs. little-endian because the first four bytes of the MD5 hash
+ * were used as a u_int32 without any byte swapping.  This broke
+ * the refid-based loop detection between mixed-endian systems.
+ * In order to preserve behavior on the more-common little-endian
+ * systems, the hash is now byte-swapped on big-endian systems to
+ * match the little-endian hash.  This is ugly but it seems better
+ * than changing the IPv6 refid calculation on the more-common
+ * systems.
  */
 u_int32
 addr2refid(sockaddr_u *addr)
@@ -288,5 +297,8 @@ addr2refid(sockaddr_u *addr)
        EVP_DigestFinal(ctx, digest, &len);
        EVP_MD_CTX_free(ctx);
        memcpy(&addr_refid, digest, sizeof(addr_refid));
-       return (addr_refid);
+#ifdef WORDS_BIGENDIAN
+       addr_refid = BYTESWAP32(addr_refid);
+#endif
+       return addr_refid;
 }

diff --git a/ntpd/ntp_io.c b/ntpd/ntp_io.c
index 36cd776b1f1ec12d3c3714e2ad04f2433f257cde..4731790d48a22e1cd83b04c682d2a87c241993c8 100644 (file)
--- a/ntpd/ntp_io.c
+++ b/ntpd/ntp_io.c
@@ -834,6 +834,11 @@ add_interface(
 
        /* Calculate the refid */
        ep->addr_refid = addr2refid(&ep->sin);
+#    ifdef WORDS_BIGENDIAN
+       if (IS_IPV6(&ep->sin)) {
+               ep->old_refid = BYTESWAP32(ep->addr_refid);
+       }
+#    endif
        /* link at tail so ntpdc -c ifstats index increases each row */
        LINK_TAIL_SLIST(ep_list, ep, elink, endpt);
        ninterfaces++;

diff --git a/ntpd/ntp_proto.c b/ntpd/ntp_proto.c
index 82074ca14510acd1ea77a24f23cb98996e312e0e..6518eb31c1cd1d3a812666d78ef022b670f351e0 100644 (file)
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -1368,7 +1368,12 @@ receive(
                if (   sys_leap == LEAP_NOTINSYNC
                    || sys_stratum >= hisstratum
                    || (!sys_cohort && sys_stratum == hisstratum + 1)
-                   || rbufp->dstadr->addr_refid == pkt->refid) {
+                   || rbufp->dstadr->addr_refid == pkt->refid
+#          ifdef WORDS_BIGENDIAN       /* see local_refid() comment */
+                   || (   IS_IPV6(&rbufp->dstadr->sin)
+                       &&rbufp->dstadr->old_refid ==  pkt->refid)
+#          endif
+                                                                 ) {
                        DPRINTF(2, ("receive: sys leap: %0x, sys_stratum %d > hisstratum+1 %d, !sys_cohort %d && sys_stratum == hisstratum+1, loop refid %#x == pkt refid %#x\n", sys_leap, sys_stratum, hisstratum + 1, !sys_cohort, rbufp->dstadr->addr_refid, pkt->refid));
                        DPRINTF(2, ("receive: AM_FXMIT drop: LEAP_NOTINSYNC || stratum || loop\n"));
                        sys_declined++;
@@ -5015,8 +5020,18 @@ key_expire(
 
 
 /*
- * local_refid(peer) - check peer refid to avoid selecting peers
+ * local_refid(peer) - Check peer refid to avoid selecting peers
  *                    currently synced to this ntpd.
+ * Note that until 4.2.8p18 and 4.3.1XX ntpd calculated the IPv6
+ * refid differently on different-endian systems.  It now calculates
+ * the refid the same on both, the same way it did on little-endian
+ * in the past.  On big-endian systems, ntpd also calculates a
+ * byte-swapped version of each of its IPv6 local addresses' refids,
+ * as endpt.old_refid and also detects a loop when seeing it.  This
+ * ensures new BE ntpd will detect loops interoperating with older
+ * BE ntpd, and keeps the more-common LE old ntpd code detecting
+ * loops with IPv6 refids correctly.  Thanks to Hal Murray for
+ * the byte-swapping idea.
  */
 static int
 local_refid(
@@ -5030,10 +5045,17 @@ local_refid(
        else
                unicast_ep = findinterface(&p->srcadr);
 
-       if (unicast_ep != NULL && p->refid == unicast_ep->addr_refid)
+       if (unicast_ep != NULL
+           && (   p->refid == unicast_ep->addr_refid
+#ifdef WORDS_BIGENDIAN
+               || (   IS_IPV6(&unicast_ep->sin)
+                   && p->refid == unicast_ep->old_refid)
+#endif
+                                                        )) {
                return TRUE;
-       else
+       } else {
                return FALSE;
+       }
 }
 
 

diff --git a/tests/libntp/a_md5encrypt.c b/tests/libntp/a_md5encrypt.c
index 87e3fa5c7b3469f5f42c4b984367048b331869c3..d7674b0129e10c92a7ba77a8982c269e27a1d0cf 100644 (file)
--- a/tests/libntp/a_md5encrypt.c
+++ b/tests/libntp/a_md5encrypt.c
@@ -75,16 +75,15 @@ test_DecryptInvalid(void) {
 
 void
 test_IPv4AddressToRefId(void) {
-       sockaddr_u addr;
-       addr.sa4.sin_family = AF_INET;
-       u_int32 address;
-
-       addr.sa4.sin_port = htons(80);
+       sockaddr_u      addr;
+       u_int32         addr4n;
 
-       address = inet_addr("192.0.2.1");
-       addr.sa4.sin_addr.s_addr = address;
+       AF(&addr) = AF_INET;
+       SET_PORT(&addr, htons(80));
+       addr4n = inet_addr("192.0.2.1");
+       NSRCADR(&addr) = addr4n;
 
-       TEST_ASSERT_EQUAL(address, addr2refid(&addr));
+       TEST_ASSERT_EQUAL_UINT32(addr4n, addr2refid(&addr));
 }
 
 void
@@ -98,16 +97,8 @@ test_IPv6AddressToRefId(void) {
        } } };
        sockaddr_u addr;
 
-       ZERO(addr);
-       addr.sa6.sin6_family = AF_INET6;
-       addr.sa6.sin6_addr = address;
+       AF(&addr) = AF_INET6;
+       SOCK_ADDR6(&addr) = address;
 
-
-#if 0
        TEST_ASSERT_EQUAL(expected, addr2refid(&addr));
-#else
-       UNUSED_LOCAL(expected);
-       UNUSED_LOCAL(addr);
-       TEST_IGNORE_MESSAGE("Skipping because of big endian problem?");
-#endif
 }