Fix dnssec-keygen key collision checking for KEY rrtype keys

When generating a new key, dnssec-keygen checks for possible
key ID collisions with existing keys. The dnssec.c:findmatchingkeys()
function, which is supposed to get the list of the existing keys,
fails to do that for the existing KEY rrtype keys (i.e. generated
using 'dnssec-keygen -T KEY') because it doesn't pass down to the
dst_key_fromnamedfile() -> dst_key_read_public() functions the type
of the keys it's interested in. Fix the issue by introducing a new
function parameter which tells in which type of keys the caller is
currently interested in.

diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c
index bea889404524dc1d812909bf481810c45801a6cd..86eb1362fc3bdf3e39d19a1420732f74bfce2e6a 100644 (file)
--- a/bin/dnssec/dnssec-ksr.c
+++ b/bin/dnssec/dnssec-ksr.c
@@ -217,7 +217,8 @@ get_dnskeys(ksr_ctx_t *ksr, dns_dnsseckeylist_t *keys) {
        ISC_LIST_INIT(*keys);
        ISC_LIST_INIT(keys_read);
        ret = dns_dnssec_findmatchingkeys(name, NULL, ksr->keydir, NULL,
-                                         ksr->now, isc_g_mctx, &keys_read);
+                                         ksr->now, false, isc_g_mctx,
+                                         &keys_read);
        if (ret != ISC_R_SUCCESS && ret != ISC_R_NOTFOUND) {
                fatal("failed to load existing keys from %s: %s", ksr->keydir,
                      isc_result_totext(ret));

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index 61bb17be4e16999935155a85ad1997756d10ce6c..d7fcc1ceafa94d427fb06d8ee1b864802b1ec826 100644 (file)
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -2738,7 +2738,8 @@ findkeys:
         * Find keys that match this zone in the key repository.
         */
        result = dns_dnssec_findmatchingkeys(gorigin, NULL, directory, NULL,
-                                            now, isc_g_mctx, &matchkeys);
+                                            now, false, isc_g_mctx,
+                                            &matchkeys);
        if (result == ISC_R_NOTFOUND) {
                result = ISC_R_SUCCESS;
        }

diff --git a/bin/dnssec/dnssectool.c b/bin/dnssec/dnssectool.c
index a0eb9f87025562ee43537dcca04ff7c5241428df..150411ee4ce494ae19fa7dcf7ecd1c4c5f01c8e3 100644 (file)
--- a/bin/dnssec/dnssectool.c
+++ b/bin/dnssec/dnssectool.c
@@ -486,10 +486,17 @@ key_collision(dst_key_t *dstkey, dns_name_t *name, const char *dir,
        }
 
        ISC_LIST_INIT(matchkeys);
-       result = dns_dnssec_findmatchingkeys(name, NULL, dir, NULL, now, mctx,
-                                            &matchkeys);
+       bool keykey = false;
+
+       /*
+        * DNSKEY and KEY both use the same file names patterns so
+        * we have to look for both sets of keys.
+        */
+again:
+       result = dns_dnssec_findmatchingkeys(name, NULL, dir, NULL, now, keykey,
+                                            mctx, &matchkeys);
        if (result == ISC_R_NOTFOUND) {
-               return false;
+               goto try_key;
        }
 
        while (!ISC_LIST_EMPTY(matchkeys) && !conflict) {
@@ -531,6 +538,11 @@ key_collision(dst_key_t *dstkey, dns_name_t *name, const char *dir,
                dns_dnsseckey_destroy(mctx, &key);
        }
 
+try_key:
+       if (!conflict && !keykey) {
+               keykey = true;
+               goto again;
+       }
        return conflict;
 }
 

diff --git a/bin/dnssec/dnssectool.h b/bin/dnssec/dnssectool.h
index 0702034c7e732d540167a6598e6ebd6aae5c9f1c..0b981b6aad2018f4c32171abfeda9f8d85f14b9f 100644 (file)
--- a/bin/dnssec/dnssectool.h
+++ b/bin/dnssec/dnssectool.h
@@ -102,7 +102,7 @@ void
 set_keyversion(dst_key_t *key);
 
 bool
