From: =?utf-8?b?VmxhZGltw61yIMSMdW7DoXQ=?= <vladimir.cunat@nic.cz>
Date: Sun, 11 Feb 2024 10:00:32 +0100
Subject: lib/cache: limit the amount of work on SHA1

That's when searching NSEC3 aggressive cache.
---
 lib/cache/nsec3.c  | 14 ++++++++++++++
 lib/dnssec/nsec3.h | 12 ++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/lib/cache/nsec3.c b/lib/cache/nsec3.c
index 9832630..2716456 100644
--- a/lib/cache/nsec3.c
+++ b/lib/cache/nsec3.c
@@ -272,8 +272,22 @@ int nsec3_encloser(struct key *k, struct answer *ans,
 	const int zname_labels = knot_dname_labels(k->zname, NULL);
 	int last_nxproven_labels = -1;
 	const knot_dname_t *name = qry->sname;
+
+	/* Avoid doing too much work on SHA1; we might consider that a part of mitigating
+	 *    CVE-2023-50868: NSEC3 closest encloser proof can exhaust CPU
+	 * As currently the code iterates from the longest name, we limit that.
+	 * Note that we don't want to limit too much, as the alternative usually includes
+	 * sending more queries upstream, which would come with nontrivial work, too.
+	 */
+	const int max_labels = zname_labels + kr_nsec3_max_depth(&ans->nsec_p.libknot);
+	if (sname_labels > max_labels)
+		VERBOSE_MSG(qry, "=> NSEC3 hashing partly skipped due to too long SNAME (CVE-2023-50868)\n");
+
 	for (int name_labels = sname_labels; name_labels >= zname_labels;
 					--name_labels, name += 1 + name[0]) {
+		if (name_labels > max_labels)
+			continue; // avoid the hashing
+
 		/* Find a previous-or-equal NSEC3 in cache covering the name,
 		 * checking TTL etc. */
 		const knot_db_val_t key = key_NSEC3_name(k, name, false, &ans->nsec_p);
diff --git a/lib/dnssec/nsec3.h b/lib/dnssec/nsec3.h
index 76ef2e9..a28d3c7 100644
--- a/lib/dnssec/nsec3.h
+++ b/lib/dnssec/nsec3.h
@@ -38,6 +38,18 @@ 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).