From: Grigorii Demidov Date: Tue, 15 Mar 2016 14:47:49 +0000 (+0100) Subject: lib/layer: CNAME chain construction improvement X-Git-Tag: v1.0.0~43 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b357f3df0f90f0a77878df2473b320a54afe0e1;p=thirdparty%2Fknot-resolver.git lib/layer: CNAME chain construction improvement --- diff --git a/lib/defines.h b/lib/defines.h index 7031f7ddb..aeda472b9 100644 --- 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 776691f7c..cb2ab6b46 100644 --- a/lib/layer/iterate.c +++ b/lib/layer/iterate.c @@ -19,6 +19,7 @@ #include #include +#include #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 8b1a6c687..e73a08545 160000 --- a/tests/deckard +++ b/tests/deckard @@ -1 +1 @@ -Subproject commit 8b1a6c6873067d78e88adda48d381142aa107bd4 +Subproject commit e73a0854552d94e8bc5306e75c3a2229bc739531