From: Matthijs Mekking Date: Fri, 17 Nov 2023 09:45:05 +0000 (+0100) Subject: Better PKCS#11 label creation X-Git-Tag: v9.19.22~70^2~5 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=934d17255eeb412446a53ed54d288cd7bf9fa2d5;p=thirdparty%2Fbind9.git Better PKCS#11 label creation When using the same PKCS#11 URI for a zone that uses different DNSSEC policies, the PKCS#11 label could collide, i.e. the same label could be used for different keys. Add the policy name to the label to make it more unique. Also, the zone name could contain characters that are interpreted as special characters when parsing the PKCS#11 URI string. Mangle the zone name through 'dns_name_tofilenametext()' to make it PKCS#11 safe. Move the creation to a separate function for clarity. Furthermore, add a log message whenever a PKCS#11 object has been successfully created. --- diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index c3b98ce0a56..d770e41c94d 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -697,10 +697,10 @@ keygen(keygen_ctx_t *ctx, isc_mem_t *mctx, int argc, char **argv) { fprintf(stderr, "Generating key pair."); } - if (ctx->keystore != NULL) { - ret = dns_keystore_keygen(ctx->keystore, name, - ctx->rdclass, mctx, ctx->alg, - ctx->size, flags, &key); + if (ctx->keystore != NULL && ctx->policy != NULL) { + ret = dns_keystore_keygen( + ctx->keystore, name, ctx->policy, ctx->rdclass, + mctx, ctx->alg, ctx->size, flags, &key); } else if (!ctx->quiet && show_progress) { ret = dst_key_generate(name, ctx->alg, ctx->size, param, flags, ctx->protocol, diff --git a/lib/dns/include/dns/keystore.h b/lib/dns/include/dns/keystore.h index bd486e541e6..6db1e9cbbb6 100644 --- a/lib/dns/include/dns/keystore.h +++ b/lib/dns/include/dns/keystore.h @@ -197,8 +197,9 @@ dns_keystore_setpkcs11uri(dns_keystore_t *keystore, const char *uri); isc_result_t dns_keystore_keygen(dns_keystore_t *keystore, const dns_name_t *origin, - dns_rdataclass_t rdclass, isc_mem_t *mctx, uint32_t alg, - int size, int flags, dst_key_t **dstkey); + const char *policy, dns_rdataclass_t rdclass, + isc_mem_t *mctx, uint32_t alg, int size, int flags, + dst_key_t **dstkey); /*%< * Create a DNSSEC key pair. Set keystore PKCS#11 URI. * @@ -208,6 +209,8 @@ dns_keystore_keygen(dns_keystore_t *keystore, const dns_name_t *origin, * *\li 'origin' is a valid DNS owner name. * + *\li 'policy' is the name of the DNSSEC policy. + * *\li 'mctx' is a valid memory context. * *\li 'dstkey' is not NULL and '*dstkey' is NULL. diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 32bbd5b9f6b..05b26261c9b 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -444,9 +444,9 @@ keymgr_keyid_conflict(dst_key_t *newkey, dns_dnsseckeylist_t *keys) { */ static isc_result_t keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin, - dns_rdataclass_t rdclass, isc_mem_t *mctx, const char *keydir, - dns_dnsseckeylist_t *keylist, dns_dnsseckeylist_t *newkeys, - dst_key_t **dst_key) { + 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_result_t result = ISC_R_SUCCESS; bool conflict = false; int flags = DNS_KEYOWNER_ZONE; @@ -465,9 +465,9 @@ keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin, DNS_KEYPROTO_DNSSEC, rdclass, NULL, mctx, &newkey, NULL)); } else { - RETERR(dns_keystore_keygen(keystore, origin, rdclass, - mctx, alg, size, flags, - &newkey)); + RETERR(dns_keystore_keygen( + keystore, origin, dns_kasp_getname(kasp), + rdclass, mctx, alg, size, flags, &newkey)); } /* Key collision? */ @@ -1812,9 +1812,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, rdclass, - mctx, keydir, keyring, - newkeys, &dst_key); + isc_result_t result = + keymgr_createkey(kaspkey, origin, kasp, rdclass, mctx, + keydir, keyring, newkeys, &dst_key); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/keystore.c b/lib/dns/keystore.c index f733310ab9c..8ca22873d86 100644 --- a/lib/dns/keystore.c +++ b/lib/dns/keystore.c @@ -16,10 +16,12 @@ #include #include +#include #include #include #include +#include #include #include @@ -143,53 +145,119 @@ dns_keystore_setpkcs11uri(dns_keystore_t *keystore, const char *uri) { : isc_mem_strdup(keystore->mctx, uri); } +static isc_result_t +buildpkcs11label(const char *uri, const dns_name_t *zname, const char *policy, + int flags, isc_buffer_t *buf) { + bool ksk = ((flags & DNS_KEYFLAG_KSK) != 0); + char timebuf[18]; + isc_time_t now = isc_time_now(); + isc_result_t result; + dns_fixedname_t fname; + dns_name_t *pname = dns_fixedname_initname(&fname); + + /* uri + object */ + if (isc_buffer_availablelength(buf) < strlen(uri) + strlen(";object=")) + { + return (ISC_R_NOSPACE); + } + isc_buffer_putstr(buf, uri); + isc_buffer_putstr(buf, ";object="); + /* zone name */ + result = dns_name_tofilenametext(zname, false, buf); + if (result != ISC_R_SUCCESS) { + return (result); + } + /* + * policy name + * + * Note that strlen(policy) is not the actual length, but if this + * already does not fit, the escaped version returned from + * dns_name_tofilenametext() certainly won't fit. + */ + if (isc_buffer_availablelength(buf) < (strlen(policy) + 1)) { + return (ISC_R_NOSPACE); + } + isc_buffer_putstr(buf, "-"); + result = dns_name_fromstring(pname, policy, dns_rootname, 0, NULL); + if (result != ISC_R_SUCCESS) { + return (result); + } + result = dns_name_tofilenametext(pname, false, buf); + if (result != ISC_R_SUCCESS) { + return (result); + } + /* key type + current time */ + isc_time_formatshorttimestamp(&now, timebuf, sizeof(timebuf)); + return isc_buffer_printf(buf, "-%s-%s", ksk ? "ksk" : "zsk", timebuf); +} + isc_result_t dns_keystore_keygen(dns_keystore_t *keystore, const dns_name_t *origin, - dns_rdataclass_t rdclass, isc_mem_t *mctx, uint32_t alg, - int size, int flags, dst_key_t **dstkey) { + const char *policy, dns_rdataclass_t rdclass, + isc_mem_t *mctx, uint32_t alg, int size, int flags, + dst_key_t **dstkey) { isc_result_t result; dst_key_t *newkey = NULL; const char *uri = NULL; REQUIRE(DNS_KEYSTORE_VALID(keystore)); REQUIRE(dns_name_isvalid(origin)); + REQUIRE(policy != NULL); REQUIRE(mctx != NULL); REQUIRE(dstkey != NULL && *dstkey == NULL); uri = dns_keystore_pkcs11uri(keystore); if (uri != NULL) { - char *label = NULL; - size_t len; - char timebuf[18]; - isc_time_t now = isc_time_now(); - bool ksk = ((flags & DNS_KEYFLAG_KSK) != 0); - char namebuf[DNS_NAME_FORMATSIZE]; - char object[DNS_NAME_FORMATSIZE + 26]; - - /* Create the PKCS11 URI */ - isc_time_formatshorttimestamp(&now, timebuf, sizeof(timebuf)); - dns_name_format(origin, namebuf, sizeof(namebuf)); - snprintf(object, sizeof(object), "%s-%s-%s", namebuf, - ksk ? "ksk" : "zsk", timebuf); - len = strlen(object) + strlen(uri) + 10; - label = isc_mem_get(mctx, len); - sprintf(label, "%s;object=%s;", uri, object); + /* + * Create the PKCS#11 label. + * The label consists of the configured URI, and the object + * parameter. The object parameter needs to be unique. We + * know that for a given point in time, there will be at most + * one key per type created for each zone in a given DNSSEC + * policy. Hence the object is constructed out of the following + * parts: the zone name, policy name, key type, and the + * current time. + * + * The object may not contain any characters that conflict with + * special characters in the PKCS#11 URI scheme syntax (see + * RFC 7512, Section 2.3). Therefore, we mangle the zone name + * and policy name through 'dns_name_tofilenametext()'. We + * could create a new function to convert a name to PKCS#11 + * text, but this existing function will suffice. + */ + char label[NAME_MAX]; + isc_buffer_t buf; + isc_buffer_init(&buf, label, sizeof(label)); + result = buildpkcs11label(uri, origin, policy, flags, &buf); + if (result != ISC_R_SUCCESS) { + char namebuf[DNS_NAME_FORMATSIZE]; + dns_name_format(origin, namebuf, sizeof(namebuf)); + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_ERROR, + "keystore: failed to create PKCS#11 object " + "for zone %s, policy %s: %s", + namebuf, policy, isc_result_totext(result)); + return (result); + } /* Generate the key */ result = dst_key_generate(origin, alg, size, 0, flags, DNS_KEYPROTO_DNSSEC, rdclass, label, mctx, &newkey, NULL); - isc_mem_put(mctx, label, len); - if (result != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, - DNS_LOGMODULE_DNSSEC, ISC_LOG_ERROR, - "keystore: failed to generate key " - "%s (ret=%d)", - object, result); + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_ERROR, + "keystore: failed to generate PKCS#11 object " + "%s: %s", + label, isc_result_totext(result)); return (result); } + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_ERROR, + "keystore: generated PKCS#11 object %s", label); } else { result = dst_key_generate(origin, alg, size, 0, flags, DNS_KEYPROTO_DNSSEC, rdclass, NULL,