From 0245f7725c40fd29637fbc83ee25bd84be25bfd2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 26 May 2011 04:25:47 +0000 Subject: [PATCH] 3118. [bug] When rolling to a new DNSSEC key, a private-type record could be created and never marked complete. [RT #23253] --- CHANGES | 4 + bin/tests/system/autosign/ns2/keygen.sh | 4 +- bin/tests/system/autosign/tests.sh | 63 +++++++++++++- bin/tests/system/dnssec/tests.sh | 45 +++++++++- lib/dns/zone.c | 107 +++++++++++------------- 5 files changed, 159 insertions(+), 64 deletions(-) diff --git a/CHANGES b/CHANGES index 05c2008f10..1514be720f 100644 --- a/CHANGES +++ b/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] diff --git a/bin/tests/system/autosign/ns2/keygen.sh b/bin/tests/system/autosign/ns2/keygen.sh index f41e8ab9af..d3bf3bee00 100644 --- a/bin/tests/system/autosign/ns2/keygen.sh +++ b/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. diff --git a/bin/tests/system/autosign/tests.sh b/bin/tests/system/autosign/tests.sh index 4df8121cf4..3ad8543720 100644 --- a/bin/tests/system/autosign/tests.sh +++ b/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 /' diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index e19bcb6b0a..f64f99825c 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/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` diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 38e3ce6ff0..88c0ebfa84 100644 --- a/lib/dns/zone.c +++ b/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 (flag) - continue; - CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD, - name, 0, &rdata, &newtuple)); - CHECK(do_one_tuple(&newtuple, db, ver, diff)); - INSIST(newtuple == NULL); + if (sign_all || tuple->op == DNS_DIFFOP_DEL) { + CHECK(rr_exists(db, ver, name, &rdata, &flag)); + if (flag) + continue; + CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD, + name, 0, &rdata, &newtuple)); + 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 */ - for (key = ISC_LIST_HEAD(dnskeys); - key != NULL; - key = ISC_LIST_NEXT(key, link)) { - if (key->first_sign) { - 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. + /* See if any pre-existing keys have newly become active; + * also, see if any new key is for a new algorithm, as 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