Merge !1659: validator: accept a confusing NODATA proof with insecure delegation

Author: vcunat

NEWS

@@ -13,6 +13,7 @@ Improvements
 - manager: allow multiple instances with different rundirs (!1656)
 - tests: disable problematic config.http test (#925, !1662)
 - /management/unix-socket: allow path relative to rundir (!1657)
+- validator: accept a confusing NODATA proof with insecure delegation (!1659)
 
 
 Knot Resolver 6.0.10 (2025-01-20)

lib/dnssec/nsec.c

@@ -161,10 +161,16 @@ int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t ty
 		break;
 	default:
 		/* Parent-side delegation record isn't authoritative for non-DS;
-		 * see RFC6840 4.1. */
+		 * see RFC6840 4.1.
+		 *
+		 * Additionally, we signal if the NODATA would belong
+		 * to an *insecure* child zone.
+		 */
 		if (dnssec_nsec_bitmap_contains(bm, bm_size, KNOT_RRTYPE_NS)
 		    && !dnssec_nsec_bitmap_contains(bm, bm_size, KNOT_RRTYPE_SOA)) {
-			return NO_PROOF;
+			return dnssec_nsec_bitmap_contains(bm, bm_size, KNOT_RRTYPE_DS)
+				? NO_PROOF
+				: KNOT_EDOWNGRADED;
 		}
 		/* LATER(opt): perhaps short-circuit test if we repeat it here. */
 	}
@@ -218,9 +224,12 @@ int kr_nsec_negative(const ranked_rr_array_t *rrrs, uint32_t qry_uid,
 			&& kr_rank_test(rrrs->at[i]->rank, KR_RANK_SECURE);
 		if (!ok) continue;
 		const int covers = nsec_covers(nsec, sname);
-		if (covers == abs(EEXIST)
-		    && no_data_response_check_rrtype(nsec, stype) == 0) {
-			return PKT_NODATA; // proven NODATA by matching NSEC
+		if (covers == abs(EEXIST)) {
+			int ret = no_data_response_check_rrtype(nsec, stype);
+			if (ret == 0)
+				return PKT_NODATA; // proven NODATA by matching NSEC
+			if (ret == KNOT_EDOWNGRADED)
+				return ret;
 		}
 		if (covers != 0) continue;
 

lib/dnssec/nsec.h

@@ -8,6 +8,8 @@
 
 #include "lib/layer/iterate.h"
 
+#define KNOT_EDOWNGRADED (KNOT_ERROR_MIN - 1)
+
 /**
  * Check bitmap that child names are contained in the same zone.
  * @note see RFC6840 4.1.
@@ -25,6 +27,7 @@ int kr_nsec_children_in_zone_check(const uint8_t *bm, uint16_t bm_size);
  * @param owner   NSEC record owner.
  * @note This includes special checks for zone cuts, e.g. from RFC 6840 sec. 4.
  * @return 0, abs(ENOENT) (no proof), kr_error(EINVAL)
+ *   KNOT_EDOWNGRADED: special case where the RR would be in an insecure child zone.
  */
 int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t type, const knot_dname_t *owner);
 
@@ -44,6 +47,7 @@ int kr_nsec_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t
  * @param rrrs       list of RRs to search; typically kr_request::auth_selected
  * @param qry_uid    only consider NSECs from this packet, for better efficiency
  * @return           negative error code, or PKT_NXDOMAIN | PKT_NODATA (both for NXDOMAIN)
+ *                   KNOT_EDOWNGRADED: special case where the RR would be in an insecure child zone.
  */
 int kr_nsec_negative(const ranked_rr_array_t *rrrs, uint32_t qry_uid,
 			const knot_dname_t *sname, uint16_t stype);

lib/dnssec/nsec3.c

@@ -507,6 +507,7 @@ int kr_nsec3_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sec
  * @param sname      Name to be checked.
  * @param stype      Type to be checked.
  * @return           0 or error code.
+ *                   KNOT_EDOWNGRADED: special case where the RR would be in an insecure child zone.
  * @note             This does NOT check the opt-out case if type is DS;
  *                   see RFC 5155 8.6.
  */
@@ -528,8 +529,9 @@ static int nodata_find(const knot_pkt_t *pkt, knot_section_t section_id,
 
 		const uint8_t *bm = knot_nsec3_bitmap(nsec3->rrs.rdata);
 		uint16_t bm_size = knot_nsec3_bitmap_len(nsec3->rrs.rdata);
-		if (kr_nsec_bitmap_nodata_check(bm, bm_size, type, nsec3->owner) == kr_ok())
-			return kr_ok();
+		int ret = kr_nsec_bitmap_nodata_check(bm, bm_size, type, nsec3->owner);
+		if (ret == kr_ok() || ret == KNOT_EDOWNGRADED)
+			return ret;
 	}
 
 	return kr_error(ENOENT);
@@ -602,8 +604,8 @@ int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id,
 {
 	/* DS record may be also matched by an existing NSEC3 RR. */
 	int ret = nodata_find(pkt, section_id, sname, stype);
-	if (ret == 0) {
-		/* Satisfies RFC5155 8.5 and 8.6, both first paragraph. */
+	if (ret == 0 || ret == KNOT_EDOWNGRADED) {
+		/* If 0, satisfies RFC5155 8.5 and 8.6, both first paragraph. */
 		return ret;
 	}
 

lib/dnssec/nsec3.h

@@ -87,6 +87,8 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_
  * @return           0 or error code:
  *                   DNSSEC_NOT_FOUND - neither ds nor nsec records
  *                   were not found.
+ *                   KNOT_EDOWNGRADED - special case where
+ *                     the RR would be in an insecure child zone.
  *                   KNOT_ERANGE - denial of existence can't be proven
  *                   due to opt-out, otherwise - bogus.
  */

lib/layer/validate.c

@@ -146,8 +146,6 @@ static bool maybe_downgrade_nsec3(const ranked_rr_array_entry_t *e, struct kr_qu
 	return true;
 }
 
-#define KNOT_EDOWNGRADED (KNOT_ERROR_MIN - 1)
-
 static int validate_section(kr_rrset_validation_ctx_t *vctx, struct kr_query *qry,
 			    knot_mm_t *pool)
 {
@@ -1323,6 +1321,12 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt)
 					 * we must continue, validate NSEC\NSEC3 and
 					 * call update_parent_keys() to mark
 					 * parent queries as insecure */
+				} else if (ret == KNOT_EDOWNGRADED) { // either NSEC3 or NSEC
+					VERBOSE_MSG(qry, "<= DNSSEC downgraded by a weird proof confusing NODATA with insecure delegation\n");
+					qry->flags.DNSSEC_WANT = false;
+					qry->flags.DNSSEC_INSECURE = true;
+					rank_records(qry, true, KR_RANK_INSECURE, qry->sname);
+					mark_insecure_parents(qry);
 				} else {
 					VERBOSE_MSG(qry, "<= bad NODATA proof\n");
 					kr_request_set_extended_error(req, KNOT_EDNS_EDE_NSEC_MISS, "AHXI");