From 33a3e1ebff7d9330acd709095c6ace8df6c1a9d8 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 2 Apr 2026 11:16:29 +0200 Subject: [PATCH 1/2] Revert isdelegation() to return boolean value again The isdelegation() was changed to return an isc_result_t because the idea was to have a separate return value DNS_R_NSEC3ITERRANGE to signal to the caller we could not verify the proof because of too many iterations in the NSEC3 record, or perhaps ISC_R_UNEXPECTED for a more generic cause that verification was not done. But this would make error handling more fragile and all we care about is whether we can reliably say the NS bit was not set. If we can not reliably say so, we have to treat it as an insecure referrral. Since the answer is either yes or no, we can revert back to returning a boolean value. (cherry picked from commit 3ac1bb1c391394d23a40facd424e49acfa5513bf) --- lib/dns/validator.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 7db102062b..d7d38df39a 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -266,13 +266,13 @@ exit_check(dns_validator_t *val) { * the delegation. * * Returns: - *\li #ISC_R_SUCCESS the NS bitmap was set in the NSEC or NSEC3 record, or - * the NSEC3 covers the name (in case of opt-out), or - * we cannot validate the insecurity proof and are going - * to treat the message as isnecure. - *\li #ISC_R_NOTFOUND the NS bitmap was not set, + *\li #true the NS bitmap was set in the NSEC or NSEC3 record, or + * the NSEC3 covers the name (in case of opt-out), or + * we cannot validate the insecurity proof and are going + * to treat the message as insecure. + *\li #false the NS bitmap was not set. */ -static isc_result_t +static bool isdelegation(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset, isc_result_t dbresult, const char *caller) { dns_fixedname_t fixed; @@ -302,7 +302,7 @@ isdelegation(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset, goto trynsec3; } if (result != ISC_R_SUCCESS) { - return ISC_R_NOTFOUND; + return false; } } @@ -316,7 +316,7 @@ isdelegation(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset, dns_rdata_reset(&rdata); } dns_rdataset_disassociate(&set); - return found ? ISC_R_SUCCESS : ISC_R_NOTFOUND; + return found; trynsec3: /* @@ -365,7 +365,7 @@ trynsec3: "%s: too many iterations", caller); dns_rdataset_disassociate(&set); - return ISC_R_SUCCESS; + return true; } length = isc_iterated_hash( hash, nsec3.hash, nsec3.iterations, nsec3.salt, @@ -378,7 +378,7 @@ trynsec3: found = dns_nsec3_typepresent(&rdata, dns_rdatatype_ns); dns_rdataset_disassociate(&set); - return found ? ISC_R_SUCCESS : ISC_R_NOTFOUND; + return found; } if ((nsec3.flags & DNS_NSEC3FLAG_OPTOUT) == 0) { continue; @@ -394,12 +394,12 @@ trynsec3: memcmp(hash, nsec3.next, length) < 0))) { dns_rdataset_disassociate(&set); - return ISC_R_SUCCESS; + return true; } } dns_rdataset_disassociate(&set); } - return found ? ISC_R_SUCCESS : ISC_R_NOTFOUND; + return found; } /*% @@ -616,8 +616,7 @@ fetch_callback_ds(isc_task_t *task, isc_event_t *event) { goto unexpected; } else if (eresult != DNS_R_CNAME && isdelegation(val, devent->foundname, &val->frdataset, - eresult, - "fetch_callback_ds") == ISC_R_SUCCESS) + eresult, "fetch_callback_ds")) { /* * Failed to find a DS while trying to prove @@ -786,8 +785,7 @@ validator_callback_ds(isc_task_t *task, isc_event_t *event) { val->frdataset.covers == dns_rdatatype_ds && NEGATIVE(&val->frdataset) && isdelegation(val, name, &val->frdataset, - DNS_R_NCACHENXRRSET, - "validator_callback_ds") == ISC_R_SUCCESS) + DNS_R_NCACHENXRRSET, "validator_callback_ds")) { result = markanswer(val, "validator_callback_ds", "no DS and this is a delegation"); @@ -2879,9 +2877,9 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { return ISC_R_COMPLETE; } - result = isdelegation(val, tname, &val->frdataset, result, - "seek_ds"); - if (result == ISC_R_SUCCESS) { + if (isdelegation(val, tname, &val->frdataset, result, + "seek_ds")) + { *resp = markanswer(val, "seek_ds (3)", "this is a delegation"); return ISC_R_COMPLETE; From b5f3e92fa75a0ceab3925da9fcbf769f121d8010 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 2 Apr 2026 11:20:32 +0200 Subject: [PATCH 2/2] Rename isdelegation() to is_insecure_referral() The name 'isdelegation()' was confusing. This function is not checking whether this message is a delegation, but whether the denial of existence proofs in this message is a proof of a referral to an unsigned zone. The name 'is_unsecure_referral()' is more appropriate. (cherry picked from commit e0f09bb3743015b405ae48c9664f46a422f546e2) --- lib/dns/validator.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index d7d38df39a..3ea4a640ae 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -256,9 +256,9 @@ exit_check(dns_validator_t *val) { } /*% - * The isdelegation() function is called as part of seeking the DS record. - * Look in the NSEC or NSEC3 record returned from a DS query to see if the - * record has the NS bitmap set. If so, we are at a delegation point. + * The is_insecure_referral() function is called as part of seeking the DS + * record. Look in the NSEC or NSEC3 record returned from a DS query to see if + * the record has the NS bitmap set. If so, we are at a delegation point. * * If the response contains NSEC3 records with too high iterations, we cannot * (or rather we are not going to) validate the insecurity proof. Instead we @@ -273,8 +273,9 @@ exit_check(dns_validator_t *val) { *\li #false the NS bitmap was not set. */ static bool -isdelegation(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset, - isc_result_t dbresult, const char *caller) { +is_insecure_referral(dns_validator_t *val, dns_name_t *name, + dns_rdataset_t *rdataset, isc_result_t dbresult, + const char *caller) { dns_fixedname_t fixed; dns_label_t hashlabel; dns_name_t nsec3name; @@ -615,8 +616,9 @@ fetch_callback_ds(isc_task_t *task, isc_event_t *event) { } else if (eresult == DNS_R_SERVFAIL) { goto unexpected; } else if (eresult != DNS_R_CNAME && - isdelegation(val, devent->foundname, &val->frdataset, - eresult, "fetch_callback_ds")) + is_insecure_referral(val, devent->foundname, + &val->frdataset, eresult, + "fetch_callback_ds")) { /* * Failed to find a DS while trying to prove @@ -784,8 +786,9 @@ validator_callback_ds(isc_task_t *task, isc_event_t *event) { if ((val->attributes & VALATTR_INSECURITY) != 0 && val->frdataset.covers == dns_rdatatype_ds && NEGATIVE(&val->frdataset) && - isdelegation(val, name, &val->frdataset, - DNS_R_NCACHENXRRSET, "validator_callback_ds")) + is_insecure_referral(val, name, &val->frdataset, + DNS_R_NCACHENXRRSET, + "validator_callback_ds")) { result = markanswer(val, "validator_callback_ds", "no DS and this is a delegation"); @@ -2877,8 +2880,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { return ISC_R_COMPLETE; } - if (isdelegation(val, tname, &val->frdataset, result, - "seek_ds")) + if (is_insecure_referral(val, tname, &val->frdataset, result, + "seek_ds")) { *resp = markanswer(val, "seek_ds (3)", "this is a delegation");