From c73d8c1b72ddc3330cfc21e2070dffabca324bf7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 13 Aug 2010 06:46:25 +0000 Subject: [PATCH] 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 | 6 ++ bin/tests/system/dnssec/clean.sh | 3 +- bin/tests/system/dnssec/dnssec_update_test.pl | 6 +- bin/tests/system/dnssec/ns2/badparam.db.in | 26 +++++++++ bin/tests/system/dnssec/ns2/named.conf | 8 ++- bin/tests/system/dnssec/ns2/sign.sh | 18 +++++- bin/tests/system/dnssec/tests.sh | 13 ++++- lib/dns/rbtdb.c | 55 +++++++++++-------- 8 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 bin/tests/system/dnssec/ns2/badparam.db.in diff --git a/CHANGES b/CHANGES index 5a18a3d62d..f5ceaba50c 100644 --- a/CHANGES +++ b/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, diff --git a/bin/tests/system/dnssec/clean.sh b/bin/tests/system/dnssec/clean.sh index 312d041043..b2877f6295 100644 --- a/bin/tests/system/dnssec/clean.sh +++ b/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 diff --git a/bin/tests/system/dnssec/dnssec_update_test.pl b/bin/tests/system/dnssec/dnssec_update_test.pl index 3a87242ef6..1a75a18b9c 100644 --- a/bin/tests/system/dnssec/dnssec_update_test.pl +++ b/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; diff --git a/bin/tests/system/dnssec/ns2/badparam.db.in b/bin/tests/system/dnssec/ns2/badparam.db.in new file mode 100644 index 0000000000..e1df253ea6 --- /dev/null +++ b/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 diff --git a/bin/tests/system/dnssec/ns2/named.conf b/bin/tests/system/dnssec/ns2/named.conf index 3160413010..fb51f42ae4 100644 --- a/bin/tests/system/dnssec/ns2/named.conf +++ b/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"; diff --git a/bin/tests/system/dnssec/ns2/sign.sh b/bin/tests/system/dnssec/ns2/sign.sh index 9a2cb5ff90..b61e9a72ee 100644 --- a/bin/tests/system/dnssec/ns2/sign.sh +++ b/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 diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 511e425104..73ea6a674f 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/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. diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 8f9de66bca..86da4827fe 100644 --- a/lib/dns/rbtdb.c +++ b/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 (ISC_R_SUCCESS); + return (result); } 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 - || result == DNS_R_PARTIALMATCH) { + } else if (result == ISC_R_NOTFOUND || + 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) - result = ISC_R_NOTFOUND; - - if (result != ISC_R_NOTFOUND) { + if (result != DNS_R_PARTIALMATCH && 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); - name = dns_fixedname_name(&fname); - dns_fixedname_init(&forigin); - origin = dns_fixedname_name(&forigin); + dns_fixedname_init(&fname); + name = dns_fixedname_name(&fname); + dns_fixedname_init(&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) - return (result); + if (result != ISC_R_SUCCESS) + 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, - NULL, NULL); + result = previous_closest_nsec(type, search, + 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, - &nsecchain, &first); + name, origin, + &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)