-key_collision(dst_key_t *key, dns_name_t *name, const char *dir,
+key_collision(dst_key_t *dstkey, dns_name_t *name, const char *dir,
              isc_mem_t *mctx, uint16_t min, uint16_t max, bool *exact);
 
 bool

diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
index dffddd9a0b192b6f2eb2e618fdc7f05d94f47c4e..7eed87ce168404d9e143eb63429a1f2f193e95a2 100644 (file)
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -1221,12 +1221,12 @@ dns_dnssec_get_hints(dns_dnsseckey_t *key, isc_stdtime_t now) {
 }
 
 static isc_result_t
-findmatchingkeys(const char *directory, char *namebuf, unsigned int len,
-                isc_mem_t *mctx, isc_stdtime_t now,
+findmatchingkeys(const char *directory, bool rrtypekey, char *namebuf,
+                unsigned int len, isc_mem_t *mctx, isc_stdtime_t now,
                 dns_dnsseckeylist_t *list) {
-       isc_result_t result = ISC_R_SUCCESS;
+       isc_result_t result;
        isc_dir_t dir;
-       bool dir_open = false;
+       bool dir_open = false, match = false;
        unsigned int i, alg;
        dns_dnsseckey_t *key = NULL;
        dst_key_t *dstkey = NULL;
@@ -1235,6 +1235,7 @@ findmatchingkeys(const char *directory, char *namebuf, unsigned int len,
        if (directory == NULL) {
                directory = ".";
        }
+
        RETERR(isc_dir_open(&dir, directory));
        dir_open = true;
 
@@ -1283,11 +1284,13 @@ findmatchingkeys(const char *directory, char *namebuf, unsigned int len,
                        continue;
                }
 
+               int type = DST_TYPE_PUBLIC | DST_TYPE_PRIVATE | DST_TYPE_STATE;
+               if (rrtypekey) {
+                       type |= DST_TYPE_KEY;
+               }
                dstkey = NULL;
-               result = dst_key_fromnamedfile(
-                       dir.entry.name, directory,
-                       DST_TYPE_PUBLIC | DST_TYPE_PRIVATE | DST_TYPE_STATE,
-                       mctx, &dstkey);
+               result = dst_key_fromnamedfile(dir.entry.name, directory, type,
+                                              mctx, &dstkey);
 
                switch (alg) {
                case DST_ALG_HMACMD5:
@@ -1319,9 +1322,11 @@ findmatchingkeys(const char *directory, char *namebuf, unsigned int len,
                        dns_dnsseckey_destroy(mctx, &key);
                } else {
                        ISC_LIST_APPEND(*list, key, link);
+                       match = true;
                        key = NULL;
                }
        }
+       result = match ? ISC_R_SUCCESS : ISC_R_NOTFOUND;
 
 failure:
        if (dir_open) {
@@ -1334,12 +1339,13 @@ failure:
 }
 
 /*%
- * Get a list of DNSSEC keys from the key repository.
+ * Get a list of KEY or DNSKEY keys from the key repository. If rrtypekey
+ * is true KEY keys will be returned otherwise DNSSEC keys.
  */
 isc_result_t
 dns_dnssec_findmatchingkeys(const dns_name_t *origin, dns_kasp_t *kasp,
                            const char *keydir, dns_keystorelist_t *keystores,
-                           isc_stdtime_t now, isc_mem_t *mctx,
+                           isc_stdtime_t now, bool rrtypekey, isc_mem_t *mctx,
                            dns_dnsseckeylist_t *keylist) {
        isc_result_t result = ISC_R_SUCCESS;
        dns_dnsseckeylist_t list;
@@ -1358,8 +1364,8 @@ dns_dnssec_findmatchingkeys(const dns_name_t *origin, dns_kasp_t *kasp,
        if (kasp == NULL || (strcmp(dns_kasp_getname(kasp), "none") == 0) ||
            (strcmp(dns_kasp_getname(kasp), "insecure") == 0))
        {
-               RETERR(findmatchingkeys(keydir, namebuf, len, mctx, now,
-                                       &list));
+               RETERR(findmatchingkeys(keydir, rrtypekey, namebuf, len, mctx,
+                                       now, &list));
        } else if (keystores != NULL) {
                ISC_LIST_FOREACH(*keystores, keystore, link) {
                        ISC_LIST_FOREACH(dns_kasp_keys(kasp), kkey, link) {
@@ -1368,8 +1374,8 @@ dns_dnssec_findmatchingkeys(const dns_name_t *origin, dns_kasp_t *kasp,
                                                dns_keystore_directory(keystore,
                                                                       keydir);
                                        RETERR(findmatchingkeys(
-                                               directory, namebuf, len, mctx,
-                                               now, &list));
+                                               directory, rrtypekey, namebuf,
+                                               len, mctx, now, &list));
                                        break;
                                }
                        }

diff --git a/lib/dns/include/dns/dnssec.h b/lib/dns/include/dns/dnssec.h
index 38883b6343dcb115d26ff9b0fe4d0dd7266041ff..eed02b00da5c8a5e27c092a650c09a2ffc667b28 100644 (file)
--- a/lib/dns/include/dns/dnssec.h
+++ b/lib/dns/include/dns/dnssec.h
@@ -295,13 +295,16 @@ dns_dnssec_get_hints(dns_dnsseckey_t *key, isc_stdtime_t now);
 isc_result_t
 dns_dnssec_findmatchingkeys(const dns_name_t *origin, dns_kasp_t *kasp,
                            const char *keydir, dns_keystorelist_t *keystores,
-                           isc_stdtime_t now, isc_mem_t *mctx,
+                           isc_stdtime_t now, bool rrtypekey, isc_mem_t *mctx,
                            dns_dnsseckeylist_t *keylist);
 /*%<
  * Search for K* key files matching the name in 'origin'. If 'kasp' is not
  * NULL, search in the directories used in 'keystores'. Otherwise search in the
  * key-directory 'keydir'.
  *
+ * If 'rrtypekey' is true, then KEY type keys are matched (e.g. for SIG(0)),
+ * otherwise DNSKEY type keys are matched for DNSSEC.
+ *
  * Append all such keys, along with use hints gleaned from their
  * metadata, onto 'keylist'.  Skip any unsupported algorithms.
  *

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index 63177b68d5557289780531deaf8714404982c47b..892fcaafda7065bf32c8be6eb5b5949fadd483ea 100644 (file)
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -496,7 +496,8 @@ static isc_result_t
 keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
                 dns_kasp_t *kasp, dns_rdataclass_t rdclass, isc_mem_t *mctx,
                 const char *keydir, dns_dnsseckeylist_t *keylist,
-                dns_dnsseckeylist_t *newkeys, dst_key_t **dst_key) {
+                isc_stdtime_t now, dns_dnsseckeylist_t *newkeys,
+                dst_key_t **dst_key) {
        isc_result_t result = ISC_R_SUCCESS;
        bool conflict = false;
        int flags = DNS_KEYOWNER_ZONE;
@@ -505,11 +506,23 @@ keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
        dns_keystore_t *keystore = dns_kasp_key_keystore(kkey);
        const char *dir = NULL;
        int size = dns_kasp_key_size(kkey);
+       dns_dnsseckeylist_t keykeys;
+
+       ISC_LIST_INIT(keykeys);
 
        if (dns_kasp_key_ksk(kkey)) {
                flags |= DNS_KEYFLAG_KSK;
        }
 
+       /*
+        * We also need to check against K* files for KEYs.
+        */
+       result = dns_dnssec_findmatchingkeys(origin, NULL, keydir, NULL, now,
+                                            true, mctx, &keykeys);
+       if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
+               goto failure;
+       }
+
        do {
                if (keystore == NULL) {
                        RETERR(dst_key_generate(origin, alg, size, 0, flags,
@@ -524,6 +537,10 @@ keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
                /* Key collision? */
                conflict = keymgr_keyid_conflict(newkey, kkey->tag_min,
                                                 kkey->tag_max, keylist);
+               if (!conflict) {
+                       conflict = keymgr_keyid_conflict(
+                               newkey, kkey->tag_min, kkey->tag_max, &keykeys);
+               }
                if (!conflict) {
                        conflict = keymgr_keyid_conflict(
                                newkey, kkey->tag_min, kkey->tag_max, newkeys);
@@ -548,9 +565,14 @@ keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
                dst_key_setdirectory(newkey, dir);
        }
        *dst_key = newkey;
-       return ISC_R_SUCCESS;
+       result = ISC_R_SUCCESS;
 
 failure:
+       while (!ISC_LIST_EMPTY(keykeys)) {
+               dns_dnsseckey_t *key = ISC_LIST_HEAD(keykeys);
+               ISC_LIST_UNLINK(keykeys, key, link);
+               dns_dnsseckey_destroy(mctx, &key);
+       }
        return result;
 }
 
@@ -1920,9 +1942,9 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key,
                bool csk = (dns_kasp_key_ksk(kaspkey) &&
                            dns_kasp_key_zsk(kaspkey));
 
-               isc_result_t result =
-                       keymgr_createkey(kaspkey, origin, kasp, rdclass, mctx,
-                                        keydir, keyring, newkeys, &dst_key);
+               isc_result_t result = keymgr_createkey(
+                       kaspkey, origin, kasp, rdclass, mctx, keydir, keyring,
+                       now, newkeys, &dst_key);
                if (result != ISC_R_SUCCESS) {
                        return result;
                }

diff --git a/lib/dns/update.c b/lib/dns/update.c
index f228a96ceac6e85e5eabe5efd351045fdcb89606..ff4ae03c61e445c71227782ebec77bc12a3d92ad 100644 (file)
--- a/lib/dns/update.c
+++ b/lib/dns/update.c
@@ -1035,8 +1035,8 @@ find_zone_keys(dns_zone_t *zone, isc_mem_t *mctx, unsigned int maxkeys,
 
        dns_zone_lock_keyfiles(zone);
        result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), kasp,
-                                            keydir, keystores, now, mctx,
-                                            &keylist);
+                                            keydir, keystores, now, false,
+                                            mctx, &keylist);
        dns_zone_unlock_keyfiles(zone);
 
        if (result != ISC_R_SUCCESS) {

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index ff9d9a4ada971ad5de8d32a507a0c427f5cee344..b14b7fffb48ea92b6abce4bda1fccc1a9855628d 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -6996,9 +6996,9 @@ dns_zone_getdnsseckeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
 
        /* Get keys from private key files. */
        dns_zone_lock_keyfiles(zone);
-       result = dns_dnssec_findmatchingkeys(origin, kasp, dir,
-                                            dns_zone_getkeystores(zone), now,
-                                            dns_zone_getmctx(zone), keys);
+       result = dns_dnssec_findmatchingkeys(
+               origin, kasp, dir, dns_zone_getkeystores(zone), now, false,
+               dns_zone_getmctx(zone), keys);
        dns_zone_unlock_keyfiles(zone);
 
        if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
@@ -16819,8 +16819,8 @@ dns_zone_dnskey_inuse(dns_zone_t *zone, dns_rdata_t *rdata, bool *inuse) {
 
        dns_zone_lock_keyfiles(zone);
        result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), kasp,
-                                            keydir, keystores, now, mctx,
-                                            &keylist);
+                                            keydir, keystores, now, false,
+                                            mctx, &keylist);
        dns_zone_unlock_keyfiles(zone);
        if (result == ISC_R_NOTFOUND) {
                return ISC_R_SUCCESS;
@@ -22512,7 +22512,7 @@ zone_rekey(dns_zone_t *zone) {
        dns_zone_lock_keyfiles(zone);
        result = dns_dnssec_findmatchingkeys(&zone->origin, kasp, dir,
                                             dns_zone_getkeystores(zone), now,
-                                            mctx, &keys);
+                                            false, mctx, &keys);
        dns_zone_unlock_keyfiles(zone);
 
        if (result != ISC_R_SUCCESS) {

diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c
index fe607b20a7d79a7c7d656e67547866923eebaec3..0e044a3b087144418282d71c709f02268fa4c17e 100644 (file)
--- a/lib/isccfg/check.c
+++ b/lib/isccfg/check.c
@@ -2962,7 +2962,7 @@ check_keydir(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 
                ISC_LIST_INIT(keys);
                ret = dns_dnssec_findmatchingkeys(origin, kasp, keydir, &kslist,
-                                                 now, mctx, &keys);
+                                                 now, false, mctx, &keys);
                if (ret != ISC_R_SUCCESS) {
                        result = ret;
                }

diff --git a/tests/dns/skr_test.c b/tests/dns/skr_test.c
index 3de994ab791b08140072f72ebf66435898e2ddbc..ec41f3b3dd3a1db6d8e78b24495831cec0826248 100644 (file)
--- a/tests/dns/skr_test.c
+++ b/tests/dns/skr_test.c
@@ -448,7 +448,7 @@ ISC_RUN_TEST_IMPL(skr_read) {
        ISC_LIST_INIT(keys);
        result = dns_dnssec_findmatchingkeys(dname, NULL,
                                             TESTS_DIR "/testdata/skr/", NULL,
-                                            0, isc_g_mctx, &keys);
+                                            0, false, isc_g_mctx, &keys);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        /* Create/read the SKR file */