Merge branch 'release-6.0.6' into 6.0v6.0.6

2 files changed, 55 insertions, 10 deletions

diff --git a/lib/dnssec/nsec3.c b/lib/dnssec/nsec3.c
index 037d5bdc..4199f25f 100644
--- a/lib/dnssec/nsec3.c
+++ b/lib/dnssec/nsec3.c
@@ -71,7 +71,7 @@ static int hash_name(dnssec_binary_t *hash, const dnssec_nsec3_params_t *params,
 		return kr_error(EINVAL);
 	if (!name)
 		return kr_error(EINVAL);
-	if (kr_fails_assert(params->iterations <= KR_NSEC3_MAX_ITERATIONS)) {
+	if (kr_fails_assert(!kr_nsec3_limited_params(params))) {
 		/* This if is mainly defensive; it shouldn't happen. */
 		return kr_error(EINVAL);
 	}
@@ -146,6 +146,18 @@ static int closest_encloser_match(int *flags, const knot_rrset_t *nsec3,
 	const knot_dname_t *encloser = knot_wire_next_label(name, NULL);
 	*skipped = 1;
 
+	/* Avoid doing too much work on SHA1, mitigating:
+	 *   CVE-2023-50868: NSEC3 closest encloser proof can exhaust CPU
+	 * We log nothing here; it wouldn't be easy from this place
+	 * and huge SNAME should be suspicious on its own.
+	 */
+	const int max_labels = knot_dname_labels(nsec3->owner, NULL) - 1
+				+ kr_nsec3_max_depth(&params);
+	for (int l = knot_dname_labels(encloser, NULL); l > max_labels; --l) {
+		encloser = knot_wire_next_label(encloser, NULL);
+		++(*skipped);
+	}
+
 	while(encloser) {
 		ret = hash_name(&name_hash, &params, encloser);
 		if (ret != 0)
@@ -565,7 +577,7 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_
 		const knot_rrset_t *rrset = knot_pkt_rr(sec, i);
 		if (rrset->type != KNOT_RRTYPE_NSEC3)
 			continue;
-		if (knot_nsec3_iters(rrset->rrs.rdata) > KR_NSEC3_MAX_ITERATIONS) {
+		if (kr_nsec3_limited_rdata(rrset->rrs.rdata)) {
 			/* Avoid hashing with too many iterations.
 			 * If we get here, the `sname` wildcard probably ends up bogus,
 			 * but it gets downgraded to KR_RANK_INSECURE when validator
diff --git a/lib/dnssec/nsec3.h b/lib/dnssec/nsec3.h
index eb0bd397..a28d3c78 100644
--- a/lib/dnssec/nsec3.h
+++ b/lib/dnssec/nsec3.h
@@ -5,18 +5,51 @@
 #pragma once
 
 #include <libknot/packet/pkt.h>
+#include <libknot/rrtype/nsec3.h>
+#include <libdnssec/nsec.h>
+
+
+static inline unsigned int kr_nsec3_price(unsigned int iterations, unsigned int salt_len)
+{
+	// SHA1 works on 64-byte chunks.
+	// On iterating we hash the salt + 20 bytes of the previous hash.
+	int chunks_per_iter = (20 + salt_len - 1) / 64 + 1;
+	return (iterations + 1) * chunks_per_iter;
+}
 
 /** High numbers in NSEC3 iterations don't really help security
  *
- * ...so we avoid doing all the work.  The value is a current compromise;
- * zones shooting over get downgraded to insecure status.
+ * ...so we avoid doing all the work.  The limit is a current compromise;
+ * answers using NSEC3 over kr_nsec3_limited* get downgraded to insecure status.
  *
- * Original restriction wasn't that strict:
-   https://datatracker.ietf.org/doc/html/rfc5155#section-10.3
- * but there is discussion about officially lowering the limits:
-   https://tools.ietf.org/id/draft-hardaker-dnsop-nsec3-guidance-02.html#section-2.3
+   https://datatracker.ietf.org/doc/html/rfc9276#name-recommendation-for-validati
  */
-#define KR_NSEC3_MAX_ITERATIONS 150
+static inline bool kr_nsec3_limited(unsigned int iterations, unsigned int salt_len)
+{
+	const int MAX_ITERATIONS = 50; // limit with short salt length
+	return kr_nsec3_price(iterations, salt_len) > MAX_ITERATIONS + 1;
+}
+static inline bool kr_nsec3_limited_rdata(const knot_rdata_t *rd)
+{
+	return kr_nsec3_limited(knot_nsec3_iters(rd), knot_nsec3_salt_len(rd));
+}
+static inline bool kr_nsec3_limited_params(const dnssec_nsec3_params_t *params)
+{
+	return kr_nsec3_limited(params->iterations, params->salt.size);
+}
+
+/** Return limit on NSEC3 depth.  The point is to avoid doing too much work on SHA1.
+ *
+ * CVE-2023-50868: NSEC3 closest encloser proof can exhaust CPU
+ *
+ * 128 is chosen so that zones with good NSEC3 parameters (giving _price() == 1)
+ * won't be limited in any way.  Performance doesn't seem too bad with that either.
+ */
+static inline int kr_nsec3_max_depth(const dnssec_nsec3_params_t *params)
+{
+	return 128 / kr_nsec3_price(params->iterations, params->salt.size);
+}
+
 
 /**
  * Name error response check (RFC5155 7.2.2).
@@ -39,7 +72,7 @@ int kr_nsec3_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sec
  *                     KNOT_ERANGE - NSEC3 RR that covers a wildcard
  *                     has been found, but has opt-out flag set;
  *                     otherwise - error.
- * Records over KR_NSEC3_MAX_ITERATIONS are skipped, so you probably get kr_error(ENOENT).
+ * Too expensive NSEC3 records are skipped, so you probably get kr_error(ENOENT).
  */
 int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t section_id,
                                             const knot_dname_t *sname, int trim_to_next);