Merge tag 'v1.5.2', bringing security fixes

lib/dnssec/nsec.c

@@ -30,6 +30,7 @@
 bool kr_nsec_bitmap_contains_type(const uint8_t *bm, uint16_t bm_size, uint16_t type)
 {
 	if (!bm || bm_size == 0) {
+		assert(bm);
 		return false;
 	}
 
@@ -59,29 +60,52 @@ bool kr_nsec_bitmap_contains_type(const uint8_t *bm, uint16_t bm_size, uint16_t
 	return false;
 }
 
+int kr_nsec_children_in_zone_check(const uint8_t *bm, uint16_t bm_size)
+{
+	if (!bm) {
+		return kr_error(EINVAL);
+	}
+	const bool parent_side =
+		kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_DNAME)
+		|| (kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_NS)
+		    && !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)
+		);
+	return parent_side ? abs(ENOENT) : kr_ok();
+	/* LATER: after refactoring, probably also check if signer name equals owner,
+	 * but even without that it's not possible to attack *correctly* signed zones.
+	 */
+}
+
 /**
  * Check whether the NSEC RR proves that there is no closer match for <SNAME, SCLASS>.
  * @param nsec  NSEC RRSet.
  * @param sname Searched name.
- * @return      0 or error code.
+ * @return      0 if proves, >0 if not (abs(ENOENT)), or error code (<0).
  */
