]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Wait with NSEC3 during a DNSSEC policy change
authorMatthijs Mekking <matthijs@isc.org>
Wed, 10 Aug 2022 13:29:59 +0000 (15:29 +0200)
committerMatthijs Mekking <matthijs@isc.org>
Mon, 22 Aug 2022 17:21:39 +0000 (19:21 +0200)
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.

(cherry picked from commit 09a81dc84ce0fee37442f03cdbd63c2398215376)

bin/dnssec/dnssec-signzone.c
lib/dns/include/dns/nsec.h
lib/dns/include/dns/zone.h
lib/dns/nsec.c
lib/dns/zone.c
lib/ns/update.c

index 330b581a2b59a5e90a7901f2a9383608971bc433..afe4797b0ee5699e396767813371a059b37683c1 100644 (file)
@@ -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 "
index 04029e2ff6b6a1ff06506c5f9288e5ce1cee8b0c..e68ea35ebfb3ca4ad1971b4d9ebeac53de013221 100644 (file)
@@ -19,6 +19,7 @@
 
 #include <isc/lang.h>
 
+#include <dns/diff.h>
 #include <dns/name.h>
 #include <dns/types.h>
 
@@ -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.
index 4f609e5462f669dabde999f9742c0b5a155a2c01..53ba560b54b1861102fdb919ba4db8e221b54db7 100644 (file)
@@ -29,6 +29,7 @@
 #include <isc/tls.h>
 
 #include <dns/catz.h>
+#include <dns/diff.h>
 #include <dns/master.h>
 #include <dns/masterdump.h>
 #include <dns/rdatastruct.h>
@@ -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.
+ */
index 390127fc98bdfd71f4662531a295ab3c7c68bb7b..d2b8a7beb3aa89444b26e939a386052d951694ba 100644 (file)
@@ -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);
index 6796f8b49900b4530d8c625cfd72257ccf1dd030..b5b564ed07c19dd89d2a738b14212ace0f17076d 100644 (file)
@@ -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;
                }
index 912c91c01c9da6e08f33408d6b0b443645210042..90578782463fce4b829cea8fa0341089c42559c7 100644 (file)
@@ -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;
                        }