3118. [bug] When rolling to a new DNSSEC key, a private-type

record could be created and never marked complete.
			[RT #23253]

CHANGES

@@ -1,3 +1,7 @@
+3119.	[bug]		When rolling to a new DNSSEC key, a private-type
+			record could be created and never marked complete.
+			[RT #23253]
+
 3118.	[bug]		nsupdate could dump core on shutdown when using
 			SIG(0) keys. [RT #24604]
 

bin/tests/system/autosign/ns2/keygen.sh

@@ -14,7 +14,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: keygen.sh,v 1.7 2010/06/07 04:45:43 marka Exp $
+# $Id: keygen.sh,v 1.8 2011/05/26 04:25:47 each Exp $
 
 SYSTEMTESTTOP=../..
 . $SYSTEMTESTTOP/conf.sh
@@ -57,5 +57,5 @@ for i in Xbar.+005+30676.key Xbar.+005+30804.key Xbar.+005+30676.private \
 do
 	cp $i `echo $i | sed s/X/K/`
 done
-$KEYGEN -3 -q -r $RANDFILE $zone > /dev/null
+$KEYGEN -q -r $RANDFILE $zone > /dev/null
 $DSFROMKEY Kbar.+005+30804.key > dsset-bar.

bin/tests/system/autosign/tests.sh

@@ -14,7 +14,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: tests.sh,v 1.26 2011/05/02 23:56:59 marka Exp $
+# $Id: tests.sh,v 1.27 2011/05/26 04:25:47 each Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
@@ -26,6 +26,34 @@ n=0
 
 DIGOPTS="+tcp +noadd +nosea +nostat +nocmd +dnssec -p 5300"
 
+# convert private-type records to readable form
+showprivate () {
+    echo "-- $@ --"
+    $DIG $DIGOPTS +nodnssec +short @$2 -t type65534 $1 | cut -f3 -d' ' |
+        while read record; do
+            perl -e 'my $rdata = pack("H*", @ARGV[0]);
+                die "invalid record" unless length($rdata) == 5;
+                my ($alg, $key, $remove, $complete) = unpack("CnCC", $rdata);
+                my $action = "signing";
+                $action = "removing" if $remove;
+                my $state = " (incomplete)";
+                $state = " (complete)" if $complete;
+                print ("$action: alg: $alg, key: $key$state\n");' $record
+        done
+}
+
+# check that signing records are marked as complete
+checkprivate () {
+    ret=0
+    x=`showprivate "$@"`
+    echo $x | grep incomplete >&- 2>&- && ret=1
+    [ $ret = 1 ] && {
+        echo "$x"
+        echo "I:failed"
+    }
+    return $ret
+}
+
 #
 #  The NSEC record at the apex of the zone and its RRSIG records are
 #  added as part of the last step in signing a zone.  We wait for the
@@ -741,13 +769,14 @@ oldfile=`cat active.key`
 oldid=`sed 's/^K.+007+0*//' < active.key`
 newfile=`cat standby.key`
 newid=`sed 's/^K.+007+0*//' < standby.key`
-$SETTIME -K ns1 -I now -D now+15 $oldfile > /dev/null
+$SETTIME -K ns1 -I now+2s -D now+15 $oldfile > /dev/null
 $SETTIME -K ns1 -i 0 -S $oldfile $newfile > /dev/null
 
 # note previous zone serial number
 oldserial=`$DIG $DIGOPTS +short soa . @10.53.0.1 | awk '{print $3}'`
 
 $RNDC -c ../common/rndc.conf -s 10.53.0.1 -p 9953 loadkeys . 2>&1 | sed 's/^/I:ns1 /'
+sleep 4
 
 echo "I:revoking key to duplicated key ID"
 $SETTIME -R now -K ns2 Kbar.+005+30676.key > /dev/null
@@ -774,6 +803,36 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
+echo "I:checking that signing records have been marked as complete ($n)"
+ret=0
+checkprivate . 10.53.0.1 || ret=1
+checkprivate bar 10.53.0.2 || ret=1
+checkprivate example 10.53.0.2 || ret=1
+checkprivate private.secure.example 10.53.0.3 || ret=1
+checkprivate nsec3.example 10.53.0.3 || ret=1
+checkprivate nsec3.nsec3.example 10.53.0.3 || ret=1
+checkprivate nsec3.optout.example 10.53.0.3 || ret=1
+checkprivate nsec3-to-nsec.example 10.53.0.3 || ret=1
+checkprivate nsec.example 10.53.0.3 || ret=1
+checkprivate oldsigs.example 10.53.0.3 || ret=1
+checkprivate optout.example 10.53.0.3 || ret=1
+checkprivate optout.nsec3.example 10.53.0.3 || ret=1
+checkprivate optout.optout.example 10.53.0.3 || ret=1
+checkprivate prepub.example 10.53.0.3 || ret=1
+checkprivate rsasha256.example 10.53.0.3 || ret=1
+checkprivate rsasha512.example 10.53.0.3 || ret=1
+checkprivate secure.example 10.53.0.3 || ret=1
+checkprivate secure.nsec3.example 10.53.0.3 || ret=1
+checkprivate secure.optout.example 10.53.0.3 || ret=1
+checkprivate secure-to-insecure2.example 10.53.0.3 || ret=1
+checkprivate secure-to-insecure.example 10.53.0.3 || ret=1
+checkprivate ttl1.example 10.53.0.3 || ret=1
+checkprivate ttl2.example 10.53.0.3 || ret=1
+checkprivate ttl3.example 10.53.0.3 || ret=1
+checkprivate ttl4.example 10.53.0.3 || ret=1
+n=`expr $n + 1`
+status=`expr $status + $ret`
+
 echo "I:forcing full sign"
 $RNDC -c ../common/rndc.conf -s 10.53.0.1 -p 9953 sign . 2>&1 | sed 's/^/I:ns1 /'
 

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.90 2011/05/23 20:10:02 each Exp $
+# $Id: tests.sh,v 1.91 2011/05/26 04:25:47 each Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
@@ -29,6 +29,34 @@ rm -f dig.out.*
 
 DIGOPTS="+tcp +noadd +nosea +nostat +nocmd +dnssec -p 5300"
 
+# convert private-type records to readable form
+showprivate () {
+    echo "-- $@ --"
+    $DIG $DIGOPTS +nodnssec +short @$2 -t type65534 $1 | cut -f3 -d' ' |
+        while read record; do
+            perl -e 'my $rdata = pack("H*", @ARGV[0]);
+                die "invalid record" unless length($rdata) == 5;
+                my ($alg, $key, $remove, $complete) = unpack("CnCC", $rdata);
+                my $action = "signing";
+                $action = "removing" if $remove;
+                my $state = " (incomplete)";
+                $state = " (complete)" if $complete;
+                print ("$action: alg: $alg, key: $key$state\n");' $record
+        done
+}
+
+# check that signing records are marked as complete
+checkprivate () {
+    ret=0
+    x=`showprivate "$@"`
+    echo $x | grep incomplete >&- 2>&- && ret=1
+    [ $ret = 1 ] && {
+        echo "$x"
+        echo "I:failed"
+    }
+    return $ret
+}
+
 # Check the example. domain
 
 echo "I:checking that zone transfer worked ($n)"
@@ -1213,7 +1241,7 @@ ret=0
 $DIG $DIGOPTS +dnssec a auto-nsec.example. @10.53.0.4 > dig.out.ns4.test$n || ret=1
 grep "NOERROR" dig.out.ns4.test$n > /dev/null || ret=1
 grep "flags:.* ad[ ;]" dig.out.ns4.test$n > /dev/null || ret=1
-grep "IN.NSEC[^3].* TYPE65534" dig.out.ns4.test$n > /dev/null || ret=1
+grep "IN.NSEC[^3].* DNSKEY" dig.out.ns4.test$n > /dev/null || ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
@@ -1223,7 +1251,18 @@ ret=0
 $DIG $DIGOPTS +dnssec a auto-nsec3.example. @10.53.0.4 > dig.out.ns4.test$n || ret=1
 grep "NOERROR" dig.out.ns4.test$n > /dev/null || ret=1
 grep "flags:.* ad[ ;]" dig.out.ns4.test$n > /dev/null || ret=1
-grep "IN.NSEC3 .* TYPE65534" dig.out.ns4.test$n > /dev/null || ret=1
+grep "IN.NSEC3 .* DNSKEY" dig.out.ns4.test$n > /dev/null || ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
+echo "I:checking that signing records have been marked as complete ($n)"
+ret=0
+checkprivate dynamic.example 10.53.0.3 || ret=1
+checkprivate update-nsec3.example 10.53.0.3 || ret=1
+checkprivate auto-nsec3.example 10.53.0.3 || ret=1
+checkprivate expiring.example 10.53.0.3 || ret=1
+checkprivate auto-nsec.example 10.53.0.3 || ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`

lib/dns/zone.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: zone.c,v 1.611 2011/05/23 20:10:02 each Exp $ */
+/* $Id: zone.c,v 1.612 2011/05/26 04:25:47 each Exp $ */
 
 /*! \file */
 
@@ -13831,7 +13831,8 @@ rr_exists(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
  */
 static isc_result_t
 add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
-		    dns_dbversion_t *ver, dns_diff_t *diff)
+		    dns_dbversion_t *ver, dns_diff_t *diff,
+		    isc_boolean_t sign_all)
 {
 	dns_difftuple_t *tuple, *newtuple = NULL;
 	dns_rdata_dnskey_t dnskey;
@@ -13870,13 +13871,16 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
 		rdata.type = privatetype;
 		rdata.rdclass = tuple->rdata.rdclass;
 
-		CHECK(rr_exists(db, ver, name, &rdata, &flag));
+		if (sign_all || tuple->op == DNS_DIFFOP_DEL) {
-		if (flag)
+			CHECK(rr_exists(db, ver, name, &rdata, &flag));
-			continue;
+			if (flag)
-		CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
+				continue;
-					   name, 0, &rdata, &newtuple));
+			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
-		CHECK(do_one_tuple(&newtuple, db, ver, diff));
+						   name, 0, &rdata, &newtuple));
-		INSIST(newtuple == NULL);
+			CHECK(do_one_tuple(&newtuple, db, ver, diff));
+			INSIST(newtuple == NULL);
+		}
+
 		/*
 		 * Remove any record which says this operation has already
 		 * completed.
@@ -14113,6 +14117,7 @@ zone_rekey(dns_zone_t *zone) {
 	dns_dnsseckey_t *key;
 	dns_diff_t diff, sig_diff;
 	isc_boolean_t commit = ISC_FALSE, newactive = ISC_FALSE;
+	isc_boolean_t newalg = ISC_FALSE;
 	isc_boolean_t fullsign;
 	dns_ttl_t ttl = 3600;
 	const char *dir;
@@ -14190,52 +14195,11 @@ zone_rekey(dns_zone_t *zone) {
 			goto trylater;
 		}
 
-		/* See if any pre-existing keys have newly become active */
+		/* See if any pre-existing keys have newly become active;
-		for (key = ISC_LIST_HEAD(dnskeys);
+		 * also, see if any new key is for a new algorithm, as in that
-		     key != NULL;
+		 * event, we need to sign the zone fully.  (If there's a new
-		     key = ISC_LIST_NEXT(key, link)) {
+		 * key, but it's for an already-existing algorithm, then
-			if (key->first_sign) {
+		 * the zone signing can be handled incrementally.)
-				newactive = ISC_TRUE;
-				break;
-			}
-		}
-
-		if ((newactive || fullsign || !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(add_signing_records(db, zone->privatetype, ver,
-						  &diff));
-			CHECK(increment_soa_serial(db, ver, &diff, mctx));
-			CHECK(add_chains(zone, db, ver, &diff));
-			CHECK(sign_apex(zone, db, ver, &diff, &sig_diff));
-			CHECK(zone_journal(zone, &sig_diff, "zone_rekey"));
-			commit = ISC_TRUE;
-		}
-	}
-
-	dns_db_closeversion(db, &ver, commit);
-
-	if (commit) {
-		isc_time_t timenow;
-		dns_difftuple_t *tuple;
-		isc_boolean_t newkey = ISC_FALSE;
-		isc_boolean_t newalg = ISC_FALSE;
-
-		LOCK_ZONE(zone);
-		DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDNOTIFY);
-
-		zone_needdump(zone, DNS_DUMP_DELAY);
-
-		TIME_NOW(&timenow);
-		zone_settimer(zone, &timenow);
-
-		/*
-		 * Has a new key become active?  If so, is it for
-		 * a new algorithm?  In that event, we need to sign the
-		 * zone fully.  If there's a new key, but it's for an
-		 * already-existing algorithm, then the zone signing
-		 * can be handled incrementally.
 		 */
 		for (key = ISC_LIST_HEAD(dnskeys);
 		     key != NULL;
@@ -14243,7 +14207,8 @@ zone_rekey(dns_zone_t *zone) {
 			if (!key->first_sign)
 				continue;
 
-			newkey = ISC_TRUE;
+			newactive = ISC_TRUE;
+
 			if (!dns_rdataset_isassociated(&keysigs)) {
 				newalg = ISC_TRUE;
 				break;
@@ -14262,6 +14227,35 @@ zone_rekey(dns_zone_t *zone) {
 			}
 		}
 
+		if ((newactive || fullsign || !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(add_signing_records(db, zone->privatetype,
+						  ver, &diff,
+						  ISC_TF(newalg || fullsign)));
+			CHECK(increment_soa_serial(db, ver, &diff, mctx));
+			CHECK(add_chains(zone, db, ver, &diff));
+			CHECK(sign_apex(zone, db, ver, &diff, &sig_diff));
+			CHECK(zone_journal(zone, &sig_diff, "zone_rekey"));
+			commit = ISC_TRUE;
+		}
+	}
+
+	dns_db_closeversion(db, &ver, commit);
+
+	if (commit) {
+		isc_time_t timenow;
+		dns_difftuple_t *tuple;
+
+		LOCK_ZONE(zone);
+		DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDNOTIFY);
+
+		zone_needdump(zone, DNS_DUMP_DELAY);
+
+		TIME_NOW(&timenow);
+		zone_settimer(zone, &timenow);
+
 		/* Remove any signatures from removed keys.  */
 		if (!ISC_LIST_EMPTY(rmkeys)) {
 			for (key = ISC_LIST_HEAD(rmkeys);
@@ -14279,7 +14273,6 @@ zone_rekey(dns_zone_t *zone) {
 			}
 		}
 
-
 		if (fullsign) {
 			/*
 			 * "rndc sign" was called, so we now sign the zone