From ffc41d1b140a8b859abf33f60ca46b5d38445d5d Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 18 Feb 2022 15:03:49 +0100 Subject: [PATCH] Store key store reference instead of name When creating the kasp structure, instead of storing the name of the key store on keys, store a reference to the key store object instead. This requires to build the keystore list prior to creating the kasp structures, in the dnssec tools, the check code and the server code. We will create a builtin keystore called "key-directory" which means use the zone's key-directory as the key store. The check code changes, because now the keystore is looked up before creating the kasp structure (and if the keystore is not found, this is an error). Instead of looking up the keystore after all 'dnssec-policy' clauses have been read. --- bin/dnssec/dnssec-keygen.c | 39 +++++++- bin/named/server.c | 33 +++++-- lib/dns/include/dns/kasp.h | 13 +-- lib/dns/kasp.c | 7 +- lib/isccfg/check.c | 138 ++++++++++----------------- lib/isccfg/include/isccfg/kaspconf.h | 14 ++- lib/isccfg/kaspconf.c | 71 +++++++++++--- 7 files changed, 191 insertions(+), 124 deletions(-) diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 6e5dd34769..9d32558ac2 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -256,13 +256,39 @@ progress(int p) { static void kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name, dns_kasp_t **kaspp) { + isc_result_t result = ISC_R_NOTFOUND; const cfg_listelt_t *element; const cfg_obj_t *kasps = NULL; dns_kasp_t *kasp = NULL, *kasp_next; - isc_result_t result = ISC_R_NOTFOUND; dns_kasplist_t kasplist; + const cfg_obj_t *keystores = NULL; + dns_keystore_t *ks = NULL, *ks_next; + dns_keystorelist_t kslist; ISC_LIST_INIT(kasplist); + ISC_LIST_INIT(kslist); + + (void)cfg_map_get(config, "key-store", &keystores); + for (element = cfg_list_first(keystores); element != NULL; + element = cfg_list_next(element)) + { + cfg_obj_t *kconfig = cfg_listelt_value(element); + ks = NULL; + result = cfg_keystore_fromconfig(kconfig, mctx, lctx, &kslist, + &ks); + if (result != ISC_R_SUCCESS) { + fatal("failed to configure key-store '%s': %s", + cfg_obj_asstring(cfg_tuple_get(kconfig, "name")), + isc_result_totext(result)); + } + INSIST(ks != NULL); + dns_keystore_detach(&ks); + } + /* Default key-directory key store. */ + ks = NULL; + (void)cfg_keystore_fromconfig(NULL, mctx, lctx, engine, &kslist, &ks); + INSIST(ks != NULL); + dns_keystore_detach(&ks); (void)cfg_map_get(config, "dnssec-policy", &kasps); for (element = cfg_list_first(kasps); element != NULL; @@ -277,7 +303,7 @@ kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name, } result = cfg_kasp_fromconfig(kconfig, NULL, true, mctx, lctx, - &kasplist, &kasp); + &kslist, &kasplist, &kasp); if (result != ISC_R_SUCCESS) { fatal("failed to configure dnssec-policy '%s': %s", cfg_obj_asstring(cfg_tuple_get(kconfig, "name")), @@ -298,6 +324,15 @@ kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name, ISC_LIST_UNLINK(kasplist, kasp, link); dns_kasp_detach(&kasp); } + + /* + * Cleanup keystore list. + */ + for (ks = ISC_LIST_HEAD(kslist); ks != NULL; ks = ks_next) { + ks_next = ISC_LIST_NEXT(ks, link); + ISC_LIST_UNLINK(kslist, ks, link); + dns_keystore_detach(&ks); + } } static void diff --git a/bin/named/server.c b/bin/named/server.c index d506c56e47..1b6037d9b3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8888,6 +8888,19 @@ load_configuration(const char *filename, named_server_t *server, */ (void)configure_session_key(maps, server, named_g_mctx, first_time); + /* + * Create the built-in key store ("key-directory"). + */ + keystore = NULL; + result = cfg_keystore_fromconfig(NULL, named_g_mctx, named_g_lctx, + named_g_engine, &keystorelist, + &keystore); + if (result != ISC_R_SUCCESS) { + goto cleanup_keystorelist; + } + INSIST(keystore != NULL); + dns_keystore_detach(&keystore); + /* * Create the DNSSEC key stores. */ @@ -8899,17 +8912,14 @@ load_configuration(const char *filename, named_server_t *server, cfg_obj_t *kconfig = cfg_listelt_value(element); keystore = NULL; result = cfg_keystore_fromconfig(kconfig, named_g_mctx, - named_g_lctx, &keystorelist, - &keystore)); + named_g_lctx, named_g_engine, + &keystorelist, &keystore); if (result != ISC_R_SUCCESS) { goto cleanup_keystorelist; } INSIST(keystore != NULL); dns_keystore_detach(&keystore); } - tmpkeystorelist = server->keystorelist; - server->keystorelist = keystorelist; - keystorelist = tmpkeystorelist; /* * Create the built-in kasp policies ("default", "insecure"). @@ -8924,7 +8934,7 @@ load_configuration(const char *filename, named_server_t *server, kasp = NULL; result = cfg_kasp_fromconfig(kconfig, default_kasp, true, named_g_mctx, named_g_lctx, - &kasplist, &kasp); + &keystorelist, &kasplist, &kasp); if (result != ISC_R_SUCCESS) { goto cleanup_kasplist; } @@ -8953,7 +8963,7 @@ load_configuration(const char *filename, named_server_t *server, kasp = NULL; result = cfg_kasp_fromconfig(kconfig, default_kasp, true, named_g_mctx, named_g_lctx, - &kasplist, &kasp); + &keystorelist, &kasplist, &kasp); if (result != ISC_R_SUCCESS) { goto cleanup_kasplist; } @@ -8961,8 +8971,15 @@ load_configuration(const char *filename, named_server_t *server, dns_kasp_freeze(kasp); dns_kasp_detach(&kasp); } - dns_kasp_detach(&default_kasp); + + /* + * Save keystore list and kasp list. + */ + tmpkeystorelist = server->keystorelist; + server->keystorelist = keystorelist; + keystorelist = tmpkeystorelist; + tmpkasplist = server->kasplist; server->kasplist = kasplist; kasplist = tmpkasplist; diff --git a/lib/dns/include/dns/kasp.h b/lib/dns/include/dns/kasp.h index 288c754c9f..42fe126396 100644 --- a/lib/dns/include/dns/kasp.h +++ b/lib/dns/include/dns/kasp.h @@ -30,6 +30,7 @@ #include #include +#include #include ISC_LANG_BEGINDECLS @@ -51,11 +52,11 @@ struct dns_kasp_key { ISC_LINK(struct dns_kasp_key) link; /* Configuration */ - const char *keystore; - uint32_t lifetime; - uint8_t algorithm; - int length; - uint8_t role; + dns_keystore_t *keystore; + uint32_t lifetime; + uint8_t algorithm; + int length; + uint8_t role; }; struct dns_kasp_nsec3param { @@ -644,7 +645,7 @@ dns_kasp_key_lifetime(dns_kasp_key_t *key); * */ -const char * +dns_keystore_t * dns_kasp_key_keystore(dns_kasp_key_t *key); /*%< * The keystore reference of this key. diff --git a/lib/dns/kasp.c b/lib/dns/kasp.c index de8d757fd4..8658fd629c 100644 --- a/lib/dns/kasp.c +++ b/lib/dns/kasp.c @@ -408,10 +408,7 @@ dns_kasp_key_destroy(dns_kasp_key_t *key) { REQUIRE(key != NULL); if (key->keystore != NULL) { - char *ks; - DE_CONST(key->keystore, ks); - isc_mem_free(key->mctx, ks); - key->keystore = NULL; + dns_keystore_detach(&key->keystore); } isc_mem_putanddetach(&key->mctx, key, sizeof(*key)); } @@ -474,7 +471,7 @@ dns_kasp_key_lifetime(dns_kasp_key_t *key) { return (key->lifetime); } -const char * +dns_keystore_t * dns_kasp_key_keystore(dns_kasp_key_t *key) { REQUIRE(key != NULL); diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index fb042d095e..fb8012cf53 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -1189,39 +1189,6 @@ check_port(const cfg_obj_t *options, isc_log_t *logctx, const char *type, return (ISC_R_SUCCESS); } -static isc_result_t -check_keystore(const cfg_obj_t *obj, isc_log_t *logctx, dns_kasp_t *kasp, - dns_keystorelist_t *kslist) { - isc_result_t result = ISC_R_SUCCESS; - dns_kasp_key_t *kkey; - dns_keystore_t *ks = NULL; - - REQUIRE(kasp != NULL); - - dns_kasp_freeze(kasp); - - for (kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; - kkey = ISC_LIST_NEXT(kkey, link)) - { - const char *keystore = dns_kasp_key_keystore(kkey); - if (keystore != NULL && strcmp("key-directory", keystore) != 0) - { - if (dns_keystorelist_find(kslist, keystore, &ks) == - ISC_R_SUCCESS) { - dns_keystore_detach(&ks); - } else { - cfg_obj_log(obj, logctx, ISC_LOG_ERROR, - "key-store '%s' not found", - keystore); - result = ISC_R_FAILURE; - } - } - } - - dns_kasp_thaw(kasp); - return (result); -} - static isc_result_t check_options(const cfg_obj_t *options, const cfg_obj_t *config, bool check_algorithms, isc_log_t *logctx, isc_mem_t *mctx, @@ -1401,12 +1368,14 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config, DNS_KEYSTORE_KEYDIRECTORY); if (result == ISC_R_SUCCESS) { result = ISC_R_FAILURE; + continue; } } kopt = cfg_tuple_get(kconfig, "options"); if (cfg_map_get(kopt, "directory", &kobj) == - ISC_R_SUCCESS) { + ISC_R_SUCCESS) + { val = cfg_obj_asstring(kobj); ret = isc_file_isdirectory(val); switch (ret) { @@ -1457,6 +1426,19 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config, } } + /* + * Add default key-store "key-directory". + */ + tresult = cfg_keystore_fromconfig(NULL, mctx, logctx, &kslist, &ks); + if (tresult != ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS) { + result = tresult; + } + } + if (ks != NULL) { + dns_keystore_detach(&ks); + } + /* * Check dnssec-policy. */ @@ -1494,13 +1476,8 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config, ret = cfg_kasp_fromconfig( kconfig, NULL, check_algorithms, - mctx, logctx, &list, &kasp); - if (ret == ISC_R_SUCCESS) { - /* Check key-stores of keys */ - ret = check_keystore( - obj, logctx, kasp, - &kslist); - } + mctx, logctx, &kslist, &list, + &kasp); if (ret != ISC_R_SUCCESS) { if (result == ISC_R_SUCCESS) { result = ret; @@ -2976,6 +2953,23 @@ check_keydir(const cfg_obj_t *config, const cfg_obj_t *zconfig, ISC_LIST_INIT(kslist); do_cleanup = true; + /* + * Build the keystore list. + */ + for (element = cfg_list_first(keystores); element != NULL; + element = cfg_list_next(element)) + { + cfg_obj_t *kcfg = cfg_listelt_value(element); + ks = NULL; + (void)cfg_keystore_fromconfig(kcfg, mctx, logctx, &kslist, &ks); + INSIST(ks != NULL); + dns_keystore_detach(&ks); + } + ks = NULL; + (void)cfg_keystore_fromconfig(NULL, mctx, logctx, &kslist, &ks); + INSIST(ks != NULL); + dns_keystore_detach(&ks); + /* * Look for the dnssec-policy by name, which is the dnssec-policy * for the zone in question. @@ -2995,8 +2989,8 @@ check_keydir(const cfg_obj_t *config, const cfg_obj_t *zconfig, continue; } - ret = cfg_kasp_fromconfig(kconfig, NULL, mctx, logctx, - &kasplist, &kasp); + ret = cfg_kasp_fromconfig(kconfig, NULL, false, mctx, logctx, + &kslist, &kasplist, &kasp); if (ret != ISC_R_SUCCESS) { kasp = NULL; } @@ -3011,45 +3005,15 @@ check_keydir(const cfg_obj_t *config, const cfg_obj_t *zconfig, for (dns_kasp_key_t *kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; kkey = ISC_LIST_NEXT(kkey, link)) { - const char *ksname = dns_kasp_key_keystore(kkey); - if (ksname == NULL || strcmp("key-directory", ksname) == 0) { + dns_keystore_t *kks = dns_kasp_key_keystore(kkey); + if (kks == NULL || strcmp(DNS_KEYSTORE_KEYDIRECTORY, + dns_keystore_name(kks)) == 0) + { dir = keydir; keystore = false; } else { - /* Look for the key-store by name */ - for (element = cfg_list_first(keystores); - element != NULL; element = cfg_list_next(element)) - { - cfg_obj_t *kconfig = cfg_listelt_value(element); - const cfg_obj_t *ksobj = NULL; - - if (!cfg_obj_istuple(kconfig)) { - continue; - } - - ksobj = cfg_tuple_get(kconfig, "name"); - if (strcmp(ksname, cfg_obj_asstring(ksobj)) != - 0) { - continue; - } - - /* Found the keystore */ - ksobj = NULL; - if (cfg_map_get(cfg_tuple_get(kconfig, "option" - "s"), - "directory", - &ksobj) == ISC_R_SUCCESS) - { - /* Check this directory in the symtable - */ - dir = cfg_obj_asstring(ksobj); - keystore = true; - } else { - dir = keydir; - keystore = false; - } - break; - } + dir = dns_keystore_directory(kks); + keystore = true; } } dns_kasp_thaw(kasp); @@ -3065,10 +3029,17 @@ check: } if (do_cleanup) { + if (kasp != NULL) { + dns_kasp_detach(&kasp); + } if (ks != NULL) { dns_keystore_detach(&ks); } - if (kasp != NULL) { + for (kasp = ISC_LIST_HEAD(kasplist); kasp != NULL; + kasp = kasp_next) + { + kasp_next = ISC_LIST_NEXT(kasp, link); + ISC_LIST_UNLINK(kasplist, kasp, link); dns_kasp_detach(&kasp); } for (ks = ISC_LIST_HEAD(kslist); ks != NULL; ks = ks_next) { @@ -3076,12 +3047,6 @@ check: ISC_LIST_UNLINK(kslist, ks, link); dns_keystore_detach(&ks); } - for (kasp = ISC_LIST_HEAD(kasplist); kasp != NULL; - kasp = kasp_next) { - kasp_next = ISC_LIST_NEXT(kasp, link); - ISC_LIST_UNLINK(kasplist, kasp, link); - dns_kasp_detach(&kasp); - } } return (result); @@ -3366,6 +3331,7 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, has_dnssecpolicy = false; kasp_inlinesigning = false; } else { + const cfg_obj_t *kasps = NULL; (void)cfg_map_get(config, "dnssec-policy", &kasps); for (element = cfg_list_first(kasps); element != NULL; element = cfg_list_next(element)) diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h index a005a39165..6f40fab072 100644 --- a/lib/isccfg/include/isccfg/kaspconf.h +++ b/lib/isccfg/include/isccfg/kaspconf.h @@ -26,13 +26,17 @@ ISC_LANG_BEGINDECLS isc_result_t cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, bool check_algorithms, isc_mem_t *mctx, isc_log_t *logctx, - dns_kasplist_t *kasplist, dns_kasp_t **kaspp); + dns_keystorelist_t *keystorelist, dns_kasplist_t *kasplist, + dns_kasp_t **kaspp); /*%< * Create and configure a KASP. If 'default_kasp' is not NULL, the built-in * default configuration is used to set values that are not explicitly set in - * the policy. If a 'kasplist' is provided, a lookup happens and if a KASP - * already exists with the same name, no new KASP is created, and no attach to - * 'kaspp' happens. + * the policy. + * + * If a 'kasplist' is provided, a lookup happens and if a KASP already exists + * with the same name, no new KASP is created, and no attach to 'kaspp' happens. + * + * The 'keystorelist' is where to lookup key stores if KASP keys are using them. * * If 'check_algorithms' is true then the dnssec-policy DNSSEC key * algorithms are checked against those supported by the crypto provider. @@ -59,7 +63,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, isc_result_t cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx, dns_keystorelist_t *keystorelist, - dns_keystore_t **kspp); + dns_keystore_t **kspp); /*%< * Create and configure a key store. If a 'keystorelist' is provided, a lookup * happens and if a keystore already exists with the same name, no new one is diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index bb0aa46f0e..9598864ef2 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -113,6 +113,7 @@ get_string(const cfg_obj_t **maps, const char *option) { static isc_result_t cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp, bool check_algorithms, isc_log_t *logctx, + dns_keystorelist_t *keystorelist, uint32_t ksk_min_lifetime, uint32_t zsk_min_lifetime) { isc_result_t result; dns_kasp_key_t *key = NULL; @@ -129,8 +130,15 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp, key->lifetime = 0; /* unlimited */ key->algorithm = DNS_KEYALG_ECDSA256; key->length = -1; + result = dns_keystorelist_find(keystorelist, + DNS_KEYSTORE_KEYDIRECTORY, + &key->keystore); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } } else { const char *rolestr = NULL; + const char *keydir = NULL; const cfg_obj_t *obj = NULL; isc_consttextregion_t alg; bool error = false; @@ -147,9 +155,26 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp, obj = cfg_tuple_get(config, "keystorage"); if (cfg_obj_isstring(obj)) { - key->keystore = isc_mem_strdup(key->mctx, - cfg_obj_asstring(obj)); + keydir = cfg_obj_asstring(obj); } + if (keydir == NULL) { + keydir = DNS_KEYSTORE_KEYDIRECTORY; + } + result = dns_keystorelist_find(keystorelist, keydir, + &key->keystore); + if (result == ISC_R_NOTFOUND) { + cfg_obj_log(obj, logctx, ISC_LOG_ERROR, + "dnssec-policy: keystore %s does not exist", + keydir); + result = ISC_R_FAILURE; + goto cleanup; + } else if (result != ISC_R_SUCCESS) { + cfg_obj_log(obj, logctx, ISC_LOG_ERROR, + "dnssec-policy: bad keystore %s", keydir); + result = ISC_R_FAILURE; + goto cleanup; + } + INSIST(key->keystore != NULL); key->lifetime = 0; /* unlimited */ obj = cfg_tuple_get(config, "lifetime"); @@ -373,7 +398,8 @@ add_digest(dns_kasp_t *kasp, const cfg_obj_t *digest, isc_log_t *logctx) { isc_result_t cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, bool check_algorithms, isc_mem_t *mctx, isc_log_t *logctx, - dns_kasplist_t *kasplist, dns_kasp_t **kaspp) { + dns_keystorelist_t *keystorelist, dns_kasplist_t *kasplist, + dns_kasp_t **kaspp) { isc_result_t result; const cfg_obj_t *maps[2]; const cfg_obj_t *koptions = NULL; @@ -548,8 +574,13 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, cfg_obj_t *kobj = cfg_listelt_value(element); result = cfg_kaspkey_fromconfig( kobj, kasp, check_algorithms, logctx, - ksk_min_lifetime, zsk_min_lifetime); + keystorelist, ksk_min_lifetime, + zsk_min_lifetime); if (result != ISC_R_SUCCESS) { + cfg_obj_log(kobj, logctx, ISC_LOG_ERROR, + "dnssec-policy: failed to " + "configure keys (%s)", + isc_result_totext(result)); goto cleanup; } } @@ -620,9 +651,12 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, new_key = NULL; result = dns_kasp_key_create(kasp, &new_key); if (result != ISC_R_SUCCESS) { + cfg_obj_log(config, logctx, ISC_LOG_ERROR, + "dnssec-policy: failed to " + "configure keys (%s)", + isc_result_totext(result)); goto cleanup; } - if (dns_kasp_key_ksk(key)) { new_key->role |= DNS_KASP_KEY_ROLE_KSK; } @@ -632,6 +666,16 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, new_key->lifetime = dns_kasp_key_lifetime(key); new_key->algorithm = dns_kasp_key_algorithm(key); new_key->length = dns_kasp_key_size(key); + result = dns_keystorelist_find( + keystorelist, DNS_KEYSTORE_KEYDIRECTORY, + &new_key->keystore); + if (result != ISC_R_SUCCESS) { + cfg_obj_log(config, logctx, ISC_LOG_ERROR, + "dnssec-policy: failed to " + "find keystore (%s)", + isc_result_totext(result)); + goto cleanup; + } dns_kasp_addkey(kasp, new_key); } } @@ -688,13 +732,17 @@ cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, const cfg_obj_t *maps[2]; const cfg_obj_t *koptions = NULL; const char *name = NULL; + const char *keydirectory = DNS_KEYSTORE_KEYDIRECTORY; dns_keystore_t *keystore = NULL; int i = 0; - REQUIRE(config != NULL); REQUIRE(kspp != NULL && *kspp == NULL); - name = cfg_obj_asstring(cfg_tuple_get(config, "name")); + if (config != NULL) { + name = cfg_obj_asstring(cfg_tuple_get(config, "name")); + } else { + name = keydirectory; + } INSIST(name != NULL); result = dns_keystorelist_find(keystorelist, name, &keystore); @@ -728,12 +776,11 @@ cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, if (config != NULL) { koptions = cfg_tuple_get(config, "options"); maps[i++] = koptions; + maps[i] = NULL; + dns_keystore_setdirectory(keystore, + get_string(maps, "directory")); + dns_keystore_setpkcs11uri(keystore, get_string(maps, "uri")); } - maps[i] = NULL; - - /* Configuration */ - dns_keystore_setdirectory(keystore, get_string(maps, "directory")); - dns_keystore_setpkcs11uri(keystore, get_string(maps, "uri")); /* Append it to the list for future lookups. */ ISC_LIST_APPEND(*keystorelist, keystore, link);