]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
layer/iterate: fix cache injection via CNAME
authorMarek Vavruša <mvavrusa@cloudflare.com>
Wed, 25 Jul 2018 18:43:19 +0000 (12:43 -0600)
committerTomas Krizek <tomas.krizek@nic.cz>
Thu, 2 Aug 2018 09:46:45 +0000 (09:46 +0000)
The current default mode doesn't check bailiwick anymore when unrolling
CNAME chains, so if an answer contains:

```
testingme.com.       3600 IN CNAME victim.com.
victim.com.         172800 IN NS attackers.ns
```

The resolver will cache both records as authoritative even though
`victim.com` isn't in the current bailiwick. This was previously
checked in 79d9931daaa5b9e6c7965f6ee29c965786a4754e, but removed
in refactoring.

lib/layer/iterate.c

index b30d451b3be99c86ef4c5da672e3cae8159cdc4c..a02acfba7a429d817a3d98671c46d3b1c9b8d01c 100644 (file)
@@ -500,7 +500,8 @@ static int unroll_cname(knot_pkt_t *pkt, struct kr_request *req, bool referral,
                                || type == KNOT_RRTYPE_CNAME || type == KNOT_RRTYPE_DNAME;
                                /* TODO: actually handle DNAMEs */
                        if (rr->rclass != KNOT_CLASS_IN || !type_OK
-                           || !knot_dname_is_equal(rr->owner, cname)) {
+                           || !knot_dname_is_equal(rr->owner, cname)
+                           || !knot_dname_in(query->zone_cut.name, rr->owner)) {
                                continue;
                        }
 
@@ -565,7 +566,14 @@ static int unroll_cname(knot_pkt_t *pkt, struct kr_request *req, bool referral,
                        cname = pending_cname;
                        break;
                }
-               /* try to unroll cname only within current zone */
+               /* Information outside bailiwick is not trusted. */
+               if (!knot_dname_in(query->zone_cut.name, pending_cname)) {
+                       cname = pending_cname;
+                       break;
+               }
+               /* The validator still can't handle multiple zones in one answer,
+                * so we only follow if a single label is replaced.
+                * TODO: this still isn't 100%, as the target may have a NS+DS. */
                const int pending_labels = knot_dname_labels(pending_cname, NULL);
                if (pending_labels != cname_labels) {
                        cname = pending_cname;