From: Juergen Perlinger Date: Tue, 26 Jul 2016 06:56:44 +0000 (+0200) Subject: [Bug 2998] sntp/tests/packetProcessing.c broken without openssl X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a295b83478dc1726ffe307e1c2cb9f3c7a5f10c;p=thirdparty%2Fntp.git [Bug 2998] sntp/tests/packetProcessing.c broken without openssl bk: 579709acyMeVsOH0zK_5r7oDuntQ9Q --- diff --git a/ChangeLog b/ChangeLog index 0805467dc..1c015d780 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,7 @@ * [Sec 3044] Spoofed server packets are partially processed. HStenn. * [Sec 3045] Bad authentication demobilizes ephemeral associations. JPerlinger. * [Sec 3046] CRYPTO_NAK crash. stenn@ntp.org +* [Bug 2998] sntp/tests/packetProcessing.c broken without openssl. JPerlinger * [Bug 3038] NTP fails to build in VS2015. perlinger@ntp.org - provide build environment - 'wint_t' and 'struct timespec' defined by VS2015 diff --git a/sntp/tests/packetProcessing.c b/sntp/tests/packetProcessing.c index 88e61cc89..660b5b6e2 100644 --- a/sntp/tests/packetProcessing.c +++ b/sntp/tests/packetProcessing.c @@ -1,10 +1,5 @@ #include "config.h" -/* need autokey for some of the tests, or the will create buffer overruns. */ -#ifndef AUTOKEY -# define AUTOKEY 1 -#endif - #include "sntptest.h" #include "networking.h" #include "ntp_stdlib.h" @@ -13,7 +8,7 @@ const char * Version = "stub unit test Version string"; -// Hacks into the key database. +/* Hacks into the key database. */ extern struct key* key_ptr; extern int key_cnt; @@ -41,9 +36,25 @@ void test_CorrectUnauthenticatedPacket(void); void test_CorrectAuthenticatedPacketMD5(void); void test_CorrectAuthenticatedPacketSHA1(void); +/* [Bug 2998] There are some issues whith the definition of 'struct pkt' + * when AUTOKEY is undefined -- the formal struct is too small to hold + * all the extension fields that are going to be tested. We have to make + * sure we have the extra bytes, or the test yield undefined results due + * to buffer overrun. + */ +#ifndef AUTOKEY +# define EXTRA_BUFSIZE 256 +#else +# define EXTRA_BUFSIZE 0 +#endif + +union tpkt { + struct pkt p; + u_char b[sizeof(struct pkt) + EXTRA_BUFSIZE]; +}; -static struct pkt testpkt; -static struct pkt testspkt; +static union tpkt testpkt; +static union tpkt testspkt; static sockaddr_u testsock; bool restoreKeyDb; @@ -95,10 +106,10 @@ setUp(void) /* Initialize the test packet and socket, * so they contain at least some valid data. */ - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, NTP_VERSION, + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, NTP_VERSION, MODE_SERVER); - testpkt.stratum = STRATUM_REFCLOCK; - memcpy(&testpkt.refid, "GPS\0", 4); + testpkt.p.stratum = STRATUM_REFCLOCK; + memcpy(&testpkt.p.refid, "GPS\0", 4); /* Set the origin timestamp of the received packet to the * same value as the transmit timestamp of the sent packet. @@ -107,8 +118,8 @@ setUp(void) tmp.l_ui = 1000UL; tmp.l_uf = 0UL; - HTONL_FP(&tmp, &testpkt.org); - HTONL_FP(&tmp, &testspkt.xmt); + HTONL_FP(&tmp, &testpkt.p.org); + HTONL_FP(&tmp, &testspkt.p.xmt); } @@ -129,11 +140,11 @@ void test_TooShortLength(void) { TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC - 1, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC - 1, + MODE_SERVER, &testspkt.p, "UnitTest")); TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC - 1, - MODE_BROADCAST, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC - 1, + MODE_BROADCAST, &testspkt.p, "UnitTest")); } @@ -141,22 +152,29 @@ void test_LengthNotMultipleOfFour(void) { TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC + 6, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC + 6, + MODE_SERVER, &testspkt.p, "UnitTest")); TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC + 3, - MODE_BROADCAST, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC + 3, + MODE_BROADCAST, &testspkt.p, "UnitTest")); } void test_TooShortExtensionFieldLength(void) { + /* [Bug 2998] We have to get around the formal specification of + * the extension field if AUTOKEY is undefined. (At least CLANG + * issues a warning in this case. It's just a warning, but + * still... + */ + uint32_t * pe = testpkt.p.exten + 7; + /* The lower 16-bits are the length of the extension field. * This lengths must be multiples of 4 bytes, which gives * a minimum of 4 byte extension field length. */ - testpkt.exten[7] = htonl(3); /* 3 bytes is too short. */ + *pe = htonl(3); /* 3 bytes is too short. */ /* We send in a pkt_len of header size + 4 byte extension * header + 24 byte MAC, this prevents the length error to @@ -165,8 +183,8 @@ test_TooShortExtensionFieldLength(void) int pkt_len = LEN_PKT_NOMAC + 4 + 24; TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -181,8 +199,8 @@ test_UnauthenticatedPacketReject(void) /* We demand authentication, but no MAC header is present. */ TEST_ASSERT_EQUAL(SERVER_AUTH_FAIL, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -196,8 +214,8 @@ test_CryptoNAKPacketReject(void) int pkt_len = LEN_PKT_NOMAC + 4; /* + 4 byte MAC = Crypto-NAK */ TEST_ASSERT_EQUAL(SERVER_AUTH_FAIL, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -211,19 +229,19 @@ test_AuthenticatedPacketInvalid(void) /* Prepare the packet. */ int pkt_len = LEN_PKT_NOMAC; - testpkt.exten[0] = htonl(50); - int mac_len = make_mac(&testpkt, pkt_len, + testpkt.p.exten[0] = htonl(50); + int mac_len = make_mac(&testpkt.p, pkt_len, MAX_MD5_LEN, key_ptr, - &testpkt.exten[1]); + &testpkt.p.exten[1]); pkt_len += 4 + mac_len; /* Now, alter the MAC so it becomes invalid. */ - testpkt.exten[1] += 1; + testpkt.p.exten[1] += 1; TEST_ASSERT_EQUAL(SERVER_AUTH_FAIL, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -239,15 +257,15 @@ test_AuthenticatedPacketUnknownKey(void) */ int pkt_len = LEN_PKT_NOMAC; - testpkt.exten[0] = htonl(50); - int mac_len = make_mac(&testpkt, pkt_len, + testpkt.p.exten[0] = htonl(50); + int mac_len = make_mac(&testpkt.p, pkt_len, MAX_MD5_LEN, key_ptr, - &testpkt.exten[1]); + &testpkt.p.exten[1]); pkt_len += 4 + mac_len; TEST_ASSERT_EQUAL(SERVER_AUTH_FAIL, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -256,16 +274,16 @@ test_ServerVersionTooOld(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, - NTP_OLDVERSION - 1, - MODE_CLIENT); - TEST_ASSERT_TRUE(PKT_VERSION(testpkt.li_vn_mode) < NTP_OLDVERSION); + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, + NTP_OLDVERSION - 1, + MODE_CLIENT); + TEST_ASSERT_TRUE(PKT_VERSION(testpkt.p.li_vn_mode) < NTP_OLDVERSION); int pkt_len = LEN_PKT_NOMAC; TEST_ASSERT_EQUAL(SERVER_UNUSEABLE, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -274,16 +292,16 @@ test_ServerVersionTooNew(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, - NTP_VERSION + 1, - MODE_CLIENT); - TEST_ASSERT_TRUE(PKT_VERSION(testpkt.li_vn_mode) > NTP_VERSION); + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, + NTP_VERSION + 1, + MODE_CLIENT); + TEST_ASSERT_TRUE(PKT_VERSION(testpkt.p.li_vn_mode) > NTP_VERSION); int pkt_len = LEN_PKT_NOMAC; TEST_ASSERT_EQUAL(SERVER_UNUSEABLE, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -292,16 +310,16 @@ test_NonWantedMode(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, - NTP_VERSION, - MODE_CLIENT); + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, + NTP_VERSION, + MODE_CLIENT); /* The packet has a mode of MODE_CLIENT, but process_pkt expects * MODE_SERVER */ TEST_ASSERT_EQUAL(SERVER_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -311,12 +329,12 @@ test_KoDRate(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.stratum = STRATUM_PKT_UNSPEC; - memcpy(&testpkt.refid, "RATE", 4); + testpkt.p.stratum = STRATUM_PKT_UNSPEC; + memcpy(&testpkt.p.refid, "RATE", 4); TEST_ASSERT_EQUAL(KOD_RATE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -325,12 +343,12 @@ test_KoDDeny(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.stratum = STRATUM_PKT_UNSPEC; - memcpy(&testpkt.refid, "DENY", 4); + testpkt.p.stratum = STRATUM_PKT_UNSPEC; + memcpy(&testpkt.p.refid, "DENY", 4); TEST_ASSERT_EQUAL(KOD_DEMOBILIZE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -339,13 +357,13 @@ test_RejectUnsyncedServer(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOTINSYNC, - NTP_VERSION, - MODE_SERVER); + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOTINSYNC, + NTP_VERSION, + MODE_SERVER); TEST_ASSERT_EQUAL(SERVER_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -357,15 +375,15 @@ test_RejectWrongResponseServerMode(void) l_fp tmp; tmp.l_ui = 1000UL; tmp.l_uf = 0UL; - HTONL_FP(&tmp, &testpkt.org); + HTONL_FP(&tmp, &testpkt.p.org); tmp.l_ui = 2000UL; tmp.l_uf = 0UL; - HTONL_FP(&tmp, &testspkt.xmt); + HTONL_FP(&tmp, &testspkt.p.xmt); TEST_ASSERT_EQUAL(PACKET_UNUSEABLE, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -374,12 +392,12 @@ test_AcceptNoSentPacketBroadcastMode(void) { TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); - testpkt.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, - NTP_VERSION, - MODE_BROADCAST); + testpkt.p.li_vn_mode = PKT_LI_VN_MODE(LEAP_NOWARNING, + NTP_VERSION, + MODE_BROADCAST); TEST_ASSERT_EQUAL(LEN_PKT_NOMAC, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, MODE_BROADCAST, NULL, "UnitTest")); } @@ -390,8 +408,8 @@ test_CorrectUnauthenticatedPacket(void) TEST_ASSERT_FALSE(ENABLED_OPT(AUTHENTICATION)); TEST_ASSERT_EQUAL(LEN_PKT_NOMAC, - process_pkt(&testpkt, &testsock, LEN_PKT_NOMAC, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, LEN_PKT_NOMAC, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -404,16 +422,16 @@ test_CorrectAuthenticatedPacketMD5(void) int pkt_len = LEN_PKT_NOMAC; /* Prepare the packet. */ - testpkt.exten[0] = htonl(10); - int mac_len = make_mac(&testpkt, pkt_len, + testpkt.p.exten[0] = htonl(10); + int mac_len = make_mac(&testpkt.p, pkt_len, MAX_MD5_LEN, key_ptr, - &testpkt.exten[1]); + &testpkt.p.exten[1]); pkt_len += 4 + mac_len; TEST_ASSERT_EQUAL(pkt_len, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } @@ -426,14 +444,14 @@ test_CorrectAuthenticatedPacketSHA1(void) int pkt_len = LEN_PKT_NOMAC; /* Prepare the packet. */ - testpkt.exten[0] = htonl(20); - int mac_len = make_mac(&testpkt, pkt_len, + testpkt.p.exten[0] = htonl(20); + int mac_len = make_mac(&testpkt.p, pkt_len, MAX_MAC_LEN, key_ptr, - &testpkt.exten[1]); + &testpkt.p.exten[1]); pkt_len += 4 + mac_len; TEST_ASSERT_EQUAL(pkt_len, - process_pkt(&testpkt, &testsock, pkt_len, - MODE_SERVER, &testspkt, "UnitTest")); + process_pkt(&testpkt.p, &testsock, pkt_len, + MODE_SERVER, &testspkt.p, "UnitTest")); } diff --git a/sntp/tests/run-packetProcessing.c b/sntp/tests/run-packetProcessing.c index ad02b7a54..38f855273 100644 --- a/sntp/tests/run-packetProcessing.c +++ b/sntp/tests/run-packetProcessing.c @@ -66,24 +66,24 @@ int main(int argc, char *argv[]) { progname = argv[0]; UnityBegin("packetProcessing.c"); - RUN_TEST(test_TooShortLength, 25); - RUN_TEST(test_LengthNotMultipleOfFour, 26); - RUN_TEST(test_TooShortExtensionFieldLength, 27); - RUN_TEST(test_UnauthenticatedPacketReject, 28); - RUN_TEST(test_CryptoNAKPacketReject, 29); - RUN_TEST(test_AuthenticatedPacketInvalid, 30); - RUN_TEST(test_AuthenticatedPacketUnknownKey, 31); - RUN_TEST(test_ServerVersionTooOld, 32); - RUN_TEST(test_ServerVersionTooNew, 33); - RUN_TEST(test_NonWantedMode, 34); - RUN_TEST(test_KoDRate, 35); - RUN_TEST(test_KoDDeny, 36); - RUN_TEST(test_RejectUnsyncedServer, 37); - RUN_TEST(test_RejectWrongResponseServerMode, 38); - RUN_TEST(test_AcceptNoSentPacketBroadcastMode, 39); - RUN_TEST(test_CorrectUnauthenticatedPacket, 40); - RUN_TEST(test_CorrectAuthenticatedPacketMD5, 41); - RUN_TEST(test_CorrectAuthenticatedPacketSHA1, 42); + RUN_TEST(test_TooShortLength, 20); + RUN_TEST(test_LengthNotMultipleOfFour, 21); + RUN_TEST(test_TooShortExtensionFieldLength, 22); + RUN_TEST(test_UnauthenticatedPacketReject, 23); + RUN_TEST(test_CryptoNAKPacketReject, 24); + RUN_TEST(test_AuthenticatedPacketInvalid, 25); + RUN_TEST(test_AuthenticatedPacketUnknownKey, 26); + RUN_TEST(test_ServerVersionTooOld, 27); + RUN_TEST(test_ServerVersionTooNew, 28); + RUN_TEST(test_NonWantedMode, 29); + RUN_TEST(test_KoDRate, 30); + RUN_TEST(test_KoDDeny, 31); + RUN_TEST(test_RejectUnsyncedServer, 32); + RUN_TEST(test_RejectWrongResponseServerMode, 33); + RUN_TEST(test_AcceptNoSentPacketBroadcastMode, 34); + RUN_TEST(test_CorrectUnauthenticatedPacket, 35); + RUN_TEST(test_CorrectAuthenticatedPacketMD5, 36); + RUN_TEST(test_CorrectAuthenticatedPacketSHA1, 37); return (UnityEnd()); } diff --git a/tests/libntp/a_md5encrypt.c b/tests/libntp/a_md5encrypt.c index d8e7ab93e..a87aa796d 100644 --- a/tests/libntp/a_md5encrypt.c +++ b/tests/libntp/a_md5encrypt.c @@ -49,9 +49,7 @@ test_Encrypt(void) { u_int32 *packetPtr; int length; - packetPtr = emalloc(totalLength * sizeof(*packetPtr)); - - memset(packetPtr + packetLength, 0, keyIdLength); + packetPtr = emalloc_zero(totalLength * sizeof(*packetPtr)); memcpy(packetPtr, packet, packetLength); cache_secretsize = keyLength; diff --git a/tests/libntp/sfptostr.c b/tests/libntp/sfptostr.c index c7616c7e3..c781c03a4 100644 --- a/tests/libntp/sfptostr.c +++ b/tests/libntp/sfptostr.c @@ -39,7 +39,7 @@ void test_PositiveInteger(void) void test_NegativeInteger(void) { - s_fp test = -200 << 16; // exact -200.000000 + s_fp test = -(200 << 16); // exact -200.000000 TEST_ASSERT_EQUAL_STRING("-200.000000", fptoa(test, SFP_MAX_PRECISION)); TEST_ASSERT_EQUAL_STRING("-200000.000", fptoms(test, SFP_MAX_PRECISION)); @@ -55,7 +55,7 @@ void test_PositiveIntegerPositiveFraction(void) void test_NegativeIntegerNegativeFraction(void) { - s_fp test = (-200 << 16) - (1 << 15); // -200 - 0.5 + s_fp test = -(200 << 16) - (1 << 15); // -200 - 0.5 TEST_ASSERT_EQUAL_STRING("-200.500000", fptoa(test, SFP_MAX_PRECISION)); TEST_ASSERT_EQUAL_STRING("-200500.000", fptoms(test, SFP_MAX_PRECISION)); @@ -71,7 +71,7 @@ void test_PositiveIntegerNegativeFraction(void) void test_NegativeIntegerPositiveFraction(void) { - s_fp test = (-200 << 16) + (1 << 14)*3; // -200 + 0.75 + s_fp test = -(200 << 16) + (1 << 14)*3; // -200 + 0.75 TEST_ASSERT_EQUAL_STRING("-199.250000", fptoa(test, SFP_MAX_PRECISION)); TEST_ASSERT_EQUAL_STRING("-199250.000", fptoms(test, SFP_MAX_PRECISION));