From f894bf661f88a273a1a4cc6bd67b839f04b6d531 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 12 Dec 2023 13:47:30 +1100 Subject: [PATCH 01/10] Test dnssec-policy dnskey-ttl behaviour If the dnskey-ttl in the dnssec-policy doesn't match the DNSKEY's ttl then the DNSKEY, CDNSKEY and CDS rrset should be updated by named to reflect the expressed policy. Check that named does this by creating a zone with a TTL that does not match the policy's TTL and check that it is correctly updated. --- bin/tests/system/kasp.sh | 5 ++- bin/tests/system/kasp/ns3/named-fips.conf.in | 9 +++++ bin/tests/system/kasp/ns3/setup.sh | 8 ++++ bin/tests/system/kasp/tests.sh | 42 ++++++++++++++++++++ bin/tests/system/nsec3/tests.sh | 1 + 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/kasp.sh b/bin/tests/system/kasp.sh index 0683786340..a1f669adf7 100644 --- a/bin/tests/system/kasp.sh +++ b/bin/tests/system/kasp.sh @@ -213,6 +213,7 @@ set_policy() { POLICY=$1 NUM_KEYS=$2 DNSKEY_TTL=$3 + KEYFILE_TTL=${4:-$3} CDS_DELETE="no" CDS_SHA256="yes" CDS_SHA384="no" @@ -332,7 +333,7 @@ check_key() { _alg_numpad=$(printf "%03d" "$_alg_num") _alg_string=$(key_get "$1" ALG_STR) _length=$(key_get "$1" "ALG_LEN") - _dnskey_ttl="$DNSKEY_TTL" + _dnskey_ttl="$KEYFILE_TTL" _lifetime=$(key_get "$1" LIFETIME) _legacy=$(key_get "$1" LEGACY) _private=$(key_get "$1" PRIVATE) @@ -1074,7 +1075,7 @@ _find_dnskey() { _flags="$(key_get $1 FLAGS)" _key_file="$(key_get $1 BASEFILE).key" - awk '$1 == "'"$_owner"'" && $2 == "'"$DNSKEY_TTL"'" && $3 == "IN" && $4 == "DNSKEY" && $5 == "'"$_flags"'" && $6 == "3" && $7 == "'"$_alg"'" { print $8 }' <"$_key_file" + awk '$1 == "'"$_owner"'" && $2 == "'"$KEYFILE_TTL"'" && $3 == "IN" && $4 == "DNSKEY" && $5 == "'"$_flags"'" && $6 == "3" && $7 == "'"$_alg"'" { print $8 }' <"$_key_file" } # Test DNSKEY query. diff --git a/bin/tests/system/kasp/ns3/named-fips.conf.in b/bin/tests/system/kasp/ns3/named-fips.conf.in index d67aa5f38a..ab0d87dcfa 100644 --- a/bin/tests/system/kasp/ns3/named-fips.conf.in +++ b/bin/tests/system/kasp/ns3/named-fips.conf.in @@ -263,6 +263,15 @@ zone "expired-sigs.autosign" { dnssec-policy "autosign"; }; +/* + * Zone that has DNSKEY TTL mismatch with the dnssec-policy. + */ +zone "dnskey-ttl-mismatch.autosign" { + type primary; + file "dnskey-ttl-mismatch.autosign.db"; + dnssec-policy "autosign"; +}; + /* * Zone that has valid, fresh signatures. */ diff --git a/bin/tests/system/kasp/ns3/setup.sh b/bin/tests/system/kasp/ns3/setup.sh index 55fcd1b5e1..4d76d250c6 100644 --- a/bin/tests/system/kasp/ns3/setup.sh +++ b/bin/tests/system/kasp/ns3/setup.sh @@ -198,6 +198,14 @@ private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >>"$infile" cp $infile $zonefile $SIGNER -PS -x -s now-2mo -e now-1mo -o $zone -O raw -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1 +# The DNSKEY's TTLs do not match the policy. +setup dnskey-ttl-mismatch.autosign +KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 30 -f KSK $ksktimes $zone 2>keygen.out.$zone.1) +ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 30 $zsktimes $zone 2>keygen.out.$zone.2) +cat template.db.in "${KSK}.key" "${ZSK}.key" >"$infile" +cp $infile $zonefile +$SIGNER -PS -x -o $zone -O raw -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1 + # These signatures are still good, and can be reused. setup fresh-sigs.autosign T="now-6mo" diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 645d77b8dd..981dd69b8e 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -1375,6 +1375,48 @@ check_rrsig_refresh() { check_rrsig_refresh +# +# Zone: dnskey-ttl-mismatch.autosign +# +set_zone "dnskey-ttl-mismatch.autosign" +set_policy "autosign" "2" "300" "30" +set_server "ns3" "10.53.0.3" +# Key properties. +key_clear "KEY1" +set_keyrole "KEY1" "ksk" +set_keylifetime "KEY1" "63072000" +set_keyalgorithm "KEY1" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS" +set_keysigning "KEY1" "yes" +set_zonesigning "KEY1" "no" + +key_clear "KEY2" +set_keyrole "KEY2" "zsk" +set_keylifetime "KEY2" "31536000" +set_keyalgorithm "KEY2" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS" +set_keysigning "KEY2" "no" +set_zonesigning "KEY2" "yes" + +# Both KSK and ZSK stay OMNIPRESENT. +set_keystate "KEY1" "GOAL" "omnipresent" +set_keystate "KEY1" "STATE_DNSKEY" "omnipresent" +set_keystate "KEY1" "STATE_KRRSIG" "omnipresent" +set_keystate "KEY1" "STATE_DS" "omnipresent" + +set_keystate "KEY2" "GOAL" "omnipresent" +set_keystate "KEY2" "STATE_DNSKEY" "omnipresent" +set_keystate "KEY2" "STATE_ZRRSIG" "omnipresent" +# Expect only two keys. +key_clear "KEY3" +key_clear "KEY4" + +check_keys +check_dnssecstatus "$SERVER" "$POLICY" "$ZONE" +set_keytimes_autosign_policy +check_keytimes +check_apex +check_subdomain +dnssec_verify + # # Zone: fresh-sigs.autosign. # diff --git a/bin/tests/system/nsec3/tests.sh b/bin/tests/system/nsec3/tests.sh index ee49d63f47..da61c8abb3 100644 --- a/bin/tests/system/nsec3/tests.sh +++ b/bin/tests/system/nsec3/tests.sh @@ -41,6 +41,7 @@ set_zone_policy() { POLICY=$2 NUM_KEYS=$3 DNSKEY_TTL=$4 + KEYFILE_TTL=${5:-$4} # The CDS digest type in these tests are all the default, # which is SHA-256 (2). CDS_SHA256="yes" From dcb7799061a7dc93d345b343145f94405d37f32b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 12 Dec 2023 13:47:30 +1100 Subject: [PATCH 02/10] Update the DNSKEY, CDNSKEY and CDS TTLs to match dnskey-ttl If the TTLs of the DNSKEY, CDNSKEY and CDS do not match the dnskey-ttl update them by removing all records and re-adding them with the correct TTL. --- lib/dns/zone.c | 113 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 105 insertions(+), 8 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 6f5cf67690..6f3ea2bc88 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -21247,6 +21247,55 @@ zone_checkds(dns_zone_t *zone) { #endif /* ifdef ENABLE_AFL */ } +static isc_result_t +update_ttl(dns_rdataset_t *rdataset, dns_name_t *name, dns_ttl_t ttl, + dns_diff_t *diff) { + isc_result_t result; + + /* + * Delete everything using the existing TTL. + */ + for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; + result = dns_rdataset_next(rdataset)) + { + dns_difftuple_t *tuple = NULL; + dns_rdata_t rdata = DNS_RDATA_INIT; + + dns_rdataset_current(rdataset, &rdata); + result = dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, name, + rdataset->ttl, &rdata, &tuple); + if (result != ISC_R_SUCCESS) { + return (result); + } + dns_diff_appendminimal(diff, &tuple); + } + if (result != ISC_R_NOMORE) { + return (result); + } + + /* + * Add everything using the new TTL. + */ + for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; + result = dns_rdataset_next(rdataset)) + { + dns_difftuple_t *tuple = NULL; + dns_rdata_t rdata = DNS_RDATA_INIT; + + dns_rdataset_current(rdataset, &rdata); + result = dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD, name, + ttl, &rdata, &tuple); + if (result != ISC_R_SUCCESS) { + return (result); + } + dns_diff_appendminimal(diff, &tuple); + } + if (result != ISC_R_NOMORE) { + return (result); + } + return (ISC_R_SUCCESS); +} + static void zone_rekey(dns_zone_t *zone) { isc_result_t result; @@ -21304,11 +21353,39 @@ zone_rekey(dns_zone_t *zone) { ttl = soaset.ttl; dns_rdataset_disassociate(&soaset); + /* + * Only update DNSKEY TTL if we have a policy. + */ + if (kasp != NULL) { + ttl = dns_kasp_dnskeyttl(kasp); + } + /* Get the DNSKEY rdataset */ result = dns_db_findrdataset(db, node, ver, dns_rdatatype_dnskey, dns_rdatatype_none, 0, &keyset, &keysigs); if (result == ISC_R_SUCCESS) { - ttl = keyset.ttl; + /* + * If we don't have a policy then use the DNSKEY ttl + * if it exists. Otherwise update the DNSKEY ttl if + * needed. + */ + if (kasp == NULL) { + ttl = keyset.ttl; + } else if (ttl != keyset.ttl) { + result = update_ttl(&keyset, &zone->origin, ttl, &diff); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_ERROR, + "Updating DNSKEY TTL from %u to %u " + "failed: %s", + keyset.ttl, ttl, + isc_result_totext(result)); + goto failure; + } + dnssec_log(zone, ISC_LOG_INFO, + "Updating DNSKEY TTL from %u to %u", + keyset.ttl, ttl); + keyset.ttl = ttl; + } dns_zone_lock_keyfiles(zone); @@ -21330,6 +21407,18 @@ zone_rekey(dns_zone_t *zone) { dns_rdatatype_none, 0, &cdsset, NULL); if (result != ISC_R_SUCCESS && dns_rdataset_isassociated(&cdsset)) { dns_rdataset_disassociate(&cdsset); + } else if (result == ISC_R_SUCCESS && kasp != NULL && ttl != cdsset.ttl) + { + result = update_ttl(&cdsset, &zone->origin, ttl, &diff); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_ERROR, + "Updating CDS TTL from %u to %u failed: %s", + cdsset.ttl, ttl, isc_result_totext(result)); + goto failure; + } + dnssec_log(zone, ISC_LOG_INFO, "Updating CDS TTL from %u to %u", + cdsset.ttl, ttl); + cdsset.ttl = ttl; } /* Get the CDNSKEY rdataset */ @@ -21337,6 +21426,21 @@ zone_rekey(dns_zone_t *zone) { dns_rdatatype_none, 0, &cdnskeyset, NULL); if (result != ISC_R_SUCCESS && dns_rdataset_isassociated(&cdnskeyset)) { dns_rdataset_disassociate(&cdnskeyset); + } else if (result == ISC_R_SUCCESS && kasp != NULL && + ttl != cdnskeyset.ttl) + { + result = update_ttl(&cdnskeyset, &zone->origin, ttl, &diff); + if (result != ISC_R_SUCCESS) { + dnssec_log( + zone, ISC_LOG_ERROR, + "Updating CDNSKEY TTL from %u to %u failed: %s", + cdnskeyset.ttl, ttl, isc_result_totext(result)); + goto failure; + } + dnssec_log(zone, ISC_LOG_INFO, + "Updating CDNSKEY TTL from %u to %u", cdnskeyset.ttl, + ttl); + cdnskeyset.ttl = ttl; } /* @@ -21475,13 +21579,6 @@ zone_rekey(dns_zone_t *zone) { digests = dns_kasp_digests(zone->defaultkasp); } - /* - * Only update DNSKEY TTL if we have a policy. - */ - if (kasp != NULL) { - ttl = dns_kasp_dnskeyttl(kasp); - } - result = dns_dnssec_updatekeys(&dnskeys, &keys, &rmkeys, &zone->origin, ttl, &diff, mctx, dnssec_report); From 21be35c54e1329556062cd772860e730bb37e1de Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 12 Dec 2023 13:51:19 +1100 Subject: [PATCH 03/10] Use the current CDS and CDNSKEY TTLs When adding new CDS and CDNSKEY records use the existing RRset TTL if they already exist. --- lib/dns/dnssec.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 92ac096134..6b45dfc117 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -2034,11 +2034,21 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys, unsigned char keybuf[DST_KEY_MAXSIZE]; isc_result_t result; dns_dnsseckey_t *key; + dns_ttl_t cdsttl = ttl; + dns_ttl_t cdnskeyttl = ttl; REQUIRE(digests != NULL); REQUIRE(keys != NULL); REQUIRE(rmkeys != NULL); + if (dns_rdataset_isassociated(cds)) { + cdsttl = cds->ttl; + } + + if (dns_rdataset_isassociated(cdnskey)) { + cdnskeyttl = cdnskey->ttl; + } + for (key = ISC_LIST_HEAD(*keys); key != NULL; key = ISC_LIST_NEXT(key, link)) { @@ -2058,7 +2068,8 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys, { RETERR(add_cds(key, &cdnskeyrdata, (const char *)keystr, cds, - alg->digest, ttl, diff, mctx)); + alg->digest, cdsttl, diff, + mctx)); } if (gencdnskey && @@ -2071,7 +2082,7 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys, "CDNSKEY for key %s is now published", keystr); RETERR(addrdata(&cdnskeyrdata, diff, origin, - ttl, mctx)); + cdnskeyttl, mctx)); } } From d601a90ea3516aee8be2ddcce5571b4faf66ec22 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 14 Dec 2023 15:02:22 +1100 Subject: [PATCH 04/10] sync_secure_db failed to handle some TTL changes If the DNSKEY, CDNSKEY or CDS RRset had different TTLs then the filtering of these RRset resulted in dns_diff_apply failing with "not exact". Identify tuple pairs that are just TTL changes and allow them through the filter. --- lib/dns/zone.c | 196 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 183 insertions(+), 13 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 6f3ea2bc88..42ca43f2d1 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -16061,6 +16061,77 @@ failure: return (result); } +/* + * Filter the key material preserving TTL changes. If kasp in effect honour the + * existing ttl. The lists returned by sync_secure_db/dns_db_diffx should be + * DNSSEC RRset order so we can process 'del' and 'add' in parallel rather than + * searching for TTL only changes first and processing them, then checking the + * 'in use' status on a subsequent pass. + */ + +static void +filter_keymaterial(dns_zone_t *zone, dns_difftuplelist_t *del, + dns_difftuplelist_t *add, bool kasp, dns_ttl_t ttl) { + dns_difftuple_t *deltuple = ISC_LIST_HEAD(*del); + dns_difftuple_t *addtuple = ISC_LIST_HEAD(*add); + isc_result_t result; + + while (deltuple != NULL || addtuple != NULL) { + dns_difftuple_t *delnext = NULL, *addnext = NULL; + bool inuse = false; + if (deltuple != NULL) { + delnext = ISC_LIST_NEXT(deltuple, link); + } + if (addtuple != NULL) { + addnext = ISC_LIST_NEXT(addtuple, link); + } + if (deltuple != NULL && addtuple != NULL) { + int n = dns_rdata_compare(&deltuple->rdata, + &addtuple->rdata); + if (n == 0) { + /* + * If the rdata is equal then the only + * difference will be a TTL change. + */ + if (kasp) { + /* TTL is managed by dnssec-policy */ + ISC_LIST_UNLINK(*del, deltuple, link); + dns_difftuple_free(&deltuple); + ISC_LIST_UNLINK(*add, addtuple, link); + dns_difftuple_free(&addtuple); + } + deltuple = delnext; + addtuple = addnext; + continue; + } + if (n < 0) { + goto checkdel; + } + goto checkadd; + } else if (deltuple != NULL) { + checkdel: + result = dns_zone_dnskey_inuse(zone, &deltuple->rdata, + &inuse); + if (result == ISC_R_SUCCESS && inuse) { + ISC_LIST_UNLINK(*del, deltuple, link); + dns_difftuple_free(&deltuple); + } + deltuple = delnext; + } else { + checkadd: + result = dns_zone_dnskey_inuse(zone, &addtuple->rdata, + &inuse); + if (result == ISC_R_SUCCESS && inuse) { + ISC_LIST_UNLINK(*add, addtuple, link); + dns_difftuple_free(&addtuple); + } else if (kasp) { + addtuple->ttl = ttl; + } + addtuple = addnext; + } + } +} + static isc_result_t sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, dns_dbversion_t *secver, dns_difftuple_t **soatuple, @@ -16071,6 +16142,16 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, dns_difftuple_t *tuple = NULL, *next; dns_difftuple_t *oldtuple = NULL, *newtuple = NULL; dns_rdata_soa_t oldsoa, newsoa; + dns_difftuplelist_t add = ISC_LIST_INITIALIZER; + dns_difftuplelist_t del = ISC_LIST_INITIALIZER; + dns_difftuplelist_t keyadd = ISC_LIST_INITIALIZER; + dns_difftuplelist_t keydel = ISC_LIST_INITIALIZER; + dns_difftuplelist_t ckeyadd = ISC_LIST_INITIALIZER; + dns_difftuplelist_t ckeydel = ISC_LIST_INITIALIZER; + dns_difftuplelist_t cdsadd = ISC_LIST_INITIALIZER; + dns_difftuplelist_t cdsdel = ISC_LIST_INITIALIZER; + dns_kasp_t *kasp = NULL; + dns_ttl_t keyttl = 0, ckeyttl = 0, cdsttl = 0; REQUIRE(DNS_ZONE_VALID(seczone)); REQUIRE(soatuple != NULL && *soatuple == NULL); @@ -16089,8 +16170,52 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, return (result); } + /* + * If kasp is in effect honour the existing DNSKEY, CDNSKEY and CDS + * TTLs. + */ + kasp = seczone->kasp; + if (kasp != NULL) { + dns_rdataset_t rdataset; + dns_dbnode_t *node = NULL; + dns_ttl_t ttl = dns_kasp_dnskeyttl(kasp); + + dns_rdataset_init(&rdataset); + + result = dns_db_getoriginnode(secdb, &node); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + + result = dns_db_findrdataset( + secdb, node, secver, dns_rdatatype_dnskey, + dns_rdatatype_none, 0, &rdataset, NULL); + keyttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl; + if (dns_rdataset_isassociated(&rdataset)) { + dns_rdataset_disassociate(&rdataset); + } + + result = dns_db_findrdataset( + secdb, node, secver, dns_rdatatype_cdnskey, + dns_rdatatype_none, 0, &rdataset, NULL); + ckeyttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl; + if (dns_rdataset_isassociated(&rdataset)) { + dns_rdataset_disassociate(&rdataset); + } + + result = dns_db_findrdataset( + secdb, node, secver, dns_rdatatype_cds, + dns_rdatatype_none, 0, &rdataset, NULL); + cdsttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl; + if (dns_rdataset_isassociated(&rdataset)) { + dns_rdataset_disassociate(&rdataset); + } + dns_db_detachnode(secdb, &node); + } + for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; tuple = next) { + dns_difftuplelist_t *al = &add, *dl = &del; + next = ISC_LIST_NEXT(tuple, link); + /* * Skip DNSSEC records that BIND maintains with inline-signing. */ @@ -16105,18 +16230,27 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, } /* - * Allow DNSKEY, CDNSKEY, CDS because users should be able to - * update the zone with these records from a different provider, - * but skip records that are under our control. + * Apex DNSKEY, CDNSKEY and CDS need special processing so + * split them out. */ - if (dns_rdatatype_iskeymaterial(tuple->rdata.type)) { - bool inuse = false; - isc_result_t r = dns_zone_dnskey_inuse( - seczone, &tuple->rdata, &inuse); - if (r == ISC_R_SUCCESS && inuse) { - ISC_LIST_UNLINK(diff->tuples, tuple, link); - dns_difftuple_free(&tuple); - continue; + if (dns_rdatatype_iskeymaterial(tuple->rdata.type) && + dns_name_equal(&tuple->name, &seczone->origin)) + { + switch (tuple->rdata.type) { + case dns_rdatatype_dnskey: + al = &keyadd; + dl = &keydel; + break; + case dns_rdatatype_cdnskey: + al = &ckeyadd; + dl = &ckeydel; + break; + case dns_rdatatype_cds: + al = &cdsadd; + dl = &cdsdel; + break; + default: + UNREACHABLE(); } } @@ -16130,6 +16264,23 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, newtuple = tuple; } } + + /* + * Split into deletions and additions. + */ + ISC_LIST_UNLINK(diff->tuples, tuple, link); + switch (tuple->op) { + case DNS_DIFFOP_DEL: + case DNS_DIFFOP_DELRESIGN: + ISC_LIST_APPEND(*dl, tuple, link); + break; + case DNS_DIFFOP_ADD: + case DNS_DIFFOP_ADDRESIGN: + ISC_LIST_APPEND(*al, tuple, link); + break; + default: + UNREACHABLE(); + } } if (oldtuple != NULL && newtuple != NULL) { @@ -16151,13 +16302,32 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb, dns_name_equal(&oldsoa.origin, &newsoa.origin) && dns_name_equal(&oldsoa.contact, &newsoa.contact)) { - ISC_LIST_UNLINK(diff->tuples, oldtuple, link); + ISC_LIST_UNLINK(del, oldtuple, link); dns_difftuple_free(&oldtuple); - ISC_LIST_UNLINK(diff->tuples, newtuple, link); + ISC_LIST_UNLINK(add, newtuple, link); dns_difftuple_free(&newtuple); } } + /* + * Filter out keys we manage but still allow TTL changes. + */ + filter_keymaterial(seczone, &keydel, &keyadd, kasp != NULL, keyttl); + filter_keymaterial(seczone, &ckeydel, &ckeyadd, kasp != NULL, ckeyttl); + filter_keymaterial(seczone, &cdsdel, &cdsadd, kasp != NULL, cdsttl); + + /* + * Rebuild the diff now that we have filtered it + */ + ISC_LIST_APPENDLIST(diff->tuples, del, link); + ISC_LIST_APPENDLIST(diff->tuples, keydel, link); + ISC_LIST_APPENDLIST(diff->tuples, ckeydel, link); + ISC_LIST_APPENDLIST(diff->tuples, cdsdel, link); + ISC_LIST_APPENDLIST(diff->tuples, add, link); + ISC_LIST_APPENDLIST(diff->tuples, keyadd, link); + ISC_LIST_APPENDLIST(diff->tuples, ckeyadd, link); + ISC_LIST_APPENDLIST(diff->tuples, cdsadd, link); + if (ISC_LIST_EMPTY(diff->tuples)) { return (DNS_R_UNCHANGED); } From 27e74b2e4b567bf9da9bad0818d94de9db2a8929 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 19 Dec 2023 15:58:49 +1100 Subject: [PATCH 05/10] Only create private records for DNSKEYs that have changed We don't need to create private records for DNSKEY records that have only had their TTL's changed. --- lib/dns/zone.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 42ca43f2d1..0e4be36948 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -19908,7 +19908,8 @@ failure: static isc_result_t add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, dns_dbversion_t *ver, dns_diff_t *diff, bool sign_all) { - dns_difftuple_t *tuple, *newtuple = NULL; + dns_difftuple_t *tuple = NULL, *newtuple = NULL, *next = NULL; + dns_difftuple_t *addtuple = NULL, *deltuple = NULL; dns_rdata_dnskey_t dnskey; dns_rdata_t rdata = DNS_RDATA_INIT; bool flag; @@ -19917,11 +19918,20 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, uint16_t keyid; unsigned char buf[5]; dns_name_t *name = dns_db_origin(db); + dns_difftuplelist_t add = ISC_LIST_INITIALIZER; + dns_difftuplelist_t del = ISC_LIST_INITIALIZER; + dns_difftuplelist_t tuples = ISC_LIST_INITIALIZER; + /* + * Move non DNSKEY and not DNSSEC DNSKEY records to tuples + * and sort the remaining DNSKEY records to add and del. + */ for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; - tuple = ISC_LIST_NEXT(tuple, link)) + tuple = ISC_LIST_HEAD(diff->tuples)) { if (tuple->rdata.type != dns_rdatatype_dnskey) { + ISC_LIST_UNLINK(diff->tuples, tuple, link); + ISC_LIST_APPEND(tuples, tuple, link); continue; } @@ -19930,9 +19940,65 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, if ((dnskey.flags & (DNS_KEYFLAG_OWNERMASK | DNS_KEYTYPE_NOAUTH)) != DNS_KEYOWNER_ZONE) { + ISC_LIST_UNLINK(diff->tuples, tuple, link); + ISC_LIST_APPEND(tuples, tuple, link); continue; } + ISC_LIST_UNLINK(diff->tuples, tuple, link); + switch (tuple->op) { + case DNS_DIFFOP_DEL: + case DNS_DIFFOP_DELRESIGN: + ISC_LIST_APPEND(del, tuple, link); + break; + case DNS_DIFFOP_ADD: + case DNS_DIFFOP_ADDRESIGN: + ISC_LIST_APPEND(add, tuple, link); + break; + default: + UNREACHABLE(); + } + } + + /* + * Put the tuples that don't need more processing back onto + * diff->tuples. + */ + ISC_LIST_APPENDLIST(diff->tuples, tuples, link); + + /* + * Filter out DNSKEY TTL changes and put them back onto diff->tuples. + */ + for (deltuple = ISC_LIST_HEAD(del); deltuple != NULL; deltuple = next) { + next = ISC_LIST_NEXT(deltuple, link); + for (addtuple = ISC_LIST_HEAD(add); addtuple != NULL; + addtuple = ISC_LIST_NEXT(addtuple, link)) + { + int n = dns_rdata_compare(&deltuple->rdata, + &addtuple->rdata); + if (n == 0) { + ISC_LIST_UNLINK(del, deltuple, link); + ISC_LIST_APPEND(diff->tuples, deltuple, link); + ISC_LIST_UNLINK(add, addtuple, link); + ISC_LIST_APPEND(diff->tuples, addtuple, link); + break; + } + } + } + + /* + * Combine any remaining DNSKEY changes together. + */ + ISC_LIST_APPENDLIST(tuples, add, link); + ISC_LIST_APPENDLIST(tuples, del, link); + + /* + * Add private records for keys that have been removed + * or added. + */ + for (tuple = ISC_LIST_HEAD(tuples); tuple != NULL; + tuple = ISC_LIST_NEXT(tuple, link)) + { dns_rdata_toregion(&tuple->rdata, &r); keyid = dst_region_computeid(&r); @@ -19972,7 +20038,15 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype, INSIST(newtuple == NULL); } } + failure: + /* + * Put the DNSKEY changes we cared about back on diff->tuples. + */ + ISC_LIST_APPENDLIST(diff->tuples, tuples, link); + INSIST(ISC_LIST_EMPTY(add)); + INSIST(ISC_LIST_EMPTY(del)); + INSIST(ISC_LIST_EMPTY(tuples)); return (result); } From b770740b44f7b91ea2a2d6714075547fe836b16f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 22 Dec 2023 15:08:45 +0100 Subject: [PATCH 06/10] Write new DNSKEY TTL to key file When the current DNSKEY TTL does not match the one from the policy, write the new TTL to disk. --- bin/tests/system/kasp.sh | 2 +- bin/tests/system/kasp/tests.sh | 10 +++++----- bin/tests/system/nsec3/tests.sh | 2 +- lib/dns/keymgr.c | 9 +++++++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/bin/tests/system/kasp.sh b/bin/tests/system/kasp.sh index a1f669adf7..5f879cbe71 100644 --- a/bin/tests/system/kasp.sh +++ b/bin/tests/system/kasp.sh @@ -213,7 +213,7 @@ set_policy() { POLICY=$1 NUM_KEYS=$2 DNSKEY_TTL=$3 - KEYFILE_TTL=${4:-$3} + KEYFILE_TTL=$3 CDS_DELETE="no" CDS_SHA256="yes" CDS_SHA384="no" diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 981dd69b8e..59dd4d391a 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -1379,7 +1379,7 @@ check_rrsig_refresh # Zone: dnskey-ttl-mismatch.autosign # set_zone "dnskey-ttl-mismatch.autosign" -set_policy "autosign" "2" "300" "30" +set_policy "autosign" "2" "300" set_server "ns3" "10.53.0.3" # Key properties. key_clear "KEY1" @@ -4079,7 +4079,7 @@ dnssec_verify # Zone: step1.going-insecure.kasp # set_zone "step1.going-insecure.kasp" -set_policy "insecure" "2" "7200" +set_policy "insecure" "2" "3600" set_server "ns6" "10.53.0.6" # Expect a CDS/CDNSKEY Delete Record. set_cdsdelete @@ -4116,7 +4116,7 @@ check_next_key_event 93600 # Zone: step2.going-insecure.kasp # set_zone "step2.going-insecure.kasp" -set_policy "insecure" "2" "7200" +set_policy "insecure" "2" "3600" set_server "ns6" "10.53.0.6" # The DS is long enough removed from the zone to be considered HIDDEN. @@ -4146,7 +4146,7 @@ check_next_key_event 7500 # set_zone "step1.going-insecure-dynamic.kasp" set_dynamic -set_policy "insecure" "2" "7200" +set_policy "insecure" "2" "3600" set_server "ns6" "10.53.0.6" # Expect a CDS/CDNSKEY Delete Record. set_cdsdelete @@ -4184,7 +4184,7 @@ check_next_key_event 93600 # set_zone "step2.going-insecure-dynamic.kasp" set_dynamic -set_policy "insecure" "2" "7200" +set_policy "insecure" "2" "3600" set_server "ns6" "10.53.0.6" # The DS is long enough removed from the zone to be considered HIDDEN. diff --git a/bin/tests/system/nsec3/tests.sh b/bin/tests/system/nsec3/tests.sh index da61c8abb3..f7ab72a7d4 100644 --- a/bin/tests/system/nsec3/tests.sh +++ b/bin/tests/system/nsec3/tests.sh @@ -41,7 +41,7 @@ set_zone_policy() { POLICY=$2 NUM_KEYS=$3 DNSKEY_TTL=$4 - KEYFILE_TTL=${5:-$4} + KEYFILE_TTL=$4 # The CDS digest type in these tests are all the default, # which is SHA-256 (2). CDS_SHA256="yes" diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index ea8dfb788b..56672a1198 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -2214,11 +2214,16 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keyring); dkey != NULL; dkey = ISC_LIST_NEXT(dkey, link)) { - if (dst_key_ismodified(dkey->key) && !dkey->purge) { + bool modified = dst_key_ismodified(dkey->key); + if (dst_key_getttl(dkey->key) != dns_kasp_dnskeyttl(kasp)) { + dst_key_setttl(dkey->key, dns_kasp_dnskeyttl(kasp)); + modified = true; + } + if (modified && !dkey->purge) { dns_dnssec_get_hints(dkey, now); RETERR(dst_key_tofile(dkey->key, options, directory)); - dst_key_setmodified(dkey->key, false); } + dst_key_setmodified(dkey->key, false); } result = ISC_R_SUCCESS; From 16a720357b9195367f19ea6e8327530a12872088 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 12 Dec 2023 16:20:53 +1100 Subject: [PATCH 07/10] Make $TTL match dnskey-ttl --- bin/tests/system/autosign/ns3/nozsk.example.db.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tests/system/autosign/ns3/nozsk.example.db.in b/bin/tests/system/autosign/ns3/nozsk.example.db.in index 1376922f0a..5d244c3fc6 100644 --- a/bin/tests/system/autosign/ns3/nozsk.example.db.in +++ b/bin/tests/system/autosign/ns3/nozsk.example.db.in @@ -9,7 +9,7 @@ ; See the COPYRIGHT file distributed with this work for additional ; information regarding copyright ownership. -$TTL 300 ; 5 minutes +$TTL 3600 ; 1 hour @ IN SOA mname1. . ( 1 ; serial 20 ; refresh (20 seconds) From 7a6570a911b5e74da626204c64d12c4a58f2a902 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 13 Dec 2023 00:43:15 +1100 Subject: [PATCH 08/10] Create keys with TTLs that match the policies TTL --- bin/tests/system/inline/ns3/sign.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/inline/ns3/sign.sh b/bin/tests/system/inline/ns3/sign.sh index da4592dc0d..1b2a905db6 100755 --- a/bin/tests/system/inline/ns3/sign.sh +++ b/bin/tests/system/inline/ns3/sign.sh @@ -49,8 +49,8 @@ $DSFROMKEY -T 1200 $keyname >>../ns1/root.db zone=updated rm -f K${zone}.+*+*.key rm -f K${zone}.+*+*.private -zsk=$($KEYGEN -q -a ${DEFAULT_ALGORITHM} -n zone $zone) -ksk=$($KEYGEN -q -a ${DEFAULT_ALGORITHM} -n zone -f KSK $zone) +zsk=$($KEYGEN -q -a ${DEFAULT_ALGORITHM} -L 3600 -n zone $zone) +ksk=$($KEYGEN -q -a ${DEFAULT_ALGORITHM} -L 3600 -n zone -f KSK $zone) $SETTIME -s -g OMNIPRESENT -k RUMOURED now -z RUMOURED now "$zsk" >settime.out.updated.1 2>&1 $SETTIME -s -g OMNIPRESENT -k RUMOURED now -r RUMOURED now -d HIDDEN now "$ksk" >settime.out.updated.2 2>&1 $DSFROMKEY -T 1200 $ksk >>../ns1/root.db From 882b1a444963e74ce779a9b7528dc62cf50564ad Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 13 Dec 2023 01:05:54 +1100 Subject: [PATCH 09/10] Set the DNSKEY TTLs to match the dnssec policy This prevents the DNSKEY records being updated and the statistics not matching as a consequence --- bin/tests/system/statschannel/ns2/sign.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bin/tests/system/statschannel/ns2/sign.sh b/bin/tests/system/statschannel/ns2/sign.sh index 77bcd41de6..a66e81c22f 100644 --- a/bin/tests/system/statschannel/ns2/sign.sh +++ b/bin/tests/system/statschannel/ns2/sign.sh @@ -19,8 +19,8 @@ set -e zone=dnssec. infile=dnssec.db.in zonefile=dnssec.db.signed -ksk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -f KSK "$zone") -zsk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" "$zone") +ksk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -L 3600 -b "$DEFAULT_BITS" -f KSK "$zone") +zsk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -L 3600 -b "$DEFAULT_BITS" "$zone") # Sign deliberately with a very short expiration date. "$SIGNER" -P -S -x -O full -e "now"+1s -o "$zone" -f "$zonefile" "$infile" >"signzone.out.$zone" 2>&1 id=$(keyfile_to_key_id "$ksk") @@ -31,12 +31,12 @@ echo "$DEFAULT_ALGORITHM_NUMBER+$id" >dnssec.zsk.id zone=manykeys. infile=manykeys.db.in zonefile=manykeys.db.signed -ksk8=$("$KEYGEN" -q -a RSASHA256 -b 2048 -f KSK "$zone") -zsk8=$("$KEYGEN" -q -a RSASHA256 -b 2048 "$zone") -ksk13=$("$KEYGEN" -q -a ECDSAP256SHA256 -b 256 -f KSK "$zone") -zsk13=$("$KEYGEN" -q -a ECDSAP256SHA256 -b 256 "$zone") -ksk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -b 384 -f KSK "$zone") -zsk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -b 384 "$zone") +ksk8=$("$KEYGEN" -q -a RSASHA256 -L 3600 -b 2048 -f KSK "$zone") +zsk8=$("$KEYGEN" -q -a RSASHA256 -L 3600 -b 2048 "$zone") +ksk13=$("$KEYGEN" -q -a ECDSAP256SHA256 -L 3600 -b 256 -f KSK "$zone") +zsk13=$("$KEYGEN" -q -a ECDSAP256SHA256 -L 3600 -b 256 "$zone") +ksk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -L 3600 -b 384 -f KSK "$zone") +zsk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -L 3600 -b 384 "$zone") # Sign deliberately with a very short expiration date. "$SIGNER" -S -x -O full -e "now"+1s -o "$zone" -f "$zonefile" "$infile" >"signzone.out.$zone" 2>&1 id=$(keyfile_to_key_id "$ksk8") From 531420bac08c2be902c3134f38b665b2db5e261a Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 19 Dec 2023 19:01:22 +1100 Subject: [PATCH 10/10] Add CHANGES note for [GL #4466] --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 6977a2e395..e635e97d99 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6313. [bug] When dnssec-policy is in effect the DNSKEY's TTLs in + the zone where not being updated to match the policy. + This lead to failures when DNSKEYs where updated as the + TTLs mismatched. [GL #4466] + 6312. [bug] Conversion from NSEC3 signed to NSEC signed could temporarily put the zone into a state where it was treated as unsigned until the NSEC chain was built.