]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Verify new key files before running keymgr
authorMatthijs Mekking <matthijs@isc.org>
Thu, 15 Aug 2024 08:36:15 +0000 (10:36 +0200)
committerMatthijs Mekking <matthijs@isc.org>
Fri, 11 Oct 2024 15:42:00 +0000 (17:42 +0200)
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
lib/dns/zone.c

index e4851737c209ac5a48e617b83d1017d613342309..40b971b261e84866f8d60b938472145c8aeacf69 100644 (file)
@@ -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"
index 51eb23890a3a44ec057b4fc9cee1eba5de031532..ada88d4608000458f0ff9ff74b0ae1c84df9a961 100644 (file)
@@ -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;