From 1bc731c6994dda2c1ab12fb2d8261246bcbc139c Mon Sep 17 00:00:00 2001 From: Mark Wooding Date: Wed, 11 Jun 2014 00:40:42 +0100 Subject: [PATCH] src/types.c: Handle inconsistent CNAME records between addr answers. It can happen, if we have multiple address queries outstanding, that they disagree about whether the original name has a CNAME record, or what the CNAME target is. This needn't be a configuration error or a server bug: the zone might have been caught during a changeover. So we should try to handle this situation gracefully. We resolve this as follows. At least one answer must have reported a CNAME record, else there is no ambiguity. We choose the first such record to arrive, and commit to it, discarding other addresses already received, if any. We then resubmit address queries for the address types still outstanding, but asking specifically for the CNAME target rather than the original name. Signed-off-by: Mark Wooding --- src/internal.h | 5 ++++ src/types.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/internal.h b/src/internal.h index a95bf4d..e07978b 100644 --- a/src/internal.h +++ b/src/internal.h @@ -109,6 +109,11 @@ typedef enum { rcode_refused } dns_rcode; +enum { + adns__qf_addr_answer= 0x01000000,/* addr query received an answer */ + adns__qf_addr_cname = 0x02000000 /* addr subquery performed on cname */ +}; + /* Shared data structures */ typedef struct { diff --git a/src/types.c b/src/types.c index d4693c2..c1d235e 100644 --- a/src/types.c +++ b/src/types.c @@ -378,6 +378,27 @@ static unsigned addr_rrtypeflag(adns_rrtype type) { return i < addr_nrrtypes ? 1 << i : 0; } +/* About CNAME handling in addr queries. + * + * A user-level addr query is translated into a number of protocol-level + * queries, and its job is to reassemble the results. This gets tricky if + * the answers aren't consistent. In particular, if the answers report + * inconsistent indirection via CNAME records (e.g., different CNAMEs, or + * some indirect via a CNAME, and some don't) then we have trouble. + * + * Once we've received an answer, even if it was NODATA, we set + * adns__qf_addr_answer on the parent query. This will let us detect a + * conflict between a no-CNAME-with-NODATA reply and a subsequent CNAME. + * + * If we detect a conflict of any kind, then at least one answer came back + * with a CNAME record, so we pick the first such answer (somewhat + * arbitrarily) as being the `right' canonical name, and set this in the + * parent query's answer->cname slot. We discard address records from the + * wrong name. And finally we cancel the outstanding child queries, and + * resubmit address queries for the address families we don't yet have, with + * adns__qf_addr_cname set so that we know that we're in the fixup state. + */ + static adns_status pap_addr(const parseinfo *pai, int rrty, size_t rrsz, int *cbyte_io, int max, adns_rr_addr *storeto) { const byte *dgram= pai->dgram; @@ -613,9 +634,55 @@ static void icb_addr(adns_query parent, adns_query child) { adns_answer *pans= parent->answer, *cans= child->answer; struct timeval now; adns_status err; + adns_queryflags qf; + int id; propagate_ttl(parent, child); + if (!(child->flags & adns__qf_addr_cname) && + (parent->flags & adns__qf_addr_answer) && + (!!pans->cname != !!cans->cname || + (pans->cname && strcmp(pans->cname, cans->cname)))) { + /* We've detected an inconsistency in CNAME records, and must deploy + * countermeasures. + */ + + if (!pans->cname) { + /* The child has a CNAME record, but the parent doesn't. We must + * discard all of the parent's addresses, and substitute the child's. + */ + + assert(pans->rrsz == cans->rrsz); + adns__free_interim(parent, pans->rrs.bytes); + adns__transfer_interim(child, parent, cans->rrs.bytes); + pans->rrs.bytes= cans->rrs.bytes; + pans->nrrs= cans->nrrs; + parent->ctx.tinfo.addr.have= 0; + done_addr_type(parent, cans->type); + err= copy_cname_from_child(parent, child); if (err) goto x_err; + } + + /* We've settled on the CNAME (now) associated with the parent, which + * already has appropriate address records. Build a query datagram for + * this name so that we can issue child queries for the missing address + * families. The child's vbuf looks handy for this. + */ + err= adns__mkquery(ads, &child->vb, &id, pans->cname, + strlen(pans->cname), &tinfo_addrsub, + adns_r_addr, parent->flags); + if (err) goto x_err; + + /* Now cancel the remaining children, and try again with the CNAME we've + * settled on. + */ + adns__cancel_children(parent); + if (gettimeofday(&now, 0)) goto x_gtod; + qf= adns__qf_addr_cname; + if (!(parent->flags & adns_qf_cname_loose)) qf |= adns_qf_cname_forbid; + addr_subqueries(parent, now, qf, child->vb.buf, child->vb.used); + return; + } + if (cans->cname && !pans->cname) { err= copy_cname_from_child(parent, child); if (err) goto x_err; @@ -648,6 +715,7 @@ static void icb_addr(adns_query parent, adns_query child) { if (parent->children.head) LIST_LINK_TAIL(ads->childw, parent); else if (!pans->nrrs) adns__query_fail(parent, adns_s_nodata); else adns__query_done(parent); + parent->flags |= adns__qf_addr_answer; return; x_gtod: -- 2.30.2