From cf73c9b1a9f8c658082b2abfc70886662a611111 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 14 Jan 2025 14:10:20 +0100 Subject: [PATCH 1/2] Test dnssec-signzone with private key file missing Add a test case for the scenario below. There is a case when signing a zone with dnssec-signzone where the private key file is moved outside the key directory (for offline ksk purposes), and then the zone is resigned. The signature of the DNSKEY needs refreshing, but is not expired. Rather than removing the signature without having a valid replacement, leave the signature in the zone (despite it needs to be refreshed). (cherry picked from commit 0a91321d787968f1fb0409023d6ab233f0a4403e) --- bin/tests/system/dnssec/tests.sh | 27 ++++++++++++++++++++++ bin/tests/system/dnssec/tests_sh_dnssec.py | 2 ++ 2 files changed, 29 insertions(+) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 22b438f857..84e08d2669 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1358,6 +1358,33 @@ n=$((n + 1)) test "$ret" -eq 0 || echo_i "failed" status=$((status + ret)) +echo_ic "two DNSKEYs, DNSKEY RRset only by KSK ($n)" +ret=0 +( +cd signer/general || exit 1 +rm -f signed.zone +$SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n +test -f signed.zone +) || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +echo_ic "two DNSKEYs, DNSKEY RRset only by KSK, private key missing ($n)" +ret=0 +( + cd signer/general || exit 1 + cp signed.zone signed.expect + grep "example\.com\..*3600.*IN.*RRSIG.*DNSKEY.*10.*2.*3600.*28633.*example\.com\." signed.expect >dnskey.expect || exit 1 + mv Kexample.com.+010+28633.private Kexample.com.+010+28633.offline + $SIGNER -P -x -f signed.zone -O full -o example.com. signed.zone >signer.out.$n + mv Kexample.com.+010+28633.offline Kexample.com.+010+28633.private + grep "$(cat dnskey.expect)" signed.zone >/dev/null || exit 1 +) || ret=1 +n=$((n + 1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status + ret)) + echo_ic "one non-KSK DNSKEY ($n)" ret=0 ( diff --git a/bin/tests/system/dnssec/tests_sh_dnssec.py b/bin/tests/system/dnssec/tests_sh_dnssec.py index b0e9a55871..91817bd0b7 100644 --- a/bin/tests/system/dnssec/tests_sh_dnssec.py +++ b/bin/tests/system/dnssec/tests_sh_dnssec.py @@ -146,7 +146,9 @@ pytestmark = pytest.mark.extra_artifacts( "signer/example.db.changed", "signer/example2.db", "signer/example3.db", + "signer/general/dnskey.expect", "signer/general/dsset-*", + "signer/general/signed.expect", "signer/general/signed.zone", "signer/general/signer.out.*", "signer/nsec3param.out", From 9d6302b32c3db6e22d1ae9fd9e81e5f5b57aff64 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 14 Jan 2025 15:18:40 +0100 Subject: [PATCH 2/2] dnssec-signzone retain signature if key is offline Track inside the dns_dnsseckey structure whether we have seen the private key, or if this key only has a public key file. If the key only has a public key file, or a DNSKEY reference in the zone, mark the key 'pubkey'. In dnssec-signzone, if the key only has a public key available, consider the key to be offline. Any signatures that should be refreshed for which the key is not available, retain the signature. So in the code, 'expired' becomes 'refresh', and the new 'expired' is only used to determine whether we need to keep the signature if the corresponding key is not available (retaining the signature if it is not expired). In the 'keysthatsigned' function, we can remove: - key->force_publish = false; - key->force_sign = false; because they are redundant ('dns_dnsseckey_create' already sets these values to false). (cherry picked from commit 5e3aef364fc3c6382a5761eb339ca780df0829ef) --- bin/dnssec/dnssec-signzone.c | 34 ++++++++++++++++++++++---------- bin/tests/system/dnssec/tests.sh | 12 +++++------ lib/dns/dnssec.c | 12 ++++++----- lib/dns/include/dns/dnssec.h | 1 + 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 0cd92cd745..7e906dfd1d 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -420,10 +420,9 @@ keythatsigned(dns_rdata_rrsig_t *rrsig) { dns_dnsseckey_create(mctx, &privkey, &key); } else { dns_dnsseckey_create(mctx, &pubkey, &key); + key->pubkey = true; } - key->force_publish = false; - key->force_sign = false; key->index = keycount++; ISC_LIST_APPEND(keylist, key, link); @@ -541,7 +540,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, } while (result == ISC_R_SUCCESS) { - bool expired, future; + bool expired, refresh, future, offline; bool keep = false, resign = false; dns_rdataset_current(&sigset, &sigrdata); @@ -552,8 +551,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, future = isc_serial_lt(now, rrsig.timesigned); key = keythatsigned(&rrsig); + offline = key->pubkey; sig_format(&rrsig, sigstr, sizeof(sigstr)); - expired = isc_serial_gt(now + cycle, rrsig.timeexpire); + expired = isc_serial_gt(now, rrsig.timeexpire); + refresh = isc_serial_gt(now + cycle, rrsig.timeexpire); if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) { /* rrsig is dropped and not replaced */ @@ -582,15 +583,21 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, } else if (issigningkey(key)) { wassignedby[key->index] = true; - if (!expired && rrsig.originalttl == set->ttl && + if (!refresh && rrsig.originalttl == set->ttl && setverifies(name, set, key->key, &sigrdata)) { vbprintf(2, "\trrsig by %s retained\n", sigstr); keep = true; + } else if (offline) { + vbprintf(2, + "\trrsig by %s retained - private key " + "missing\n", + sigstr); + keep = true; } else { vbprintf(2, "\trrsig by %s dropped - %s\n", sigstr, - expired ? "expired" + refresh ? "refresh" : rrsig.originalttl != set->ttl ? "ttl change" : "failed to " @@ -603,25 +610,32 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, } else if (iszonekey(key)) { wassignedby[key->index] = true; - if (!expired && rrsig.originalttl == set->ttl && + if (!refresh && rrsig.originalttl == set->ttl && setverifies(name, set, key->key, &sigrdata)) { vbprintf(2, "\trrsig by %s retained\n", sigstr); keep = true; + } else if (offline) { + vbprintf(2, + "\trrsig by %s retained - private key " + "missing\n", + sigstr); + keep = true; } else { vbprintf(2, "\trrsig by %s dropped - %s\n", sigstr, - expired ? "expired" + refresh ? "refresh" : rrsig.originalttl != set->ttl ? "ttl change" : "failed to " "verify"); } - } else if (!expired) { + } else if (!refresh) { vbprintf(2, "\trrsig by %s retained\n", sigstr); keep = true; } else { - vbprintf(2, "\trrsig by %s expired\n", sigstr); + vbprintf(2, "\trrsig by %s %s\n", sigstr, + expired ? "expired" : "needs refresh"); } if (keep) { diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 84e08d2669..37fc64de29 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1361,14 +1361,14 @@ status=$((status + ret)) echo_ic "two DNSKEYs, DNSKEY RRset only by KSK ($n)" ret=0 ( -cd signer/general || exit 1 -rm -f signed.zone -$SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n -test -f signed.zone + cd signer/general || exit 1 + rm -f signed.zone + $SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n + test -f signed.zone ) || ret=1 -n=$((n+1)) +n=$((n + 1)) test "$ret" -eq 0 || echo_i "failed" -status=$((status+ret)) +status=$((status + ret)) echo_ic "two DNSKEYs, DNSKEY RRset only by KSK, private key missing ($n)" ret=0 diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 4b2261d17e..f80f317eea 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -1123,6 +1123,7 @@ dns_dnsseckey_create(isc_mem_t *mctx, dst_key_t **dstkey, dk->hint_remove = false; dk->first_sign = false; dk->is_active = false; + dk->pubkey = false; dk->purge = false; dk->prepublish = 0; dk->source = dns_keysource_unknown; @@ -1411,7 +1412,7 @@ failure: */ static void addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys, - isc_mem_t *mctx) { + bool pubkey_only, isc_mem_t *mctx) { dns_dnsseckey_t *key = NULL; /* Skip duplicates */ @@ -1446,6 +1447,7 @@ addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys, } dns_dnsseckey_create(mctx, newkey, &key); + key->pubkey = pubkey_only; if (key->legacy || savekeys) { key->force_publish = true; key->force_sign = dst_key_isprivate(key->key); @@ -1586,7 +1588,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp, } if (publickey) { - addkey(keylist, &dnskey, savekeys, mctx); + addkey(keylist, &dnskey, savekeys, true, mctx); goto skip; } @@ -1674,9 +1676,9 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp, addkey: if (result == ISC_R_FILENOTFOUND || result == ISC_R_NOPERM) { if (pubkey != NULL) { - addkey(keylist, &pubkey, savekeys, mctx); + addkey(keylist, &pubkey, savekeys, true, mctx); } else { - addkey(keylist, &dnskey, savekeys, mctx); + addkey(keylist, &dnskey, savekeys, false, mctx); } goto skip; } @@ -1693,7 +1695,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp, */ dst_key_setttl(privkey, dst_key_getttl(dnskey)); - addkey(keylist, &privkey, savekeys, mctx); + addkey(keylist, &privkey, savekeys, false, mctx); skip: if (dnskey != NULL) { dst_key_free(&dnskey); diff --git a/lib/dns/include/dns/dnssec.h b/lib/dns/include/dns/dnssec.h index 3525672dcf..55e7fb842b 100644 --- a/lib/dns/include/dns/dnssec.h +++ b/lib/dns/include/dns/dnssec.h @@ -71,6 +71,7 @@ struct dns_dnsseckey { * an older version of BIND9) and * should be ignored when searching * for keys to import into the zone */ + bool pubkey; /*% public key only */ unsigned int index; /*% position in list */ ISC_LINK(dns_dnsseckey_t) link; };