From 5fdad05a8a56323a24ad8939b9eef5142388331f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 15 Aug 2024 10:36:15 +0200 Subject: [PATCH] Verify new key files before running keymgr Prior to running the keymgr, first make sure that existing keys are present in the new keylist. If not, treat this as an operational error where the keys are made offline (temporarily), possibly unwanted. --- bin/tests/system/kasp/tests.sh | 13 +++++- lib/dns/zone.c | 76 ++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index e4851737c2..40b971b261 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -385,7 +385,7 @@ echo_i "test that if private key files are inaccessible this doesn't trigger a r basefile=$(key_get KEY1 BASEFILE) mv "${basefile}.private" "${basefile}.offline" rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed" -wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:verify keys failed: some key files are missing" $DIR/named.run || ret=1 +wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run || ret=1 mv "${basefile}.offline" "${basefile}.private" test "$ret" -eq 0 || echo_i "failed" status=$((status + ret)) @@ -1647,6 +1647,15 @@ check_subdomain dnssec_verify check_rrsig_refresh +# Load again, make sure the purged key is not an issue when verifying keys. +echo_i "load keys for $ZONE, making sure a recently purged key is not an issue when verifying keys ($n)" +ret=0 +rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed" +wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run +grep "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run && ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status + ret)) + # # Zone: legacy-keys.kasp. # @@ -1796,7 +1805,7 @@ rm_keyfiles "KEY1" rm_keyfiles "KEY2" rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed" -wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:verify keys failed: some key files are missing" $DIR/named.run || ret=1 +wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run || ret=1 # Check keys again, make sure no new keys are created. set_policy "autosign" "0" "300" key_clear "KEY1" diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 51eb23890a..ada88d4608 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -371,6 +371,7 @@ struct dns_zone { dns_view_t *prev_view; dns_kasp_t *kasp; dns_kasp_t *defaultkasp; + dns_dnsseckeylist_t keyring; dns_checkmxfunc_t checkmx; dns_checksrvfunc_t checksrv; dns_checknsfunc_t checkns; @@ -1177,6 +1178,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid) { zone->parentals = r; zone->notify = r; zone->defaultkasp = NULL; + ISC_LIST_INIT(zone->keyring); isc_stats_create(mctx, &zone->gluecachestats, dns_gluecachestatscounter_max); @@ -1279,6 +1281,9 @@ zone_free(dns_zone_t *zone) { if (zone->defaultkasp != NULL) { dns_kasp_detach(&zone->defaultkasp); } + if (!ISC_LIST_EMPTY(zone->keyring)) { + clear_keylist(&zone->keyring, zone->mctx); + } if (!ISC_LIST_EMPTY(zone->checkds_ok)) { clear_keylist(&zone->checkds_ok, zone->mctx); } @@ -21914,6 +21919,47 @@ update_ttl(dns_rdataset_t *rdataset, dns_name_t *name, dns_ttl_t ttl, return (ISC_R_SUCCESS); } +static isc_result_t +zone_verifykeys(dns_zone_t *zone, dns_dnsseckeylist_t *newkeys) { + dns_dnsseckey_t *key1, *key2, *next; + + /* + * Make sure that the existing keys are also present in the new keylist. + */ + for (key1 = ISC_LIST_HEAD(zone->keyring); key1 != NULL; key1 = next) { + bool found = false; + next = ISC_LIST_NEXT(key1, link); + + if (dst_key_is_unused(key1->key)) { + continue; + } + if (key1->purge) { + continue; + } + + for (key2 = ISC_LIST_HEAD(*newkeys); key2 != NULL; + key2 = ISC_LIST_NEXT(key2, link)) + { + if (dst_key_compare(key1->key, key2->key)) { + found = true; + break; + } + } + + if (!found) { + char keystr[DST_KEY_FORMATSIZE]; + dst_key_format(key1->key, keystr, sizeof(keystr)); + dnssec_log(zone, ISC_LOG_DEBUG(1), + "verifykeys: key %s - not available", + keystr); + return (ISC_R_NOTFOUND); + } + } + + /* All good. */ + return (ISC_R_SUCCESS); +} + static void remove_rdataset(dns_zone_t *zone, dns_diff_t *diff, dns_rdataset_t *rdataset) { if (!dns_rdataset_isassociated(rdataset)) { @@ -22202,6 +22248,16 @@ zone_rekey(dns_zone_t *zone) { } if (kasp != NULL && !offlineksk) { + /* Verify new keys. */ + isc_result_t ret = zone_verifykeys(zone, &keys); + if (ret != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_ERROR, + "zone_rekey:zone_verifykeys failed: " + "some key files are missing"); + KASP_UNLOCK(kasp); + goto failure; + } + /* * Check DS at parental agents. Clear ongoing checks. */ @@ -22211,8 +22267,8 @@ zone_rekey(dns_zone_t *zone) { ISC_LIST_INIT(zone->checkds_ok); UNLOCK_ZONE(zone); - isc_result_t ret = dns_zone_getdnsseckeys(zone, db, ver, now, - &zone->checkds_ok); + ret = dns_zone_getdnsseckeys(zone, db, ver, now, + &zone->checkds_ok); if (ret == ISC_R_SUCCESS) { zone_checkds(zone); } else { @@ -22224,7 +22280,7 @@ zone_rekey(dns_zone_t *zone) { isc_result_totext(ret)); } - /* Run keymgr */ + /* Run keymgr. */ if (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND) { dns_zone_lock_keyfiles(zone); result = dns_keymgr_run(&zone->origin, zone->rdclass, @@ -22695,10 +22751,14 @@ zone_rekey(dns_zone_t *zone) { } UNLOCK_ZONE(zone); - if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { - for (key = ISC_LIST_HEAD(dnskeys); key != NULL; - key = ISC_LIST_NEXT(key, link)) - { + /* + * Remember which keys have been used. + */ + if (!ISC_LIST_EMPTY(zone->keyring)) { + clear_keylist(&zone->keyring, zone->mctx); + } + while ((key = ISC_LIST_HEAD(dnskeys)) != NULL) { + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { /* This debug log is used in the kasp system test */ char algbuf[DNS_SECALG_FORMATSIZE]; dns_secalg_format(dst_key_alg(key->key), algbuf, @@ -22707,6 +22767,8 @@ zone_rekey(dns_zone_t *zone) { "zone_rekey done: key %d/%s", dst_key_id(key->key), algbuf); } + ISC_LIST_UNLINK(dnskeys, key, link); + ISC_LIST_APPEND(zone->keyring, key, link); } result = ISC_R_SUCCESS;