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 1bd3bb505a8e5cf3873e7f6c2dbd8485ccccf147..4be656b1f167c38bf1516390d97698f1d99edf3c 100644 (file)
--- 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 de9ffda6206de170bea2f0876de881101bd31a28..c417c25a8f5d2fff14ad14f7d105724786054228 100644 (file)
--- 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 6bd84823e24bec89ff879b73f317fc269c52fece..71ef7dde467c77805988a5609c2f136260cc9edc 100644 (file)
--- 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 2a702765fc4e8fd8537374a3fc970e9b783c306d..2900281a727c91f302c0df617a8d40c9073bceba 100644 (file)
--- 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 f621e15898ad6faada00a987ff0365755e487424..bea3e709b6a3dff860388cbed5d19fbd48863f77 100644 (file)
--- 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;