-static int nsec_nonamematch(const knot_rrset_t *nsec, const knot_dname_t *sname)
+static int nsec_covers(const knot_rrset_t *nsec, const knot_dname_t *sname)
 {
 	assert(nsec && sname);
 	const knot_dname_t *next = knot_nsec_next(&nsec->rrs);
+	if (knot_dname_cmp(sname, nsec->owner) <= 0) {
+		return abs(ENOENT); /* 'sname' before 'owner', so can't be covered */
+	}
 	/* If NSEC 'owner' >= 'next', it means that there is nothing after 'owner' */
-	const bool is_last_nsec = (knot_dname_cmp(nsec->owner, next) >= 0);
-	if (is_last_nsec) { /* SNAME is after owner => provably doesn't exist */
-		if (knot_dname_cmp(nsec->owner, sname) < 0) {
-			return kr_ok();
-		}
-	} else {
-		/* Prove that SNAME is between 'owner' and 'next' */
-		if ((knot_dname_cmp(nsec->owner, sname) < 0) && (knot_dname_cmp(sname, next) < 0)) {
-			return kr_ok();
-		}
+	const bool is_last_nsec = knot_dname_cmp(nsec->owner, next) >= 0;
+	const bool in_range = is_last_nsec || knot_dname_cmp(sname, next) < 0;
+	if (!in_range) {
+		return abs(ENOENT);
 	}
-	return kr_error(EINVAL);
+	/* Before returning kr_ok(), we have to check a special case:
+	 * sname might be under delegation from owner and thus
+	 * not in the zone of this NSEC at all.
+	 */
+	if (!knot_dname_is_sub(sname, nsec->owner)) {
+		return kr_ok();
+	}
+	uint8_t *bm = NULL;
+	uint16_t bm_size = 0;
+	knot_nsec_bitmap(&nsec->rrs, &bm, &bm_size);
+	return kr_nsec_children_in_zone_check(bm, bm_size);
 }
 
 #define FLG_NOEXIST_RRTYPE (1 << 0) /**< <SNAME, SCLASS> exists, <SNAME, SCLASS, STYPE> does not exist. */
@@ -128,7 +152,7 @@ static int name_error_response_check_rr(int *flags, const knot_rrset_t *nsec,
 {
 	assert(flags && nsec && name);
 
-	if (nsec_nonamematch(nsec, name) == 0) {
+	if (nsec_covers(nsec, name) == 0) {
 		*flags |= FLG_NOEXIST_RRSET;
 	}
 
@@ -147,7 +171,7 @@ static int name_error_response_check_rr(int *flags, const knot_rrset_t *nsec,
 		*(--ptr) = '*';
 		*(--ptr) = 1;
 		/* True if this wildcard provably doesn't exist. */
-		if (nsec_nonamematch(nsec, ptr) == 0) {
+		if (nsec_covers(nsec, ptr) == 0) {
 			*flags |= FLG_NOEXIST_WILDCARD;
 			break;
 		}
@@ -219,41 +243,75 @@ static int coverign_rrsig_labels(const knot_rrset_t *nsec, const knot_pktsection
 	return ret;
 }
 
-/**
- * Perform check of RR type existence denial according to RFC4035 5.4, bullet 1.
- * @param flags Flags to be set according to check outcome.
- * @param nsec  NSEC RR.
- * @param type  Type to be checked.
- * @return      0 or error code.
- */
-static int no_data_response_check_rrtype(int *flags, const knot_rrset_t *nsec,
-                                         uint16_t type)
-{
-	assert(flags && nsec);
 
-	uint8_t *bm = NULL;
-	uint16_t bm_size;
-	knot_nsec_bitmap(&nsec->rrs, &bm, &bm_size);
+int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t type)
+{
+	const int NO_PROOF = abs(ENOENT);
 	if (!bm) {
 		return kr_error(EINVAL);
 	}
+	if (kr_nsec_bitmap_contains_type(bm, bm_size, type)) {
+		return NO_PROOF;
+	}
 
-	if (!kr_nsec_bitmap_contains_type(bm, bm_size, type)) {
-		/* The type is not listed in the NSEC bitmap. */
+	if (type != KNOT_RRTYPE_CNAME
+	    && kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_CNAME)) {
+		return NO_PROOF;
+	}
+	/* Special behavior around zone cuts. */
+	switch (type) {
+	case KNOT_RRTYPE_DS:
 		/* Security feature: in case of DS also check for SOA
 		 * non-existence to be more certain that we don't hold
 		 * a child-side NSEC by some mistake (e.g. when forwarding).
 		 * See RFC4035 5.2, next-to-last paragraph. */
-		if (type != KNOT_RRTYPE_DS
-		    || !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) {
-			*flags |= FLG_NOEXIST_RRTYPE;
+		if (kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) {
+			return NO_PROOF;
+		}
+		break;
+	case KNOT_RRTYPE_CNAME:
+		/* Exception from the `default` rule.  It's perhaps disputable,
+		 * but existence of CNAME at zone apex is not allowed, so we
+		 * consider a parent-side record to be enough to prove non-existence. */
+		break;
+	default:
+		/* Parent-side delegation record isn't authoritative for non-DS;
+		 * see RFC6840 4.1. */
+		if (kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_NS)
+		    && !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) {
+			return NO_PROOF;
 		}
+		/* LATER(opt): perhaps short-circuit test if we repeat it here. */
 	}
 
 	return kr_ok();
 }
 
 /**
+ * Attempt to prove NODATA given a matching NSEC.
+ * @param flags Flags to be set according to check outcome.
+ * @param nsec  NSEC RR.
+ * @param type  Type to be checked.
+ * @return      0 on success, abs(ENOENT) for no proof, or error code (<0).
+ * @note        It's not a *full* proof, of course (wildcards, etc.)
+ * @TODO returning result via `flags` is just ugly.
+ */
+static int no_data_response_check_rrtype(int *flags, const knot_rrset_t *nsec,
+                                         uint16_t type)
+{
+	assert(flags && nsec);
+
+	uint8_t *bm = NULL;
+	uint16_t bm_size = 0;
+	knot_nsec_bitmap(&nsec->rrs, &bm, &bm_size);
+	int ret = kr_nsec_bitmap_nodata_check(bm, bm_size, type);
+	if (ret == kr_ok()) {
+		*flags |= FLG_NOEXIST_RRTYPE;
+	}
+	return ret <= 0 ? ret : kr_ok();
+}
+
+/**
  * Perform check for RR type wildcard existence denial according to RFC4035 5.4, bullet 1.
  * @param flags Flags to be set according to check outcome.
  * @param nsec  NSEC RR.
@@ -360,7 +418,7 @@ int kr_nsec_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t
 		if (rrset->type != KNOT_RRTYPE_NSEC) {
 			continue;
 		}
-		if (nsec_nonamematch(rrset, sname) == 0) {
+		if (nsec_covers(rrset, sname) == 0) {
 			return kr_ok();
 		}
 	}
@@ -476,11 +534,15 @@ int kr_nsec_ref_to_unsigned(const knot_pkt_t *pkt)
 int kr_nsec_matches_name_and_type(const knot_rrset_t *nsec,
 				   const knot_dname_t *name, uint16_t type)
 {
-	if (!nsec || !name) {
-		return (EINVAL);
+	/* It's not secure enough to just check a single bit for (some) other types,
+	 * but we don't (currently) only use this API for NS.  See RFC 6840 sec. 4.
+	 */
+	if (type != KNOT_RRTYPE_NS || !nsec || !name) {
+		assert(!EINVAL);
+		return kr_error(EINVAL);
 	}
 	if (!knot_dname_is_equal(nsec->owner, name)) {
-		return (ENOENT);
+		return kr_error(ENOENT);
 	}
 	uint8_t *bm = NULL;
 	uint16_t bm_size = 0;
@@ -488,8 +550,9 @@ int kr_nsec_matches_name_and_type(const knot_rrset_t *nsec,
 	if (!bm) {
 		return kr_error(EINVAL);
 	}
-	if (!kr_nsec_bitmap_contains_type(bm, bm_size, type)) {
-		return (ENOENT);
+	if (kr_nsec_bitmap_contains_type(bm, bm_size, type)) {
+		return kr_ok();
+	} else {
+		return kr_error(ENOENT);
 	}
-	return kr_ok();
 }