From: Matthijs Mekking Date: Wed, 10 Aug 2022 13:29:59 +0000 (+0200) Subject: Wait with NSEC3 during a DNSSEC policy change X-Git-Tag: v9.19.5~26^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=501dc87d75386d50bb713ae3c23c1122e6a5af57;p=thirdparty%2Fbind9.git Wait with NSEC3 during a DNSSEC policy change When doing a dnssec-policy reconfiguration from a zone with NSEC only keys to a zone that uses NSEC3, figure out to wait with building the NSEC3 chain. Previously, BIND 9 would attempt to sign such a zone, but failed to do so because the NSEC3 chain conflicted with existing DNSKEY records in the zone that were not compatible with NSEC3. There exists logic for detecting such a case in the functions dnskey_sane() (in lib/dns/zone.c) and check_dnssec() (in lib/ns/update.c). Both functions look very similar so refactor them to use the same code and call the new function (called dns_zone_check_dnskey_nsec3()). Also update the dns_nsec_nseconly() function to take an additional parameter 'diff' that, if provided, will be checked whether an offending NSEC only DNSKEY will be deleted from the zone. If so, this key will not be considered when checking the zone for NSEC only DNSKEYs. This is needed to allow a transition from an NSEC zone with NSEC only DNSKEYs to an NSEC3 zone. --- diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 889ca79ab5a..1780a93ae81 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3901,7 +3901,7 @@ main(int argc, char *argv[]) { hashlist_init(&hashlist, dns_db_nodecount(gdb, dns_dbtree_main) * 2, hash_length); - result = dns_nsec_nseconly(gdb, gversion, &answer); + result = dns_nsec_nseconly(gdb, gversion, NULL, &answer); if (result == ISC_R_NOTFOUND) { fprintf(stderr, "%s: warning: NSEC3 generation " diff --git a/lib/dns/include/dns/nsec.h b/lib/dns/include/dns/nsec.h index 04029e2ff6b..e68ea35ebfb 100644 --- a/lib/dns/include/dns/nsec.h +++ b/lib/dns/include/dns/nsec.h @@ -19,6 +19,7 @@ #include +#include #include #include @@ -60,11 +61,15 @@ dns_nsec_typepresent(dns_rdata_t *nsec, dns_rdatatype_t type); */ isc_result_t -dns_nsec_nseconly(dns_db_t *db, dns_dbversion_t *version, bool *answer); +dns_nsec_nseconly(dns_db_t *db, dns_dbversion_t *version, dns_diff_t *diff, + bool *answer); /* * Report whether the DNSKEY RRset has a NSEC only algorithm. Unknown * algorithms are assumed to support NSEC3. If DNSKEY is not found, * *answer is set to false, and ISC_R_NOTFOUND is returned. + * If 'diff' is provided, check if the NSEC only DNSKEY will be deleted. + * If so, and there are no other NSEC only DNSKEYs that will stay in 'db', + * consider the DNSKEY RRset to have no NSEC only DNSKEYs. * * Requires: * 'answer' to be non NULL. diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index c9a1487928a..0f035cf5a8c 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -2761,3 +2762,20 @@ dns_zone_gettid(dns_zone_t *zone); * * \return thread id associated with the zone */ + +bool +dns_zone_check_dnskey_nsec3(dns_zone_t *zone, dns_db_t *db, + dns_dbversion_t *ver, dns_diff_t *diff, + dst_key_t **keys, unsigned int numkeys); +/**< + * Return whether the zone would enter an inconsistent state where NSEC only + * DNSKEYs are present along NSEC3 chains. + * + * Requires: + * \li 'zone' to be a valid zone. + * \li 'db'is not NULL. + * + * Returns: + * \li 'true' if the check passes, that is the zone remains consistent, + * 'false' if the zone would have NSEC only DNSKEYs and an NSEC3 chain. + */ diff --git a/lib/dns/nsec.c b/lib/dns/nsec.c index afbe9c80f9e..84c47d6c215 100644 --- a/lib/dns/nsec.c +++ b/lib/dns/nsec.c @@ -247,7 +247,8 @@ dns_nsec_typepresent(dns_rdata_t *nsec, dns_rdatatype_t type) { } isc_result_t -dns_nsec_nseconly(dns_db_t *db, dns_dbversion_t *version, bool *answer) { +dns_nsec_nseconly(dns_db_t *db, dns_dbversion_t *version, dns_diff_t *diff, + bool *answer) { dns_dbnode_t *node = NULL; dns_rdataset_t rdataset; dns_rdata_dnskey_t dnskey; @@ -282,8 +283,35 @@ dns_nsec_nseconly(dns_db_t *db, dns_dbversion_t *version, bool *answer) { RUNTIME_CHECK(result == ISC_R_SUCCESS); if (dnskey.algorithm == DST_ALG_RSAMD5 || - dnskey.algorithm == DST_ALG_RSASHA1) { - break; + dnskey.algorithm == DST_ALG_DH || + dnskey.algorithm == DST_ALG_DSA || + dnskey.algorithm == DST_ALG_RSASHA1) + { + bool deleted = false; + if (diff != NULL) { + for (dns_difftuple_t *tuple = + ISC_LIST_HEAD(diff->tuples); + tuple != NULL; + tuple = ISC_LIST_NEXT(tuple, link)) + { + if (tuple->rdata.type != + dns_rdatatype_dnskey || + tuple->op != DNS_DIFFOP_DEL) { + continue; + } + + if (dns_rdata_compare( + &rdata, &tuple->rdata) == 0) + { + deleted = true; + break; + } + } + } + + if (!deleted) { + break; + } } } dns_rdataset_disassociate(&rdataset); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index d932b1b8413..600fad94e50 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -3726,7 +3726,7 @@ zone_addnsec3chain(dns_zone_t *zone, dns_rdata_nsec3param_t *nsec3param) { * latter to exist in the first place. */ dns_db_currentversion(db, &version); - result = dns_nsec_nseconly(db, version, &nseconly); + result = dns_nsec_nseconly(db, version, NULL, &nseconly); nsec3ok = (result == ISC_R_SUCCESS && !nseconly); dns_db_closeversion(db, &version, false); if (!nsec3ok && (nsec3param->flags & DNS_NSEC3FLAG_REMOVE) == 0) { @@ -3918,7 +3918,7 @@ resume_addnsec3chain(dns_zone_t *zone) { * In order to create NSEC3 chains we need the DNSKEY RRset at zone * apex to exist and contain no keys using NSEC-only algorithms. */ - result = dns_nsec_nseconly(db, version, &nseconly); + result = dns_nsec_nseconly(db, version, NULL, &nseconly); nsec3ok = (result == ISC_R_SUCCESS && !nseconly); /* @@ -8186,7 +8186,7 @@ try_private: goto add; } - result = dns_nsec_nseconly(db, ver, &nseconly); + result = dns_nsec_nseconly(db, ver, diff, &nseconly); nsec3ok = (result == ISC_R_SUCCESS && !nseconly); /* @@ -9508,6 +9508,105 @@ failure: return (result); } +/* + * Prevent the zone entering a inconsistent state where + * NSEC only DNSKEYs are present with NSEC3 chains. + */ +bool +dns_zone_check_dnskey_nsec3(dns_zone_t *zone, dns_db_t *db, + dns_dbversion_t *ver, dns_diff_t *diff, + dst_key_t **keys, unsigned int numkeys) { + uint8_t alg; + dns_rdatatype_t privatetype; + ; + bool nseconly = false, nsec3 = false; + isc_result_t result; + + REQUIRE(DNS_ZONE_VALID(zone)); + REQUIRE(db != NULL); + + privatetype = dns_zone_getprivatetype(zone); + + /* Scan the tuples for an NSEC-only DNSKEY */ + if (diff != NULL) { + for (dns_difftuple_t *tuple = ISC_LIST_HEAD(diff->tuples); + tuple != NULL; tuple = ISC_LIST_NEXT(tuple, link)) + { + if (nseconly && nsec3) { + break; + } + + if (tuple->op != DNS_DIFFOP_ADD) { + continue; + } + + if (tuple->rdata.type == dns_rdatatype_nsec3param) { + nsec3 = true; + } + + if (tuple->rdata.type != dns_rdatatype_dnskey) { + continue; + } + + alg = tuple->rdata.data[3]; + if (alg == DNS_KEYALG_RSAMD5 || alg == DNS_KEYALG_DH || + alg == DNS_KEYALG_DSA || alg == DNS_KEYALG_RSASHA1) + { + nseconly = true; + } + } + } + /* Scan the zone keys for an NSEC-only DNSKEY */ + if (keys != NULL && !nseconly) { + for (unsigned int i = 0; i < numkeys; i++) { + alg = dst_key_alg(keys[i]); + if (alg == DNS_KEYALG_RSAMD5 || alg == DNS_KEYALG_DH || + alg == DNS_KEYALG_DSA || alg == DNS_KEYALG_RSASHA1) + { + nseconly = true; + break; + } + } + } + + /* Check DB for NSEC-only DNSKEY */ + if (!nseconly) { + result = dns_nsec_nseconly(db, ver, diff, &nseconly); + /* + * Adding an NSEC3PARAM record can proceed without a + * DNSKEY (it will trigger a delayed change), so we can + * ignore ISC_R_NOTFOUND here. + */ + if (result == ISC_R_NOTFOUND) { + result = ISC_R_SUCCESS; + } + CHECK(result); + } + + /* Check existing DB for NSEC3 */ + if (!nsec3) { + CHECK(dns_nsec3_activex(db, ver, false, privatetype, &nsec3)); + } + + /* Check kasp for NSEC3PARAM settings */ + if (!nsec3) { + dns_kasp_t *kasp = dns_zone_getkasp(zone); + if (kasp != NULL) { + nsec3 = dns_kasp_nsec3(kasp); + } + } + + /* Refuse to allow NSEC3 with NSEC-only keys */ + if (nseconly && nsec3) { + goto failure; + } + + return (true); + +failure: + return (false); +} + /* * Incrementally sign the zone using the keys requested. * Builds the NSEC chain if required. @@ -9643,6 +9742,15 @@ zone_sign(dns_zone_t *zone) { /* Determine which type of chain to build */ if (use_kasp) { build_nsec3 = dns_kasp_nsec3(kasp); + if (!dns_zone_check_dnskey_nsec3(zone, db, version, NULL, + (dst_key_t **)&zone_keys, + nkeys)) + { + dnssec_log(zone, ISC_LOG_INFO, + "wait building NSEC3 chain until NSEC only " + "DNSKEYs are removed"); + build_nsec3 = false; + } build_nsec = !build_nsec3; } else { CHECK(dns_private_chains(db, version, zone->privatetype, @@ -20506,63 +20614,6 @@ failure: return (result); } -/* - * Prevent the zone entering a inconsistent state where - * NSEC only DNSKEYs are present with NSEC3 chains. - * See update.c:check_dnssec() - */ -static bool -dnskey_sane(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, - dns_diff_t *diff) { - isc_result_t result; - dns_difftuple_t *tuple; - bool nseconly = false, nsec3 = false; - dns_rdatatype_t privatetype = dns_zone_getprivatetype(zone); - - /* Scan the tuples for an NSEC-only DNSKEY */ - for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; - tuple = ISC_LIST_NEXT(tuple, link)) - { - uint8_t alg; - if (tuple->rdata.type != dns_rdatatype_dnskey || - tuple->op != DNS_DIFFOP_ADD) { - continue; - } - - alg = tuple->rdata.data[3]; - if (alg == DST_ALG_RSASHA1) { - nseconly = true; - break; - } - } - - /* Check existing DB for NSEC-only DNSKEY */ - if (!nseconly) { - result = dns_nsec_nseconly(db, ver, &nseconly); - if (result == ISC_R_NOTFOUND) { - result = ISC_R_SUCCESS; - } - CHECK(result); - } - - /* Check existing DB for NSEC3 */ - if (!nsec3) { - CHECK(dns_nsec3_activex(db, ver, false, privatetype, &nsec3)); - } - - /* Refuse to allow NSEC3 with NSEC-only keys */ - if (nseconly && nsec3) { - dnssec_log(zone, ISC_LOG_ERROR, - "NSEC only DNSKEYs and NSEC3 chains not allowed"); - goto failure; - } - - return (true); - -failure: - return (false); -} - static isc_result_t clean_nsec3param(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_diff_t *diff) { @@ -21558,6 +21609,7 @@ zone_rekey(dns_zone_t *zone) { if (result == ISC_R_SUCCESS) { bool cdsdel = false; bool cdnskeydel = false; + bool sane_diff, sane_dnskey; isc_stdtime_t when; /* @@ -21726,9 +21778,21 @@ zone_rekey(dns_zone_t *zone) { } } - if ((newactive || fullsign || !ISC_LIST_EMPTY(diff.tuples)) && - dnskey_sane(zone, db, ver, &diff)) - { + /* + * A sane diff is one that is not empty, and that does not + * introduce a zone with NSEC only DNSKEYs along with NSEC3 + * chains. + */ + sane_dnskey = dns_zone_check_dnskey_nsec3(zone, db, ver, &diff, + NULL, 0); + sane_diff = !ISC_LIST_EMPTY(diff.tuples) && sane_dnskey; + if (!sane_dnskey) { + dnssec_log(zone, ISC_LOG_ERROR, + "NSEC only DNSKEYs and NSEC3 chains not " + "allowed"); + } + + if (newactive || fullsign || sane_diff) { CHECK(dns_diff_apply(&diff, db, ver)); CHECK(clean_nsec3param(zone, db, ver, &diff)); CHECK(add_signing_records(db, zone->privatetype, ver, @@ -22913,7 +22977,7 @@ rss_post(dns_zone_t *zone, isc_event_t *event) { dns_rdata_init(&rdata); np->data[2] |= DNS_NSEC3FLAG_CREATE; - result = dns_nsec_nseconly(db, newver, &nseconly); + result = dns_nsec_nseconly(db, newver, NULL, &nseconly); if (result == ISC_R_NOTFOUND || nseconly) { np->data[2] |= DNS_NSEC3FLAG_INITIAL; } diff --git a/lib/ns/update.c b/lib/ns/update.c index 28b59a54844..4b32b7508f8 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -2096,56 +2096,12 @@ failure: static isc_result_t check_dnssec(ns_client_t *client, dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_diff_t *diff) { - dns_difftuple_t *tuple; - bool nseconly = false, nsec3 = false; isc_result_t result; unsigned int iterations = 0; dns_rdatatype_t privatetype = dns_zone_getprivatetype(zone); - /* Scan the tuples for an NSEC-only DNSKEY or an NSEC3PARAM */ - for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; - tuple = ISC_LIST_NEXT(tuple, link)) - { - if (tuple->op != DNS_DIFFOP_ADD) { - continue; - } - - if (tuple->rdata.type == dns_rdatatype_dnskey) { - uint8_t alg; - alg = tuple->rdata.data[3]; - if (alg == DST_ALG_RSASHA1) { - nseconly = true; - break; - } - } else if (tuple->rdata.type == dns_rdatatype_nsec3param) { - nsec3 = true; - break; - } - } - - /* Check existing DB for NSEC-only DNSKEY */ - if (!nseconly) { - result = dns_nsec_nseconly(db, ver, &nseconly); - - /* - * An NSEC3PARAM update can proceed without a DNSKEY (it - * will trigger a delayed change), so we can ignore - * ISC_R_NOTFOUND here. - */ - if (result == ISC_R_NOTFOUND) { - result = ISC_R_SUCCESS; - } - - CHECK(result); - } - - /* Check existing DB for NSEC3 */ - if (!nsec3) { - CHECK(dns_nsec3_activex(db, ver, false, privatetype, &nsec3)); - } - /* Refuse to allow NSEC3 with NSEC-only keys */ - if (nseconly && nsec3) { + if (!dns_zone_check_dnskey_nsec3(zone, db, ver, diff, NULL, 0)) { update_log(client, zone, ISC_LOG_ERROR, "NSEC only DNSKEYs and NSEC3 chains not allowed"); result = DNS_R_REFUSED; @@ -2346,8 +2302,11 @@ add_nsec3param_records(ns_client_t *client, dns_zone_t *zone, dns_db_t *db, * supporting an NSEC3 chain, then we set the * INITIAL flag to indicate that these parameters * are to be used later. + * + * Don't provide a 'diff' here because we want to + * know the capability of the current database. */ - result = dns_nsec_nseconly(db, ver, &nseconly); + result = dns_nsec_nseconly(db, ver, NULL, &nseconly); if (result == ISC_R_NOTFOUND || nseconly) { buf[2] |= DNS_NSEC3FLAG_INITIAL; }