[master] address rpz bugs

3877.	[bug]		Inserting and deleting parent and child nodes
			in response policy zones could trigger an assertion
			failure. [RT #36272]

CHANGES

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

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

bin/tests/system/rpz/ns5/empty.db.in

+; 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

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; };
+};

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; };
+};

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

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

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;

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,