diff --git a/CHANGES b/CHANGES index e7bb6801fe..b282c74c60 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +3877. [bug] Inserting and deleting parent and child nodes + in response policy zones could trigger an assertion + failure. [RT #36272] + 3876. [bug] Improve efficiency of DLZ redirect zones by suppressing unnecessary database lookups. [RT #35835] diff --git a/bin/tests/system/rpz/clean.sh b/bin/tests/system/rpz/clean.sh index 3a83ff2ff6..51f1d033ee 100644 --- a/bin/tests/system/rpz/clean.sh +++ b/bin/tests/system/rpz/clean.sh @@ -16,6 +16,7 @@ rm -f proto.* dsset-* trusted.conf dig.out* nsupdate.tmp ns*/*tmp rm -f ns*/*.key ns*/*.private ns2/tld2s.db ns2/bl.tld2.db -rm -f ns3/bl*.db ns*/*switch ns5/requests ns5/example.db ns5/bl.db ns5/*.perf +rm -f ns3/bl*.db ns*/*switch ns*/empty.db ns*/empty.db.jnl +rm -f ns5/requests ns5/example.db ns5/bl.db ns5/*.perf rm -f */named.memstats */named.run */named.stats */session.key rm -f */*.jnl */*.core */*.pid diff --git a/bin/tests/system/rpz/ns5/empty.db.in b/bin/tests/system/rpz/ns5/empty.db.in new file mode 100644 index 0000000000..0d58df82b8 --- /dev/null +++ b/bin/tests/system/rpz/ns5/empty.db.in @@ -0,0 +1,24 @@ +; 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. + +$TTL 120 +@ SOA . hostmaster.ns.example.tld5. ( 1 3600 1200 604800 60 ) + NS ns + NS ns1 + NS ns2 + NS ns3 +ns A 10.53.0.5 +ns1 A 10.53.0.5 +ns2 A 10.53.0.6 +ns3 A 10.53.0.6 diff --git a/bin/tests/system/rpz/ns5/named.conf b/bin/tests/system/rpz/ns5/named.conf index 448084d763..5735a1e512 100644 --- a/bin/tests/system/rpz/ns5/named.conf +++ b/bin/tests/system/rpz/ns5/named.conf @@ -32,7 +32,9 @@ options { session-keyfile "session.key"; listen-on { 10.53.0.5; }; listen-on-v6 { none; }; - notify no; + ixfr-from-differences yes; + notify-delay 1; + notify yes; # turn rpz on or off include "rpz-switch"; @@ -73,3 +75,11 @@ zone "bl16." {type master; file "bl.db"; }; zone "bl17." {type master; file "bl.db"; }; zone "bl18." {type master; file "bl.db"; }; zone "bl19." {type master; file "bl.db"; }; + +zone "policy1" { + type master; + file "empty.db"; + also-notify { 10.53.0.6; }; + allow-update { any; }; + allow-transfer { any; }; +}; diff --git a/bin/tests/system/rpz/ns6/named.conf b/bin/tests/system/rpz/ns6/named.conf index 5c9530b82d..4d396b5a55 100644 --- a/bin/tests/system/rpz/ns6/named.conf +++ b/bin/tests/system/rpz/ns6/named.conf @@ -24,9 +24,10 @@ options { session-keyfile "session.key"; listen-on { 10.53.0.6; }; listen-on-v6 { none; }; - notify no; forward only; forwarders { 10.53.0.3; }; + + response-policy { zone "policy1"; }; }; key rndc_key { @@ -39,3 +40,10 @@ controls { }; include "../trusted.conf"; + +zone "policy1" { + type slave; + masters { 10.53.0.5; }; + file "empty.db"; + allow-transfer { any; }; +}; diff --git a/bin/tests/system/rpz/setup.sh b/bin/tests/system/rpz/setup.sh index 6db6fb2dfa..e44351626c 100644 --- a/bin/tests/system/rpz/setup.sh +++ b/bin/tests/system/rpz/setup.sh @@ -115,3 +115,4 @@ $PERL -e 'for ($cnt = $val = 1; $cnt <= 3000; ++$cnt) { }' >ns5/requests cp ns2/bl.tld2.db.in ns2/bl.tld2.db +cp ns5/empty.db.in ns5/empty.db diff --git a/bin/tests/system/rpz/tests.sh b/bin/tests/system/rpz/tests.sh index 85451ce331..596f3eae09 100644 --- a/bin/tests/system/rpz/tests.sh +++ b/bin/tests/system/rpz/tests.sh @@ -606,5 +606,28 @@ $DIG +noall +answer -p 5300 @$ns3 any a3-2.tld2 > dig.out.any ttl=`awk '/a3-2 tld2 text/ {print $2}' dig.out.any` if test ${ttl:=0} -eq 0; then setret I:failed; fi +echo "I:checking rpz updates/transfers with parent nodes added after children" +# regression test for RT #36272: the success condition +# is the slave server not crashing. +nsd() { + nsupdate -p 5300 << EOF +server $1 +ttl 300 +update $2 $3 IN CNAME . +update $2 $4 IN CNAME . +send +EOF + sleep 2 +} + +for i in 1 2 3 4 5; do + nsd $ns5 add example.com.policy1. '*.example.com.policy1.' + nsd $ns5 delete example.com.policy1. '*.example.com.policy1.' +done +for i in 1 2 3 4 5; do + nsd $ns5 add '*.example.com.policy1.' example.com.policy1. + nsd $ns5 delete '*.example.com.policy1.' example.com.policy1. +done + echo "I:exit status: $status" exit $status diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index b905bad90a..36fd348cd6 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -2719,20 +2719,6 @@ findnodeintree(dns_rbtdb_t *rbtdb, dns_rbt_t *tree, dns_name_t *name, node = NULL; result = dns_rbt_addnode(tree, name, &node); if (result == ISC_R_SUCCESS) { - if (rbtdb->rpzs != NULL && tree == rbtdb->tree) { - dns_fixedname_t fnamef; - dns_name_t *fname; - - dns_fixedname_init(&fnamef); - fname = dns_fixedname_name(&fnamef); - dns_rbt_fullnamefromnode(node, fname); - result = dns_rpz_add(rbtdb->rpzs, - rbtdb->rpz_num, fname); - if (result != ISC_R_SUCCESS) { - RWUNLOCK(&rbtdb->tree_lock, locktype); - return (result); - } - } dns_rbt_namefromnode(node, &nodename); #ifdef DNS_RBT_USEHASH node->locknum = node->hashval % rbtdb->node_lock_count; @@ -2763,6 +2749,33 @@ findnodeintree(dns_rbtdb_t *rbtdb, dns_rbt_t *tree, dns_name_t *name, INSIST(node->nsec == DNS_RBT_NSEC_NSEC3); reactivate_node(rbtdb, node, locktype); + + /* + * Always try to add the policy zone data, because this node might + * already have been implicitly created by the previous addition of + * a longer domain. A common example is adding *.example.com + * (implicitly creating example.com) followed by explicitly adding + * example.com. + */ + if (create && rbtdb->rpzs != NULL && tree == rbtdb->tree) { + dns_fixedname_t fnamef; + dns_name_t *fname; + + dns_fixedname_init(&fnamef); + fname = dns_fixedname_name(&fnamef); + dns_rbt_fullnamefromnode(node, fname); + result = dns_rpz_add(rbtdb->rpzs, rbtdb->rpz_num, fname); + if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { + /* + * It is too late to give up, so merely complain. + */ + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, + DNS_LOGMODULE_RBTDB, DNS_RPZ_ERROR_LEVEL, + "dns_rpz_add(): %s", + isc_result_totext(result)); + } + } + RWUNLOCK(&rbtdb->tree_lock, locktype); *nodep = (dns_dbnode_t *)node; diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 5f4078e277..5c997721d4 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -1111,6 +1111,13 @@ add_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, if (result != ISC_R_SUCCESS) { char namebuf[DNS_NAME_FORMATSIZE]; + /* + * Do not worry if the radix tree already exists, + * because diff_apply() likes to add nodes before deleting. + */ + if (result == ISC_R_EXISTS) + return (ISC_R_SUCCESS); + /* * bin/tests/system/rpz/tests.sh looks for "rpz.*failed". */ @@ -1159,18 +1166,8 @@ add_nm(dns_rpz_zones_t *rpzs, dns_name_t *trig_name, if ((nm_data->set.qname & new_data->set.qname) != 0 || (nm_data->set.ns & new_data->set.ns) != 0 || (nm_data->wild.qname & new_data->wild.qname) != 0 || - (nm_data->wild.ns & new_data->wild.ns) != 0) { - char namebuf[DNS_NAME_FORMATSIZE]; - - /* - * bin/tests/system/rpz/tests.sh looks for "rpz.*failed". - */ - dns_name_format(trig_name, namebuf, sizeof(namebuf)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, - DNS_LOGMODULE_RBTDB, DNS_RPZ_ERROR_LEVEL, - "rpz add_nm(%s): bits already set", namebuf); + (nm_data->wild.ns & new_data->wild.ns) != 0) return (ISC_R_EXISTS); - } nm_data->set.qname |= new_data->set.qname; nm_data->set.ns |= new_data->set.ns; @@ -1188,11 +1185,26 @@ add_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_name_t *trig_name; isc_result_t result; + /* + * No need for a summary database of names with only 1 policy zone. + */ + if (rpzs->p.num_zones <= 1) { + adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, ISC_TRUE); + return (ISC_R_SUCCESS); + } + dns_fixedname_init(&trig_namef); trig_name = dns_fixedname_name(&trig_namef); name2data(rpzs, rpz_num, rpz_type, src_name, trig_name, &new_data); result = add_nm(rpzs, trig_name, &new_data); + + /* + * Do not worry if the node already exists, + * because diff_apply() likes to add nodes before deleting. + */ + if (result == ISC_R_EXISTS) + return (ISC_R_SUCCESS); if (result == ISC_R_SUCCESS) adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, ISC_TRUE); return (result); @@ -1793,10 +1805,6 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_nm_data_t *nm_data, del_data; isc_result_t result; - dns_fixedname_init(&trig_namef); - trig_name = dns_fixedname_name(&trig_namef); - name2data(rpzs, rpz_num, rpz_type, src_name, trig_name, &del_data); - /* * No need for a summary database of names with only 1 policy zone. */ @@ -1805,6 +1813,10 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, return; } + dns_fixedname_init(&trig_namef); + trig_name = dns_fixedname_name(&trig_namef); + name2data(rpzs, rpz_num, rpz_type, src_name, trig_name, &del_data); + nmnode = NULL; result = dns_rbt_findnode(rpzs->rbt, trig_name, NULL, &nmnode, NULL, 0, NULL, NULL); @@ -1815,7 +1827,8 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, * that were later added for (often empty) wildcards * and then to the RBTDB deferred cleanup list. */ - if (result == ISC_R_NOTFOUND) + if (result == ISC_R_NOTFOUND || + result == DNS_R_PARTIALMATCH) return; dns_name_format(src_name, namebuf, sizeof(namebuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ,