From bf9d2fd260b1c8bb707209345f4a28d7924e18d7 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 23 Jan 2013 14:57:18 -0800 Subject: [PATCH] [v9_9] fix incorrect nsec3 check - check for NSEC3 in empty nodes when not due to optout delegations - fixed typo in output ("Bad record NSEC record") - incidentally fixed an error in signzone that caused an incorrect warning about missing DNSKEYs when using -S and -3 together 3473. [bug] dnssec-signzone/verify could incorrectly report an error condition due to an empty node above an opt-out delegation lacking an NSEC3. [RT #32072] (cherry picked from commit 9a0dd99a757c469d9530acd5cb11789b3b0af5ce) --- CHANGES | 4 + bin/dnssec/dnssec-signzone.c | 35 ++++--- bin/dnssec/dnssectool.c | 116 +++++++++++++++++----- bin/tests/system/dnssec/tests.sh | 26 +++++ bin/tests/system/verify/tests.sh | 2 +- bin/tests/system/verify/zones/genzones.sh | 9 +- 6 files changed, 149 insertions(+), 43 deletions(-) diff --git a/CHANGES b/CHANGES index c48887a99a0..c8b360f97b8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +3473. [bug] dnssec-signzone/verify could incorrectly report + an error condition due to an empty node above an + opt-out delegation lacking an NSEC3. [RT #32072] + 3471. [bug] The number of UDP dispatches now defaults to the number of CPUs even if -n has been set to a higher value. [RT #30964] diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 4965a661b37..47976495d7f 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -2329,7 +2329,7 @@ nsec3ify(unsigned int hashalg, unsigned int iterations, continue; } if (is_delegation(gdb, gversion, gorigin, - nextname, nextnode, NULL)) + nextname, nextnode, NULL)) { zonecut = dns_fixedname_name(&fzonecut); dns_name_copy(nextname, zonecut, NULL); @@ -3481,23 +3481,6 @@ main(int argc, char *argv[]) { else set_nsec3params(update_chain, set_salt, set_optout, set_iter); - if (IS_NSEC3) { - isc_boolean_t answer; - hash_length = dns_nsec3_hashlength(dns_hash_sha1); - hashlist_init(&hashlist, dns_db_nodecount(gdb) * 2, - hash_length); - result = dns_nsec_nseconly(gdb, gversion, &answer); - if (result == ISC_R_NOTFOUND) - fprintf(stderr, "%s: warning: NSEC3 generation " - "requested with no DNSKEY; ignoring\n", - program); - else if (result != ISC_R_SUCCESS) - check_result(result, "dns_nsec_nseconly"); - else if (answer) - fatal("NSEC3 generation requested with " - "NSEC-only DNSKEY"); - } - /* * We need to do this early on, as we start messing with the list * of keys rather early. @@ -3550,6 +3533,22 @@ main(int argc, char *argv[]) { if (IS_NSEC3) { unsigned int max; + isc_boolean_t answer; + + hash_length = dns_nsec3_hashlength(dns_hash_sha1); + hashlist_init(&hashlist, dns_db_nodecount(gdb) * 2, + hash_length); + result = dns_nsec_nseconly(gdb, gversion, &answer); + if (result == ISC_R_NOTFOUND) + fprintf(stderr, "%s: warning: NSEC3 generation " + "requested with no DNSKEY; ignoring\n", + program); + else if (result != ISC_R_SUCCESS) + check_result(result, "dns_nsec_nseconly"); + else if (answer) + fatal("NSEC3 generation requested with " + "NSEC-only DNSKEY"); + result = dns_nsec3_maxiterations(gdb, NULL, mctx, &max); check_result(result, "dns_nsec3_maxiterations()"); if (nsec3iter > max) diff --git a/bin/dnssec/dnssectool.c b/bin/dnssec/dnssectool.c index 0f987b65791..1c75c0db050 100644 --- a/bin/dnssec/dnssectool.c +++ b/bin/dnssec/dnssectool.c @@ -583,7 +583,7 @@ verifynsec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_name_format(name, namebuf, sizeof(namebuf)); dns_name_format(nextname, nextbuf, sizeof(nextbuf)); dns_name_format(&nsec.next, found, sizeof(found)); - fprintf(stderr, "Bad record NSEC record for %s, next name " + fprintf(stderr, "Bad NSEC record for %s, next name " "mismatch (expected:%s, found:%s)\n", namebuf, nextbuf, found); goto failure; @@ -594,7 +594,7 @@ verifynsec(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, check_result(result, "dns_nsec_buildrdata()"); if (dns_rdata_compare(&rdata, &tmprdata) != 0) { dns_name_format(name, namebuf, sizeof(namebuf)); - fprintf(stderr, "Bad record NSEC record for %s, bit map " + fprintf(stderr, "Bad NSEC record for %s, bit map " "mismatch\n", namebuf); goto failure; } @@ -770,7 +770,7 @@ match_nsec3(dns_name_t *name, isc_mem_t *mctx, len = dns_nsec_compressbitmap(cbm, types, maxtype); if (nsec3.len != len || memcmp(cbm, nsec3.typebits, len) != 0) { dns_name_format(name, namebuf, sizeof(namebuf)); - fprintf(stderr, "Bad record NSEC3 record for %s, bit map " + fprintf(stderr, "Bad NSEC3 record for %s, bit map " "mismatch\n", namebuf); return (ISC_R_FAILURE); } @@ -891,11 +891,64 @@ record_found(dns_db_t *db, dns_dbversion_t *ver, isc_mem_t *mctx, return (ISC_R_SUCCESS); } +static isc_boolean_t +isoptout(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, + dns_rdata_t *nsec3rdata) +{ + dns_rdataset_t rdataset; + dns_rdata_t rdata = DNS_RDATA_INIT; + dns_rdata_nsec3_t nsec3; + dns_rdata_nsec3param_t nsec3param; + dns_fixedname_t fixed; + dns_name_t *hashname; + isc_result_t result; + dns_dbnode_t *node = NULL; + unsigned char rawhash[NSEC3_MAX_HASH_LENGTH]; + size_t rhsize = sizeof(rawhash); + isc_boolean_t ret; + + result = dns_rdata_tostruct(nsec3rdata, &nsec3param, NULL); + check_result(result, "dns_rdata_tostruct()"); + + dns_fixedname_init(&fixed); + result = dns_nsec3_hashname(&fixed, rawhash, &rhsize, origin, origin, + nsec3param.hash, nsec3param.iterations, + nsec3param.salt, nsec3param.salt_length); + check_result(result, "dns_nsec3_hashname()"); + + dns_rdataset_init(&rdataset); + hashname = dns_fixedname_name(&fixed); + result = dns_db_findnsec3node(db, hashname, ISC_FALSE, &node); + if (result == ISC_R_SUCCESS) + result = dns_db_findrdataset(db, node, ver, dns_rdatatype_nsec3, + 0, 0, &rdataset, NULL); + if (result != ISC_R_SUCCESS) + return (ISC_FALSE); + + result = dns_rdataset_first(&rdataset); + check_result(result, "dns_rdataset_first()"); + + dns_rdataset_current(&rdataset, &rdata); + + result = dns_rdata_tostruct(&rdata, &nsec3, NULL); + if (result != ISC_R_SUCCESS) + ret = ISC_FALSE; + else + ret = ISC_TF((nsec3.flags & DNS_NSEC3FLAG_OPTOUT) != 0); + + if (dns_rdataset_isassociated(&rdataset)) + dns_rdataset_disassociate(&rdataset); + if (node != NULL) + dns_db_detachnode(db, &node); + + return (ret); +} + static isc_result_t verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name, dns_rdata_t *rdata, - isc_boolean_t delegation, unsigned char types[8192], - unsigned int maxtype) + isc_boolean_t delegation, isc_boolean_t empty, + unsigned char types[8192], unsigned int maxtype) { char namebuf[DNS_NAME_FORMATSIZE]; char hashbuf[DNS_NAME_FORMATSIZE]; @@ -907,6 +960,7 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, dns_dbnode_t *node = NULL; unsigned char rawhash[NSEC3_MAX_HASH_LENGTH]; size_t rhsize = sizeof(rawhash); + isc_boolean_t optout; result = dns_rdata_tostruct(rdata, &nsec3param, NULL); check_result(result, "dns_rdata_tostruct()"); @@ -917,6 +971,8 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, if (!dns_nsec3_supportedhash(nsec3param.hash)) return (ISC_R_SUCCESS); + optout = isoptout(db, ver, origin, rdata); + dns_fixedname_init(&fixed); result = dns_nsec3_hashname(&fixed, rawhash, &rhsize, name, origin, nsec3param.hash, nsec3param.iterations, @@ -936,16 +992,22 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, result = dns_db_findrdataset(db, node, ver, dns_rdatatype_nsec3, 0, 0, &rdataset, NULL); if (result != ISC_R_SUCCESS && - (!delegation || dns_nsec_isset(types, dns_rdatatype_ds))) { + (!delegation || (empty && !optout) || + (!empty && dns_nsec_isset(types, dns_rdatatype_ds)))) + { dns_name_format(name, namebuf, sizeof(namebuf)); dns_name_format(hashname, hashbuf, sizeof(hashbuf)); fprintf(stderr, "Missing NSEC3 record for %s (%s)\n", namebuf, hashbuf); + } else if (result == ISC_R_NOTFOUND && + delegation && (!empty || optout)) + { + result = ISC_R_SUCCESS; } else if (result == ISC_R_SUCCESS) { result = match_nsec3(name, mctx, &nsec3param, &rdataset, types, maxtype, rawhash, rhsize); - } else if (result == ISC_R_NOTFOUND && delegation) - result = ISC_R_SUCCESS; + } + if (dns_rdataset_isassociated(&rdataset)) dns_rdataset_disassociate(&rdataset); if (node != NULL) @@ -957,8 +1019,8 @@ verifynsec3(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, static isc_result_t verifynsec3s(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, isc_mem_t *mctx, dns_name_t *name, dns_rdataset_t *nsec3paramset, - isc_boolean_t delegation, unsigned char types[8192], - unsigned int maxtype) + isc_boolean_t delegation, isc_boolean_t empty, + unsigned char types[8192], unsigned int maxtype) { isc_result_t result; @@ -969,7 +1031,7 @@ verifynsec3s(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, dns_rdataset_current(nsec3paramset, &rdata); result = verifynsec3(db, ver, origin, mctx, name, &rdata, - delegation, types, maxtype); + delegation, empty, types, maxtype); if (result != ISC_R_SUCCESS) break; } @@ -1114,8 +1176,8 @@ verifynode(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, if (nsec3paramset != NULL && dns_rdataset_isassociated(nsec3paramset)) { tresult = verifynsec3s(db, ver, origin, mctx, name, - nsec3paramset, delegation, types, - maxtype); + nsec3paramset, delegation, ISC_FALSE, + types, maxtype); if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) result = tresult; } @@ -1304,8 +1366,8 @@ verify_nsec3_chains(isc_mem_t *mctx) { static isc_result_t verifyemptynodes(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, - isc_mem_t *mctx, dns_name_t *name, dns_name_t *nextname, - dns_rdataset_t *nsec3paramset) + isc_mem_t *mctx, dns_name_t *name, dns_name_t *prevname, + isc_boolean_t isdelegation, dns_rdataset_t *nsec3paramset) { dns_namereln_t reln; int order; @@ -1313,23 +1375,24 @@ verifyemptynodes(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *origin, dns_name_t suffix; isc_result_t result = ISC_R_SUCCESS, tresult; - reln = dns_name_fullcompare(name, nextname, &order, &labels); + reln = dns_name_fullcompare(prevname, name, &order, &labels); if (order >= 0) return (result); - nlabels = dns_name_countlabels(nextname); + nlabels = dns_name_countlabels(name); if (reln == dns_namereln_commonancestor || reln == dns_namereln_contains) { dns_name_init(&suffix, NULL); for (i = labels + 1; i < nlabels; i++) { - dns_name_getlabelsequence(nextname, nlabels - i, i, + dns_name_getlabelsequence(name, nlabels - i, i, &suffix); if (nsec3paramset != NULL && dns_rdataset_isassociated(nsec3paramset)) { tresult = verifynsec3s(db, ver, origin, mctx, &suffix, nsec3paramset, - ISC_FALSE, NULL, 0); + isdelegation, ISC_TRUE, + NULL, 0); if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) result = tresult; @@ -1359,8 +1422,8 @@ verifyzone(dns_db_t *db, dns_dbversion_t *ver, char algbuf[80]; dns_dbiterator_t *dbiter = NULL; dns_dbnode_t *node = NULL, *nextnode = NULL; - dns_fixedname_t fname, fnextname, fzonecut; - dns_name_t *name, *nextname, *zonecut; + dns_fixedname_t fname, fnextname, fprevname, fzonecut; + dns_name_t *name, *nextname, *prevname, *zonecut; dns_rdata_dnskey_t dnskey; dns_rdata_t rdata = DNS_RDATA_INIT; dns_rdataset_t keyset, soaset; @@ -1572,6 +1635,8 @@ verifyzone(dns_db_t *db, dns_dbversion_t *ver, name = dns_fixedname_name(&fname); dns_fixedname_init(&fnextname); nextname = dns_fixedname_name(&fnextname); + dns_fixedname_init(&fprevname); + prevname = NULL; dns_fixedname_init(&fzonecut); zonecut = NULL; @@ -1638,8 +1703,13 @@ verifyzone(dns_db_t *db, dns_dbversion_t *ver, vresult = ISC_R_SUCCESS; if (vresult == ISC_R_SUCCESS && result != ISC_R_SUCCESS) vresult = result; - result = verifyemptynodes(db, ver, origin, mctx, name, - nextname, &nsec3paramset); + if (prevname != NULL) { + result = verifyemptynodes(db, ver, origin, mctx, name, + prevname, isdelegation, + &nsec3paramset); + } else + prevname = dns_fixedname_name(&fprevname); + dns_name_copy(name, prevname, NULL); if (vresult == ISC_R_SUCCESS && result != ISC_R_SUCCESS) vresult = result; dns_db_detachnode(db, &node); diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index e250c8e34d8..efc3d7b2e2c 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1050,6 +1050,32 @@ n=`expr $n + 1` if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` +echo "I:checking NSEC3 signing with empty nonterminals above a delegation ($n)" +ret=0 +zone=example +key1=`$KEYGEN -K signer -q -r $RANDFILE -a NSEC3RSASHA1 -b 1024 -n zone $zone` +key2=`$KEYGEN -K signer -q -r $RANDFILE -f KSK -a NSEC3RSASHA1 -b 1024 -n zone $zone` +( +cd signer +cat example.db.in $key1.key $key2.key > example3.db +echo "some.empty.nonterminal.nodes.example 60 IN NS ns.example.tld" >> example3.db +$SIGNER -3 - -A -H 10 -o example -f example3.db example3.db > /dev/null 2>&1 +awk '/^IQF9LQTLK/ { + printf("%s", $0); + while (!index($0, ")")) { + if (getline <= 0) + break; + printf (" %s", $0); + } + printf("\n"); + }' example.db | sed 's/[ ][ ]*/ /g' > nsec3param.out + +grep "IQF9LQTLKKNFK0KVIFELRAK4IC4QLTMG.example. 0 IN NSEC3 1 0 10 - ( IQF9LQTLKKNFK0KVIFELRAK4IC4QLTMG A NS SOA RRSIG DNSKEY NSEC3PARAM )" nsec3param.out > /dev/null +) || ret=1 +n=`expr $n + 1` +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + echo "I:checking that dnsssec-signzone updates originalttl on ttl changes ($n)" ret=0 zone=example diff --git a/bin/tests/system/verify/tests.sh b/bin/tests/system/verify/tests.sh index b6ea42cfceb..80993643c46 100644 --- a/bin/tests/system/verify/tests.sh +++ b/bin/tests/system/verify/tests.sh @@ -69,7 +69,7 @@ do expect1="unexpected NSEC RRset at" ;; *.nsec.broken-chain) - expect1="Bad record NSEC record for.*, next name mismatch" + expect1="Bad NSEC record for.*, next name mismatch" ;; *.bad-bitmap) expect1="bit map mismatch" diff --git a/bin/tests/system/verify/zones/genzones.sh b/bin/tests/system/verify/zones/genzones.sh index d2fde9a4f34..b36627c97b1 100644 --- a/bin/tests/system/verify/zones/genzones.sh +++ b/bin/tests/system/verify/zones/genzones.sh @@ -170,12 +170,19 @@ $SIGNER -Px -Z nonsecify -O full -o ${zone} -f ${file}.tmp ${file} $zsk > s.out$ awk '$1 ~ /^ns.sub/ && $4 == "RRSIG" && $5 != "NSEC" { next; } { print; }' ${file}.tmp > ${file} # missing NSEC3 record at empty node +# extract the hash fields from the empty node's NSEC 3 record then fix up +# the NSEC3 chain to remove it setup ksk+zsk.nsec3.missing-empty bad zsk=`$KEYGEN -3 -r $RANDFILE ${zone} 2> kg1.out$n` || dumpit kg1.out$n ksk=`$KEYGEN -3 -r $RANDFILE -fK ${zone} 2> kg2.out$n` || dumpit kg2.out$n cat unsigned.db $ksk.key $zsk.key > $file $SIGNER -3 - -P -O full -o ${zone} -f ${file} ${file} $ksk > s.out$n 2>&1 || dumpit s.out$n -awk '$4 == "NSEC3" && NF == 9 { next; } { print; }' ${file} > ${file}.tmp +a=`awk '$4 == "NSEC3" && NF == 9 { split($1, a, "."); print a[1]; }' ${file}` +b=`awk '$4 == "NSEC3" && NF == 9 { print $9; }' ${file}` +awk ' +$4 == "NSEC3" && $9 == "'$a'" { $9 = "'$b'"; print; next; } +$4 == "NSEC3" && NF == 9 { next; } +{ print; }' ${file} > ${file}.tmp $SIGNER -3 - -Px -Z nonsecify -O full -o ${zone} -f ${file} ${file}.tmp $zsk > s.out$n 2>&1 || dumpit s.out$n # extra NSEC3 record -- 2.47.3