2938. [bug] When skipping NSEC3 records that don't match the

current NSEC3PARAM record in use for zone named
                        could dereference a uninitialised pointer attempting
                        to obtain a lock. [RT# 21868]

CHANGES

@@ -1,3 +1,9 @@
+2938.	[bug]		When generating signed responses, from a signed zone
+			that uses NSEC3, named would use a uninitialised
+			pointer if it needed to skip a NSEC3 record because
+			it didn't match the selected NSEC3PARAM record for
+			zone. [RT# 21868]
+
 2937.	[bug]		Worked around an apparent race condition in over
 			memory conditions.  Without this fix a DNS cache DB or
 			ADB could incorrectly stay in an over memory state,

bin/tests/system/dnssec/clean.sh

@@ -15,7 +15,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: clean.sh,v 1.28 2010/06/25 03:24:05 marka Exp $
+# $Id: clean.sh,v 1.29 2010/08/13 06:46:24 marka Exp $
 
 rm -f */K* */keyset-* */dsset-* */dlvset-* */signedkey-* */*.signed */trusted.conf */tmp* */*.jnl */*.bk
 rm -f ns1/root.db ns2/example.db ns3/secure.example.db
@@ -23,6 +23,7 @@ rm -f ns3/unsecure.example.db ns3/bogus.example.db ns3/keyless.example.db
 rm -f ns3/dynamic.example.db ns3/dynamic.example.db.signed.jnl
 rm -f ns3/rsasha256.example.db ns3/rsasha512.example.db
 rm -f ns2/private.secure.example.db
+rm -f ns2/badparam.db ns2/badparam.db.bad
 rm -f */example.bk
 rm -f dig.out.*
 rm -f random.data

bin/tests/system/dnssec/dnssec_update_test.pl

@@ -32,7 +32,7 @@
 #
 #    perl -MCPAN -e "install Net::DNS"
 #
-# $Id: dnssec_update_test.pl,v 1.5 2007/06/19 23:47:02 tbox Exp $
+# $Id: dnssec_update_test.pl,v 1.6 2010/08/13 06:46:24 marka Exp $
 #
 
 use Getopt::Std;
@@ -97,9 +97,9 @@ section("Delete the name");
 test("NOERROR", ["update", rr_del("a.$zone")]);
 
 if ($failures) {
-    print "I:$failures tests failed.\n";
+    print "I:$failures update tests failed.\n";
 } else {
-    print "I:All tests successful.\n";
+    print "I:All update tests successful.\n";
 }
 
 exit $failures;

bin/tests/system/dnssec/ns2/badparam.db.in

@@ -0,0 +1,26 @@
+; Copyright (C) 2010  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.
+
+; $Id: badparam.db.in,v 1.2 2010/08/13 06:46:24 marka Exp $
+
+$TTL 300	; 5 minutes
+@			IN SOA	mname1. . (
+				2010081000 ; serial
+				20         ; refresh (20 seconds)
+				20         ; retry (20 seconds)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns2
+ns2			A	10.53.0.2

bin/tests/system/dnssec/ns2/named.conf

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: named.conf,v 1.30 2008/09/25 04:02:38 tbox Exp $ */
+/* $Id: named.conf,v 1.31 2010/08/13 06:46:24 marka Exp $ */
 
 // NS2
 
@@ -80,4 +80,10 @@ zone "child.optout.example" {
 	allow-update { none; };
 };
 
+
+zone "badparam" {
+	type master;
+	file "badparam.db.bad";
+};
+
 include "trusted.conf";

bin/tests/system/dnssec/ns2/sign.sh

@@ -15,7 +15,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: sign.sh,v 1.38 2010/01/18 23:48:40 tbox Exp $
+# $Id: sign.sh,v 1.39 2010/08/13 06:46:25 marka Exp $
 
 SYSTEMTESTTOP=../..
 . $SYSTEMTESTTOP/conf.sh
@@ -114,3 +114,19 @@ dlvkeyname=`$KEYGEN -q -r $RANDFILE -a RSAMD5 -b 768 -n zone $dlvzone`
 cat $dlvinfile $dlvkeyname.key dlvset-$privzone > $dlvzonefile
 
 $SIGNER -P -g -r $RANDFILE -o $dlvzone $dlvzonefile > /dev/null
+
+
+# Sign the badparam secure file
+
+zone=badparam.
+infile=badparam.db.in
+zonefile=badparam.db
+
+keyname1=`$KEYGEN -q -r $RANDFILE -a RSASHA256 -b 1024 -n zone -f KSK $zone`
+keyname2=`$KEYGEN -q -r $RANDFILE -a RSASHA256 -b 1024 -n zone $zone`
+
+cat $infile $keyname1.key $keyname2.key >$zonefile
+
+$SIGNER -P -3 - -H 1 -g -r $RANDFILE -o $zone -k $keyname1 $zonefile $keyname2 > /dev/null
+
+sed 's/IN NSEC3 1 0 1 /IN NSEC3 1 0 10 /' $zonefile.signed > $zonefile.bad

bin/tests/system/dnssec/tests.sh

@@ -15,7 +15,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: tests.sh,v 1.68 2010/08/09 22:34:56 each Exp $
+# $Id: tests.sh,v 1.69 2010/08/13 06:46:24 marka Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
@@ -995,6 +995,17 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
+#
+# RT21868 regression test.
+#
+echo "I:checking NSEC3 zone with mismatched NSEC3PARAM / NSEC parameters ($n)"
+ret=0
+$DIG $DIGOPTS non-exist.badparam. @10.53.0.2 a > dig.out.ns2.test$n || ret=1
+grep "status: NXDOMAIN" dig.out.ns2.test$n > /dev/null || ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 # Run a minimal update test if possible.  This is really just
 # a regression test for RT #2399; more tests should be added.
 

lib/dns/rbtdb.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: rbtdb.c,v 1.302 2010/08/11 22:54:58 jinmei Exp $ */
+/* $Id: rbtdb.c,v 1.303 2010/08/13 06:46:24 marka Exp $ */
 
 /*! \file */
 
@@ -3275,6 +3275,9 @@ matchparams(rdatasetheader_t *header, rbtdb_search_t *search)
 	return (ISC_FALSE);
 }
 
+/*
+ * Find node of the NSEC/NSEC3 record that is 'name'.
+ */
 static inline isc_result_t
 previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 		    dns_name_t *name, dns_name_t *origin,
@@ -3286,15 +3289,15 @@ previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 	dns_rbtnode_t *nsecnode;
 	isc_result_t result;
 
+	REQUIRE(nodep != NULL && *nodep == NULL);
+
 	if (type == dns_rdatatype_nsec3) {
 		result = dns_rbtnodechain_prev(&search->chain, NULL, NULL);
 		if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN)
 			return (result);
 		result = dns_rbtnodechain_current(&search->chain, name, origin,
 						  nodep);
-		if (result != ISC_R_SUCCESS)
+		return (result);
-			return (result);
-		return (ISC_R_SUCCESS);
 	}
 
 	dns_fixedname_init(&ftarget);
@@ -3327,11 +3330,11 @@ previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 				 * Try the previous node in the NSEC tree.
 				 */
 				result = dns_rbtnodechain_prev(nsecchain,
-							name, origin);
+							       name, origin);
 				if (result == DNS_R_NEWORIGIN)
 					result = ISC_R_SUCCESS;
