From 837adb93d3fe6ffad7aa8a7af58718930978545f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 20 Jul 2021 08:17:22 +0200 Subject: [PATCH] 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