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

[master] geoip security fixes

Browse Source 
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.	[security]	Lookups in GeoIP databases that were not
			loaded could cause an assertion failure.
			[RT #37679]

4001.	[security]	The caching of GeoIP lookups did not always
			handle address families correctly, potentially
			resulting in an assertion failure. [RT #37672]
This commit is contained in:
Evan Hunt
2014-11-16 08:43:22 -08:00
parent aee6c351d3
commit a0b4f6d952
8 changed files with 177 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
CHANGES
View File

@@ -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]

1
bin/named/geoip.c
View File

@@ -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

4
bin/tests/system/geoip/clean.sh
View File

@@ -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

55
bin/tests/system/geoip/ns2/named15.conf Normal file
View File

@@ -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";
};
};

2
bin/tests/system/geoip/ns2/named6.conf
View File

@@ -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";
};

3
bin/tests/system/geoip/setup.sh
View File

@@ -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/

52
bin/tests/system/geoip/tests.sh
View File

@@ -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

93
lib/dns/geoip.c
View File

@@ -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