From 3d05c99abbfc34644ffeffe2884b6335fc12e055 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Mon, 10 Jan 2022 17:18:47 +0100 Subject: [PATCH] Update dns_dnssec_syncdelete() function Update the function that synchronizes the CDS and CDNSKEY DELETE records. It now allows for the possibility that the CDS DELETE record is published and the CDNSKEY DELETE record is not, and vice versa. Also update the code in zone.c how 'dns_dnssec_syncdelete()' is called. With KASP, we still maintain the DELETE records our self. Otherwise, we publish the CDS and CDNSKEY DELETE record only if they are added to the zone. We do still check if these records can be signed by a KSK. This change will allow users to add a CDS and/or CDNSKEY DELETE record manually, without BIND removing them on the next zone sign. Note that this commit removes the check whether the key is a KSK, this check is redundant because this check is also made in 'dst_key_is_signing()' when the role is set to DST_BOOL_KSK. --- lib/dns/dnssec.c | 48 ++++++++++---------- lib/dns/include/dns/dnssec.h | 9 ++-- lib/dns/zone.c | 87 +++++++++++++++++++++++++++++------- 3 files changed, 101 insertions(+), 43 deletions(-) diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 75ea6f3218..d5d96fc7df 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -2135,7 +2135,7 @@ isc_result_t dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey, dns_name_t *origin, dns_rdataclass_t zclass, dns_ttl_t ttl, dns_diff_t *diff, isc_mem_t *mctx, - bool dnssec_insecure) { + bool expect_cds_delete, bool expect_cdnskey_delete) { unsigned char dsbuf[5] = { 0, 0, 0, 0, 0 }; /* CDS DELETE rdata */ unsigned char keybuf[5] = { 0, 0, 3, 0, 0 }; /* CDNSKEY DELETE rdata */ char namebuf[DNS_NAME_FORMATSIZE]; @@ -2155,7 +2155,30 @@ dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey, dns_name_format(origin, namebuf, sizeof(namebuf)); - if (dnssec_insecure) { + if (expect_cds_delete) { + if (!dns_rdataset_isassociated(cds) || + !exists(cds, &cds_delete)) { + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO, + "CDS (DELETE) for zone %s is now " + "published", + namebuf); + RETERR(addrdata(&cds_delete, diff, origin, ttl, mctx)); + } + } else { + if (dns_rdataset_isassociated(cds) && exists(cds, &cds_delete)) + { + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO, + "CDS (DELETE) for zone %s is now " + "deleted", + namebuf); + RETERR(delrdata(&cds_delete, diff, origin, cds->ttl, + mctx)); + } + } + + if (expect_cdnskey_delete) { if (!dns_rdataset_isassociated(cdnskey) || !exists(cdnskey, &cdnskey_delete)) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -2166,16 +2189,6 @@ dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey, RETERR(addrdata(&cdnskey_delete, diff, origin, ttl, mctx)); } - - if (!dns_rdataset_isassociated(cds) || - !exists(cds, &cds_delete)) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO, - "CDS (DELETE) for zone %s is now " - "published", - namebuf); - RETERR(addrdata(&cds_delete, diff, origin, ttl, mctx)); - } } else { if (dns_rdataset_isassociated(cdnskey) && exists(cdnskey, &cdnskey_delete)) { @@ -2187,17 +2200,6 @@ dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey, RETERR(delrdata(&cdnskey_delete, diff, origin, cdnskey->ttl, mctx)); } - - if (dns_rdataset_isassociated(cds) && exists(cds, &cds_delete)) - { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO, - "CDS (DELETE) for zone %s is now " - "deleted", - namebuf); - RETERR(delrdata(&cds_delete, diff, origin, cds->ttl, - mctx)); - } } result = ISC_R_SUCCESS; diff --git a/lib/dns/include/dns/dnssec.h b/lib/dns/include/dns/dnssec.h index ed9927c732..719e723812 100644 --- a/lib/dns/include/dns/dnssec.h +++ b/lib/dns/include/dns/dnssec.h @@ -365,11 +365,14 @@ isc_result_t dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey, dns_name_t *origin, dns_rdataclass_t zclass, dns_ttl_t ttl, dns_diff_t *diff, isc_mem_t *mctx, - bool dnssec_insecure); + bool expect_cds_delete, bool expect_cdnskey_delete); /*%< * Add or remove the CDS DELETE record and the CDNSKEY DELETE record. - * If 'dnssec_insecure' is true, the DELETE records should be present. - * Otherwise, the DELETE records must be removed from the RRsets (if present). + * If 'expect_cds_delete' is true, the CDS DELETE record should be present. + * Otherwise, the CDS DELETE record must be removed from the RRsets (if + * present). If 'expect_cdnskey_delete' is true, the CDNSKEY DELETE record + * should be present. Otherwise, the CDNSKEY DELETE record must be removed + * from the RRsets (if present). * * Returns: *\li ISC_R_SUCCESS diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 76f6a01eef..b0dd2c2819 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -21641,16 +21641,69 @@ zone_rekey(dns_zone_t *zone) { KASP_UNLOCK(kasp); if (result == ISC_R_SUCCESS) { - bool cds_delete = false; + bool cdsdel = false; + bool cdnskeydel = false; isc_stdtime_t when; /* * Publish CDS/CDNSKEY DELETE records if the zone is * transitioning from secure to insecure. */ - if (kasp != NULL && - strcmp(dns_kasp_getname(kasp), "insecure") == 0) { - cds_delete = true; + if (kasp != NULL) { + if (strcmp(dns_kasp_getname(kasp), "insecure") == 0) { + cdsdel = true; + cdnskeydel = true; + } + } else { + /* Check if there is a CDS DELETE record. */ + if (dns_rdataset_isassociated(&cdsset)) { + for (result = dns_rdataset_first(&cdsset); + result == ISC_R_SUCCESS; + result = dns_rdataset_next(&cdsset)) + { + dns_rdata_t crdata = DNS_RDATA_INIT; + dns_rdataset_current(&cdsset, &crdata); + /* + * CDS deletion record has this form + * "0 0 0 00" which is 5 zero octets. + */ + if (crdata.length == 5U && + memcmp(crdata.data, + (unsigned char[5]){ 0, 0, 0, + 0, 0 }, + 5) == 0) + { + cdsdel = true; + break; + } + } + } + + /* Check if there is a CDNSKEY DELETE record. */ + if (dns_rdataset_isassociated(&cdnskeyset)) { + for (result = dns_rdataset_first(&cdnskeyset); + result == ISC_R_SUCCESS; + result = dns_rdataset_next(&cdnskeyset)) + { + dns_rdata_t crdata = DNS_RDATA_INIT; + dns_rdataset_current(&cdnskeyset, + &crdata); + /* + * CDNSKEY deletion record has this form + * "0 3 0 AA==" which is 2 zero octets, + * a 3, and 2 zero octets. + */ + if (crdata.length == 5U && + memcmp(crdata.data, + (unsigned char[5]){ 0, 0, 3, + 0, 0 }, + 5) == 0) + { + cdnskeydel = true; + break; + } + } + } } /* @@ -21687,36 +21740,36 @@ zone_rekey(dns_zone_t *zone) { goto failure; } - if (cds_delete) { + if (cdsdel || cdnskeydel) { /* * 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; + bool allow = 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) && + if (dst_key_is_published(dstk, now, &when) && dst_key_is_signing(dstk, DST_BOOL_KSK, now, &when)) { - cds_delete = true; + allow = true; break; } } + if (cdsdel) { + cdsdel = allow; + } + if (cdnskeydel) { + cdnskeydel = allow; + } } - result = dns_dnssec_syncdelete(&cdsset, &cdnskeyset, - &zone->origin, zone->rdclass, - ttl, &diff, mctx, cds_delete); + result = dns_dnssec_syncdelete( + &cdsset, &cdnskeyset, &zone->origin, zone->rdclass, ttl, + &diff, mctx, cdsdel, cdnskeydel); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_rekey:couldn't update CDS/CDNSKEY "