lib/layer: CNAME chain construction improvement

diff --git a/lib/defines.h b/lib/defines.h
index 7031f7ddb5fe8d9fc270ae0d1ee6e7f73a9620c2..aeda472b959d4c7d378eb6f83db18ddf8fd6661c 100644 (file)
--- a/lib/defines.h
+++ b/lib/defines.h
@@ -53,6 +53,7 @@ static inline int __attribute__((__cold__)) kr_error(int x) {
 #define KR_CONN_RTT_MAX 3000 /* Timeout for network activity */
 #define KR_CONN_RETRY 300    /* Retry interval for network activity */
 #define KR_ITER_LIMIT 50     /* Built-in iterator limit */
+#define KR_CNAME_CHAIN_LIMIT 40 /* Built-in maximum CNAME chain length */
 
 /*
  * Defines.

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 776691f7cb7df12d36055ac0cbe30aef0bb5284e..cb2ab6b460e72dd383edb217bd59a789daa3377b 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -19,6 +19,7 @@
 
 #include <libknot/descriptor.h>
 #include <libknot/rrtype/rdname.h>
+#include <libknot/rrtype/rrsig.h>
 
 #include "lib/layer/iterate.h"
 #include "lib/resolve.h"
@@ -110,19 +111,6 @@ int kr_response_classify(knot_pkt_t *pkt)
        }
 }
 
-static void follow_cname_chain(const knot_dname_t **cname, const knot_rrset_t *rr,
-                               struct kr_query *cur)
-{
-       if (rr->type == KNOT_RRTYPE_CNAME) {
-               const knot_dname_t *next_cname = knot_cname_name(&rr->rrs);
-               if (next_cname)
-                       *cname = next_cname;
-       } else if (rr->type != KNOT_RRTYPE_RRSIG) {
-               /* Terminate CNAME chain (if not RRSIG). */
-               *cname = cur->sname;
-       }
-}
-
 /** @internal Filter ANY or loopback addresses. */
 static bool is_valid_addr(const uint8_t *addr, size_t len)
 {
@@ -346,7 +334,6 @@ static void finalize_answer(knot_pkt_t *pkt, struct kr_query *qry, struct kr_req
 static int process_answer(knot_pkt_t *pkt, struct kr_request *req)
 {
        struct kr_query *query = req->current_query;
-
        /* Response for minimized QNAME.
         * NODATA   => may be empty non-terminal, retry (found zone cut)
         * NOERROR  => found zone cut, retry
@@ -371,39 +358,75 @@ static int process_answer(knot_pkt_t *pkt, struct kr_request *req)
 
        /* Process answer type */
        const knot_pktsection_t *an = knot_pkt_section(pkt, KNOT_ANSWER);
-       bool follow_chain = (query->stype != KNOT_RRTYPE_CNAME);
-       const knot_dname_t *cname = query->sname;
-       for (unsigned i = 0; i < an->count; ++i) {
-               /* @todo construct a CNAME chain closure and accept all names from that set */ 
-               const knot_rrset_t *rr = knot_pkt_rr(an, i);
-               if (!knot_dname_is_equal(rr->owner, query->sname) &&
-                       !(follow_chain && knot_dname_is_equal(rr->owner, cname))) {
-                       continue;
-               }
-               unsigned hint = 0;
-               if(knot_dname_is_equal(cname, knot_pkt_qname(req->answer))) {
-                       hint = KNOT_COMPR_HINT_QNAME;
-               }
-               int state = is_final ? update_answer(rr, hint, req->answer) : update_parent(rr, query);
-               if (state == KNOT_STATE_FAIL) {
-                       return state;
-               }
-               /* Follow chain only within current cut (if secure). */
-               if (follow_chain) {
-                       follow_cname_chain(&cname, rr, query);
-                       if (!(query->flags & QUERY_DNSSEC_WANT) || !knot_dname_in(query->zone_cut.name, cname)) {
-                               follow_chain = false; 
+       const knot_dname_t *cname = NULL;
+       const knot_dname_t *pending_cname = query->sname;
+       unsigned cname_chain_len = 0;
+       while (pending_cname) {
+               /* CNAME was found at previous iteration, but records may not follow the correct order.
+                * Try to find records for pending_cname owner from section start. */
+               cname = pending_cname;
+               pending_cname = NULL;
+               for (unsigned i = 0; i < an->count; ++i) {
+                       const knot_rrset_t *rr = knot_pkt_rr(an, i);
+                       if (!knot_dname_is_equal(rr->owner, cname)) {
+                               continue;
+                       }
+                       /* Process records matching current SNAME */
+                       unsigned hint = 0;
+                       if(knot_dname_is_equal(cname, knot_pkt_qname(req->answer))) {
+                               hint = KNOT_COMPR_HINT_QNAME;
+                       }
+                       int state = is_final ? update_answer(rr, hint, req->answer) : update_parent(rr, query);
+                       if (state == KNOT_STATE_FAIL) {
+                               return state;
+                       }
+                       /* Jump to next CNAME target */
+                       if ((query->stype == KNOT_RRTYPE_CNAME) || (rr->type != KNOT_RRTYPE_CNAME)) {
+                               continue;
+                       }
+                       cname_chain_len += 1;
+                       pending_cname = knot_cname_name(&rr->rrs);
+                       if (!pending_cname) {
+                               break;
+                       }
+                       if (cname_chain_len > an->count || cname_chain_len > KR_CNAME_CHAIN_LIMIT) {
+                               DEBUG_MSG("<= too long cname chain\n");
+                               return KNOT_STATE_FAIL;
+                       }
+                       /* If secure, don't use pending_cname immediately.
+                        * There are can be RRSIG for "old" cname.
+                        */
+                       if (query->flags & QUERY_DNSSEC_WANT) {
+                               /* Follow chain only within current cut (if secure). */
+                               if (pending_cname && !knot_dname_in(query->zone_cut.name, pending_cname)) {
+                                       pending_cname = NULL;
+                               }
+                       } else {
+                               /* Try to find next cname */
+                               cname = pending_cname;
                        }
                }
        }
 
-       /* Make sure that this is an authoritative naswer (even with AA=0) for other layers */
+       /* Make sure that this is an authoritative answer (even with AA=0) for other layers */
        knot_wire_set_aa(pkt->wire);
        /* Either way it resolves current query. */
        query->flags |= QUERY_RESOLVED;
        /* Follow canonical name as next SNAME. */
        if (!knot_dname_is_equal(cname, query->sname)) {
                DEBUG_MSG("<= cname chain, following\n");
+               /* Check if already resolved */
+               if (cname && !knot_dname_is_equal(cname, query->sname)) {
+                       for (int i = 0; i < req->rplan.resolved.len; ++i) {
+                               struct kr_query * q = req->rplan.resolved.at[i];
+                               if (q->sclass == query->sclass &&
+                                   q->stype == query->stype   &&
+                                   knot_dname_is_equal(q->sname, cname)) {
+                                       DEBUG_MSG("<= cname chain loop\n");
+                                       return KNOT_STATE_FAIL;
+                               }
+                       }
+               }
                struct kr_query *next = kr_rplan_push(&req->rplan, query->parent, cname, query->sclass, query->stype);
                if (!next) {
                        return KNOT_STATE_FAIL;

diff --git a/tests/deckard b/tests/deckard
index 8b1a6c6873067d78e88adda48d381142aa107bd4..e73a0854552d94e8bc5306e75c3a2229bc739531 160000 (submodule)
--- a/tests/deckard
+++ b/tests/deckard
@@ -1 +1 @@
-Subproject commit 8b1a6c6873067d78e88adda48d381142aa107bd4
+Subproject commit e73a0854552d94e8bc5306e75c3a2229bc739531