From: Mark Wooding <mdw@distorted.org.uk>
Date: Sun, 1 Jun 2014 20:50:59 +0000 (+0100)
Subject: src/types.c, regress/: Use checklabel hook to parse PTR query domains.
X-Git-Tag: wip.ipv6.2014-10-13.reentrancy-base~22
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~mdw/git/adns/commitdiff_plain/938b0228d88a9166f9e808fe9fe8313f8d940977

src/types.c, regress/: Use checklabel hook to parse PTR query domains.

A PTR with forward-lookup checking needs to parse the original question
into an address, so as to have something to compare the results of the
forward queries against.  Previously this was done in pa_ptr, when the
first PTR answer came in.  But parsing can fail, and is a simple
syntactic computation on the input domain name, so it seems rather
unfortunate to delay checking the name until an answer has come in.  Now
that we have the checklabel hook, then, we might as well use it here.
(Indeed, this is in fact the reason for introducing it.)

Unfortunately, this check now rejects a number of queries early, which
previously would have resulted in a query and maybe a NODATA or NXDOMAIN
answer, but in exactly the way we just rigged all of the test cases.  So
we remove the temporary hack to adns_submit and everything stays good.
Right?

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---

diff --git a/src/internal.h b/src/internal.h
index 5f8892e..c2876bf 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -125,7 +125,7 @@ typedef struct {
 } parseinfo;
 
 union checklabel_state {
-  int dummy;
+  struct { byte ipv[4]; } ptr;
 };
 
 typedef struct {
diff --git a/src/query.c b/src/query.c
index dd2a47a..a3ce68d 100644
--- a/src/query.c
+++ b/src/query.c
@@ -294,17 +294,7 @@ int adns_submit(adns_state ads,
     ol--;
   }
 
-/* temporary hack */
-#define CHECK_PTR do {							\
-  if (type == adns_r_ptr && (ol < 5 ||					\
-			     strncmp(owner + ol - 5, ".arpa", 5))) {	\
-    ads->nextid++; adns__autosys(ads,now);				\
-    stat= adns_s_querydomainwrong; goto x_adnsfail;			\
-  }									\
-} while (0)
-
   if (flags & adns_qf_search) {
-    CHECK_PTR;
     r= adns__vbuf_append(&qu->search_vb,owner,ol);
     if (!r) { stat= adns_s_nomemory; goto x_adnsfail; }
 
@@ -316,15 +306,12 @@ int adns_submit(adns_state ads,
     if (flags & adns_qf_owner) {
       if (!save_owner(qu,owner,ol)) { stat= adns_s_nomemory; goto x_adnsfail; }
     }
-    CHECK_PTR;
     query_simple(ads,qu, owner,ol, typei,flags, now);
   }
   adns__autosys(ads,now);
   adns__consistency(ads,qu,cc_entex);
   return 0;
 
-#undef CHECK_PTR
-
  x_adnsfail:
   adns__query_fail(qu,stat);
   adns__consistency(ads,qu,cc_entex);
diff --git a/src/types.c b/src/types.c
index 675442c..d463b7f 100644
--- a/src/types.c
+++ b/src/types.c
@@ -58,7 +58,7 @@
  * _mx                        (pa,di)
  * _inthostaddr               (mf,cs)
  * _inthost		      (cs)
- * _ptr                       (pa)
+ * _ptr                       (ckl,pa +icb_ptr)
  * _strpair                   (mf)
  * _intstrpair                (mf)
  * _hinfo                     (pa)
@@ -704,9 +704,43 @@ static adns_status cs_inthost(vbuf *vb, const void *datap) {
 }
 
 /*
- * _ptr   (pa, +icb_ptr)
+ * _ptr   (ckl,pa +icb_ptr)
  */
 
+static adns_status ckl_ptr(adns_state ads, adns_queryflags flags,
+			   union checklabel_state *cls, qcontext *ctx,
+			   int labnum, const char *label, int lablen) {
+  static const char *const (expectdomain[])= { DNS_INADDR_ARPA };
+  adns_rr_addr *ap;
+  char *ep;
+  const char *ed;
+  char labbuf[4];
+  int l;
+
+  if (labnum < 4) {
+    if (lablen<=0 || lablen>3) return adns_s_querydomainwrong;
+    memcpy(labbuf, label, lablen);
+    labbuf[lablen]= 0;
+    cls->ptr.ipv[3-labnum]= strtoul(labbuf,&ep,10);
+    if (*ep) return adns_s_querydomainwrong;
+    if (lablen>1 && *label=='0') return adns_s_querydomainwrong;
+  } else if (labnum < 4 + sizeof(expectdomain)/sizeof(*expectdomain)) {
+    ed= expectdomain[labnum-4];
+    l= strlen(ed);
+    if (lablen != l || memcmp(label, ed, l)) return adns_s_querydomainwrong;
+  } else {
+    if (lablen) return adns_s_querydomainwrong;
+    ap= &ctx->tinfo.ptr_addr;
+    ap->len= sizeof(struct sockaddr_in);
+    memset(&ap->addr,0,sizeof(ap->addr.inet));
+    ap->addr.inet.sin_family= AF_INET;
+    ap->addr.inet.sin_addr.s_addr=
+      htonl((cls->ptr.ipv[0]<<24) | (cls->ptr.ipv[1]<<16) |
+	    (cls->ptr.ipv[2]<< 8) | (cls->ptr.ipv[3]));
+  }
+  return adns_s_ok;
+}
+
 static void icb_ptr(adns_query parent, adns_query child) {
   adns_answer *cans= child->answer;
   const adns_rr_addr *queried, *found;
@@ -740,16 +774,9 @@ static void icb_ptr(adns_query parent, adns_query child) {
 
 static adns_status pa_ptr(const parseinfo *pai, int dmstart,
 			  int max, void *datap) {
-  static const char *const (expectdomain[])= { DNS_INADDR_ARPA };
-  
   char **rrp= datap;
   adns_status st;
-  adns_rr_addr *ap;
-  findlabel_state fls;
-  char *ep;
-  byte ipv[4];
-  char labbuf[4];
-  int cbyte, i, lablen, labstart, l, id;
+  int cbyte, id;
   adns_query nqu;
   qcontext ctx;
 
@@ -759,38 +786,6 @@ static adns_status pa_ptr(const parseinfo *pai, int dmstart,
   if (st) return st;
   if (cbyte != max) return adns_s_invaliddata;
 
-  ap= &pai->qu->ctx.tinfo.ptr_addr;
-  if (!ap->len) {
-    adns__findlabel_start(&fls, pai->ads, -1, pai->qu,
-			  pai->qu->query_dgram, pai->qu->query_dglen,
-			  pai->qu->query_dglen, DNS_HDRSIZE, 0);
-    for (i=0; i<4; i++) {
-      st= adns__findlabel_next(&fls,&lablen,&labstart); assert(!st);
-      if (lablen<=0 || lablen>3) return adns_s_querydomainwrong;
-      memcpy(labbuf, pai->qu->query_dgram + labstart, lablen);
-      labbuf[lablen]= 0;
-      ipv[3-i]= strtoul(labbuf,&ep,10);
-      if (*ep) return adns_s_querydomainwrong;
-      if (lablen>1 && pai->qu->query_dgram[labstart]=='0')
-	return adns_s_querydomainwrong;
-    }
-    for (i=0; i<sizeof(expectdomain)/sizeof(*expectdomain); i++) {
-      st= adns__findlabel_next(&fls,&lablen,&labstart); assert(!st);
-      l= strlen(expectdomain[i]);
-      if (lablen != l ||
-	  memcmp(pai->qu->query_dgram + labstart, expectdomain[i], l))
-	return adns_s_querydomainwrong;
-    }
-    st= adns__findlabel_next(&fls,&lablen,0); assert(!st);
-    if (lablen) return adns_s_querydomainwrong;
-    
-    ap->len= sizeof(struct sockaddr_in);
-    memset(&ap->addr,0,sizeof(ap->addr.inet));
-    ap->addr.inet.sin_family= AF_INET;
-    ap->addr.inet.sin_addr.s_addr=
-      htonl((ipv[0]<<24) | (ipv[1]<<16) | (ipv[2]<<8) | (ipv[3]));
-  }
-
   st= adns__mkquery_frdgram(pai->ads, &pai->qu->vb, &id,
 			    pai->dgram, pai->dglen, dmstart,
 			    adns_r_addr, adns_qf_quoteok_query);
@@ -1260,7 +1255,8 @@ DEEP_TYPE(srv_raw,"SRV",  "raw",srvraw ,   srvraw,  srv,   srvraw,
 
 FLAT_TYPE(addr,   "A",  "addr", addr,      addr,    addr,  addr            ),
 DEEP_TYPE(ns,     "NS", "+addr",hostaddr,  hostaddr,hostaddr,hostaddr      ),
-DEEP_TYPE(ptr,    "PTR","checked",str,     ptr,     0,     domain          ),
+DEEP_TYPE(ptr,    "PTR","checked",str,     ptr,     0,     domain,
+						       .checklabel= ckl_ptr),
 DEEP_TYPE(mx,     "MX", "+addr",inthostaddr,mx,     mx,    inthostaddr,    ),
 DEEP_TYPE(srv,    "SRV","+addr",srvha,     srvha,   srv,   srvha,
 			      .checklabel= ckl_srv, .postsort= postsort_srv),