diff --git a/CHANGES b/CHANGES index 5554412d12..bce9536c10 100644 --- a/CHANGES +++ b/CHANGES @@ -9,11 +9,19 @@ reference could be leaked causing an assertion failure on shutdown. [RT #37796] -4003. [placeholder] +4003. [security] When geoip-directory was reconfigured during + named run-time, the previously loaded GeoIP + data could remain, potentially causing wrong + ACLs to be used or wrong results to be served + based on geolocation. [RT #37720] -4002. [placeholder] +4002. [security] Lookups in GeoIP databases that were not + loaded could cause an assertion failure. + [RT #37679] -4001. [placeholder] +4001. [security] The caching of GeoIP lookups did not always + handle address families correctly, potentially + resulting in an assertion failure. [RT #37672] 4000. [bug] NXDOMAIN redirection incorrectly handled NXRRSET from the redirect zone. [RT #37722] diff --git a/bin/named/geoip.c b/bin/named/geoip.c index 3a9ab1b68c..11fc96efed 100644 --- a/bin/named/geoip.c +++ b/bin/named/geoip.c @@ -87,6 +87,7 @@ ns_geoip_init(void) { #ifndef HAVE_GEOIP return; #else + GeoIP_cleanup(); if (ns_g_geoip == NULL) ns_g_geoip = &geoip_table; #endif diff --git a/bin/tests/system/geoip/clean.sh b/bin/tests/system/geoip/clean.sh index 6fa60e3e93..84c3f70da0 100644 --- a/bin/tests/system/geoip/clean.sh +++ b/bin/tests/system/geoip/clean.sh @@ -17,3 +17,7 @@ rm -f ns2/named.conf rm -f ns2/example*.db rm -f dig.out.* rndc.out.* +rm -f data2/*dat +[ -d data2 ] && rmdir data2 +rm -f ns?/named.run +rm -f ns?/named.memstats diff --git a/bin/tests/system/geoip/ns2/named15.conf b/bin/tests/system/geoip/ns2/named15.conf new file mode 100644 index 0000000000..6ac837b141 --- /dev/null +++ b/bin/tests/system/geoip/ns2/named15.conf @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH + * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY + * AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, + * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM + * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE + * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR + * PERFORMANCE OF THIS SOFTWARE. + */ + +// NS2 + +controls { /* empty */ }; + +options { + query-source address 10.53.0.2; + notify-source 10.53.0.2; + transfer-source 10.53.0.2; + port 5300; + pid-file "named.pid"; + listen-on { 10.53.0.2; }; + listen-on-v6 { fd92:7065:b8e:ffff::2; }; + recursion no; + geoip-directory "../data2"; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.2 port 9953 allow { any; } keys { rndc_key; }; +}; + +view two { + match-clients { geoip country US; }; + zone "example" { + type master; + file "../ns2/example2.db"; + }; +}; + +view none { + zone "example" { + type master; + file "examplebogus.db"; + }; +}; diff --git a/bin/tests/system/geoip/ns2/named6.conf b/bin/tests/system/geoip/ns2/named6.conf index 3e54a0c61f..feab552111 100644 --- a/bin/tests/system/geoip/ns2/named6.conf +++ b/bin/tests/system/geoip/ns2/named6.conf @@ -25,7 +25,7 @@ options { port 5300; pid-file "named.pid"; listen-on { 10.53.0.2; }; - listen-on-v6 { none; }; + listen-on-v6 { fd92:7065:b8e:ffff::1; }; recursion no; geoip-directory "../data"; }; diff --git a/bin/tests/system/geoip/setup.sh b/bin/tests/system/geoip/setup.sh index 2aaeaca15f..4019181536 100644 --- a/bin/tests/system/geoip/setup.sh +++ b/bin/tests/system/geoip/setup.sh @@ -25,3 +25,6 @@ for i in 1 2 3 4 5 6 7 other bogus; do cp ns2/example.db.in ns2/example${i}.db echo "@ IN TXT \"$i\"" >> ns2/example$i.db done + +mkdir -p data2 +cp data/GeoIPv6.dat data2/ diff --git a/bin/tests/system/geoip/tests.sh b/bin/tests/system/geoip/tests.sh index 19d05ce66f..9befdf4768 100644 --- a/bin/tests/system/geoip/tests.sh +++ b/bin/tests/system/geoip/tests.sh @@ -23,6 +23,7 @@ n=0 rm -f dig.out.* DIGOPTS="+tcp +short -p 5300 @10.53.0.2" +DIGOPTS6="+tcp +short -p 5300 @fd92:7065:b8e:ffff::2" n=`expr $n + 1` echo "I:checking GeoIP country database by code ($n)" @@ -159,6 +160,18 @@ cp -f ns2/named6.conf ns2/named.conf $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 2>&1 | sed 's/^/I:ns2 /' sleep 3 +if $TESTSOCK6 fd92:7065:b8e:ffff::3 +then + n=`expr $n + 1` + echo "I:checking GeoIP city database by city name using IPv6 ($n)" + ret=0 + $DIG +tcp +short -p 5300 @fd92:7065:b8e:ffff::1 -6 txt example -b fd92:7065:b8e:ffff::2 > dig.out.ns2.test$n || ret=1 + [ $ret -eq 0 ] || echo "I:failed" + status=`expr $status + $ret` +else + echo "I:IPv6 unavailable; skipping" +fi + n=`expr $n + 1` echo "I:checking GeoIP city database by city name ($n)" ret=0 @@ -306,7 +319,7 @@ done status=`expr $status + $ret` n=`expr $n + 1` -echo "I:checking GeoIP domain database (using client subnet) ($n)" +echo "I:checking GeoIP asnum database - ASNNNN only (using client subnet) ($n)" ret=0 lret=0 for i in 1 2 3 4 5 6 7; do @@ -338,6 +351,20 @@ done [ $ret -eq 0 ] || echo "I:failed" status=`expr $status + $ret` +n=`expr $n + 1` +echo "I:checking GeoIP domain database (using client subnet) ($n)" +ret=0 +lret=0 +for i in 1 2 3 4 5 6 7; do + $DIG $DIGOPTS txt example -b 127.0.0.1 +subnet="10.53.0.$i/32" > dig.out.ns2.test$n.$i || lret=1 + j=`cat dig.out.ns2.test$n.$i | tr -d '"'` + [ "$i" = "$j" ] || lret=1 + [ $lret -eq 1 ] && break +done +[ $lret -eq 1 ] && ret=1 +[ $ret -eq 0 ] || echo "I:failed" +status=`expr $status + $ret` + echo "I:reloading server" cp -f ns2/named12.conf ns2/named.conf $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 2>&1 | sed 's/^/I:ns2 /' @@ -427,5 +454,28 @@ done [ $ret -eq 0 ] || echo "I:failed" status=`expr $status + $ret` +n=`expr $n + 1` +echo "I:reloading server with different geoip-directory ($n)" +cp -f ns2/named15.conf ns2/named.conf +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reload 2>&1 | sed 's/^/I:ns2 /' +sleep 3 +awk '/using "..\/data2" as GeoIP directory/ {m=1} ; { if (m>0) { print } }' ns2/named.run | grep "GeoIP City .* DB not available" > /dev/null || ret=1 +[ $ret -eq 0 ] || echo "I:failed" +status=`expr $status + $ret` + +n=`expr $n + 1` +echo "I:checking GeoIP v4/v6 when only IPv6 database is available ($n)" +ret=0 +$DIG $DIGOPTS -4 txt example -b 10.53.0.2 > dig.out.ns2.test$n.1 || ret=1 +j=`cat dig.out.ns2.test$n.1 | tr -d '"'` +[ "$j" = "bogus" ] || ret=1 +if $TESTSOCK6 fd92:7065:b8e:ffff::2; then + $DIG $DIGOPTS6 txt example -b fd92:7065:b8e:ffff::2 > dig.out.ns2.test$n.2 || ret=1 + j=`cat dig.out.ns2.test$n.2 | tr -d '"'` + [ "$j" = "2" ] || ret=1 +fi +[ $ret -eq 0 ] || echo "I:failed" +status=`expr $status + $ret` + echo "I:exit status: $status" exit $status diff --git a/lib/dns/geoip.c b/lib/dns/geoip.c index 42109fedcb..95f2b623a2 100644 --- a/lib/dns/geoip.c +++ b/lib/dns/geoip.c @@ -48,7 +48,7 @@ * so that successive lookups for the same data from the same IP * address will not require repeated calls into the GeoIP library * to look up data in the database. This should improve performance - * somwhat. + * somewhat. * * For lookups in the City and Region databases, we preserve pointers * to the GeoIPRecord and GeoIPregion structures; these will need to be @@ -145,12 +145,18 @@ clean_state(geoip_state_t *state) { if (state == NULL) return; - if (state->record != NULL) + if (state->record != NULL) { GeoIPRecord_delete(state->record); - if (state->region != NULL) + state->record = NULL; + } + if (state->region != NULL) { GeoIPRegion_delete(state->region); - if (state->name != NULL) + state->region = NULL; + } + if (state->name != NULL) { free (state->name); + state->name = NULL; + } state->ipnum = 0; state->text = NULL; state->id = 0; @@ -188,8 +194,7 @@ set_state(unsigned int family, isc_uint32_t ipnum, const geoipv6_t *ipnum6, clean_state(state); #else state = &prev_state; - if (state->ipnum != ipnum) - clean_state(state); + clean_state(state); #endif if (family == AF_INET) @@ -210,20 +215,32 @@ set_state(unsigned int family, isc_uint32_t ipnum, const geoipv6_t *ipnum6, } static geoip_state_t * -get_state(void) { +get_state_for(unsigned int family, isc_uint32_t ipnum, + const geoipv6_t *ipnum6) +{ + geoip_state_t *state; + #ifdef ISC_PLATFORM_USETHREADS isc_result_t result; - geoip_state_t *state; result = state_key_init(); if (result != ISC_R_SUCCESS) return (NULL); state = (geoip_state_t *) isc_thread_key_getspecific(state_key); - return (state); + if (state == NULL) + return (NULL); #else - return (&prev_state); + state = &prev_state; #endif + + if (state->family == family && + ((state->family == AF_INET && state->ipnum == ipnum) || + (state->family == AF_INET6 && ipnum6 != NULL && + memcmp(state->ipnum6.s6_addr, ipnum6->s6_addr, 16) == 0))) + return (state); + + return (NULL); } /* @@ -249,15 +266,8 @@ country_lookup(GeoIP *db, dns_geoip_subtype_t subtype, return (NULL); #endif - prev_state = get_state(); - - if (prev_state != NULL && - prev_state->subtype == subtype && - prev_state->family == family && - ((prev_state->family == AF_INET && prev_state->ipnum == ipnum) || - (prev_state->family == AF_INET6 && ipnum6 != NULL && - memcmp(prev_state->ipnum6.s6_addr, ipnum6->s6_addr, 16) == 0))) - { + prev_state = get_state_for(family, ipnum, ipnum6); + if (prev_state != NULL && prev_state->subtype == subtype) { text = prev_state->text; if (scope != NULL) *scope = prev_state->scope; @@ -409,14 +419,8 @@ city_lookup(GeoIP *db, dns_geoip_subtype_t subtype, return (NULL); #endif - prev_state = get_state(); - - if (prev_state != NULL && - is_city(prev_state->subtype) && - ((prev_state->family == AF_INET && prev_state->ipnum == ipnum) || - (prev_state->family == AF_INET6 && - memcmp(prev_state->ipnum6.s6_addr, ipnum6->s6_addr, 16) == 0))) - { + prev_state = get_state_for(family, ipnum, ipnum6); + if (prev_state != NULL && is_city(prev_state->subtype)) { record = prev_state->record; if (scope != NULL) *scope = record->netmask; @@ -493,11 +497,8 @@ region_lookup(GeoIP *db, dns_geoip_subtype_t subtype, REQUIRE(db != NULL); - prev_state = get_state(); - - if (prev_state != NULL && prev_state->ipnum == ipnum && - is_region(prev_state->subtype)) - { + prev_state = get_state_for(AF_INET, ipnum, NULL); + if (prev_state != NULL && is_region(prev_state->subtype)) { region = prev_state->region; if (scope != NULL) *scope = prev_state->scope; @@ -533,11 +534,8 @@ name_lookup(GeoIP *db, dns_geoip_subtype_t subtype, REQUIRE(db != NULL); - prev_state = get_state(); - - if (prev_state != NULL && prev_state->ipnum == ipnum && - prev_state->subtype == subtype) - { + prev_state = get_state_for(AF_INET, ipnum, NULL); + if (prev_state != NULL && prev_state->subtype == subtype) { name = prev_state->name; if (scope != NULL) *scope = prev_state->scope; @@ -574,10 +572,8 @@ netspeed_lookup(GeoIP *db, dns_geoip_subtype_t subtype, REQUIRE(db != NULL); - prev_state = get_state(); - - if (prev_state != NULL && prev_state->ipnum == ipnum && - prev_state->subtype == subtype) { + prev_state = get_state_for(AF_INET, ipnum, NULL); + if (prev_state != NULL && prev_state->subtype == subtype) { id = prev_state->id; if (scope != NULL) *scope = prev_state->scope; @@ -854,6 +850,16 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, isc_uint8_t *scope, return (ISC_TRUE); break; + case dns_geoip_countrycode: + case dns_geoip_countrycode3: + case dns_geoip_countryname: + case dns_geoip_regionname: + /* + * If these were not remapped by fix_subtype(), + * the database was unavailable. Always return false. + */ + break; + default: INSIST(0); } @@ -864,9 +870,12 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, isc_uint8_t *scope, void dns_geoip_shutdown(void) { -#if defined(HAVE_GEOIP) && defined(ISC_PLATFORM_USETHREADS) +#ifdef HAVE_GEOIP + GeoIP_cleanup(); +#ifdef ISC_PLATFORM_USETHREADS if (state_mctx != NULL) isc_mem_detach(&state_mctx); +#endif #else return; #endif