From 95187ee3deb9f260fc1498e1e0cfbca1c8b74da6 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 9 Dec 2025 14:12:08 +0100 Subject: [PATCH 1/4] Update optout test to reconfig to NSEC If we change from NSEC3 to NSEC we should not produce a zone with missing NSEC records. The code only considered having seen a record if there was previously a signature present at the owner name. However with opt-out, insecure delegations don't have a RRSIG record. Reconfiguring to NSEC causes all insecure delegations to have a missing NSEC record. Add a DNAME record to the test zone to also cover DNAME delegations. (cherry picked from commit 3679bd48885273819c02341ff0392c7a54c4b668) --- bin/tests/system/optout/ns2/named.conf.j2 | 16 +++++++++ bin/tests/system/optout/ns2/small.test.db | 25 ++++++++++++++ bin/tests/system/optout/ns2/test.db | 3 ++ bin/tests/system/optout/setup.sh | 14 -------- bin/tests/system/optout/tests_optout.py | 41 +++++++++++++++++++++-- 5 files changed, 83 insertions(+), 16 deletions(-) create mode 100644 bin/tests/system/optout/ns2/small.test.db delete mode 100644 bin/tests/system/optout/setup.sh diff --git a/bin/tests/system/optout/ns2/named.conf.j2 b/bin/tests/system/optout/ns2/named.conf.j2 index 4d9aed3ed0..6bfe881451 100644 --- a/bin/tests/system/optout/ns2/named.conf.j2 +++ b/bin/tests/system/optout/ns2/named.conf.j2 @@ -11,6 +11,9 @@ * information regarding copyright ownership. */ +{% set reconfiged = reconfiged | default(False) %} +{% set policy = "optout" if not reconfiged else "nsec" %} + options { port @PORT@; pid-file "named.pid"; @@ -33,9 +36,22 @@ dnssec-policy "optout" { nsec3param iterations 0 optout yes salt-length 0; }; +dnssec-policy "nsec" { + keys { + csk lifetime unlimited algorithm ecdsa256; + }; +}; + zone "test" { type primary; file "test.db"; dnssec-policy "optout"; inline-signing yes; }; + +zone "small.test" { + type primary; + file "small.test.db"; + dnssec-policy "@policy@"; + inline-signing yes; +}; diff --git a/bin/tests/system/optout/ns2/small.test.db b/bin/tests/system/optout/ns2/small.test.db new file mode 100644 index 0000000000..9b67ef4e80 --- /dev/null +++ b/bin/tests/system/optout/ns2/small.test.db @@ -0,0 +1,25 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, you can obtain one at https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +$TTL 3600 +@ IN SOA ns2.small.test. hostmaster.small.test. 1 7200 3600 24796800 3600 + IN NS ns2 + +ns2 IN A 10.53.0.2 + +a IN A 127.0.0.1 + +dname IN DNAME branch.example. +under.dname IN TXT "occluded" + +$GENERATE 1-10 child$ IN NS ns.example. + +child5 IN DS 7250 13 2 A30B3F78B6DDE9A4A9A2AD0C805518B4F49EC62E7D3F4531D33DE697 CDA01CB2 diff --git a/bin/tests/system/optout/ns2/test.db b/bin/tests/system/optout/ns2/test.db index d3a930229f..a864d6f1e8 100644 --- a/bin/tests/system/optout/ns2/test.db +++ b/bin/tests/system/optout/ns2/test.db @@ -17,6 +17,9 @@ ns2 IN A 10.53.0.2 a IN A 127.0.0.1 +dname IN DNAME branch.example. +under.dname IN TXT "occluded" + $GENERATE 1-50000 child$ IN NS ns.example. child303 IN DS 7250 13 2 A30B3F78B6DDE9A4A9A2AD0C805518B4F49EC62E7D3F4531D33DE697 CDA01CB2 diff --git a/bin/tests/system/optout/setup.sh b/bin/tests/system/optout/setup.sh deleted file mode 100644 index bb08b9c092..0000000000 --- a/bin/tests/system/optout/setup.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/sh -e - -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -. ../conf.sh diff --git a/bin/tests/system/optout/tests_optout.py b/bin/tests/system/optout/tests_optout.py index 67628c20e8..3f0df0bb53 100755 --- a/bin/tests/system/optout/tests_optout.py +++ b/bin/tests/system/optout/tests_optout.py @@ -94,14 +94,51 @@ def verify_zone(zone, transfer): def test_optout(ns2): zone = "test" + expect_nsec3param = True # Wait until the provided zone is signed and then verify its DNSSEC data. def check_nsec3param(): response = do_query(ns2, zone, "NSEC3PARAM") - return has_nsec3param(zone, response) + if expect_nsec3param: + return has_nsec3param(zone, response) + return not has_nsec3param(zone, response) # check zone is fully signed. - isctest.run.retry_with_timeout(check_nsec3param, timeout=300) + isctest.run.retry_with_timeout(check_nsec3param, timeout=100) + + # check if zone if DNSSEC valid. + transfer = do_xfr(ns2, zone) + assert verify_zone(zone, transfer) + + +def test_optout_to_nsec(ns2, templates): + zone = "small.test" + expect_nsec3param = True + + # Wait until the provided zone is signed and then verify its DNSSEC data. + def check_nsec3param(): + response = do_query(ns2, zone, "NSEC3PARAM") + if expect_nsec3param: + return has_nsec3param(zone, response) + return not has_nsec3param(zone, response) + + # check zone is fully signed. + isctest.run.retry_with_timeout(check_nsec3param, timeout=100) + + # check if zone if DNSSEC valid. + transfer = do_xfr(ns2, zone) + assert verify_zone(zone, transfer) + + # reconfigure to NSEC. + data = { + "reconfiged": True, + } + templates.render(f"{ns2.identifier}/named.conf", data) + ns2.reconfigure() + + # wait until NSEC3PARAM is removed. + expect_nsec3param = False + isctest.run.retry_with_timeout(check_nsec3param, timeout=100) # check if zone if DNSSEC valid. transfer = do_xfr(ns2, zone) From d3e74983bbae8d3ae001dca908aec70bb90faba8 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 9 Dec 2025 14:13:58 +0100 Subject: [PATCH 2/4] Nit fix removing a newline in the logs (cherry picked from commit 780e8e8f1cc864527253fce2b4dd61d8c375e9b4) --- lib/dns/zone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index b4a7c0166f..605dfbd21d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -8803,7 +8803,7 @@ zone_nsec3chain(dns_zone_t *zone) { if (first) { dnssec_log(zone, ISC_LOG_DEBUG(3), - "zone_nsec3chain:buildnsecchain = %u\n", + "zone_nsec3chain:buildnsecchain = %u", buildnsecchain); } From 07a4d63fd97e9ff665860499bfe91e6d1d6f5033 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 9 Dec 2025 18:03:13 +0100 Subject: [PATCH 3/4] Add NSEC for opt-out names When switching from NSEC3 opt-out to NSEC, add NSEC records if we saw an RR. This corrects a mistake in style cleanups done in commit 308ab1b4a5c5239860ca06c64b0def9b98ae4b17. (cherry picked from commit 6f285bff6a5f79574529848082c2e7acc08ba1f0) --- lib/dns/zone.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 605dfbd21d..e9c630862f 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -8893,7 +8893,8 @@ zone_nsec3chain(dns_zone_t *zone) { seen_nsec = true; } else if (rdataset.type == dns_rdatatype_nsec3) { seen_nsec3 = true; - } else if (rdataset.type != dns_rdatatype_rrsig) { + } + if (rdataset.type != dns_rdatatype_rrsig) { seen_rr = true; } dns_rdataset_disassociate(&rdataset); From 1b3fb1b9666557e6ac44096f85d7d9d71ae9cd24 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 10 Dec 2025 11:42:41 +0100 Subject: [PATCH 4/4] Refactor code that checks if records are seen There are three places that do roughly the same. Refactor the code to a helper function. (cherry picked from commit ae151a7a761099b85117c5d028f6a25ad3f60fde) --- lib/dns/zone.c | 154 ++++++++++++++++++++++--------------------------- 1 file changed, 70 insertions(+), 84 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index e9c630862f..ed515b5909 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -7627,6 +7627,58 @@ cleanup: return result; } +typedef struct seen { + bool rr; + bool soa; + bool ns; + bool nsec; + bool nsec3; + bool ds; + bool dname; +} seen_t; + +static isc_result_t +allrdatasets(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, + dns_rdatasetiter_t **iterp, seen_t *seen) { + isc_result_t result; + dns_rdataset_t rdataset = DNS_RDATASET_INIT; + + *seen = (seen_t){}; + + RETERR(dns_db_allrdatasets(db, node, version, 0, 0, iterp)); + + for (result = dns_rdatasetiter_first(*iterp); result == ISC_R_SUCCESS; + result = dns_rdatasetiter_next(*iterp)) + { + dns_rdatasetiter_current(*iterp, &rdataset); + + if (rdataset.type == dns_rdatatype_rrsig) { + dns_rdataset_disassociate(&rdataset); + continue; + } + + (*seen).rr = true; + + if (rdataset.type == dns_rdatatype_soa) { + (*seen).soa = true; + } else if (rdataset.type == dns_rdatatype_ns) { + (*seen).ns = true; + } else if (rdataset.type == dns_rdatatype_ds) { + (*seen).ds = true; + } else if (rdataset.type == dns_rdatatype_dname) { + (*seen).dname = true; + } else if (rdataset.type == dns_rdatatype_nsec) { + (*seen).nsec = true; + } else if (rdataset.type == dns_rdatatype_nsec3) { + (*seen).nsec3 = true; + } + + dns_rdataset_disassociate(&rdataset); + } + + return ISC_R_SUCCESS; +} + static isc_result_t sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, dns_dbnode_t *node, dns_dbversion_t *version, bool build_nsec3, @@ -7643,13 +7695,13 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, bool offlineksk = false; isc_buffer_t buffer; unsigned char data[1024]; - bool seen_soa, seen_ns, seen_rr, seen_nsec, seen_nsec3, seen_ds; + seen_t seen; if (zone->kasp != NULL) { offlineksk = dns_kasp_offlineksk(zone->kasp); } - result = dns_db_allrdatasets(db, node, version, 0, 0, &iterator); + result = allrdatasets(db, node, version, &iterator, &seen); if (result != ISC_R_SUCCESS) { if (result == ISC_R_NOTFOUND) { result = ISC_R_SUCCESS; @@ -7659,36 +7711,13 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, dns_rdataset_init(&rdataset); isc_buffer_init(&buffer, data, sizeof(data)); - seen_rr = seen_soa = seen_ns = seen_nsec = seen_nsec3 = seen_ds = false; - for (result = dns_rdatasetiter_first(iterator); result == ISC_R_SUCCESS; - result = dns_rdatasetiter_next(iterator)) - { - dns_rdatasetiter_current(iterator, &rdataset); - if (rdataset.type == dns_rdatatype_soa) { - seen_soa = true; - } else if (rdataset.type == dns_rdatatype_ns) { - seen_ns = true; - } else if (rdataset.type == dns_rdatatype_ds) { - seen_ds = true; - } else if (rdataset.type == dns_rdatatype_nsec) { - seen_nsec = true; - } else if (rdataset.type == dns_rdatatype_nsec3) { - seen_nsec3 = true; - } - if (rdataset.type != dns_rdatatype_rrsig) { - seen_rr = true; - } - dns_rdataset_disassociate(&rdataset); - } - if (result != ISC_R_NOMORE) { - goto cleanup; - } + /* * Going from insecure to NSEC3. * Don't generate NSEC3 records for NSEC3 records. */ - if (build_nsec3 && !seen_nsec3 && seen_rr) { - bool unsecure = !seen_ds && seen_ns && !seen_soa; + if (build_nsec3 && !seen.nsec3 && seen.rr) { + bool unsecure = !seen.ds && seen.ns && !seen.soa; CHECK(dns_nsec3_addnsec3s(db, version, name, nsecttl, unsecure, diff)); (*signatures)--; @@ -7697,7 +7726,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, * Going from insecure to NSEC. * Don't generate NSEC records for NSEC3 records. */ - if (build_nsec && !seen_nsec3 && !seen_nsec && seen_rr) { + if (build_nsec && !seen.nsec3 && !seen.nsec && seen.rr) { /* * Build a NSEC record except at the origin. */ @@ -7739,7 +7768,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, } } - if (seen_ns && !seen_soa && rdataset.type != dns_rdatatype_ds && + if (seen.ns && !seen.soa && rdataset.type != dns_rdatatype_ds && rdataset.type != dns_rdatatype_nsec) { goto next_rdataset; @@ -8437,8 +8466,7 @@ zone_nsec3chain(dns_zone_t *zone) { unsigned int nkeys = 0; uint32_t nodes; bool unsecure = false; - bool seen_soa, seen_ns, seen_dname, seen_ds; - bool seen_nsec, seen_nsec3, seen_rr; + seen_t seen; dns_rdatasetiter_t *iterator = NULL; bool buildnsecchain; bool updatensec = false; @@ -8606,45 +8634,27 @@ zone_nsec3chain(dns_zone_t *zone) { /* * Check to see if this is a bottom of zone node. */ - result = dns_db_allrdatasets(db, node, version, 0, 0, - &iterator); + result = allrdatasets(db, node, version, &iterator, &seen); if (result == ISC_R_NOTFOUND) { /* Empty node? */ goto next_addnode; } CHECK(result); - seen_soa = seen_ns = seen_dname = seen_ds = seen_nsec = false; - for (result = dns_rdatasetiter_first(iterator); - result == ISC_R_SUCCESS; - result = dns_rdatasetiter_next(iterator)) - { - dns_rdatasetiter_current(iterator, &rdataset); - INSIST(rdataset.type != dns_rdatatype_nsec3); - if (rdataset.type == dns_rdatatype_soa) { - seen_soa = true; - } else if (rdataset.type == dns_rdatatype_ns) { - seen_ns = true; - } else if (rdataset.type == dns_rdatatype_dname) { - seen_dname = true; - } else if (rdataset.type == dns_rdatatype_ds) { - seen_ds = true; - } else if (rdataset.type == dns_rdatatype_nsec) { - seen_nsec = true; - } - dns_rdataset_disassociate(&rdataset); - } + INSIST(!seen.nsec3); + dns_rdatasetiter_destroy(&iterator); /* * Is there a NSEC chain than needs to be cleaned up? */ - if (seen_nsec) { + if (seen.nsec) { nsec3chain->seen_nsec = true; } - if (seen_ns && !seen_soa && !seen_ds) { + + if (seen.ns && !seen.soa && !seen.ds) { unsecure = true; } - if ((seen_ns && !seen_soa) || seen_dname) { + if ((seen.ns && !seen.soa) || seen.dname) { delegation = true; } @@ -8868,43 +8878,19 @@ zone_nsec3chain(dns_zone_t *zone) { /* * Check to see if this is a bottom of zone node. */ - result = dns_db_allrdatasets(db, node, version, 0, 0, - &iterator); + result = allrdatasets(db, node, version, &iterator, &seen); if (result == ISC_R_NOTFOUND) { /* Empty node? */ goto next_removenode; } CHECK(result); - seen_soa = seen_ns = seen_dname = seen_nsec3 = seen_nsec = - seen_rr = false; - for (result = dns_rdatasetiter_first(iterator); - result == ISC_R_SUCCESS; - result = dns_rdatasetiter_next(iterator)) - { - dns_rdatasetiter_current(iterator, &rdataset); - if (rdataset.type == dns_rdatatype_soa) { - seen_soa = true; - } else if (rdataset.type == dns_rdatatype_ns) { - seen_ns = true; - } else if (rdataset.type == dns_rdatatype_dname) { - seen_dname = true; - } else if (rdataset.type == dns_rdatatype_nsec) { - seen_nsec = true; - } else if (rdataset.type == dns_rdatatype_nsec3) { - seen_nsec3 = true; - } - if (rdataset.type != dns_rdatatype_rrsig) { - seen_rr = true; - } - dns_rdataset_disassociate(&rdataset); - } dns_rdatasetiter_destroy(&iterator); - if (!seen_rr || seen_nsec3 || seen_nsec) { + if (!seen.rr || seen.nsec3 || seen.nsec) { goto next_removenode; } - if ((seen_ns && !seen_soa) || seen_dname) { + if ((seen.ns && !seen.soa) || seen.dname) { delegation = true; }