ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Properly detect MMDB lookup failures

Browse Source 
Only comparing the value of the integer passed as the last argument to
MMDB_lookup_sockaddr() against MMDB_SUCCESS is not enough to ensure that
an MMDB lookup was successful - the 'found_entry' field of the
MMDB_lookup_result_s structure returned by that function also needs to
be true or else the remaining contents of that structure should be
ignored as the lookup failed.  Extend the relevant logical condition in
get_entry_for() to ensure the latter does not return incorrect MMDB
entries for IP addresses which do not belong to any subnet defined in a
given GeoIP2 database.

(cherry picked from commit ec8334fb74)
This commit is contained in:
Michał Kępień
2020-01-13 14:36:58 +01:00
parent 8c7faf623d
commit 814da1c808
2 changed files with 79 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/dns/geoip2.c
+1 -1
View File
@@ -194,7 +194,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);
}
lib/dns/tests/geoip_test.c
+78
View File
@@ -38,6 +38,8 @@
#if defined(HAVE_GEOIP2)
#include <maxminddb.h>
#include "../geoip2.c"
/* Use GeoIP2 databases from the 'geoip2' system test */
#define TEST_GEOIP_DATA "../../../bin/tests/system/geoip2/data"
#elif defined(HAVE_GEOIP)
@@ -219,6 +221,79 @@ do_lookup_int(const char *addr, dns_geoip_subtype_t subtype, int id) {
#endif /* HAVE_GEOIP */
#ifdef HAVE_GEOIP2
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"));
}
#endif /* HAVE_GEOIP2 */
#if defined(HAVE_GEOIP) || defined(HAVE_GEOIP2)
static bool
do_lookup_string(const char *addr, dns_geoip_subtype_t subtype,
@@ -617,6 +692,9 @@ int
main(void) {
#if defined(HAVE_GEOIP) || defined(HAVE_GEOIP2)
const struct CMUnitTest tests[] = {
#ifdef HAVE_GEOIP2
cmocka_unit_test_setup_teardown(baseline, _setup, _teardown),
#endif /* HAVE_GEOIP2 */
cmocka_unit_test_setup_teardown(country, _setup, _teardown),
cmocka_unit_test_setup_teardown(country_v6, _setup, _teardown),
cmocka_unit_test_setup_teardown(city, _setup, _teardown),