From bd852b1f97c2cc6ded1cd54127024959508a5943 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 | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index a75825896a..bf9a364a32 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -268,13 +268,13 @@ validator_done(dns_validator_t *val, isc_result_t result) { * 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; @@ -304,7 +304,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; } } @@ -318,7 +318,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: /* @@ -367,7 +367,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, @@ -380,7 +380,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; @@ -396,12 +396,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; } static void @@ -615,10 +615,9 @@ fetch_callback_ds(void *arg) { break; case DNS_R_NXRRSET: case DNS_R_NCACHENXRRSET: - result = isdelegation(val, resp->foundname, - &val->frdataset, eresult, - "fetch_callback_ds"); - if (result == ISC_R_SUCCESS) { + if (isdelegation(val, resp->foundname, &val->frdataset, + eresult, "fetch_callback_ds")) + { /* * Failed to find a DS while trying to prove * insecurity. If this is a zone cut, that @@ -739,8 +738,7 @@ validator_callback_ds(void *arg) { 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"); @@ -3223,9 +3221,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 f58554d05a37ce1a21e3edab2a1401c236ef7d84 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 bf9a364a32..f770a9cc70 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -258,9 +258,9 @@ validator_done(dns_validator_t *val, isc_result_t result) { } /*% - * 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 @@ -275,8 +275,9 @@ validator_done(dns_validator_t *val, isc_result_t result) { *\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(void *arg) { break; case DNS_R_NXRRSET: case DNS_R_NCACHENXRRSET: - if (isdelegation(val, resp->foundname, &val->frdataset, - eresult, "fetch_callback_ds")) + if (is_insecure_referral(val, resp->foundname, + &val->frdataset, eresult, + "fetch_callback_ds")) { /* * Failed to find a DS while trying to prove @@ -737,8 +739,9 @@ validator_callback_ds(void *arg) { 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"); @@ -3221,8 +3224,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");