From 0a91321d787968f1fb0409023d6ab233f0a4403e 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). --- 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 d94c267074..1c3876e0fb 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 5e3aef364fc3c6382a5761eb339ca780df0829ef 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). --- 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 fcdfe15e04..748f28f3bc 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -419,10 +419,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); @@ -540,7 +539,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); @@ -551,8 +550,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 */ @@ -581,15 +582,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 " @@ -602,25 +609,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 1c3876e0fb..400d9db806 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 dcc418f955..ef3aefd3d7 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -1121,6 +1121,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; @@ -1409,7 +1410,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 */ @@ -1444,6 +1445,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); @@ -1584,7 +1586,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; } @@ -1672,9 +1674,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; } @@ -1691,7 +1693,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 09281eb9cc..daacd69a5c 100644 --- a/lib/dns/include/dns/dnssec.h +++ b/lib/dns/include/dns/dnssec.h @@ -68,6 +68,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; };