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.

(cherry picked from commit 49b7ce9a54790aa3adcf2f7028d8e62576bd527a)

diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c
index 3d355887084aadb6af5387997f8e8dd845077613..584965dae1fe9d230f6c4bfac2a4b486e1f24d11 100644 (file)
--- a/bin/dnssec/dnssec-ksr.c
+++ b/bin/dnssec/dnssec-ksr.c
@@ -224,7 +224,7 @@ 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, mctx, &keys_read);
+                                         ksr->now, false, 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 49d013afef3cd0f1f1223897610a0b7b27171191..10f7f7d242c233b0f7cd252c4d233317a1225e0e 100644 (file)
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -2852,7 +2852,7 @@ findkeys:
         * Find keys that match this zone in the key repository.
         */
        result = dns_dnssec_findmatchingkeys(gorigin, NULL, directory, NULL,
-                                            now, mctx, &matchkeys);
+                                            now, false, mctx, &matchkeys);
        if (result == ISC_R_NOTFOUND) {
                result = ISC_R_SUCCESS;
        }

diff --git a/bin/dnssec/dnssectool.c b/bin/dnssec/dnssectool.c
index 723abd8f25578f51163caa1ad792f6a911f21ebb..5a0f81c97ca4b668be1bac4bf10e57ea43036c74 100644 (file)
--- a/bin/dnssec/dnssectool.c
+++ b/bin/dnssec/dnssectool.c
@@ -511,10 +511,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) {
@@ -558,6 +565,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 8b0f0386e0e6253e3676eba604c1d727a762bebd..1df15a9a115535a01197a5325a60b0a8e3159484 100644 (file)
--- a/bin/dnssec/dnssectool.h
+++ b/bin/dnssec/dnssectool.h
@@ -108,7 +108,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 bc17c2c3a29aa90eadfaf325a0f7ae660905201e..c6f5d5c7fabfdded82237a6d30ac514f091f2e50 100644 (file)
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -1199,12 +1199,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;
@@ -1213,6 +1213,7 @@ findmatchingkeys(const char *directory, char *namebuf, unsigned int len,
        if (directory == NULL) {
                directory = ".";
        }
+
        RETERR(isc_dir_open(&dir, directory));
        dir_open = true;
 
@@ -1261,11 +1262,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:
@@ -1297,9 +1300,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) {
@@ -1312,12 +1317,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;
@@ -1337,8 +1343,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) {
                for (dns_keystore_t *keystore = ISC_LIST_HEAD(*keystores);
                     keystore != NULL; keystore = ISC_LIST_NEXT(keystore, link))
@@ -1352,8 +1358,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 55e7fb842b90db04f4a02e836482df940715383f..9be260bac1b82b5e8f958d5925a2dd6317a07bd5 100644 (file)
--- a/lib/dns/include/dns/dnssec.h
+++ b/lib/dns/include/dns/dnssec.h
@@ -285,13 +285,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 99ff4de571a16ebc0832a894f8198e84bc3e23fe..42d3ffa93d938277ec105ed733ad5191b291a503 100644 (file)
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -500,7 +500,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;
@@ -509,11 +510,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,
@@ -528,6 +541,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);
@@ -552,9 +569,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;
 }
 
@@ -1945,9 +1967,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 4d554f263b3ab998c9162b98afa9958c13b9ead1..a821bf4f24b68fbfe64ee5d7128148b8d3207ac2 100644 (file)
--- a/lib/dns/update.c
+++ b/lib/dns/update.c
@@ -1068,8 +1068,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 cd90250bb2888d0360b4c77c7687eecb53653c4a..6188e1fb3c6bb10cc2c7f385f55f06fa1b7084a4 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -6590,7 +6590,8 @@ 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, zone->keystores,
-                                            now, dns_zone_getmctx(zone), keys);
+                                            now, false, dns_zone_getmctx(zone),
+                                            keys);
        dns_zone_unlock_keyfiles(zone);
 
        if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
@@ -16596,8 +16597,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;
@@ -22622,7 +22623,8 @@ zone_rekey(dns_zone_t *zone) {
 
        dns_zone_lock_keyfiles(zone);
        result = dns_dnssec_findmatchingkeys(&zone->origin, kasp, dir,
-                                            zone->keystores, now, mctx, &keys);
+                                            zone->keystores, now, 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 fccfd1d206e1c3fe3e0934a0d3c7b416f42c3007..74a48da8fdca63a945419d0901bec54ca1a9e1ed 100644 (file)
--- a/lib/isccfg/check.c
+++ b/lib/isccfg/check.c
@@ -3174,7 +3174,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 7230155b453a999d1f1d616dc071a62a1055f105..4a40410c718dabe9ecf7a16495aa88298b870841 100644 (file)
--- a/tests/dns/skr_test.c
+++ b/tests/dns/skr_test.c
@@ -453,8 +453,9 @@ ISC_RUN_TEST_IMPL(skr_read) {
 
        /* Get the KSK */
        ISC_LIST_INIT(keys);
-       result = dns_dnssec_findmatchingkeys(
-               dname, NULL, TESTS_DIR "/testdata/skr/", NULL, 0, mctx, &keys);
+       result = dns_dnssec_findmatchingkeys(dname, NULL,
+                                            TESTS_DIR "/testdata/skr/", NULL,
+                                            0, false, mctx, &keys);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        /* Create/read the SKR file */