From cff610acb61b5dba89bef88e4d2f72e1b74d5881 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 Mar 2019 09:32:20 +0100 Subject: [PATCH 1/4] Add test for ZSK rollover while KSK offline This commit adds a lengthy test where the ZSK is rolled but the KSK is offline (except for when the DNSKEY RRset is changed). The specific scenario has the `dnskey-kskonly` configuration option set meaning the DNSKEY RRset should only be signed with the KSK. A new zone `updatecheck-kskonly.secure` is added to test against, that can be dynamically updated, and that can be controlled with rndc to load the DNSSEC keys. There are some pre-checks for this test to make sure everything is fine before the ZSK roll, after the new ZSK is published, and after the old ZSK is deleted. Note there are actually two ZSK rolls in quick succession. When the latest added ZSK becomes active and its predecessor becomes inactive, the KSK is offline. However, the DNSKEY RRset did not change and it has a good signature that is valid for long enough. The expected behavior is that the DNSKEY RRset stays signed with the KSK only (signature does not need to change). However, the test will fail because after reconfiguring the keys for the zone, it wants to add re-sign tasks for the new active keys (in sign_apex). Because the KSK is offline, named determines that the only other active key, the latest ZSK, will be used to resign the DNSKEY RRset, in addition to keeping the RRSIG of the KSK. The question is: Why do we need to resign the DNSKEY RRset immediately when a new key becomes active? This is not required, only once the next resign task is triggered the new active key should replace signatures that are in need of refreshing. --- bin/tests/system/dnssec/clean.sh | 3 +- bin/tests/system/dnssec/ns2/named.conf.in | 21 ++ bin/tests/system/dnssec/ns2/sign.sh | 17 ++ .../system/dnssec/ns2/template.secure.db.in | 12 + bin/tests/system/dnssec/tests.sh | 217 +++++++++++++++++- 5 files changed, 267 insertions(+), 3 deletions(-) create mode 100644 bin/tests/system/dnssec/ns2/template.secure.db.in diff --git a/bin/tests/system/dnssec/clean.sh b/bin/tests/system/dnssec/clean.sh index f67c61d52fa..a6de2faf4a6 100644 --- a/bin/tests/system/dnssec/clean.sh +++ b/bin/tests/system/dnssec/clean.sh @@ -15,7 +15,7 @@ rm -f ./*/K* ./*/keyset-* ./*/dsset-* ./*/dlvset-* ./*/signedkey-* ./*/*.signed rm -f ./*/example.bk rm -f ./*/named.conf rm -f ./*/named.memstats -rm -f ./*/named.run +rm -f ./*/named.run ./*/named.run.prev rm -f ./*/named.secroots rm -f ./*/tmp* ./*/*.jnl ./*/*.bk ./*/*.jbk rm -f ./*/trusted.conf ./*/managed.conf ./*/revoked.conf @@ -48,6 +48,7 @@ rm -f ./ns2/in-addr.arpa.db rm -f ./ns2/nsec3chain-test.db rm -f ./ns2/private.secure.example.db rm -f ./ns2/single-nsec3.db +rm -f ./ns2/updatecheck-kskonly.secure.* rm -f ./ns3/auto-nsec.example.db ./ns3/auto-nsec3.example.db rm -f ./ns3/badds.example.db rm -f ./ns3/dname-at-apex-nsec3.example.db diff --git a/bin/tests/system/dnssec/ns2/named.conf.in b/bin/tests/system/dnssec/ns2/named.conf.in index 4f056328181..b98c06c1305 100644 --- a/bin/tests/system/dnssec/ns2/named.conf.in +++ b/bin/tests/system/dnssec/ns2/named.conf.in @@ -27,6 +27,15 @@ options { minimal-responses no; }; +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.2 port 5309 allow { any; } keys { rndc_key; }; +}; + zone "." { type hint; file "../../common/root.hint"; @@ -158,6 +167,18 @@ zone "cdnskey-auto.secure" { allow-update { any; }; }; +zone "updatecheck-kskonly.secure" { + type master; + auto-dnssec maintain; + key-directory "."; + dnssec-dnskey-kskonly yes; + update-check-ksk yes; + sig-validity-interval 10; + dnskey-sig-validity 40; + file "updatecheck-kskonly.secure.db.signed"; + allow-update { any; }; +}; + zone "corp" { type master; file "corp.db"; diff --git a/bin/tests/system/dnssec/ns2/sign.sh b/bin/tests/system/dnssec/ns2/sign.sh index 8b9b20af0ab..65ff46b5365 100644 --- a/bin/tests/system/dnssec/ns2/sign.sh +++ b/bin/tests/system/dnssec/ns2/sign.sh @@ -279,3 +279,20 @@ key1=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone -f KSK "$ key2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone") sed 's/DNSKEY/CDNSKEY/' "$key1.key" > "$key1.cds" cat "$infile" "$key1.cds" > "$zonefile.signed" + +zone=updatecheck-kskonly.secure +infile=template.secure.db.in +zonefile=${zone}.db +key1=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone -f KSK "$zone") +key2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone") +# Save key id's for checking active key usage +echo "$key1" | sed -e 's/.*[+]//' -e 's/^0*//' > $zone.ksk.id +echo "$key2" | sed -e 's/.*[+]//' -e 's/^0*//' > $zone.zsk.id +echo "${key1}" > $zone.ksk.key +echo "${key2}" > $zone.zsk.key +# Add CDS and CDNSKEY records +sed 's/DNSKEY/CDNSKEY/' "$key1.key" > "$key1.cdnskey" +"$DSFROMKEY" -C "$key1.key" > "$key1.cds" +cat "$infile" "$key1.key" "$key2.key" "$key1.cdnskey" "$key1.cds" > "$zonefile" +# Don't sign, let auto-dnssec maintain do it. +mv $zonefile "$zonefile.signed" diff --git a/bin/tests/system/dnssec/ns2/template.secure.db.in b/bin/tests/system/dnssec/ns2/template.secure.db.in new file mode 100644 index 00000000000..e42cb4a29eb --- /dev/null +++ b/bin/tests/system/dnssec/ns2/template.secure.db.in @@ -0,0 +1,12 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, You can obtain one at http://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +$TTL 3600 +@ SOA ns2.example. . 1 3600 1200 86400 1200 +@ NS ns2.example. diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 81865b626e3..5efc9af075a 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -40,6 +40,26 @@ rndccmd() { "$RNDC" -c "$SYSTEMTESTTOP/common/rndc.conf" -p "$CONTROLPORT" -s "$@" } +# TODO: Move wait_for_log and loadkeys_on to conf.sh.common +wait_for_log() { + msg=$1 + file=$2 + for i in 1 2 3 4 5 6 7 8 9 10; do + nextpart "$file" | grep "$msg" > /dev/null && return + sleep 1 + done + echo_i "exceeded time limit waiting for '$msg' in $file" + ret=1 +} + +dnssec_loadkeys_on() { + nsidx=$1 + zone=$2 + nextpart ns${nsidx}/named.run > /dev/null + rndccmd 10.53.0.${nsidx} loadkeys ${zone} | sed "s/^/ns${nsidx} /" | cat_i + wait_for_log "next key event" ns${nsidx}/named.run +} + # convert private-type records to readable form showprivate () { echo "-- $* --" @@ -2599,7 +2619,7 @@ my_dig() { "$DIG" +noadd +nosea +nostat +noquest +nocomm +nocmd -p "$PORT" @10.53.0.4 "$@" } -echo_i "checking dnskey query with no data still gets put in cache ($n)" +echo_i "checking DNSKEY query with no data still gets put in cache ($n)" ret=0 firstVal=$(my_dig insecure.example. dnskey| awk '$1 != ";;" { print $2 }') sleep 1 @@ -2655,7 +2675,7 @@ do fi echo_i "sleeping ...." sleep 3 -done; +done grep "ANSWER: 3," dig.out.ns2.test$n > /dev/null || ret=1 if [ "$ret" -ne 0 ]; then echo_i "nsec3 chain generation not complete"; fi dig_with_opts +noauth +nodnssec soa nsec3chain-test @10.53.0.2 > dig.out.ns2.test$n || ret=1 @@ -3648,6 +3668,199 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +### +### Additional checks for when the KSK is offline. +### + +# Save some useful information +zone="updatecheck-kskonly.secure" +KSK=`cat ns2/${zone}.ksk.key` +ZSK=`cat ns2/${zone}.zsk.key` +KSK_ID=`cat ns2/${zone}.ksk.id` +ZSK_ID=`cat ns2/${zone}.zsk.id` +SECTIONS="+answer +noauthority +noadditional" +echo_i "testing zone $zone KSK=$KSK_ID ZSK=$ZSK_ID" + +# Basic checks to make sure everything is fine before the KSK is made offline. +for qtype in "DNSKEY" "CDNSKEY" "CDS" +do + echo_i "checking $qtype RRset is signed with KSK only (update-check-ksk, dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + test "$lines" -eq 1 || ret=1 + grep $KSK_ID dig.out.test$n > /dev/null || ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + +echo_i "checking SOA RRset is signed with ZSK only (update-check-ksk and dnssec-ksk-only) ($n)" +ret=0 +dig_with_opts $SECTIONS @10.53.0.2 soa $zone > dig.out.test$n +lines=$(awk '$4 == "RRSIG" && $5 == "SOA" {print}' dig.out.test$n | wc -l) +grep $KSK_ID dig.out.test$n > /dev/null && ret=1 +grep $ZSK_ID dig.out.test$n > /dev/null || ret=1 +test "$lines" -eq 1 || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +# Roll the ZSK. +sleep 1 +zsk2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone") +echo_i "new ZSK $zsk2 created for zone $zone" +echo "$zsk2" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id2 +ZSK_ID2=`cat ns2/$zone.zsk.id2` +dnssec_loadkeys_on 2 $zone + +# Wait until new ZSK becomes active. +sleep 1 +echo_i "make ZSK $ZSK inactive and make new ZSK $zsk2 active for zone $zone" +$SETTIME -I now -K ns2 $ZSK > /dev/null +$SETTIME -A now -K ns2 $zsk2 > /dev/null +dnssec_loadkeys_on 2 $zone + +# Remove the KSK from disk. +sleep 1 +echo_i "remove the KSK $KSK for zone $zone from disk" +mv ns2/$KSK.key ns2/$KSK.key.bak +mv ns2/$KSK.private ns2/$KSK.private.bak + +# Update the zone that requires a resign of the SOA RRset. +sleep 1 +echo_i "update the zone with $zone IN TXT nsupdate added me" +( +echo zone $zone +echo server 10.53.0.2 "$PORT" +echo update add $zone. 300 in txt "nsupdate added me" +echo send +) | $NSUPDATE + +# Redo the tests now that the zone is updated and the KSK is offline. +for qtype in "DNSKEY" "CDNSKEY" "CDS" +do + echo_i "checking $qtype RRset is signed with KSK only, KSK offline (update-check-ksk, dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + test "$lines" -eq 1 || ret=1 + grep $KSK_ID dig.out.test$n > /dev/null || ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + +for qtype in "SOA" "TXT" +do + echo_i "checking $qtype RRset is signed with ZSK only, KSK offline (update-check-ksk and dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + grep $KSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID2 dig.out.test$n > /dev/null || ret=1 + test "$lines" -eq 1 || ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + +# Put back the KSK. +sleep 1 +echo_i "put back the KSK $KSK for zone $zone from disk" +mv ns2/$KSK.key.bak ns2/$KSK.key +mv ns2/$KSK.private.bak ns2/$KSK.private + +# Roll the ZSK again. +sleep 1 +zsk3=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone") +echo_i "new ZSK $zsk3 created for zone $zone" +echo "$zsk3" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id3 +ZSK_ID3=`cat ns2/$zone.zsk.id3` +dnssec_loadkeys_on 2 $zone + +# Wait until new ZSK becomes active. +sleep 1 +echo_i "delete old ZSK $ZSK make ZSK $ZSK2 inactive and make new ZSK $zsk3 active for zone $zone" +$SETTIME -D now -K ns2 $ZSK > /dev/null +$SETTIME -I +5 -K ns2 $zsk2 > /dev/null +$SETTIME -A +5 -K ns2 $zsk3 > /dev/null +dnssec_loadkeys_on 2 $zone + +# Remove the KSK from disk. +sleep 1 +echo_i "remove the KSK $KSK for zone $zone from disk" +mv ns2/$KSK.key ns2/$KSK.key.bak +mv ns2/$KSK.private ns2/$KSK.private.bak + +# Update the zone that requires a resign of the SOA RRset. +sleep 1 +echo_i "update the zone with $zone IN TXT nsupdate added me again" +( +echo zone $zone +echo server 10.53.0.2 "$PORT" +echo update add $zone. 300 in txt "nsupdate added me again" +echo send +) | $NSUPDATE + +# Redo the tests now that the ZSK roll has deleted the old key. +for qtype in "DNSKEY" "CDNSKEY" "CDS" +do + echo_i "checking $qtype RRset is signed with KSK only, old ZSK deleted (update-check-ksk, dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + test "$lines" -eq 1 || ret=1 + grep $KSK_ID dig.out.test$n > /dev/null || ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + +for qtype in "SOA" "TXT" +do + echo_i "checking $qtype RRset is signed with ZSK only, old ZSK deleted (update-check-ksk and dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + grep $KSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID2 dig.out.test$n > /dev/null || ret=1 + grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1 + test "$lines" -eq 1 || ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + +# Wait for newest ZSK to become active. +echo_i "sleep 6 to make new ZSK $zsk3 active and ZSK $zsk2 inactive" +sleep 6 + +# Redo the tests one more time. +for qtype in "DNSKEY" "CDNSKEY" "CDS" +do + echo_i "checking $qtype RRset is signed with KSK only, new ZSK active (update-check-ksk, dnssec-ksk-only) ($n)" + ret=0 + dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n + lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l) + test "$lines" -eq 1 || ret=1 + grep $KSK_ID dig.out.test$n > /dev/null || ret=1 + grep $ZSK_ID dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1 + grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1 + n=$((n+1)) + test "$ret" -eq 0 || echo_i "failed" + status=$((status+ret)) +done + # Note: after this check, ns4 will not be validating any more; do not add any # further validation tests employing ns4 below this check. echo_i "check that validation defaults to off when dnssec-enable is off ($n)" -- GitLab From 9a66fc03ed98c6ed8eaaa2ba271e6f0aab80ba9d Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 Mar 2019 09:43:14 +0100 Subject: [PATCH 2/4] Add detail on echo message in autosign test --- bin/tests/system/autosign/tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tests/system/autosign/tests.sh b/bin/tests/system/autosign/tests.sh index 69798cb5d36..9f60714d57b 100755 --- a/bin/tests/system/autosign/tests.sh +++ b/bin/tests/system/autosign/tests.sh @@ -993,7 +993,7 @@ $RNDCCMD 10.53.0.2 loadkeys bar. 2>&1 | sed 's/^/ns2 /' | cat_i echo_i "waiting for changes to take effect" sleep 5 -echo_i "checking former standby key is now active ($n)" +echo_i "checking former standby key $newid is now active ($n)" ret=0 $DIG $DIGOPTS dnskey . @10.53.0.1 > dig.out.ns1.test$n || ret=1 grep 'RRSIG.*'" $newid "'\. ' dig.out.ns1.test$n > /dev/null || ret=1 -- GitLab From e7a4b21ac27c0a07bdf14aebc1f64c0fb47544c6 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 Mar 2019 09:44:01 +0100 Subject: [PATCH 3/4] Style: some curly brackets --- lib/dns/zone.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 735928e379d..fa82848d9d2 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -6463,10 +6463,11 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, * If there is not a matching DNSKEY then * delete the RRSIG. */ - if (!found) + if (!found) { result = update_one_rr(db, ver, zonediff->diff, DNS_DIFFOP_DELRESIGN, name, rdataset.ttl, &rdata); + } if (result != ISC_R_SUCCESS) break; } @@ -6545,6 +6546,7 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, have_ksk = false; have_nonksk = true; } + for (j = 0; j < nkeys; j++) { if (j == i || ALG(keys[i]) != ALG(keys[j])) continue; @@ -6559,10 +6561,12 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, else have_nonksk = true; both = have_ksk && have_nonksk; - if (both) + if (both) { break; + } } } + if (both) { /* * CDS and CDNSKEY are signed with KSK (RFC 7344, 4.1). @@ -10487,14 +10491,17 @@ zone_maintenance(dns_zone_t *zone) { if (zone->rss_event != NULL) break; if (!isc_time_isepoch(&zone->signingtime) && - isc_time_compare(&now, &zone->signingtime) >= 0) + isc_time_compare(&now, &zone->signingtime) >= 0) { zone_sign(zone); + } else if (!isc_time_isepoch(&zone->resigntime) && - isc_time_compare(&now, &zone->resigntime) >= 0) + isc_time_compare(&now, &zone->resigntime) >= 0) { zone_resigninc(zone); + } else if (!isc_time_isepoch(&zone->nsec3chaintime) && - isc_time_compare(&now, &zone->nsec3chaintime) >= 0) + isc_time_compare(&now, &zone->nsec3chaintime) >= 0) { zone_nsec3chain(zone); + } /* * Do we need to issue a key expiry warning? */ @@ -18018,15 +18025,18 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; tuple = ISC_LIST_NEXT(tuple, link)) { - if (tuple->rdata.type != dns_rdatatype_dnskey) + if (tuple->rdata.type != dns_rdatatype_dnskey) { continue; + } result = dns_rdata_tostruct(&tuple->rdata, &dnskey, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); if ((dnskey.flags & (DNS_KEYFLAG_OWNERMASK|DNS_KEYTYPE_NOAUTH)) != DNS_KEYOWNER_ZONE) + { continue; + } dns_rdata_toregion(&tuple->rdata, &r); @@ -18044,8 +18054,10 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, if (sign_all || tuple->op == DNS_DIFFOP_DEL) { CHECK(rr_exists(db, ver, name, &rdata, &flag)); - if (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)); @@ -18371,7 +18383,6 @@ zone_rekey(dns_zone_t *zone) { goto failure; } - /* Get the CDS rdataset */ result = dns_db_findrdataset(db, node, ver, dns_rdatatype_cds, dns_rdatatype_none, 0, &cdsset, NULL); @@ -18397,7 +18408,6 @@ zone_rekey(dns_zone_t *zone) { if (result == ISC_R_SUCCESS) { bool check_ksk; check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK); - result = dns_dnssec_updatekeys(&dnskeys, &keys, &rmkeys, &zone->origin, ttl, &diff, !check_ksk, mctx, -- GitLab From e8ed98c0c2107d9bf6561e4b0ed487d9f5b108df Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 Mar 2019 09:53:46 +0100 Subject: [PATCH 4/4] Propose a fix for the kskonly bug This commit has two ideas for fixing the case where the DNSKEY RRset is signed with the ZSK when the KSK is offline (but the DNSKEY does not need to be resigned). 1. In sign_apex, don't enforce updating the DNSKEY signatures and cause them to sign so that the newly activated keys are used. This works, but breaks the autosign test because that has a test that verifies a former standby key is now active by checking if it has signed the DNSKEY RRset. If we don't require resigning the DNSKEY RRset immediately after activating a new key, this test needs to adapt and check it another way. Also it is not clear if sign_apex assumes that the apex is being resigned regardless of existing signatures. Only if that was documented somewhere. 2. In zone_rekey, when we detect a new active key, don't use it to determine whether we need to update apply diff, clean NSEC3PARAM, add signing records, update SOA SERIAL, add NSEC(3) chains, sign the apex and write a journal. It makes sense when doing a full sign or when we have a diff, but a change in activation of keys should not require all those steps to be done. Also this breaks the autosign test the same way as idea 1. --- lib/dns/zone.c | 44 ++------------------------------------------ 1 file changed, 2 insertions(+), 42 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index fa82848d9d2..a3e070fdbcb 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -18090,7 +18090,6 @@ sign_apex(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, bool check_ksk, keyset_kskonly; dst_key_t *zone_keys[DNS_MAXZONEKEYS]; unsigned int nkeys = 0, i; - dns_difftuple_t *tuple; result = dns__zone_findkeys(zone, db, ver, now, zone->mctx, DNS_MAXZONEKEYS, zone_keys, &nkeys); @@ -18114,43 +18113,6 @@ sign_apex(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK); keyset_kskonly = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_DNSKEYKSKONLY); - /* - * See if dns__zone_updatesigs() will update DNSKEY signature and if - * not cause them to sign so that newly activated keys are used. - */ - for (tuple = ISC_LIST_HEAD(diff->tuples); - tuple != NULL; - tuple = ISC_LIST_NEXT(tuple, link)) - { - if (tuple->rdata.type == dns_rdatatype_dnskey && - dns_name_equal(&tuple->name, &zone->origin)) - { - break; - } - } - - if (tuple == NULL) { - result = del_sigs(zone, db, ver, &zone->origin, - dns_rdatatype_dnskey, zonediff, - zone_keys, nkeys, now, false); - if (result != ISC_R_SUCCESS) { - dnssec_log(zone, ISC_LOG_ERROR, - "sign_apex:del_sigs -> %s", - dns_result_totext(result)); - goto failure; - } - result = add_sigs(db, ver, &zone->origin, dns_rdatatype_dnskey, - zonediff->diff, zone_keys, nkeys, zone->mctx, - inception, keyexpire, check_ksk, - keyset_kskonly); - if (result != ISC_R_SUCCESS) { - dnssec_log(zone, ISC_LOG_ERROR, - "sign_apex:add_sigs -> %s", - dns_result_totext(result)); - goto failure; - } - } - result = dns__zone_updatesigs(diff, db, ver, zone_keys, nkeys, zone, inception, soaexpire, keyexpire, now, check_ksk, keyset_kskonly, zonediff); @@ -18326,7 +18288,7 @@ zone_rekey(dns_zone_t *zone) { dns_dnsseckey_t *key = NULL; dns_diff_t diff, _sig_diff; dns__zonediff_t zonediff; - bool commit = false, newactive = false; + bool commit = false; bool newalg = false; bool fullsign; dns_ttl_t ttl = 3600; @@ -18451,8 +18413,6 @@ zone_rekey(dns_zone_t *zone) { continue; } - newactive = true; - if (!dns_rdataset_isassociated(&keysigs)) { newalg = true; break; @@ -18471,7 +18431,7 @@ zone_rekey(dns_zone_t *zone) { } } - if ((newactive || fullsign || !ISC_LIST_EMPTY(diff.tuples)) && + if ((newalg || fullsign || !ISC_LIST_EMPTY(diff.tuples)) && dnskey_sane(zone, db, ver, &diff)) { CHECK(dns_diff_apply(&diff, db, ver)); -- GitLab