From: Matthijs Mekking Date: Mon, 19 Apr 2021 14:32:40 +0000 (+0200) Subject: Lock kasp when looking for zone keys X-Git-Tag: v9.17.14~39^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=252a1ae0a17317318a95d90768689a8807074623;p=thirdparty%2Fbind9.git Lock kasp when looking for zone keys We should also lock kasp when reading key files, because at the same time the zone in another view may be updating the key file. --- diff --git a/bin/named/server.c b/bin/named/server.c index 1bd3bb505a8..4be656b1f16 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -15162,10 +15162,10 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, CHECK(dns_db_findnode(db, origin, false, &node)); dns_db_currentversion(db, &version); /* Get keys from private key files. */ - LOCK(&kasp->lock); - result = dns_dnssec_findmatchingkeys(origin, dir, now, + dns_zone_lock_keyfiles(zone); + result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), dir, now, dns_zone_getmctx(zone), &keys); - UNLOCK(&kasp->lock); + dns_zone_unlock_keyfiles(zone); if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) { goto cleanup; } diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index de9ffda6206..c417c25a8f5 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -363,6 +363,24 @@ dns_zone_getmaxttl(dns_zone_t *zone); *\li dns_ttl_t maxttl. */ +void +dns_zone_lock_keyfiles(dns_zone_t *zone); +/*%< + * Lock associated keyfiles for this zone. + * + * Require: + *\li 'zone' to be a valid zone. + */ + +void +dns_zone_unlock_keyfiles(dns_zone_t *zone); +/*%< + * Unlock associated keyfiles for this zone. + * + * Require: + *\li 'zone' to be a valid zone. + */ + isc_result_t dns_zone_load(dns_zone_t *zone, bool newonly); diff --git a/lib/dns/update.c b/lib/dns/update.c index 6bd84823e24..71ef7dde467 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1064,11 +1064,16 @@ find_zone_keys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, isc_stdtime_t 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_stdtime_get(&now); - CHECK(dns_dnssec_findzonekeys(db, ver, node, dns_db_origin(db), - directory, now, mctx, maxkeys, keys, - nkeys)); + + dns_zone_lock_keyfiles(zone); + result = dns_dnssec_findzonekeys(db, ver, node, dns_db_origin(db), + directory, now, mctx, maxkeys, keys, + nkeys); + dns_zone_unlock_keyfiles(zone); + failure: if (node != NULL) { dns_db_detachnode(db, &node); diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 2a702765fc4..2900281a727 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1276,6 +1276,7 @@ dns_zone_keydone dns_zone_link dns_zone_load dns_zone_loadandthaw +dns_zone_lock_keyfiles dns_zone_log dns_zone_logc dns_zone_logv @@ -1376,6 +1377,7 @@ dns_zone_setzeronosoattl dns_zone_signwithkey dns_zone_synckeyzone dns_zone_unload +dns_zone_unlock_keyfiles dns_zone_verifydb dns_zonekey_iszonekey dns_zonemgr_attach diff --git a/lib/dns/zone.c b/lib/dns/zone.c index f621e15898a..bea3e709b6a 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -123,6 +123,19 @@ #define ID(x) dst_key_id(x) #define ALG(x) dst_key_alg(x) +/*% + * KASP flags + */ +#define KASP_LOCK(k) \ + if ((k) != NULL) { \ + LOCK((&((k)->lock))); \ + } + +#define KASP_UNLOCK(k) \ + if ((k) != NULL) { \ + UNLOCK((&((k)->lock))); \ + } + /* * Default values. */ @@ -191,6 +204,9 @@ typedef struct dns_include dns_include_t; #define ZONEDB_LOCK(l, t) RWLOCK((l), (t)) #define ZONEDB_UNLOCK(l, t) RWUNLOCK((l), (t)) +#define LOCK_KEYFILES(z) LOCK(&(z)->keyflock) +#define UNLOCK_KEYFILES(z) UNLOCK(&(z)->keyflock) + #ifdef ENABLE_AFL extern bool dns_fuzzing_resolver; #endif /* ifdef ENABLE_AFL */ @@ -199,6 +215,7 @@ struct dns_zone { /* Unlocked */ unsigned int magic; isc_mutex_t lock; + isc_mutex_t keyflock; #ifdef DNS_ZONE_CHECKLOCK bool locked; #endif /* ifdef DNS_ZONE_CHECKLOCK */ @@ -1041,6 +1058,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { zone->mctx = NULL; isc_mem_attach(mctx, &zone->mctx); isc_mutex_init(&zone->lock); + isc_mutex_init(&zone->keyflock); ZONEDB_INITLOCK(&zone->dblock); /* XXX MPA check that all elements are initialised */ #ifdef DNS_ZONE_CHECKLOCK @@ -1102,6 +1120,7 @@ free_refs: isc_refcount_destroy(&zone->erefs); isc_refcount_destroy(&zone->irefs); ZONEDB_DESTROYLOCK(&zone->dblock); + isc_mutex_destroy(&zone->keyflock); isc_mutex_destroy(&zone->lock); isc_mem_putanddetach(&zone->mctx, zone, sizeof(*zone)); return (result); @@ -1276,6 +1295,7 @@ zone_free(dns_zone_t *zone) { /* last stuff */ ZONEDB_DESTROYLOCK(&zone->dblock); + isc_mutex_destroy(&zone->keyflock); isc_mutex_destroy(&zone->lock); zone->magic = 0; isc_mem_putanddetach(&zone->mctx, zone, sizeof(*zone)); @@ -6341,6 +6361,58 @@ was_dumping(dns_zone_t *zone) { return (false); } +static void +dns__zone_lockunlock_keyfiles(dns_zone_t *zone, bool lock) { + dns_viewlist_t *vlist = NULL; + dns_view_t *v = NULL; + + REQUIRE(DNS_ZONE_VALID(zone)); + + if (zone->kasp == NULL) { + /* No need to lock, nothing is writing key files. */ + return; + } + + if (zone->view == NULL || zone->view->viewlist == NULL) { + if (lock) { + LOCK_KEYFILES(zone); + } else { + UNLOCK_KEYFILES(zone); + } + return; + } + + /* + * Also lock keyfiles for zones with the same name in a different view. + */ + vlist = zone->view->viewlist; + for (v = ISC_LIST_HEAD(*vlist); v != NULL; v = ISC_LIST_NEXT(v, link)) { + dns_zone_t *z = NULL; + isc_result_t ret = dns_view_findzone(v, &zone->origin, &z); + if (ret == ISC_R_SUCCESS) { + INSIST(DNS_ZONE_VALID(z)); + + /* WMM check if policy is the same? */ + if (lock) { + LOCK_KEYFILES(z); + } else { + UNLOCK_KEYFILES(z); + } + dns_zone_detach(&z); + } + } +} + +void +dns_zone_lock_keyfiles(dns_zone_t *zone) { + dns__zone_lockunlock_keyfiles(zone, true); +} + +void +dns_zone_unlock_keyfiles(dns_zone_t *zone) { + dns__zone_lockunlock_keyfiles(zone, false); +} + /*% * Find up to 'maxkeys' DNSSEC keys used for signing version 'ver' of database * 'db' for zone 'zone' in its key directory, then load these keys into 'keys'. @@ -6357,13 +6429,21 @@ dns__zone_findkeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, CHECK(dns_db_findnode(db, dns_db_origin(db), false, &node)); memset(keys, 0, sizeof(*keys) * maxkeys); + + dns_zone_lock_keyfiles(zone); + result = dns_dnssec_findzonekeys(db, ver, node, dns_db_origin(db), directory, now, mctx, maxkeys, keys, nkeys); + + dns_zone_unlock_keyfiles(zone); + if (result == ISC_R_NOTFOUND) { result = ISC_R_SUCCESS; } + failure: + if (node != NULL) { dns_db_detachnode(db, &node); } @@ -7303,7 +7383,7 @@ signed_with_good_key(dns_zone_t *zone, dns_db_t *db, dns_dbnode_t *node, int zsk_count = 0; bool approved; - LOCK(&kasp->lock); + KASP_LOCK(kasp); for (kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; kkey = ISC_LIST_NEXT(kkey, link)) { @@ -7314,7 +7394,7 @@ signed_with_good_key(dns_zone_t *zone, dns_db_t *db, dns_dbnode_t *node, zsk_count++; } } - UNLOCK(&kasp->lock); + KASP_UNLOCK(kasp); if (type == dns_rdatatype_dnskey || type == dns_rdatatype_cdnskey || type == dns_rdatatype_cds) @@ -19988,6 +20068,8 @@ zone_rekey(dns_zone_t *zone) { TIME_NOW(&timenow); now = isc_time_seconds(&timenow); + kasp = dns_zone_getkasp(zone); + dnssec_log(zone, ISC_LOG_INFO, "reconfiguring zone keys"); /* Get the SOA record's TTL */ @@ -20001,9 +20083,18 @@ zone_rekey(dns_zone_t *zone) { dns_rdatatype_none, 0, &keyset, &keysigs); if (result == ISC_R_SUCCESS) { ttl = keyset.ttl; - CHECK(dns_dnssec_keylistfromrdataset( + + dns_zone_lock_keyfiles(zone); + + result = dns_dnssec_keylistfromrdataset( &zone->origin, dir, mctx, &keyset, &keysigs, &soasigs, - false, false, &dnskeys)); + false, false, &dnskeys); + + dns_zone_unlock_keyfiles(zone); + + if (result != ISC_R_SUCCESS) { + goto failure; + } } else if (result != ISC_R_NOTFOUND) { goto failure; } @@ -20028,10 +20119,8 @@ zone_rekey(dns_zone_t *zone) { */ fullsign = DNS_ZONEKEY_OPTION(zone, DNS_ZONEKEY_FULLSIGN); - kasp = dns_zone_getkasp(zone); - if (kasp != NULL) { - LOCK(&kasp->lock); - } + KASP_LOCK(kasp); + dns_zone_lock_keyfiles(zone); result = dns_dnssec_findmatchingkeys(&zone->origin, dir, now, mctx, &keys); @@ -20051,13 +20140,16 @@ zone_rekey(dns_zone_t *zone) { "zone_rekey:dns_dnssec_keymgr " "failed: %s", isc_result_totext(result)); - UNLOCK(&kasp->lock); + dns_zone_unlock_keyfiles(zone); + KASP_UNLOCK(kasp); goto failure; } } - UNLOCK(&kasp->lock); } + dns_zone_unlock_keyfiles(zone); + KASP_UNLOCK(kasp); + if (result == ISC_R_SUCCESS) { bool cds_delete = false; isc_stdtime_t when;