]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
cache: fix caching NSEC ranges with \000 in *ending*
authorVladimír Čunát <vladimir.cunat@nic.cz>
Tue, 6 Apr 2021 10:11:41 +0000 (12:11 +0200)
committerVladimír Čunát <vladimir.cunat@nic.cz>
Sat, 10 Apr 2021 10:06:42 +0000 (12:06 +0200)
Our aggressive NSEC cache doesn't handle these well and the case
with only the end-label being like this was forgotten.
See the parent commit for a test case.

Also, larger NSEC* sets are now considered weird.

lib/cache/api.c

index 922149d1c16d0b1d4765e376ed95effa60e0f820..4c2aa82dc575e0af1270a34eae9c35269a60bdbb 100644 (file)
@@ -465,21 +465,24 @@ static int stash_rrset_precond(const knot_rrset_t *rr, const struct kr_query *qr
  * Also include some abnormal RR cases; qry is just for logging. */
 static bool rrset_has_min_range_or_weird(const knot_rrset_t *rr, const struct kr_query *qry)
 {
-       if (!rr->rrs.count) {
-               assert(!EINVAL);
-               return true; /*< weird */
+       if (rr->rrs.count != 1) {
+               assert(rr->rrs.count > 0);
+               if (rr->type == KNOT_RRTYPE_NSEC || rr->type == KNOT_RRTYPE_NSEC3
+                               || rr->rrs.count == 0) {
+                       return true; /*< weird */
+               }
        }
        bool ret; /**< NOT used for the weird cases */
        if (rr->type == KNOT_RRTYPE_NSEC) {
-               /* NSEC: name -> \000.name
-                * Note: we have to lower-case it; the reasons are better explained
-                * on other knot_nsec_next() call sites. */
-               knot_dname_t next[KNOT_DNAME_MAXLEN];
-               if (knot_dname_to_wire(next, knot_nsec_next(rr->rrs.rdata), sizeof(next)) < 0)
-                       return true; /*< weird */
-               knot_dname_to_lower(next);
-               ret = next[0] == '\1' && next[1] == '\0'
-                       && knot_dname_is_equal(next + 2, rr->owner);
+               if (!check_dname_for_lf(rr->owner, qry))
+                       return true; /*< weird, probably filtered even before this point */
+               ret = !check_dname_for_lf(knot_nsec_next(rr->rrs.rdata), qry);
+               /* ^^ Zero inside the next-name label means it's probably a minimal range,
+                * and anyway it's problematic for our aggressive cache (comparisons).
+                * Real-life examples covered:
+                *   NSEC: name -> \000.name (e.g. typical foobar.CloudFlare.net)
+                *   NSEC: name -> name\000 (CloudFlare on delegations)
+                */
        } else if (rr->type == KNOT_RRTYPE_NSEC3) {
                if (knot_nsec3_next_len(rr->rrs.rdata) != NSEC3_HASH_LEN
                    || *rr->owner != NSEC3_HASH_TXT_LEN) {