-			} else if (result == ISC_R_NOTFOUND
+			} else if (result == ISC_R_NOTFOUND ||
-				   || result == DNS_R_PARTIALMATCH) {
+				   result == DNS_R_PARTIALMATCH) {
 				result = dns_rbtnodechain_current(nsecchain,
 							name, origin, NULL);
 				if (result == ISC_R_NOTFOUND)
@@ -3348,8 +3351,6 @@ previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 			result = dns_rbtnodechain_prev(nsecchain, name, origin);
 			if (result == DNS_R_NEWORIGIN)
 				result = ISC_R_SUCCESS;
-			if (result != ISC_R_SUCCESS)
-				return (result);
 		}
 		if (result != ISC_R_SUCCESS)
 			return (result);
@@ -3373,10 +3374,7 @@ previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 		 * same name as the node in the auxiliary NSEC tree, except for
 		 * nodes in the auxiliary tree that are awaiting deletion.
 		 */
-		if (result == DNS_R_PARTIALMATCH)
+		if (result != DNS_R_PARTIALMATCH && result != ISC_R_NOTFOUND) {
-			result = ISC_R_NOTFOUND;
-
-		if (result != ISC_R_NOTFOUND) {
 			isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
 				      DNS_LOGMODULE_CACHE, ISC_LOG_ERROR,
 				      "previous_closest_nsec(): %s",
@@ -3386,6 +3384,11 @@ previous_closest_nsec(dns_rdatatype_t type, rbtdb_search_t *search,
 	}
 }
 
+/*
+ * Find the NSEC/NSEC3 which is or before the current point on the
+ * search chain.  For NSEC3 records only NSEC3 records that match the
+ * current NSEC3PARAM record are considered.
+ */
 static inline isc_result_t
 find_closest_nsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
 		  dns_name_t *foundname, dns_rdataset_t *rdataset,
@@ -3419,15 +3422,16 @@ find_closest_nsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
 	 * Use the auxiliary tree only starting with the second node in the
 	 * hope that the original node will be right much of the time.
 	 */
-		dns_fixedname_init(&fname);
+	dns_fixedname_init(&fname);
-		name = dns_fixedname_name(&fname);
+	name = dns_fixedname_name(&fname);
-		dns_fixedname_init(&forigin);
+	dns_fixedname_init(&forigin);
-		origin = dns_fixedname_name(&forigin);
+	origin = dns_fixedname_name(&forigin);
  again:
 	node = NULL;
+	prevnode = NULL;
 	result = dns_rbtnodechain_current(&search->chain, name, origin, &node);
-		if (result != ISC_R_SUCCESS)
+	if (result != ISC_R_SUCCESS)
-			return (result);
+		return (result);
 	do {
 		NODE_LOCK(&(search->rbtdb->node_locks[node->locknum].lock),
 			  isc_rwlocktype_read);
@@ -3478,8 +3482,10 @@ find_closest_nsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
 				empty_node = ISC_TRUE;
 				found = NULL;
 				foundsig = NULL;
-				result = dns_rbtnodechain_prev(&search->chain,
+				result = previous_closest_nsec(type, search,
-							       NULL, NULL);
+							       name, origin,	
+							       &prevnode, NULL,
+							       NULL);
 			} else if (found != NULL &&
 				   (foundsig != NULL || !need_sig)) {
 				/*
@@ -3519,8 +3525,10 @@ find_closest_nsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
 				 */
 				empty_node = ISC_TRUE;
 				result = previous_closest_nsec(type, search,
-							name, origin, &prevnode,
+							       name, origin,
-							&nsecchain, &first);
+							       &prevnode,
+							       &nsecchain,
+							       &first);
 			} else {
 				/*
 				 * We found an active node, but either the
@@ -3541,6 +3549,7 @@ find_closest_nsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
 		NODE_UNLOCK(&(search->rbtdb->node_locks[node->locknum].lock),
 			    isc_rwlocktype_read);
 		node = prevnode;
+		prevnode = NULL;
 	} while (empty_node && result == ISC_R_SUCCESS);
 
 	if (!first)