From cda1ae69acffa89f88c11039b2035b43b6c30294 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 13 Jun 2023 15:59:53 +0200 Subject: [PATCH 1/5] Add log check in multisigner system test When we add DNSKEY records via dynamic update, this should no longer trigger signing the zone with these keys. This currently happens when 'find_zone_keys()' looks up the keys by inspecting the DNSKEY RRset, then attempting to read the corresponding key files. Add checks that inspect the logs whether an attempt to read the key files for the newly added keys was done (and failed because these files are not available). --- bin/tests/system/multisigner/tests.sh | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/bin/tests/system/multisigner/tests.sh b/bin/tests/system/multisigner/tests.sh index aac1c2ecdc..5a209e7946 100644 --- a/bin/tests/system/multisigner/tests.sh +++ b/bin/tests/system/multisigner/tests.sh @@ -132,10 +132,19 @@ echo server "${SERVER}" "${PORT}" echo update add $(cat "ns4/${ZONE}.zsk") echo send ) | $NSUPDATE +# Check the new DNSKEY RRset. +n=$((n+1)) echo_i "check zone ${ZONE} DNSKEY RRset after update ($n)" retry_quiet 10 zsks_are_published || ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +# Check the logs for find zone keys errors. +n=$((n+1)) +ret=0 +echo_i "make sure we did not try to sign with the keys added with nsupdate for zone ${ZONE} ($n)" +grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) # Verify again. dnssec_verify @@ -149,10 +158,19 @@ echo server "${SERVER}" "${PORT}" echo update add $(cat "ns3/${ZONE}.zsk") echo send ) | $NSUPDATE +# Check the new DNSKEY RRset. +n=$((n+1)) echo_i "check zone ${ZONE} DNSKEY RRset after update ($n)" retry_quiet 10 zsks_are_published || ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +# Check the logs for find zone keys errors. +n=$((n+1)) +ret=0 +echo_i "make sure we did not try to sign with the keys added with nsupdate for zone ${ZONE} ($n)" +grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) # Verify again. dnssec_verify no_dnssec_in_journal @@ -446,6 +464,9 @@ test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) dnssec_verify no_dnssec_in_journal +grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) # NS4 set_server "ns4" "10.53.0.4" echo_i "check server ${DIR} zone ${ZONE} DNSKEY RRset after update ($n)" @@ -454,6 +475,9 @@ test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) dnssec_verify no_dnssec_in_journal +grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) n=$((n+1)) echo_i "remove dnskey record: remove ns3 and ns4 DNSKEY records from primary ns5 ($n)" From 88e5bc06872d8c64e4c4d1064bc3ef7d5325cd20 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 4 Oct 2022 15:54:30 +0200 Subject: [PATCH 2/5] Update find_zone_keys for dynamic update The find_zone_keys() function was not working properly for inline-signed zones. It only worked if the DNSKEY records were also published in the unsigned version of the zone. But this is not the case when you use dnssec-policy, the DNSKEY records will only occur in the signed version of the zone. Therefor, when looking for keys to sign the zone, only the newly added keys in the dynamic update were found (which could be zero), ignoring existing keys. Also, if a DNSKEY was added, it would try to sign the zone with just this new key, and this would only work if the key files for that key were imported into the key-directory. This is a design error, because the goal is to sign the zone with the keys for which we actually have key files for. So instead of looking for DNSKEY records to then search for the matching key files, call dns_dnssec_findmatchingkeys() which just looks for the keys we have on disk for the given zone. It will also set the correct DNSSEC signing hints. --- lib/dns/update.c | 58 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/dns/update.c b/lib/dns/update.c index 43a30a311f..710e98f28e 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1055,26 +1055,49 @@ failure: } static isc_result_t -find_zone_keys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, - isc_mem_t *mctx, unsigned int maxkeys, dst_key_t **keys, - unsigned int *nkeys) { +find_zone_keys(dns_zone_t *zone, isc_mem_t *mctx, unsigned int maxkeys, + dst_key_t **keys, unsigned int *nkeys) { + dns_dnsseckeylist_t keylist; + dns_dnsseckey_t *k = NULL; + unsigned int count = 0; isc_result_t result; isc_stdtime_t now = isc_stdtime_now(); - dns_dbnode_t *node = NULL; - const char *directory = dns_zone_getkeydirectory(zone); - CHECK(dns_db_findnode(db, dns_db_origin(db), false, &node)); + ISC_LIST_INIT(keylist); dns_zone_lock_keyfiles(zone); - result = dns_dnssec_findzonekeys(db, ver, node, dns_db_origin(db), - directory, now, mctx, maxkeys, keys, - nkeys); + result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), + dns_zone_getkeydirectory(zone), + now, mctx, &keylist); dns_zone_unlock_keyfiles(zone); -failure: - if (node != NULL) { - dns_db_detachnode(db, &node); + if (result != ISC_R_SUCCESS) { + *nkeys = 0; + return (result); } + + /* Add new 'dnskeys' to 'keys' */ + while ((k = ISC_LIST_HEAD(keylist)) != NULL) { + if (count >= maxkeys) { + result = ISC_R_NOSPACE; + goto next; + } + + /* Detect inactive keys */ + if (!dns_dnssec_keyactive(k->key, now)) { + dst_key_setinactive(k->key, true); + } + + keys[count] = k->key; + k->key = NULL; + count++; + + next: + ISC_LIST_UNLINK(keylist, k, link); + dns_dnsseckey_destroy(mctx, &k); + } + + *nkeys = count; return (result); } @@ -1544,10 +1567,13 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, state->nkeys = 0; state->build_nsec3 = false; - result = find_zone_keys(zone, db, newver, diff->mctx, - DNS_MAXZONEKEYS, state->zone_keys, - &state->nkeys); - if (result != ISC_R_SUCCESS) { + result = find_zone_keys(zone, diff->mctx, DNS_MAXZONEKEYS, + state->zone_keys, &state->nkeys); + if (result == ISC_R_NOSPACE) { + update_log(log, zone, ISC_LOG_ERROR, + "too many zone keys for secure " + "dynamic update"); + } else if (result != ISC_R_SUCCESS) { update_log(log, zone, ISC_LOG_ERROR, "could not get zone keys for secure " "dynamic update"); From 5c9a7ffbdb99f743c4246013d5064ceac4980955 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 2 Mar 2023 11:04:59 +0100 Subject: [PATCH 3/5] Add CHANGES for find_zone_keys() function update Probably a useful point in history. --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 04e5aad1ff..cba5df42ed 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6194. [func] Change function 'find_zone_keys()' to look for signing + keys by looking for key files instead of a DNSKEY + RRset lookup. [GL #4141] + 6193. [bug] Fix a catz db update notification callback registration logic error, which could crash named when receiving an AXFR update for a catalog zone while the previous update From 5cf91728d1124fa4a3638f09321523006c81ab39 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 13 Jun 2023 17:45:08 +0200 Subject: [PATCH 4/5] Add dynamic update prepub and doubleksk test case Add two test cases for zones that use auto-dnssec, but not inline-signing, and make sure that the change for find_zone_keys() do not affect introducing a new key that is intended for signing. See note https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7638#note_355944 --- bin/tests/system/nsupdate/clean.sh | 4 ++ .../system/nsupdate/ns3/doubleksk.test.db.in | 15 +++++++ bin/tests/system/nsupdate/ns3/named.conf.in | 26 ++++++++++++ .../system/nsupdate/ns3/prepub.test.db.in | 15 +++++++ bin/tests/system/nsupdate/ns3/sign.sh | 22 ++++++++++ bin/tests/system/nsupdate/tests.sh | 42 +++++++++++++++++++ 6 files changed, 124 insertions(+) create mode 100644 bin/tests/system/nsupdate/ns3/doubleksk.test.db.in create mode 100644 bin/tests/system/nsupdate/ns3/prepub.test.db.in diff --git a/bin/tests/system/nsupdate/clean.sh b/bin/tests/system/nsupdate/clean.sh index d7cc60df03..f2a9f1ef59 100644 --- a/bin/tests/system/nsupdate/clean.sh +++ b/bin/tests/system/nsupdate/clean.sh @@ -21,9 +21,11 @@ rm -f */named.memstats rm -f */named.run */ans.run rm -f */named.run.prev rm -f Kxxx.* +rm -f doubleksk.key prepub.key rm -f check.out.* rm -f dig.out.* rm -f jp.out.ns3.* +rm -f keygen.out.* rm -f nextpart.out.* rm -f ns*/managed-keys.bind* ns*/*.mkeys* rm -f ns*/named.lock @@ -45,12 +47,14 @@ rm -f ns3/*.signed rm -f ns3/K* rm -f ns3/delegation.test.db rm -f ns3/dnskey.test.db +rm -f ns3/doubleksk.test.db rm -f ns3/dsset-* rm -f ns3/example.db rm -f ns3/relaxed.db rm -f ns3/multisigner.test.db rm -f ns3/many.test.bk rm -f ns3/nsec3param.test.db +rm -f ns3/prepub.test.db rm -f ns3/too-big.test.db rm -f ns5/local.db rm -f ns6/in-addr.db diff --git a/bin/tests/system/nsupdate/ns3/doubleksk.test.db.in b/bin/tests/system/nsupdate/ns3/doubleksk.test.db.in new file mode 100644 index 0000000000..9430fb775a --- /dev/null +++ b/bin/tests/system/nsupdate/ns3/doubleksk.test.db.in @@ -0,0 +1,15 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; 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 https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +doubleksk.test. 10 IN SOA doubleksk.test. hostmaster.doubleksk.test. 1 3600 900 2419200 3600 +doubleksk.test. 10 IN NS doubleksk.test. +doubleksk.test. 10 IN A 10.53.0.3 +doubleksk.test. 10 IN NSEC3PARAM 1 1 0 - diff --git a/bin/tests/system/nsupdate/ns3/named.conf.in b/bin/tests/system/nsupdate/ns3/named.conf.in index ffc240c75d..c678fe9d2c 100644 --- a/bin/tests/system/nsupdate/ns3/named.conf.in +++ b/bin/tests/system/nsupdate/ns3/named.conf.in @@ -26,6 +26,16 @@ options { dnssec-validation yes; }; +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + + zone "example" { type primary; allow-update { any; }; @@ -71,6 +81,22 @@ zone "too-big.test" { file "too-big.test.db"; }; +zone "prepub.test" { + type primary; + allow-update { any; }; + auto-dnssec maintain; + dnssec-dnskey-kskonly yes; + file "prepub.test.db.signed"; +}; + +zone "doubleksk.test" { + type primary; + allow-update { any; }; + auto-dnssec maintain; + dnssec-dnskey-kskonly yes; + file "doubleksk.test.db.signed"; +}; + /* Zone for testing CDS and CDNSKEY updates from other provider */ zone "multisigner.test" { type primary; diff --git a/bin/tests/system/nsupdate/ns3/prepub.test.db.in b/bin/tests/system/nsupdate/ns3/prepub.test.db.in new file mode 100644 index 0000000000..916a0b2440 --- /dev/null +++ b/bin/tests/system/nsupdate/ns3/prepub.test.db.in @@ -0,0 +1,15 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; 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 https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +prepub.test. 10 IN SOA prepub.test. hostmaster.prepub.test. 1 3600 900 2419200 3600 +prepub.test. 10 IN NS prepub.test. +prepub.test. 10 IN A 10.53.0.3 +prepub.test. 10 IN NSEC3PARAM 1 1 0 - diff --git a/bin/tests/system/nsupdate/ns3/sign.sh b/bin/tests/system/nsupdate/ns3/sign.sh index 519497c6da..19105e5f1a 100644 --- a/bin/tests/system/nsupdate/ns3/sign.sh +++ b/bin/tests/system/nsupdate/ns3/sign.sh @@ -46,5 +46,27 @@ cat $infile $keyname1.key $keyname2.key >$zonefile $SIGNER -A -3 - -P -o $zone -k $keyname1 $zonefile $keyname2 > /dev/null +zone=prepub.test. +infile=prepub.test.db.in +zonefile=prepub.test.db + +keyname1=$($KEYGEN -q -L 3600 -a ${DEFAULT_ALGORITHM} -f KSK $zone) +keyname2=$($KEYGEN -q -L 3600 -a ${DEFAULT_ALGORITHM} $zone) + +cat $infile $keyname1.key $keyname2.key >$zonefile + +$SIGNER -A -x -3 - -P -o $zone -k $keyname1 $zonefile $keyname2 > /dev/null + +zone=doubleksk.test. +infile=doubleksk.test.db.in +zonefile=doubleksk.test.db + +keyname1=$($KEYGEN -q -L 3600 -a ${DEFAULT_ALGORITHM} -f KSK $zone) +keyname2=$($KEYGEN -q -L 3600 -a ${DEFAULT_ALGORITHM} $zone) + +cat $infile $keyname1.key $keyname2.key >$zonefile + +$SIGNER -A -x -3 - -P -o $zone -k $keyname1 $zonefile $keyname2 > /dev/null + # Just copy multisigner.db.in because it is signed with dnssec-policy. cp multisigner.test.db.in multisigner.test.db diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index d57c357195..8ba497e44c 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -1614,6 +1614,48 @@ END retry_quiet 5 has_positive_response multisigner.test CDNSKEY 10.53.0.3 || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } +n=$((n + 1)) +ret=0 +echo_i "check that DNSKEY can be prepublished with dynamic update ($n)" +$DIG $DIGOPTS +tcp +norec prepub.test DNSKEY @10.53.0.3 > dig.out.pre.test$n || ret=1 +grep "status: NOERROR" dig.out.pre.test$n > /dev/null || ret=1 +grep "ANSWER: 2," dig.out.pre.test$n > /dev/null || ret=1 +zsk=$($KEYGEN -a $DEFAULT_ALGORITHM -K ns3 -L 3600 -P now -A now+1w prepub.test 2> keygen.out.prepub.test.out$n) +cat "ns3/${zsk}.key" | grep -v ";.*" > prepub.key +$NSUPDATE -d < nsupdate.out.test$n 2>&1 || ret=1 +server 10.53.0.3 ${PORT} +zone prepub.test +update add $(cat prepub.key) +send +END +$RNDCCMD 10.53.0.3 loadkeys prepub.test. 2>&1 || ret=1 +$DIG $DIGOPTS +tcp +norec prepub.test DNSKEY @10.53.0.3 > dig.out.post.test$n || ret=1 +grep "status: NOERROR" dig.out.post.test$n > /dev/null || ret=1 +grep "ANSWER: 3," dig.out.post.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check that DNSKEY can be added as a signing key with dynamic update ($n)" +$DIG $DIGOPTS +dnssec +tcp +norec doubleksk.test DNSKEY @10.53.0.3 > dig.out.pre.test$n || ret=1 +grep "status: NOERROR" dig.out.pre.test$n > /dev/null || ret=1 +# 2x DNSKEY, 1x RRSIG +grep "ANSWER: 3," dig.out.pre.test$n > /dev/null || ret=1 +ksk=$($KEYGEN -a $DEFAULT_ALGORITHM -K ns3 -L 3600 -fk -P now -A now doubleksk.test 2> keygen.out.doubleksk.test.out$n) +cat "ns3/${ksk}.key" | grep -v ";.*" > doubleksk.key +$NSUPDATE -d < nsupdate.out.test$n 2>&1 || ret=1 +server 10.53.0.3 ${PORT} +zone doubleksk.test +update add $(cat doubleksk.key) +send +END +$RNDCCMD 10.53.0.3 loadkeys doubleksk.test. 2>&1 || ret=1 +$DIG $DIGOPTS +dnssec +tcp +norec doubleksk.test DNSKEY @10.53.0.3 > dig.out.post.test$n || ret=1 +grep "status: NOERROR" dig.out.post.test$n > /dev/null || ret=1 +# 3x DNSKEY, 2x RRSIG +grep "ANSWER: 5," dig.out.post.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + n=$((n + 1)) ret=0 echo_i "check that excessive NSEC3PARAM iterations are rejected by nsupdate ($n)" From e59c6a5adc299ee7470aca6591f2e9a94c517abd Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 14 Jun 2023 09:05:31 +0200 Subject: [PATCH 5/5] Update findzonekeys function name in log message The "dns_dnssec_findzonekeys2" log message is a leftover from when that was the name of the function. Rename to match the current name of the function. --- bin/tests/system/multisigner/tests.sh | 8 ++++---- lib/dns/dnssec.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/tests/system/multisigner/tests.sh b/bin/tests/system/multisigner/tests.sh index 5a209e7946..1b3d6cbc90 100644 --- a/bin/tests/system/multisigner/tests.sh +++ b/bin/tests/system/multisigner/tests.sh @@ -142,7 +142,7 @@ status=$((status+ret)) n=$((n+1)) ret=0 echo_i "make sure we did not try to sign with the keys added with nsupdate for zone ${ZONE} ($n)" -grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +grep "dns_dnssec_findzonekeys: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) # Verify again. @@ -168,7 +168,7 @@ status=$((status+ret)) n=$((n+1)) ret=0 echo_i "make sure we did not try to sign with the keys added with nsupdate for zone ${ZONE} ($n)" -grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +grep "dns_dnssec_findzonekeys: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) # Verify again. @@ -464,7 +464,7 @@ test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) dnssec_verify no_dnssec_in_journal -grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +grep "dns_dnssec_findzonekeys: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) # NS4 @@ -475,7 +475,7 @@ test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) dnssec_verify no_dnssec_in_journal -grep "dns_dnssec_findzonekeys2: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 +grep "dns_dnssec_findzonekeys: error reading ./K${ZONE}.*\.private: file not found" "${DIR}/named.run" && ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index ca0bf413f4..8240080a8e 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -858,7 +858,7 @@ dns_dnssec_findzonekeys(dns_db_t *db, dns_dbversion_t *ver, dns_dbnode_t *node, isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_DNSSEC, ISC_LOG_WARNING, - "dns_dnssec_findzonekeys2: error " + "dns_dnssec_findzonekeys: error " "reading %s: %s", filename, isc_result_totext(result)); }