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 <mdw@distorted.org.uk>

diff --git a/src/internal.h b/src/internal.h
index a95bf4d6a15b79f5ff9955ef015f0a7e863ad835..e07978b701d7cf6dc5bc124bb5ed843791bfd444 100644 (file)
--- 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 d4693c251bcf54cb2b262b6e34f94c1523beb794..c1d235ebb389fb27e2f9881d2e01afb18964d867 100644 (file)
--- 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: