[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)

diff --git a/CHANGES b/CHANGES
index c48887a99a02d64df2e2726a34b5d73224766a05..c8b360f97b8384f9ce79cef3afe5aa85d276b3e7 100644 (file)
--- 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 4965a661b3778ae904268079e045ba8bb72da95f..47976495d7fdc148a815d947df1b288a6b11f8d2 100644 (file)
--- 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 0f987b6579195c9233c7800ef1350967c49652cd..1c75c0db0503f39dca27e69669df2880b55ee46c 100644 (file)
--- 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 e250c8e34d8a74cc86d1182a92fe150503661e53..efc3d7b2e2c5b1fae3238631d90b6682006284a4 100644 (file)
--- 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 b6ea42cfceb5f7af3c66fdde062b2b52084ea987..80993643c462cdf3eff8af95317cbbf5a767cc4a 100644 (file)
--- 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 d2fde9a4f34d282f24780ad8fee5a7cda5c6bfa9..b36627c97b10283bf64982e9b6a235204a7ebdf5 100644 (file)
--- 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