2897. [bug] NSEC3 chains could be left behind when transitioning

to insecure. [RT #21040]

CHANGES

@@ -1,3 +1,6 @@
+2897.	[bug]		NSEC3 chains could be left behind when transitioning
+			to insecure. [RT #21040]
+			
 2896.	[bug]		"rndc sign" failed to properly update the zone
 			when adding a DNSKEY for publication only. [RT #21045]
 

bin/named/update.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: update.c,v 1.181 2010/02/26 23:50:59 tbox Exp $ */
+/* $Id: update.c,v 1.182 2010/05/18 01:39:41 marka Exp $ */
 
 #include <config.h>
 
@@ -3397,125 +3397,6 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
 	return (result);
 }
 
-/*
- * Mark all NSEC3 chains for deletion without creating a NSEC chain as
- * a side effect of deleting the last chain.
- */
-static isc_result_t
-delete_chains(dns_db_t *db, dns_dbversion_t *ver, dns_zone_t *zone,
-	      dns_diff_t *diff)
-{
-	dns_dbnode_t *node = NULL;
-	dns_difftuple_t *tuple = NULL;
-	dns_name_t next;
-	dns_rdata_t rdata = DNS_RDATA_INIT;
-	dns_rdataset_t rdataset;
-	isc_boolean_t flag;
-	isc_result_t result = ISC_R_SUCCESS;
-	unsigned char buf[DNS_NSEC3PARAM_BUFFERSIZE + 1];
-	dns_name_t *origin = dns_zone_getorigin(zone);
-	dns_rdatatype_t privatetype = dns_zone_getprivatetype(zone);
-
-	dns_name_init(&next, NULL);
-	dns_rdataset_init(&rdataset);
-
-	result = dns_db_getoriginnode(db, &node);
-	if (result != ISC_R_SUCCESS)
-		return (result);
-
-	/*
-	 * Cause all NSEC3 chains to be deleted.
-	 */
-	result = dns_db_findrdataset(db, node, ver, dns_rdatatype_nsec3param,
-				     0, (isc_stdtime_t) 0, &rdataset, NULL);
-	if (result == ISC_R_NOTFOUND)
-		goto try_private;
-	if (result != ISC_R_SUCCESS)
-		goto failure;
-
-	for (result = dns_rdataset_first(&rdataset);
-	     result == ISC_R_SUCCESS;
-	     result = dns_rdataset_next(&rdataset)) {
-		dns_rdata_t private = DNS_RDATA_INIT;
-
-		dns_rdataset_current(&rdataset, &rdata);
-
-		CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, origin,
-					   rdataset.ttl, &rdata, &tuple));
-		CHECK(do_one_tuple(&tuple, db, ver, diff));
-		INSIST(tuple == NULL);
-
-		dns_nsec3param_toprivate(&rdata, &private, privatetype,
-					 buf, sizeof(buf));
-		buf[2] = DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC;
-
-		CHECK(rr_exists(db, ver, origin, &rdata, &flag));
-
-		if (!flag) {
-			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
-						   origin, 0, &rdata, &tuple));
-			CHECK(do_one_tuple(&tuple, db, ver, diff));
-			INSIST(tuple == NULL);
-		}
-		dns_rdata_reset(&rdata);
-	}
-	if (result != ISC_R_NOMORE)
-		goto failure;
-
-	dns_rdataset_disassociate(&rdataset);
-
- try_private:
-	if (privatetype == 0)
-		goto success;
-	result = dns_db_findrdataset(db, node, ver, privatetype, 0,
-				     (isc_stdtime_t) 0, &rdataset, NULL);
-	if (result == ISC_R_NOTFOUND)
-		goto success;
-	if (result != ISC_R_SUCCESS)
-		goto failure;
-
-	for (result = dns_rdataset_first(&rdataset);
-	     result == ISC_R_SUCCESS;
-	     result = dns_rdataset_next(&rdataset)) {
-		dns_rdataset_current(&rdataset, &rdata);
-		INSIST(rdata.length <= sizeof(buf));
-		memcpy(buf, rdata.data, rdata.length);
-
-		if (buf[0] != 0 ||
-		    buf[2] == (DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC)) {
-			dns_rdata_reset(&rdata);
-			continue;
-		}
-
-		CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, origin,
-					   0, &rdata, &tuple));
-		CHECK(do_one_tuple(&tuple, db, ver, diff));
-		INSIST(tuple == NULL);
-
-		buf[2] = DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC;
-
-		CHECK(rr_exists(db, ver, origin, &rdata, &flag));
-
-		if (!flag) {
-			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
-						   origin, 0, &rdata, &tuple));
-			CHECK(do_one_tuple(&tuple, db, ver, diff));
-			INSIST(tuple == NULL);
-		}
-		dns_rdata_reset(&rdata);
-	}
-	if (result != ISC_R_NOMORE)
-		goto failure;
- success:
-	result = ISC_R_SUCCESS;
-
- failure:
-	if (dns_rdataset_isassociated(&rdataset))
-		dns_rdataset_disassociate(&rdataset);
-	dns_db_detachnode(db, &node);
-	return (result);
-}
-
 static isc_boolean_t
 isdnssec(dns_db_t *db, dns_dbversion_t *ver, dns_rdatatype_t privatetype) {
 	isc_result_t result;
@@ -4175,7 +4056,8 @@ update_action(isc_task_t *task, isc_event_t *event) {
 			 * the last signature for the DNSKEY records are
 			 * remove any NSEC chain present will also be removed.
 			 */
-			 CHECK(delete_chains(db, ver, zone, &diff));
+			 CHECK(dns_nsec3param_deletechains(db, ver, zone,
+							   &diff));
 		} else if (has_dnskey && isdnssec(db, ver, privatetype)) {
 			isc_uint32_t interval;
 			interval = dns_zone_getsigvalidityinterval(zone);

lib/dns/include/dns/nsec3.h

@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: nsec3.h,v 1.10 2009/10/08 23:48:10 tbox Exp $ */
+/* $Id: nsec3.h,v 1.11 2010/05/18 01:39:41 marka Exp $ */
 
 #ifndef DNS_NSEC3_H
 #define DNS_NSEC3_H 1
@@ -239,6 +239,15 @@ dns_nsec3param_toprivate(dns_rdata_t *src, dns_rdata_t *target,
  * 'buf' should be at least src->length + 1 in size.
  */
 
+isc_result_t
+dns_nsec3param_deletechains(dns_db_t *db, dns_dbversion_t *ver,
+                            dns_zone_t *zone, dns_diff_t *diff);
+
+/*%<
+ * Mark NSEC3PARAM for deletion.
+ */
+
+
 ISC_LANG_ENDDECLS
 
 #endif /* DNS_NSEC3_H */

lib/dns/nsec3.c

@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: nsec3.c,v 1.15 2010/01/04 23:48:51 tbox Exp $ */
+/* $Id: nsec3.c,v 1.16 2010/05/18 01:39:41 marka Exp $ */
 
 #include <config.h>
 
@@ -28,6 +28,7 @@
 #include <dst/dst.h>
 
 #include <dns/db.h>
+#include <dns/zone.h>
 #include <dns/compress.h>
 #include <dns/dbiterator.h>
 #include <dns/diff.h>
@@ -1004,6 +1005,165 @@ dns_nsec3param_toprivate(dns_rdata_t *src, dns_rdata_t *target,
 	ISC_LINK_INIT(target, link);
 }
 
+static isc_result_t
+rr_exists(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
+	  const dns_rdata_t *rdata, isc_boolean_t *flag)
+{
+	dns_rdataset_t rdataset;
+	dns_dbnode_t *node = NULL;
+	isc_result_t result;
+			
+	dns_rdataset_init(&rdataset);
+	if (rdata->type == dns_rdatatype_nsec3)
+		CHECK(dns_db_findnsec3node(db, name, ISC_FALSE, &node));
+	else
+		CHECK(dns_db_findnode(db, name, ISC_FALSE, &node));
+	result = dns_db_findrdataset(db, node, ver, rdata->type, 0,
+				     (isc_stdtime_t) 0, &rdataset, NULL);
+	if (result == ISC_R_NOTFOUND) {
+		*flag = ISC_FALSE;
+		result = ISC_R_SUCCESS;
+		goto failure;
+	}
+
+	for (result = dns_rdataset_first(&rdataset);
+	     result == ISC_R_SUCCESS;
+	     result = dns_rdataset_next(&rdataset)) {
+		dns_rdata_t myrdata = DNS_RDATA_INIT;
+		dns_rdataset_current(&rdataset, &myrdata);
+		if (!dns_rdata_casecompare(&myrdata, rdata))
+			break;
+	}
+	dns_rdataset_disassociate(&rdataset);
+	if (result == ISC_R_SUCCESS) {
+		*flag = ISC_TRUE;
+	} else if (result == ISC_R_NOMORE) {
+		*flag = ISC_FALSE;
+		result = ISC_R_SUCCESS;
+	}
+
+ failure:
+	if (node != NULL)
+		dns_db_detachnode(db, &node);
+	return (result);
+}
+
+isc_result_t
+dns_nsec3param_deletechains(dns_db_t *db, dns_dbversion_t *ver,
+			    dns_zone_t *zone, dns_diff_t *diff)
+{
+	dns_dbnode_t *node = NULL;
+	dns_difftuple_t *tuple = NULL;
+	dns_name_t next;
+	dns_rdata_t rdata = DNS_RDATA_INIT;
+	dns_rdataset_t rdataset;
+	isc_boolean_t flag;
+	isc_result_t result = ISC_R_SUCCESS;
+	unsigned char buf[DNS_NSEC3PARAM_BUFFERSIZE + 1];
+	dns_name_t *origin = dns_zone_getorigin(zone);
+	dns_rdatatype_t privatetype = dns_zone_getprivatetype(zone);
+
+	dns_name_init(&next, NULL);
+	dns_rdataset_init(&rdataset);
+
+	result = dns_db_getoriginnode(db, &node);
+	if (result != ISC_R_SUCCESS)
+		return (result);
+
+	/*
+	 * Cause all NSEC3 chains to be deleted.
+	 */
+	result = dns_db_findrdataset(db, node, ver, dns_rdatatype_nsec3param,
+				     0, (isc_stdtime_t) 0, &rdataset, NULL);
+	if (result == ISC_R_NOTFOUND)
+		goto try_private;
+	if (result != ISC_R_SUCCESS)
+		goto failure;
+
+	for (result = dns_rdataset_first(&rdataset);
+	     result == ISC_R_SUCCESS;
+	     result = dns_rdataset_next(&rdataset)) {
+		dns_rdata_t private = DNS_RDATA_INIT;
+
+		dns_rdataset_current(&rdataset, &rdata);
+
+		CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, origin,
+					   rdataset.ttl, &rdata, &tuple));
+		CHECK(do_one_tuple(&tuple, db, ver, diff));
+		INSIST(tuple == NULL);
+
+		dns_nsec3param_toprivate(&rdata, &private, privatetype,
+					 buf, sizeof(buf));
+		buf[2] = DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC;
+
+		CHECK(rr_exists(db, ver, origin, &private, &flag));
+
+		if (!flag) {
+			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
+						   origin, 0, &private,
+						   &tuple));
+			CHECK(do_one_tuple(&tuple, db, ver, diff));
+			INSIST(tuple == NULL);
+		}
+		dns_rdata_reset(&rdata);
+	}
+	if (result != ISC_R_NOMORE)
+		goto failure;
+
+	dns_rdataset_disassociate(&rdataset);
+
+ try_private:
+	if (privatetype == 0)
+		goto success;
+	result = dns_db_findrdataset(db, node, ver, privatetype, 0,
+				     (isc_stdtime_t) 0, &rdataset, NULL);
+	if (result == ISC_R_NOTFOUND)
+		goto success;
+	if (result != ISC_R_SUCCESS)
+		goto failure;
+
+	for (result = dns_rdataset_first(&rdataset);
+	     result == ISC_R_SUCCESS;
+	     result = dns_rdataset_next(&rdataset)) {
+		dns_rdataset_current(&rdataset, &rdata);
+		INSIST(rdata.length <= sizeof(buf));
+		memcpy(buf, rdata.data, rdata.length);
+
+		if (buf[0] != 0 ||
+		    buf[2] == (DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC)) {
+			dns_rdata_reset(&rdata);
+			continue;
+		}
+
+		CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, origin,
+					   0, &rdata, &tuple));
+		CHECK(do_one_tuple(&tuple, db, ver, diff));
+		INSIST(tuple == NULL);
+
+		buf[2] = DNS_NSEC3FLAG_REMOVE | DNS_NSEC3FLAG_NONSEC;
+
+		CHECK(rr_exists(db, ver, origin, &rdata, &flag));
+
+		if (!flag) {
+			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
+						   origin, 0, &rdata, &tuple));
+			CHECK(do_one_tuple(&tuple, db, ver, diff));
+			INSIST(tuple == NULL);
+		}
+		dns_rdata_reset(&rdata);
+	}
+	if (result != ISC_R_NOMORE)
+		goto failure;
+ success:
+	result = ISC_R_SUCCESS;
+
+ failure:
+	if (dns_rdataset_isassociated(&rdataset))
+		dns_rdataset_disassociate(&rdataset);
+	dns_db_detachnode(db, &node);
+	return (result);
+}
+
 isc_result_t
 dns_nsec3_addnsec3sx(dns_db_t *db, dns_dbversion_t *version,
 		     dns_name_t *name, dns_ttl_t nsecttl,

lib/dns/zone.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: zone.c,v 1.565 2010/05/18 01:03:26 marka Exp $ */
+/* $Id: zone.c,v 1.566 2010/05/18 01:39:41 marka Exp $ */
 
 /*! \file */
 
@@ -13668,6 +13668,32 @@ dnskey_sane(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
 	return (ISC_FALSE);
 }
 
+static isc_result_t
+clean_nsec3param(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
+		 dns_diff_t *diff)
+{
+	isc_result_t result;
+	dns_dbnode_t *node = NULL;
+	dns_rdataset_t rdataset;
+
+	dns_rdataset_init(&rdataset);
+	CHECK(dns_db_getoriginnode(db, &node));
+
+	result = dns_db_findrdataset(db, node, ver, dns_rdatatype_dnskey,
+                                     dns_rdatatype_none, 0, &rdataset, NULL);
+	if (dns_rdataset_isassociated(&rdataset))
+		dns_rdataset_disassociate(&rdataset);
+	if (result != ISC_R_NOTFOUND)
+		goto failure;
+
+	result = dns_nsec3param_deletechains(db, ver, zone, diff);
+
+ failure:
+	if (node != NULL)
+		dns_db_detachnode(db, &node);
+	return (result);
+}
+
 static void
 zone_rekey(dns_zone_t *zone) {
 	isc_result_t result;
@@ -13757,6 +13783,7 @@ zone_rekey(dns_zone_t *zone) {
 		if ((newactive || !ISC_LIST_EMPTY(diff.tuples)) &&
 		    dnskey_sane(zone, db, ver, &diff)) {
 			CHECK(dns_diff_apply(&diff, db, ver));
+			CHECK(clean_nsec3param(zone, db, ver, &diff));
 			CHECK(sign_apex(zone, db, ver, dns_rdatatype_dnskey,
 					&diff));
 			CHECK(add_signing_records(db, zone->privatetype, ver,
@@ -13809,6 +13836,36 @@ zone_rekey(dns_zone_t *zone) {
 					      dns_result_totext(result));
 			}
 		}
+
+		/*
+		 * Cause the zone to add/delete NSEC3 chains for the
+		 * deferred NSEC3PARAM changes.
+		 */
+		for (tuple = ISC_LIST_HEAD(diff.tuples);
+		     tuple != NULL;
+		     tuple = ISC_LIST_NEXT(tuple, link)) {
+			unsigned char buf[DNS_NSEC3PARAM_BUFFERSIZE];
+			dns_rdata_t rdata = DNS_RDATA_INIT;
+			dns_rdata_nsec3param_t nsec3param;
+
+			if (tuple->rdata.type != zone->privatetype ||
+			    tuple->op != DNS_DIFFOP_ADD)
+				continue;
+
+			if (!dns_nsec3param_fromprivate(&tuple->rdata, &rdata,
+							buf, sizeof(buf)))
+				continue;
+			dns_rdata_tostruct(&rdata, &nsec3param, NULL);
+			if (nsec3param.flags == 0)
+				continue;
+
+			result = zone_addnsec3chain(zone, &nsec3param);
+			if (result != ISC_R_SUCCESS) {
+				dns_zone_log(zone, ISC_LOG_ERROR,
+					     "zone_addnsec3chain failed: %s",
+					     dns_result_totext(result));
+			}
+		}
 		UNLOCK_ZONE(zone);
 	}