diff --git a/CHANGES b/CHANGES index 1434df9a5b..23e465e6af 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5339. [bug] With some libmaxminddb versions, named could erroneously + match an IP address not belonging to any subnet defined + in a given GeoIP2 database to one of the existing + entries in that database. [GL #1552] + 5338. [bug] Fix line spacing in `rndc secroots`. Thanks to Tony Finch. [GL !2478] diff --git a/lib/dns/geoip2.c b/lib/dns/geoip2.c index 098a8844d5..e65d6fb570 100644 --- a/lib/dns/geoip2.c +++ b/lib/dns/geoip2.c @@ -97,7 +97,7 @@ get_entry_for(MMDB_s * const db, const isc_netaddr_t *addr) { isc_sockaddr_fromnetaddr(&sa, addr, 0); match = MMDB_lookup_sockaddr(db, &sa.type.sa, &err); - if (err != MMDB_SUCCESS) { + if (err != MMDB_SUCCESS || !match.found_entry) { return (NULL); } diff --git a/lib/dns/tests/geoip_test.c b/lib/dns/tests/geoip_test.c index 21a887bae2..bc581a01a2 100644 --- a/lib/dns/tests/geoip_test.c +++ b/lib/dns/tests/geoip_test.c @@ -36,6 +36,8 @@ #if defined(HAVE_GEOIP2) #include +#include "../geoip2.c" + /* Use GeoIP2 databases from the 'geoip2' system test */ #define TEST_GEOIP_DATA "../../../bin/tests/system/geoip2/data" @@ -105,6 +107,77 @@ close_geoip(void) { MMDB_close(&geoip_domain); } +static bool +/* Check if an MMDB entry of a given subtype exists for the given IP */ +entry_exists(dns_geoip_subtype_t subtype, const char *addr) { + struct in6_addr in6; + struct in_addr in4; + isc_netaddr_t na; + MMDB_s *db; + + if (inet_pton(AF_INET6, addr, &in6) == 1) { + isc_netaddr_fromin6(&na, &in6); + } else if (inet_pton(AF_INET, addr, &in4) == 1) { + isc_netaddr_fromin(&na, &in4); + } else { + INSIST(0); + ISC_UNREACHABLE(); + } + + db = geoip2_database(&geoip, fix_subtype(&geoip, subtype)); + + return (db != NULL && get_entry_for(db, &na) != NULL); +} + +/* + * Baseline test - check if get_entry_for() works as expected, i.e. that its + * return values are consistent with the contents of the test MMDBs found in + * bin/tests/system/geoip2/data/ (10.53.0.1 and fd92:7065:b8e:ffff::1 should be + * present in all databases, 192.0.2.128 should only be present in the country + * database, ::1 should be absent from all databases). + */ +static void +baseline(void **state) { + dns_geoip_subtype_t subtype; + + UNUSED(state); + + subtype = dns_geoip_city_name; + + assert_true(entry_exists(subtype, "10.53.0.1")); + assert_false(entry_exists(subtype, "192.0.2.128")); + assert_true(entry_exists(subtype, "fd92:7065:b8e:ffff::1")); + assert_false(entry_exists(subtype, "::1")); + + subtype = dns_geoip_country_name; + + assert_true(entry_exists(subtype, "10.53.0.1")); + assert_true(entry_exists(subtype, "192.0.2.128")); + assert_true(entry_exists(subtype, "fd92:7065:b8e:ffff::1")); + assert_false(entry_exists(subtype, "::1")); + + subtype = dns_geoip_domain_name; + + assert_true(entry_exists(subtype, "10.53.0.1")); + assert_false(entry_exists(subtype, "192.0.2.128")); + assert_true(entry_exists(subtype, "fd92:7065:b8e:ffff::1")); + assert_false(entry_exists(subtype, "::1")); + + subtype = dns_geoip_isp_name; + + assert_true(entry_exists(subtype, "10.53.0.1")); + assert_false(entry_exists(subtype, "192.0.2.128")); + assert_true(entry_exists(subtype, "fd92:7065:b8e:ffff::1")); + assert_false(entry_exists(subtype, "::1")); + + subtype = dns_geoip_as_asnum; + + assert_true(entry_exists(subtype, "10.53.0.1")); + assert_false(entry_exists(subtype, "192.0.2.128")); + assert_true(entry_exists(subtype, "fd92:7065:b8e:ffff::1")); + assert_false(entry_exists(subtype, "::1")); +} + static bool do_lookup_string(const char *addr, dns_geoip_subtype_t subtype, const char *string) @@ -334,6 +407,7 @@ int main(void) { #if defined(HAVE_GEOIP2) const struct CMUnitTest tests[] = { + cmocka_unit_test(baseline), cmocka_unit_test(country), cmocka_unit_test(country_v6), cmocka_unit_test(city),