From: Juergen Perlinger Date: Sat, 30 Dec 2017 06:49:54 +0000 (+0100) Subject: address matching: cleanup & unit tests X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=df00f0e9250473bd9418f1d6e8f621c3b6669cf7;p=thirdparty%2Fntp.git address matching: cleanup & unit tests bk: 5a473712stj-JWZM0Lc-eOQJc8-gCQ --- diff --git a/libntp/authkeys.c b/libntp/authkeys.c index 294e049a79..e3c1bf0a03 100644 --- a/libntp/authkeys.c +++ b/libntp/authkeys.c @@ -189,8 +189,14 @@ keyacc_contains( * The ISC lib contains a similar function with not entirely specified * semantics, so it seemed somewhat cleaner to do this from scratch. * - * Note: It *is* assumed that the addresses are stored in network byte + * Note 1: It *is* assumed that the addresses are stored in network byte * order, that is, most significant byte first! + * + * Note 2: "no address" compares unequal to all other addresses, even to + * itself. This has the same semantics as NaNs have for floats: *any* + * relational or equality operation involving a NaN returns FALSE, even + * equality with itself. "no address" is either a NULL pointer argument + * or an address of type AF_UNSPEC. */ int/*BOOL*/ keyacc_amatch( @@ -203,10 +209,22 @@ keyacc_amatch( const uint8_t * pm2; uint8_t msk; unsigned int len; + + /* 1st check: If any address is not an address, it's inequal. */ + if ( !a1 || (AF_UNSPEC == AF(a1)) || + !a2 || (AF_UNSPEC == AF(a2)) ) + return FALSE; + + /* We could check pointers for equality here and shortcut the + * other checks if we find object identity. But that use case is + * too rare to care for it. + */ + /* 2nd check: Address families must be the same. */ if (AF(a1) != AF(a2)) return FALSE; + /* type check: address family determines buffer & size */ switch (AF(a1)) { case AF_INET: /* IPv4 is easy: clamp size, get byte pointers */ @@ -233,15 +251,23 @@ keyacc_amatch( return FALSE; } - /* split bit length into byte length and partial byte mask */ + /* Split bit length into byte length and partial byte mask. + * Note that the byte mask extends from the MSB of a byte down, + * and that zero shift (--> mbits % 8 == 0) results in an + * all-zero mask. + */ msk = 0xFFu ^ (0xFFu >> (mbits & 7)); len = mbits >> 3; + /* 3rd check: Do memcmp() over full bytes, if any */ if (len && memcmp(pm1, pm2, len)) return FALSE; + + /* 4th check: compare last incomplete byte, if any */ if (msk && ((pm1[len] ^ pm2[len]) & msk)) return FALSE; - + + /* If none of the above failed, we're successfully through. */ return TRUE; } diff --git a/tests/libntp/authkeys.c b/tests/libntp/authkeys.c index fd11ef623d..ac44f93dd6 100644 --- a/tests/libntp/authkeys.c +++ b/tests/libntp/authkeys.c @@ -21,19 +21,12 @@ int counter = 0; void setUp(void); void tearDown(void); void AddTrustedKey(keyid_t keyno); -void AddUntrustedKey(keyid_t keyno); -void test_AddTrustedKeys(void); -void test_AddUntrustedKey(void); -void test_HaveKeyCorrect(void); -void test_HaveKeyIncorrect(void); -void test_AddWithAuthUseKey(void); -void test_EmptyKey(void); -void test_auth_log2(void); +void AddUntrustedKey(keyid_t keyno); void setUp(void) -{ +{ if (counter == 0) { counter++; init_auth(); // causes segfault if called more than once @@ -53,17 +46,17 @@ setUp(void) cache_flags = 0; cache_secret = NULL; cache_secretsize = 0; - - return; } void tearDown(void) { - return; + /*NOP*/ } + static const int KEYTYPE = KEY_TYPE_MD5; +static char msgbuf[128]; void AddTrustedKey(keyid_t keyno) @@ -75,20 +68,16 @@ AddTrustedKey(keyid_t keyno) MD5auth_setkey(keyno, KEYTYPE, NULL, 0, NULL); authtrust(keyno, TRUE); - - return; } void AddUntrustedKey(keyid_t keyno) { authtrust(keyno, FALSE); - - return; } -void -test_AddTrustedKeys(void) +void test_AddTrustedKeys(void); +void test_AddTrustedKeys(void) { const keyid_t KEYNO1 = 5; const keyid_t KEYNO2 = 8; @@ -98,24 +87,20 @@ test_AddTrustedKeys(void) TEST_ASSERT_TRUE(authistrusted(KEYNO1)); TEST_ASSERT_TRUE(authistrusted(KEYNO2)); - - return; } -void -test_AddUntrustedKey(void) +void test_AddUntrustedKey(void); +void test_AddUntrustedKey(void) { const keyid_t KEYNO = 3; - + AddUntrustedKey(KEYNO); TEST_ASSERT_FALSE(authistrusted(KEYNO)); - - return; } -void -test_HaveKeyCorrect(void) +void test_HaveKeyCorrect(void); +void test_HaveKeyCorrect(void) { const keyid_t KEYNO = 3; @@ -123,42 +108,33 @@ test_HaveKeyCorrect(void) TEST_ASSERT_TRUE(auth_havekey(KEYNO)); TEST_ASSERT_TRUE(authhavekey(KEYNO)); - - return; } -void -test_HaveKeyIncorrect(void) +void test_HaveKeyIncorrect(void); +void test_HaveKeyIncorrect(void) { const keyid_t KEYNO = 2; TEST_ASSERT_FALSE(auth_havekey(KEYNO)); TEST_ASSERT_FALSE(authhavekey(KEYNO)); - - return; } -void -test_AddWithAuthUseKey(void) +void test_AddWithAuthUseKey(void); +void test_AddWithAuthUseKey(void) { const keyid_t KEYNO = 5; const char* KEY = "52a"; TEST_ASSERT_TRUE(authusekey(KEYNO, KEYTYPE, (const u_char*)KEY)); - - return; } -void -test_EmptyKey(void) +void test_EmptyKey(void); +void test_EmptyKey(void) { const keyid_t KEYNO = 3; const char* KEY = ""; - TEST_ASSERT_FALSE(authusekey(KEYNO, KEYTYPE, (const u_char*)KEY)); - - return; } /* test the implementation of 'auth_log2' -- use a local copy of the code */ @@ -181,12 +157,12 @@ auth_log2( return (u_short)r; } -void -test_auth_log2(void) +void test_auth_log2(void); +void test_auth_log2(void) { int l2; size_t tv; - + TEST_ASSERT_EQUAL_INT(0, auth_log2(0)); TEST_ASSERT_EQUAL_INT(0, auth_log2(1)); for (l2 = 1; l2 < sizeof(size_t)*CHAR_BIT; ++l2) { @@ -196,3 +172,155 @@ test_auth_log2(void) TEST_ASSERT_EQUAL_INT(l2, auth_log2(2*tv - 1)); } } + +/* Converting a string to a host address. Here we use 'getaddrinfo()' in + * an independent implementation to avoid cross-reactions with the + * object under test. 'inet_pton' is too dangerous to handle it + * properly, and ultimate performance is *not* the goal here. + */ +static int/*BOOL*/ +getaddr( + int af, + const char *astr, + sockaddr_u * addr) +{ + struct addrinfo hint; + struct addrinfo *ares; + + memset(&hint, 0, sizeof(hint)); + hint.ai_flags = AI_NUMERICHOST; + hint.ai_family = af; + if (getaddrinfo(astr, NULL, &hint, &ares)) + return FALSE; + if (ares->ai_addrlen > sizeof(*addr)) + memcpy(addr, ares->ai_addr, sizeof(*addr)); + else + memcpy(addr, ares->ai_addr, ares->ai_addrlen); + freeaddrinfo(ares); + return TRUE; +} + +void test_AddrMatch_anull(void); +void test_AddrMatch_anull(void) +{ + /* Check the not-an-address logic with a prefix/check length of + * zero bits. Any compare with a NULL or AF_UNSPEC address + * returns inequality (aka FALSE). + */ + sockaddr_u ip4, ip6, ipn; + + memset(&ipn, 0, sizeof(ipn)); + AF(&ipn) = AF_UNSPEC; + + TEST_ASSERT_TRUE(getaddr(AF_INET , "192.128.1.1", &ip4)); + TEST_ASSERT_TRUE(getaddr(AF_INET6, "::1" , &ip6)); + + TEST_ASSERT_FALSE(keyacc_amatch(NULL, NULL, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(NULL, &ipn, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(NULL, &ip4, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(NULL, &ip6, 0)); + + TEST_ASSERT_FALSE(keyacc_amatch(&ipn, NULL, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ipn, &ipn, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ipn, &ip4, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ipn, &ip6, 0)); + + TEST_ASSERT_FALSE(keyacc_amatch(&ip4, NULL, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ip4, &ipn, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ip6, NULL, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ip6, &ipn, 0)); +} + +void test_AddrMatch_self4(void); +void test_AddrMatch_self4(void) +{ + sockaddr_u ip4; + unsigned int bits; + + TEST_ASSERT_TRUE(getaddr(AF_INET, "192.128.1.1", &ip4)); + for (bits = 0; bits < 40; ++bits) + TEST_ASSERT_TRUE(keyacc_amatch(&ip4, &ip4, bits)); +} + +void test_AddrMatch_self6(void); +void test_AddrMatch_self6(void) +{ + sockaddr_u ip6; + unsigned int bits; + + TEST_ASSERT_TRUE(getaddr(AF_INET6, "::1" , &ip6)); + for (bits = 0; bits < 136; ++bits) + TEST_ASSERT_TRUE(keyacc_amatch(&ip6, &ip6, bits)); +} + +void test_AddrMatch_afmix(void); +void test_AddrMatch_afmix(void) +{ + sockaddr_u ip6, ip4; + + TEST_ASSERT_TRUE(getaddr(AF_INET , "192.128.1.1", &ip4)); + TEST_ASSERT_TRUE(getaddr(AF_INET6, "::1" , &ip6)); + + TEST_ASSERT_FALSE(keyacc_amatch(&ip4, &ip6, 0)); + TEST_ASSERT_FALSE(keyacc_amatch(&ip6, &ip4, 0)); +} + +void test_AddrMatch_ipv4(void); +void test_AddrMatch_ipv4(void) +{ + sockaddr_u a1, a2; + unsigned int bits; + int want; + + TEST_ASSERT_TRUE(getaddr(AF_INET, "192.128.2.1", &a1)); + TEST_ASSERT_TRUE(getaddr(AF_INET, "192.128.3.1", &a2)); + + /* the first 23 bits are equal, so any prefix <= 23 should match */ + for (bits = 0; bits < 40; ++bits) { + snprintf(msgbuf, sizeof(msgbuf), + "keyacc_amatch(*,*,%u) wrong", bits); + want = (bits <= 23); + TEST_ASSERT_EQUAL_MESSAGE(want, keyacc_amatch(&a1, &a2, bits), msgbuf); + } + + TEST_ASSERT_TRUE(getaddr(AF_INET, "192.128.2.127", &a1)); + TEST_ASSERT_TRUE(getaddr(AF_INET, "192.128.2.128", &a2)); + + /* the first 24 bits are equal, so any prefix <= 24 should match */ + for (bits = 0; bits < 40; ++bits) { + snprintf(msgbuf, sizeof(msgbuf), + "keyacc_amatch(*,*,%u) wrong", bits); + want = (bits <= 24); + TEST_ASSERT_EQUAL_MESSAGE(want, keyacc_amatch(&a1, &a2, bits), msgbuf); + } +} + +void test_AddrMatch_ipv6(void); +void test_AddrMatch_ipv6(void) +{ + sockaddr_u a1, a2; + unsigned int bits; + int want; + + TEST_ASSERT_TRUE(getaddr(AF_INET6, "FEDC:BA98:7654:3210::2:FFFF", &a1)); + TEST_ASSERT_TRUE(getaddr(AF_INET6, "FEDC:BA98:7654:3210::3:FFFF", &a2)); + + /* the first 111 bits are equal, so any prefix <= 111 should match */ + for (bits = 0; bits < 136; ++bits) { + snprintf(msgbuf, sizeof(msgbuf), + "keyacc_amatch(*,*,%u) wrong", bits); + want = (bits <= 111); + TEST_ASSERT_EQUAL_MESSAGE(want, keyacc_amatch(&a1, &a2, bits), msgbuf); + } + + TEST_ASSERT_TRUE(getaddr(AF_INET6, "FEDC:BA98:7654:3210::2:7FFF", &a1)); + TEST_ASSERT_TRUE(getaddr(AF_INET6, "FEDC:BA98:7654:3210::2:8000", &a2)); + + /* the first 112 bits are equal, so any prefix <= 112 should match */ + for (bits = 0; bits < 136; ++bits) { + snprintf(msgbuf, sizeof(msgbuf), + "keyacc_amatch(*,*,%u) wrong", bits); + want = (bits <= 112); + TEST_ASSERT_EQUAL_MESSAGE(want, keyacc_amatch(&a1, &a2, bits), msgbuf); + } +} diff --git a/tests/libntp/run-authkeys.c b/tests/libntp/run-authkeys.c index b2665e4151..bdf48f0e15 100644 --- a/tests/libntp/run-authkeys.c +++ b/tests/libntp/run-authkeys.c @@ -38,6 +38,12 @@ extern void test_HaveKeyIncorrect(void); extern void test_AddWithAuthUseKey(void); extern void test_EmptyKey(void); extern void test_auth_log2(void); +extern void test_AddrMatch_anull(void); +extern void test_AddrMatch_self4(void); +extern void test_AddrMatch_self6(void); +extern void test_AddrMatch_afmix(void); +extern void test_AddrMatch_ipv4(void); +extern void test_AddrMatch_ipv6(void); //=======Suite Setup===== @@ -64,13 +70,19 @@ int main(int argc, char *argv[]) progname = argv[0]; suite_setup(); UnityBegin("authkeys.c"); - RUN_TEST(test_AddTrustedKeys, 25); - RUN_TEST(test_AddUntrustedKey, 26); - RUN_TEST(test_HaveKeyCorrect, 27); - RUN_TEST(test_HaveKeyIncorrect, 28); - RUN_TEST(test_AddWithAuthUseKey, 29); - RUN_TEST(test_EmptyKey, 30); - RUN_TEST(test_auth_log2, 31); + RUN_TEST(test_AddTrustedKeys, 79); + RUN_TEST(test_AddUntrustedKey, 92); + RUN_TEST(test_HaveKeyCorrect, 102); + RUN_TEST(test_HaveKeyIncorrect, 113); + RUN_TEST(test_AddWithAuthUseKey, 122); + RUN_TEST(test_EmptyKey, 131); + RUN_TEST(test_auth_log2, 160); + RUN_TEST(test_AddrMatch_anull, 203); + RUN_TEST(test_AddrMatch_self4, 234); + RUN_TEST(test_AddrMatch_self6, 245); + RUN_TEST(test_AddrMatch_afmix, 256); + RUN_TEST(test_AddrMatch_ipv4, 268); + RUN_TEST(test_AddrMatch_ipv6, 298); return (UnityEnd()); }