From 837adb93d3fe6ffad7aa8a7af58718930978545f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 20 Jul 2021 08:17:22 +0200 Subject: [PATCH 1/4] dnssec-signzone ZSK smooth rollover When signing with a ZSK, check if it has a predecessor. If so, and if the predecessor key is sane (same algorithm, key id matches predecessor value, is zsk), check if the RRset is signed with this key. If so, skip signing with this successor key. Otherwise, do sign with the successor key. This change means we also need to apply the interval to keys that are not actively signing. In other words, 'expired' is always 'isc_serial_gt(now + cycle, rrsig.timeexpire)'. Fix a print style issue ("removing signature by ..." was untabbed). --- bin/dnssec/dnssec-signzone.c | 93 ++++++++++++++++----- bin/tests/system/dnssec/signer/prepub.db.in | 15 ++++ 2 files changed, 88 insertions(+), 20 deletions(-) create mode 100644 bin/tests/system/dnssec/signer/prepub.db.in 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 From 35efbc270fdc4e18efc7d38652a871f921084faf Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 20 Jul 2021 08:34:10 +0200 Subject: [PATCH 2/4] Add test for dnssec-signzone smooth ZSK roll Add a test case to the dnssec system test to check that: - a zone with a prepublished key is only signed with the active key. - a zone with an inactive key but valid signatures retains those signatures and does not add signatures from successor key. - signatures are swapped in a zone when signatures of predecessor inactive key are within the refresh interval. --- bin/tests/system/dnssec/tests.sh | 118 +++++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 23 deletions(-) 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)) From 94bb545087af7b9a7730ed12941dee2f1ace6ae2 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 20 Jul 2021 11:12:07 +0200 Subject: [PATCH 3/4] Fix bug in dst_key_copymetadata When copying metadata from one dst_key to another, when the source dst_key has a boolean metadata unset, the destination dst_key will have a numeric metadata unset instead. This means that if a key has KSK or ZSK unset, we may be clearing the Predecessor or Successor metadata in the destination dst_key. --- lib/dns/dst_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); } } From 1befaa5d450d80b8775c58b45bb3c5d5d2cdea97 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 20 Jul 2021 11:40:39 +0200 Subject: [PATCH 4/4] Add release note and change entry for [#1551] --- CHANGES | 7 +++++++ doc/notes/notes-current.rst | 5 +++++ 2 files changed, 12 insertions(+) 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/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 ~~~~~~~~~