From: Dave Hart Date: Wed, 23 Aug 2023 10:58:43 +0000 (+0000) Subject: [Bug 3864] ntpd IPv6 refid different for big-endian and little-endian. X-Git-Tag: NTP_4_2_8P18_RC1~27^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=38e264877a5ea6f55a79468121182d3429015f3b;p=thirdparty%2Fntp.git [Bug 3864] ntpd IPv6 refid different for big-endian and little-endian. bk: 64e5e663hRLWRqC-38IAdckA7hZt3A --- diff --git a/ChangeLog b/ChangeLog index aa7444bda..8509fa550 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ --- +* [Bug 3864] ntpd IPv6 refid different for big-endian and little-endian. + * [Bug 3856] Enable Edit & Continue debugging with Visual Studio. * [Bug 3853] Clean up warnings with modern compilers. diff --git a/include/ntp.h b/include/ntp.h index a066d6ee7..441f5889d 100644 --- 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 7a3729691..ad6c28c17 100644 --- 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 36cd776b1..4731790d4 100644 --- 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 82074ca14..6518eb31c 100644 --- 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 87e3fa5c7..d7674b012 100644 --- 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 }