From f211c7c2a1f563ddffbf462c2abcace047fe3d52 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 17 Mar 2021 10:02:22 +0100 Subject: [PATCH 1/3] Allow CDS/CDNSKEY DELETE records in unsigned zone While not useful, having a CDS/CDNSKEY DELETE record in an unsigned zone is not an error and "named-checkzone" should not complain. --- .../checkzone/zones/good-cds-unsigned.db | 5 ++++ lib/dns/zone.c | 23 ++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 bin/tests/system/checkzone/zones/good-cds-unsigned.db diff --git a/bin/tests/system/checkzone/zones/good-cds-unsigned.db b/bin/tests/system/checkzone/zones/good-cds-unsigned.db new file mode 100644 index 0000000000..affb60039f --- /dev/null +++ b/bin/tests/system/checkzone/zones/good-cds-unsigned.db @@ -0,0 +1,5 @@ +example. 0 SOA . . 0 0 0 0 0 +example. 0 NS . +example. 0 CDS 0 0 0 00 +example. 0 CDNSKEY 0 3 0 AA== + diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 782959b62a..6083b058e6 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -20524,6 +20524,7 @@ dns_zone_cdscheck(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *version) { unsigned char buffer[DNS_DS_BUFFERSIZE]; unsigned char algorithms[256]; unsigned int i; + bool empty = false; enum { notexpected = 0, expected = 1, found = 2 }; @@ -20559,14 +20560,8 @@ dns_zone_cdscheck(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *version) { result = dns_db_findrdataset(db, node, version, dns_rdatatype_dnskey, dns_rdatatype_none, 0, &dnskey, NULL); if (result == ISC_R_NOTFOUND) { - if (dns_rdataset_isassociated(&cds)) { - result = DNS_R_BADCDS; - } else { - result = DNS_R_BADCDNSKEY; - } - goto failure; - } - if (result != ISC_R_SUCCESS) { + empty = true; + } else if (result != ISC_R_SUCCESS) { goto failure; } @@ -20596,6 +20591,12 @@ dns_zone_cdscheck(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *version) { delete = true; continue; } + + if (empty) { + result = DNS_R_BADCDS; + goto failure; + } + CHECK(dns_rdata_tostruct(&crdata, &structcds, NULL)); if (algorithms[structcds.algorithm] == 0) { algorithms[structcds.algorithm] = expected; @@ -20663,6 +20664,12 @@ dns_zone_cdscheck(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *version) { delete = true; continue; } + + if (empty) { + result = DNS_R_BADCDNSKEY; + goto failure; + } + CHECK(dns_rdata_tostruct(&crdata, &structcdnskey, NULL)); if (algorithms[structcdnskey.algorithm] == 0) { From 6f31f62d69e7516722f1b6cf0c1ba32421be3246 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 17 Mar 2021 10:04:00 +0100 Subject: [PATCH 2/3] Delete CDS/CDNSKEY records when zone is unsigned CDS/CDNSKEY DELETE records are only useful if they are signed, otherwise the parent cannot verify these RRsets anyway. So once the DS has been removed (and signaled to BIND), we can remove the DNSKEY and RRSIG records, and at this point we can also remove the CDS/CDNSKEY records. --- bin/tests/system/kasp/tests.sh | 4 ---- lib/dns/zone.c | 36 ++++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 43343e1879..05025e5af2 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -3623,8 +3623,6 @@ check_next_key_event 93600 set_zone "step2.going-insecure.kasp" set_policy "none" "2" "7200" set_server "ns6" "10.53.0.6" -# Expect a CDS/CDNSKEY Delete Record. -set_cdsdelete # The DS is long enough removed from the zone to be considered HIDDEN. # This means the DNSKEY and the KSK signatures can be removed. @@ -3693,8 +3691,6 @@ set_zone "step2.going-insecure-dynamic.kasp" set_dynamic set_policy "none" "2" "7200" set_server "ns6" "10.53.0.6" -# Expect a CDS/CDNSKEY Delete Record. -set_cdsdelete # The DS is long enough removed from the zone to be considered HIDDEN. # This means the DNSKEY and the KSK signatures can be removed. diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 6083b058e6..4f98f38a6a 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -20123,7 +20123,12 @@ zone_rekey(dns_zone_t *zone) { } if (result == ISC_R_SUCCESS) { - bool insecure = dns_zone_secure_to_insecure(zone, false); + /* + * Publish CDS/CDNSKEY DELETE records if the zone is + * transitioning from secure to insecure. + */ + bool cds_delete = dns_zone_secure_to_insecure(zone, false); + isc_stdtime_t when; /* * Only update DNSKEY TTL if we have a policy. @@ -20160,9 +20165,36 @@ zone_rekey(dns_zone_t *zone) { goto failure; } + if (cds_delete) { + /* + * Only publish CDS/CDNSKEY DELETE records if there is + * a KSK that can be used to verify the RRset. This + * means there must be a key with the KSK role that is + * published and is used for signing. + */ + cds_delete = false; + for (key = ISC_LIST_HEAD(dnskeys); key != NULL; + key = ISC_LIST_NEXT(key, link)) { + dst_key_t *dstk = key->key; + bool ksk = false; + (void)dst_key_getbool(dstk, DST_BOOL_KSK, &ksk); + if (!ksk) { + continue; + } + + if (dst_key_haskasp(dstk) && + dst_key_is_published(dstk, now, &when) && + dst_key_is_signing(dstk, DST_BOOL_KSK, now, + &when)) + { + cds_delete = true; + break; + } + } + } result = dns_dnssec_syncdelete(&cdsset, &cdnskeyset, &zone->origin, zone->rdclass, - ttl, &diff, mctx, insecure); + ttl, &diff, mctx, cds_delete); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_rekey:couldn't update CDS/CDNSKEY " From 841e90c6fcf0c55f3a52e1abd587b17c7f13080a Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 17 Mar 2021 10:09:59 +0100 Subject: [PATCH 3/3] Add CHANGES and notes for [#2517] --- CHANGES | 5 +++++ doc/notes/notes-current.rst | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/CHANGES b/CHANGES index 11a16ebdc8..313883173b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5606. [bug] CDS/CDNSKEY DELETE records were not removed when a zone + transitioned from secure to insecure. "named-checkzone" + should not complain if such records exist in an + unsigned zone. [GL #2517] + 5605. [bug] "dig -u" now uses CLOCK_REALTIME for more accurate time reporting. [GL #2592] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 149cf43fd0..2ebdff3f27 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -42,6 +42,10 @@ Bug Fixes - Dynamic zones with ``dnssec-policy`` that were frozen could not be thawed. This has been fixed. [GL #2523] +- CDS/CDNSKEY DELETE records are now removed when a zone transitioned from + secure to insecure. "named-checkzone" no longer complains if such records + exist in an unsigned zone. [GL #2517] + - Fix a crash when transferring a zone over TLS, after "named" previously skipped a master. [GL #2562]