From 40911771811a7dc535762203f76809da145c0cf8 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. (cherry picked from commit 5fdad05a8a56323a24ad8939b9eef5142388331f) --- 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 461e2b9cbb..08ba2dcfe2 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -379,7 +379,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)) @@ -1592,6 +1592,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. # @@ -1741,7 +1750,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 763be1ec72..c40df00b2c 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -376,6 +376,7 @@ struct dns_zone { dns_view_t *view; dns_view_t *prev_view; dns_kasp_t *kasp; + dns_dnsseckeylist_t keyring; dns_checkmxfunc_t checkmx; dns_checksrvfunc_t checksrv; dns_checknsfunc_t checkns; @@ -1190,6 +1191,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { ISC_LIST_INIT(zone->forwards); ISC_LIST_INIT(zone->rss_events); ISC_LIST_INIT(zone->rss_post); + ISC_LIST_INIT(zone->keyring); result = isc_stats_create(mctx, &zone->gluecachestats, dns_gluecachestatscounter_max); @@ -1315,6 +1317,9 @@ zone_free(dns_zone_t *zone) { if (zone->kasp != NULL) { dns_kasp_detach(&zone->kasp); } + 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); } @@ -21992,6 +21997,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 zone_rekey(dns_zone_t *zone) { isc_result_t result; @@ -22159,6 +22205,16 @@ zone_rekey(dns_zone_t *zone) { } if (kasp != NULL) { + /* 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. */ @@ -22168,8 +22224,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 { @@ -22181,7 +22237,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, @@ -22624,10 +22680,14 @@ zone_rekey(dns_zone_t *zone) { } UNLOCK_ZONE(zone); - if (isc_log_wouldlog(dns_lctx, 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(dns_lctx, 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, @@ -22636,6 +22696,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;