2683. [bug] dnssec-signzone should clean out old NSEC3 chains when

                        the NSEC3 parameters used to sign the zone change.
                        [RT #20246]

diff --git a/CHANGES b/CHANGES
index 07e85c19aa9869c5562d2b090009e183f87e2c36..9bb222ac010c15d8df02c5895efdeb1d131453e5 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+2683.  [bug]           dnssec-signzone should clean out old NSEC3 chains when
+                       the NSEC3 parameters used to sign the zone change.
+                       [RT #20246]
+
 2682.  [bug]           "configure --enable-symtable=all" failed to
                        build. [RT #20282]
 

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index a9e356423efbdb93e39bbba9fa08351075bb4d2a..674423d0e6822f3d5f7aaeaf6848ec7a5a561d72 100644 (file)
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -29,7 +29,7 @@
  * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dnssec-signzone.c,v 1.229 2009/09/02 06:29:00 each Exp $ */
+/* $Id: dnssec-signzone.c,v 1.230 2009/09/23 04:30:16 marka Exp $ */
 
 /*! \file */
 
@@ -174,8 +174,9 @@ static dns_ttl_t keyttl;
 static void
 sign(isc_task_t *task, isc_event_t *event);
 
-static isc_boolean_t
-nsec3only(dns_dbnode_t *node);
+#define check_dns_dbiterator_current(result) \
+       check_result((result == DNS_R_NEWORIGIN) ? ISC_R_SUCCESS : result, \
+                    "dns_dbiterator_current()")
 
 static void
 dumpnode(dns_name_t *name, dns_dbnode_t *node) {
@@ -1618,7 +1619,8 @@ verifyzone(void) {
        while (!done) {
                isc_boolean_t isdelegation = ISC_FALSE;
 
-               dns_dbiterator_current(dbiter, &node, name);
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
                if (delegation(name, node, NULL)) {
                        zonecut = dns_fixedname_name(&fzonecut);
                        dns_name_copy(name, zonecut, NULL);
@@ -1631,8 +1633,7 @@ verifyzone(void) {
                while (result == ISC_R_SUCCESS) {
                        result = dns_dbiterator_current(dbiter, &nextnode,
                                                        nextname);
-                       if (result != ISC_R_SUCCESS)
-                               break;
+                       check_dns_dbiterator_current(result);
                        if (!dns_name_issubdomain(nextname, gorigin) ||
                            (zonecut != NULL &&
                             dns_name_issubdomain(nextname, zonecut)))
@@ -1660,7 +1661,8 @@ verifyzone(void) {
        for (result = dns_dbiterator_first(dbiter);
             result == ISC_R_SUCCESS;
             result = dns_dbiterator_next(dbiter) ) {
-               dns_dbiterator_current(dbiter, &node, name);
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
                verifynode(name, node, ISC_FALSE, &rdataset,
                           ksk_algorithms, bad_algorithms);
                dns_db_detachnode(gdb, &node);
@@ -1725,7 +1727,7 @@ signapex(void) {
        result = dns_dbiterator_seek(gdbiter, gorigin);
        check_result(result, "dns_dbiterator_seek()");
        result = dns_dbiterator_current(gdbiter, &node, name);
-       check_result(result, "dns_dbiterator_current()");
+       check_dns_dbiterator_current(result);
        signname(node, name);
        dumpnode(name, node);
        cleannode(gdb, gversion, node);
@@ -1777,9 +1779,7 @@ assignwork(isc_task_t *task, isc_task_t *worker) {
        found = ISC_FALSE;
        while (!found) {
                result = dns_dbiterator_current(gdbiter, &node, name);
-               if (result != ISC_R_SUCCESS)
-                       fatal("failure iterating database: %s",
-                             isc_result_totext(result));
+               check_dns_dbiterator_current(result);
                /*
                 * The origin was handled by signapex().
                 */
@@ -1974,7 +1974,8 @@ nsecify(void) {
        check_result(result, "dns_dbiterator_first()");
 
        while (!done) {
-               dns_dbiterator_current(dbiter, &node, name);
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
                if (delegation(name, node, &nsttl)) {
                        zonecut = dns_fixedname_name(&fzonecut);
                        dns_name_copy(name, zonecut, NULL);
@@ -1987,8 +1988,7 @@ nsecify(void) {
                        isc_boolean_t active = ISC_FALSE;
                        result = dns_dbiterator_current(dbiter, &nextnode,
                                                        nextname);
-                       if (result != ISC_R_SUCCESS)
-                               break;
+                       check_dns_dbiterator_current(result);
                        active = active_node(nextnode);
                        if (!active) {
                                dns_db_detachnode(gdb, &nextnode);
@@ -2021,37 +2021,6 @@ nsecify(void) {
        dns_dbiterator_destroy(&dbiter);
 }
 
-/*%
- * Does this node only contain NSEC3 records or RRSIG records or is empty.
- */
-static isc_boolean_t
-nsec3only(dns_dbnode_t *node) {
-       dns_rdatasetiter_t *rdsiter = NULL;
-       isc_result_t result;
-       dns_rdataset_t rdataset;
-       isc_boolean_t answer = ISC_TRUE;
-
-       dns_rdataset_init(&rdataset);
-       result = dns_db_allrdatasets(gdb, node, gversion, 0, &rdsiter);
-       check_result(result, "dns_db_allrdatasets()");
-       result = dns_rdatasetiter_first(rdsiter);
-       while (result == ISC_R_SUCCESS) {
-               dns_rdatasetiter_current(rdsiter, &rdataset);
-               if (rdataset.type != dns_rdatatype_nsec3 &&
-                   rdataset.type != dns_rdatatype_rrsig) {
-                       answer = ISC_FALSE;
-                       result = ISC_R_NOMORE;
-               } else
-                       result = dns_rdatasetiter_next(rdsiter);
-               dns_rdataset_disassociate(&rdataset);
-       }
-       if (result != ISC_R_NOMORE)
-               fatal("rdataset iteration failed: %s",
-                     isc_result_totext(result));
-       dns_rdatasetiter_destroy(&rdsiter);
-       return (answer);
-}
-
 static void
 addnsec3param(const unsigned char *salt, size_t salt_length,
              unsigned int iterations)
@@ -2092,6 +2061,16 @@ addnsec3param(const unsigned char *salt, size_t salt_length,
 
        result = dns_db_findnode(gdb, gorigin, ISC_TRUE, &node);
        check_result(result, "dns_db_find(gorigin)");
+
+       /*
+        * Delete any current NSEC3PARAM records.
+        */
+       result = dns_db_deleterdataset(gdb, node, gversion,
+                                      dns_rdatatype_nsec3param, 0);
+       if (result == DNS_R_UNCHANGED)
+               result = ISC_R_SUCCESS;
+       check_result(result, "dddnsec3param: dns_db_deleterdataset()");
+
        result = dns_db_addrdataset(gdb, node, gversion, 0, &rdataset,
                                    DNS_DBADD_MERGE, NULL);
        if (result == DNS_R_UNCHANGED)
@@ -2180,6 +2159,7 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node,
        isc_buffer_t target;
        isc_result_t result;
        unsigned char hash[NSEC3_MAX_HASH_LENGTH + 1];
+       isc_boolean_t exists;
 
        /*
         * Get the first label.
@@ -2201,8 +2181,7 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node,
 
        hash[isc_buffer_usedlength(&target)] = 0;
 
-       if (hashlist_exists(hashlist, hash))
-               return;
+       exists = hashlist_exists(hashlist, hash);
 
        /*
         * Verify that the NSEC3 parameters match the current ones
@@ -2217,8 +2196,8 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node,
                return;
 
        /*
-        * Delete any matching NSEC3 records which have parameters that
-        * match the NSEC3 chain we are building.
+        * Delete any NSEC3 records which are not part of the current
+        * NSEC3 chain.
         */
        for (result = dns_rdataset_first(&rdataset);
             result == ISC_R_SUCCESS;
@@ -2227,11 +2206,11 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node,
                dns_rdataset_current(&rdataset, &rdata);
                result = dns_rdata_tostruct(&rdata, &nsec3, NULL);
                check_result(result, "dns_rdata_tostruct");
-               if (nsec3.hash == hashalg &&
+               if (exists && nsec3.hash == hashalg &&
                    nsec3.iterations == iterations &&
                    nsec3.salt_length == salt_length &&
                    !memcmp(nsec3.salt, salt, salt_length))
-                       break;
+                       continue;
                rdatalist.rdclass = rdata.rdclass;
                rdatalist.type = rdata.type;
                rdatalist.covers = 0;
@@ -2245,7 +2224,7 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node,
                result = dns_db_subtractrdataset(gdb, node, gversion,
                                                 &delrdataset, 0, NULL);
                dns_rdataset_disassociate(&delrdataset);
-               if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED)
+               if (result != ISC_R_SUCCESS && result != DNS_R_NXRRSET)
                        check_result(result, "dns_db_subtractrdataset(NSEC3)");
                delete_rrsigs = ISC_TRUE;
        }
@@ -2300,14 +2279,14 @@ nsec3ify(unsigned int hashalg, unsigned int iterations,
        check_result(result, "dns_dbiterator_first()");
 
        while (!done) {
-               dns_dbiterator_current(dbiter, &node, name);
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
                result = dns_dbiterator_next(dbiter);
                nextnode = NULL;
                while (result == ISC_R_SUCCESS) {
                        result = dns_dbiterator_current(dbiter, &nextnode,
                                                        nextname);
-                       if (result != ISC_R_SUCCESS)
-                               break;
+                       check_dns_dbiterator_current(result);
                        active = active_node(nextnode);
                        if (!active) {
                                dns_db_detachnode(gdb, &nextnode);
@@ -2389,6 +2368,26 @@ nsec3ify(unsigned int hashalg, unsigned int iterations,
 
        addnsec3param(salt, salt_length, iterations);
 
+       /*
+        * Clean out NSEC3 records which don't match this chain.
+        */
+       result = dns_db_createiterator(gdb, DNS_DB_NSEC3ONLY, &dbiter);
+       check_result(result, "dns_db_createiterator()");
+
+       for (result = dns_dbiterator_first(dbiter);
+            result == ISC_R_SUCCESS;
+            result = dns_dbiterator_next(dbiter)) {
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
+               nsec3clean(name, node, hashalg, iterations, salt, salt_length,
+                          hashlist);
+               dns_db_detachnode(gdb, &node);
+       }
+       dns_dbiterator_destroy(&dbiter);
+
+       /*
+        * Generate / complete the new chain.
+        */
        result = dns_db_createiterator(gdb, DNS_DB_NONSEC3, &dbiter);
        check_result(result, "dns_db_createiterator()");
 
@@ -2396,25 +2395,16 @@ nsec3ify(unsigned int hashalg, unsigned int iterations,
        check_result(result, "dns_dbiterator_first()");
 
        while (!done) {
-               dns_dbiterator_current(dbiter, &node, name);
+               result = dns_dbiterator_current(dbiter, &node, name);
+               check_dns_dbiterator_current(result);
                result = dns_dbiterator_next(dbiter);
                nextnode = NULL;
                while (result == ISC_R_SUCCESS) {
                        result = dns_dbiterator_current(dbiter, &nextnode,
                                                        nextname);
-                       if (result != ISC_R_SUCCESS)
-                               break;
-                       /*
-                        * Cleanout NSEC3 RRsets which don't exist in the
-                        * hash table.
-                        */
-                       nsec3clean(nextname, nextnode, hashalg, iterations,
-                                  salt, salt_length, hashlist);
-                       /*
-                        * Skip NSEC3 only nodes when looking for the next
-                        * node in the zone.  Also skips now empty nodes.
-                        */
-                       if (nsec3only(nextnode)) {
+                       check_dns_dbiterator_current(result);
+                       active = active_node(nextnode);
+                       if (!active) {
                                dns_db_detachnode(gdb, &nextnode);
                                result = dns_dbiterator_next(dbiter);
                                continue;