diff --git a/CHANGES b/CHANGES index 9707cda419..85e4fd90b9 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5690. [func] Change "dnssec-signzone" to honor the Predecessor and + Successor metadata values, and allow for gradual + replacement of RRSIGs. In other words, don't sign + with the successor key if there is an RRSIG from the + predecessor key that does not need to be refreshed. + [GL #1551] + 5689. [placeholder] 5688. [bug] Inline and dnssec-policy zones could fail to apply diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 6cea5138d2..b42d30ac22 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -555,11 +555,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, key = keythatsigned(&rrsig); sig_format(&rrsig, sigstr, sizeof(sigstr)); - if (key != NULL && issigningkey(key)) { - expired = isc_serial_gt(now + cycle, rrsig.timeexpire); - } else { - expired = isc_serial_gt(now, rrsig.timeexpire); - } + expired = isc_serial_gt(now + cycle, rrsig.timeexpire); if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) { /* rrsig is dropped and not replaced */ @@ -649,7 +645,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, } } else { tuple = NULL; - vbprintf(2, "removing signature by %s\n", sigstr); + vbprintf(2, "\tremoving signature by %s\n", sigstr); result = dns_difftuple_create( mctx, DNS_DIFFOP_DELRESIGN, name, sigset.ttl, &sigrdata, &tuple); @@ -695,20 +691,20 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, dns_name_equal(name, gorigin)) { bool have_ksk; - dns_dnsseckey_t *tmpkey; + dns_dnsseckey_t *curr; have_ksk = isksk(key); - for (tmpkey = ISC_LIST_HEAD(keylist); tmpkey != NULL; - tmpkey = ISC_LIST_NEXT(tmpkey, link)) + for (curr = ISC_LIST_HEAD(keylist); curr != NULL; + curr = ISC_LIST_NEXT(curr, link)) { if (dst_key_alg(key->key) != - dst_key_alg(tmpkey->key)) { + dst_key_alg(curr->key)) { continue; } - if (REVOKE(tmpkey->key)) { + if (REVOKE(curr->key)) { continue; } - if (isksk(tmpkey)) { + if (isksk(curr)) { have_ksk = true; } } @@ -718,8 +714,65 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, "signing with dnskey"); } } else if (iszsk(key)) { - signwithkey(name, set, key->key, ttl, add, - "signing with dnskey"); + /* + * Sign with the ZSK unless there is a predecessor + * key that already signs this RRset. + */ + bool have_pre_sig = false; + dns_dnsseckey_t *curr; + uint32_t pre; + isc_result_t ret = dst_key_getnum( + key->key, DST_NUM_PREDECESSOR, &pre); + if (ret == ISC_R_SUCCESS) { + /* + * This key has a predecessor, look for the + * corresponding key in the keylist. The + * key we are looking for must be: + * - From the same cryptographic algorithm. + * - Have the ZSK type (iszsk). + * - Have key ID equal to the predecessor id. + * - Have a successor that matches 'key' id. + */ + for (curr = ISC_LIST_HEAD(keylist); + curr != NULL; + curr = ISC_LIST_NEXT(curr, link)) + { + uint32_t suc; + + if (dst_key_alg(key->key) != + dst_key_alg(curr->key) || + !iszsk(curr) || + dst_key_id(curr->key) != pre) + { + continue; + } + ret = dst_key_getnum(curr->key, + DST_NUM_SUCCESSOR, + &suc); + if (ret != ISC_R_SUCCESS || + dst_key_id(key->key) != suc) { + continue; + } + + /* + * curr is the predecessor we were + * looking for. Check if this key + * signs this RRset. + */ + if (nowsignedby[curr->index]) { + have_pre_sig = true; + } + } + } + + /* + * If we have a signature of a predecessor key, + * skip signing with this key. + */ + if (!have_pre_sig) { + signwithkey(name, set, key->key, ttl, add, + "signing with dnskey"); + } } } @@ -3012,7 +3065,7 @@ writeset(const char *prefix, dns_rdatatype_t type) { isc_buffer_t namebuf; isc_region_t r; isc_result_t result; - dns_dnsseckey_t *key, *tmpkey; + dns_dnsseckey_t *key, *curr; unsigned char dsbuf[DNS_DS_BUFFERSIZE]; unsigned char keybuf[DST_KEY_MAXSIZE]; unsigned int filenamelen; @@ -3053,16 +3106,16 @@ writeset(const char *prefix, dns_rdatatype_t type) { have_ksk = false; have_non_ksk = true; } - for (tmpkey = ISC_LIST_HEAD(keylist); tmpkey != NULL; - tmpkey = ISC_LIST_NEXT(tmpkey, link)) + for (curr = ISC_LIST_HEAD(keylist); curr != NULL; + curr = ISC_LIST_NEXT(curr, link)) { - if (dst_key_alg(key->key) != dst_key_alg(tmpkey->key)) { + if (dst_key_alg(key->key) != dst_key_alg(curr->key)) { continue; } - if (REVOKE(tmpkey->key)) { + if (REVOKE(curr->key)) { continue; } - if (isksk(tmpkey)) { + if (isksk(curr)) { have_ksk = true; } else { have_non_ksk = true; diff --git a/bin/tests/system/dnssec/signer/prepub.db.in b/bin/tests/system/dnssec/signer/prepub.db.in new file mode 100644 index 0000000000..ff6c4b7af0 --- /dev/null +++ b/bin/tests/system/dnssec/signer/prepub.db.in @@ -0,0 +1,15 @@ +; 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 60 +prepub. 60 IN SOA prepub. . 0 0 0 0 0 +prepub. 60 IN NS prepub. +prepub. 60 IN A 1.2.3.4 +; out of zone record +out-of-zone. 60 IN A 1.2.3.4 diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index bada872181..e41fe685ca 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1417,6 +1417,93 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +get_rsasha1_key_ids_from_sigs() { + zone=$1 + + tr -d '\r' < signer/$zone.db.signed | \ + awk ' + NF < 8 { next } + $(NF-5) != "RRSIG" { next } + $(NF-3) != "5" { next } + $NF != "(" { next } + { + getline; + print $3; + } + ' | \ + sort -u +} + +# Test dnssec-signzone ZSK prepublish smooth rollover. +echo_i "check dnssec-signzone doesn't sign with prepublished zsk ($n)" +ret=0 +zone=prepub +# Generate keys. +ksk=$("$KEYGEN" -K signer -f KSK -q -a RSASHA1 -b 1024 -n zone "$zone") +zsk1=$("$KEYGEN" -K signer -q -a RSASHA1 -b 1024 -n zone "$zone") +zsk2=$("$KEYGEN" -K signer -q -a RSASHA1 -b 1024 -n zone "$zone") +zskid1=$(keyfile_to_key_id "$zsk1") +zskid2=$(keyfile_to_key_id "$zsk2") +( +cd signer || exit 1 +# Set times such that the current set of keys are introduced 60 days ago and +# start signing now. The successor key is prepublished now and will be active +# next day. +$SETTIME -P now-60d -A now $ksk > /dev/null +$SETTIME -P now-60d -A now -I now+1d -D now+60d $zsk1 > /dev/null +$SETTIME -S $zsk1 -i 1h $zsk2.key > /dev/null +$SETTIME -P now -A now+1d $zsk2.key > /dev/null +# Sign the zone with initial keys and prepublish successor. The zone signatures +# are valid for 30 days and the DNSKEY signature is valid for 60 days. +cp -f $zone.db.in $zone.db +$SIGNER -SDx -e +2592000 -X +5184000 -o $zone $zone.db > /dev/null +echo "\$INCLUDE \"$zone.db.signed\"" >> $zone.db +) +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null && ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed: missing signatures from key $zskid1" +status=$((status+ret)) + +echo_i "check dnssec-signzone retains signatures of predecessor zsk ($n)" +ret=0 +zone=prepub +( +cd signer || exit 1 +# Roll the ZSK. The predecessor is inactive from now on and the successor is +# activated. The zone signatures are valid for 30 days and the DNSKEY +# signature is valid for 60 days. Because of the predecessor/successor +# relationship, the signatures of the predecessor are retained and no new +# signatures with the successor should be generated. +$SETTIME -A now-30d -I now -D now+30d $zsk1 > /dev/null +$SETTIME -A now $zsk2 > /dev/null +$SIGNER -SDx -e +2592000 -X +5184000 -o $zone $zone.db > /dev/null +) +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null && ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +echo_i "check dnssec-signzone swaps zone signatures after interval ($n)" +ret=0 +zone=prepub +( +cd signer || exit 1 +# After some time the signatures should be replaced. When signing, set the +# interval to 30 days plus one second, meaning all predecessor signatures +# are within the refresh interval and should be replaced with successor +# signatures. +$SETTIME -A now-50d -I now-20d -D now+10d $zsk1 > /dev/null +$SETTIME -A now-20d $zsk2 > /dev/null +$SIGNER -SDx -e +2592000 -X +5184000 -i 2592001 -o $zone $zone.db > /dev/null +) +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null && ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + echo_i "checking that a key using an unsupported algorithm cannot be generated ($n)" ret=0 zone=example @@ -1458,21 +1545,6 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) -get_rsasha1_key_ids_from_sigs() { - tr -d '\r' < signer/example.db.signed | \ - awk ' - NF < 8 { next } - $(NF-5) != "RRSIG" { next } - $(NF-3) != "5" { next } - $NF != "(" { next } - { - getline; - print $3; - } - ' | \ - sort -u -} - echo_i "checking that we can sign a zone with out-of-zone records ($n)" ret=0 zone=example @@ -1573,8 +1645,8 @@ cat example.db.in "$key1.key" "$key3.key" > example.db echo "\$INCLUDE \"example.db.signed\"" >> example.db $SIGNER -D -o example example.db > /dev/null ) || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1 n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -1585,8 +1657,8 @@ ret=0 cd signer || exit 1 $SIGNER -RD -o example example.db > /dev/null ) || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null && ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null && ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1 n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -1603,8 +1675,8 @@ echo "\$INCLUDE \"example.db.signed\"" >> example.db $SETTIME -I now "$key2" > /dev/null 2>&1 $SIGNER -SD -o example example.db > /dev/null ) || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1 n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -1615,8 +1687,8 @@ ret=0 cd signer || exit 1 $SIGNER -SDQ -o example example.db > /dev/null ) || ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null && ret=1 -get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null && ret=1 +get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1 n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c6a5892d0e..b0fa7eaab8 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -66,6 +66,11 @@ Feature Changes record. This allows a clean rollover from one DNS provider to another when using a multiple-signer DNSSEC configuration. :gl:`#2710` +- ``dnssec-signzone`` is now able to retain signatures from inactive + predecessor keys without introducing additional signatures from the successor + key. This allows for a gradual replacement of RRSIGs as they reach expiry. + :gl:`#1551` + Bug Fixes ~~~~~~~~~ diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index a510934d25..b0d5162cc6 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -2708,7 +2708,7 @@ dst_key_copy_metadata(dst_key_t *to, dst_key_t *from) { if (result == ISC_R_SUCCESS) { dst_key_setbool(to, i, yesno); } else { - dst_key_unsetnum(to, i); + dst_key_unsetbool(to, i); } }