diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 330b581a2b..afe4797b0e 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3899,7 +3899,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 04029e2ff6..e68ea35ebf 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 4f609e5462..53ba560b54 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 @@ -2764,3 +2765,20 @@ dns_zonetype_name(dns_zonetype_t type); /*%< * Return the name of the zone type 'type'. */ + +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 390127fc98..d2b8a7beb3 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 6796f8b499..b5b564ed07 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -3716,7 +3716,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) { @@ -3908,7 +3908,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); /* @@ -8181,7 +8181,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); /* @@ -9504,6 +9504,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. @@ -9640,6 +9739,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, @@ -20637,63 +20745,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) { @@ -21711,6 +21762,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; /* @@ -21879,9 +21931,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, @@ -23075,7 +23139,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 912c91c01c..9057878246 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -2097,56 +2097,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; @@ -2347,8 +2303,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; }