chiark / gitweb /
src/: Change how query domain names are checked.
authorMark Wooding <mdw@distorted.org.uk>
Sat, 31 May 2014 16:26:34 +0000 (17:26 +0100)
committerMark Wooding <mdw@distorted.org.uk>
Fri, 13 Jun 2014 08:57:41 +0000 (09:57 +0100)
The `qdparselabel' hook is no more.  Unescaping of the query domain is
now done in one place, and always in the same way.  Checking of the
domain name is done through a new `checklabel' hook, with a more
convenient interface.

The checklabel hook does not have the original possibly-escaped version
of the query domain available to it; it can only check the name being
submitted on the wire, not the form in which it was provided to the
library.  This eliminates the inconsistency in SRV record handling,
where

\095http._tcp.distorted.org.uk

is acceptable, but

_\104ttp._tcp.distorted.org.uk

is not.  It also now prohibits SRV queries on `_wrong', which I think
was always intended but didn't actually work.

All queries are checked, including internally generated ones, so this
can be used to store information about the query domain for later use,
so we let the hook store information in the qcontext structure.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
src/internal.h
src/query.c
src/transmit.c
src/types.c

index 3816881..5f8892e 100644 (file)
@@ -124,6 +124,10 @@ typedef struct {
   struct timeval now;
 } parseinfo;
 
+union checklabel_state {
+  int dummy;
+};
+
 typedef struct {
   void *ext;
   void (*callback)(adns_query parent, adns_query child);
@@ -173,16 +177,17 @@ typedef struct typeinfo {
    * 0 otherwise.  Must not fail.
    */
 
-  adns_status (*qdparselabel)(adns_state ads,
-                             const char **p_io, const char *pe, int labelnum,
-                             char label_r[DNS_MAXDOMAIN], int *ll_io,
-                             adns_queryflags flags,
-                             const struct typeinfo *typei);
-  /* Parses one label from the query domain string.  On entry, *p_io
-   * points to the next character to parse and *ll_io is the size of
-   * the buffer.  pe points just after the end of the query domain
-   * string.  On successful return, label_r[] and *ll_io are filled in
-   * and *p_io points to *pe or just after the label-ending `.'.  */
+  adns_status (*checklabel)(adns_state ads, adns_queryflags flags,
+                           union checklabel_state *cls, qcontext *ctx,
+                           int labnum, const char *label, int lablen);
+  /* Check a label from the query domain string.  The label is not
+   * necessarily null-terminated.  The hook can refuse the query's submission
+   * by returning a nonzero status.  State can be stored in *cls between
+   * calls, and useful information can be stashed in ctx->tinfo, to be stored
+   * with the query (e.g., it will be available to the parse hook).  This
+   * hook can detect a first call because labnum is zero, and a final call
+   * because lablen is zero.
+   */
 
   void (*postsort)(adns_state ads, void *array, int nrrs,
                   const struct typeinfo *typei);
@@ -192,13 +197,13 @@ typedef struct typeinfo {
    */
 } typeinfo;
 
-adns_status adns__qdpl_normal(adns_state ads,
-                             const char **p_io, const char *pe, int labelnum,
-                             char label_r[], int *ll_io,
-                             adns_queryflags flags,
-                             const typeinfo *typei);
-  /* implemented in transmit.c, used by types.c as default
-   * and as part of implementation for some fancier types */
+adns_status adns__ckl_hostname(adns_state ads, adns_queryflags flags,
+                              union checklabel_state *cls,
+                              qcontext *ctx, int labnum,
+                              const char *label, int lablen);
+  /* implemented in query.c, used by types.c as default
+   * and as part of implementation for some fancier types
+   * doesn't require any state */
 
 typedef struct allocnode {
   struct allocnode *next, *back;
index 336def7..a3ce68d 100644 (file)
@@ -108,21 +108,67 @@ static void query_submit(adns_state ads, adns_query qu,
   adns__query_send(qu,now);
 }
 
+adns_status adns__ckl_hostname(adns_state ads, adns_queryflags flags,
+                              union checklabel_state *cls,
+                              qcontext *ctx, int labnum,
+                              const char *label, int lablen)
+{
+  int i, c;
+
+  if (flags & adns_qf_quoteok_query) return adns_s_ok;
+  for (i=0; i<lablen; i++) {
+    c= label[i];
+    if (c == '-') {
+      if (!i) return adns_s_querydomaininvalid;
+    } else if (!ctype_alpha(c) && !ctype_digit(c)) {
+      return adns_s_querydomaininvalid;
+    }
+  }
+  return adns_s_ok;
+}
+
+static adns_status check_domain_name(adns_state ads, adns_queryflags flags,
+                                    qcontext *ctx, const typeinfo *typei,
+                                    const byte *dgram, int dglen)
+{
+  findlabel_state fls;
+  adns_status err;
+  int labnum= 0, labstart, lablen;
+  union checklabel_state cls;
+
+  adns__findlabel_start(&fls,ads, -1,0, dgram,dglen,dglen, DNS_HDRSIZE,0);
+  do {
+    err= adns__findlabel_next(&fls, &lablen,&labstart);
+    assert(!err); assert(lablen >= 0);
+    err= typei->checklabel(ads,flags, &cls,ctx,
+                          labnum++, dgram+labstart,lablen);
+    if (err) return err;
+  } while (lablen);
+  return adns_s_ok;
+}
+
 adns_status adns__internal_submit(adns_state ads, adns_query *query_r,
                                  const typeinfo *typei, vbuf *qumsg_vb,
                                  int id,
                                  adns_queryflags flags, struct timeval now,
                                  qcontext *ctx) {
   adns_query qu;
+  adns_status err;
 
+  err= check_domain_name(ads, flags,ctx,typei, qumsg_vb->buf,qumsg_vb->used);
+  if (err) goto x_err;
   qu= query_alloc(ads,typei,typei->typekey,flags,now);
-  if (!qu) { adns__vbuf_free(qumsg_vb); return adns_s_nomemory; }
+  if (!qu) { err = adns_s_nomemory; goto x_err; }
   *query_r= qu;
 
   memcpy(&qu->ctx,ctx,sizeof(qu->ctx));
   query_submit(ads,qu, typei,qumsg_vb,id,flags,now);
   
   return adns_s_ok;
+
+x_err:
+  adns__vbuf_free(qumsg_vb);
+  return err;
 }
 
 static void query_simple(adns_state ads, adns_query qu,
@@ -145,6 +191,9 @@ static void query_simple(adns_state ads, adns_query qu,
     }
   }
 
+  stat= check_domain_name(ads, flags,&qu->ctx,typei, qu->vb.buf,qu->vb.used);
+  if (stat) { adns__query_fail(qu,stat); return; }
+
   vb_new= qu->vb;
   adns__vbuf_init(&qu->vb);
   query_submit(ads,qu, typei,&vb_new,id, flags,now);
index 7afb90f..e91250a 100644 (file)
@@ -74,11 +74,10 @@ static adns_status mkquery_footer(vbuf *vb, adns_rrtype type) {
   return adns_s_ok;
 }
 
-adns_status adns__qdpl_normal(adns_state ads,
-                             const char **p_io, const char *pe, int labelnum,
-                             char label_r[], int *ll_io,
-                             adns_queryflags flags,
-                             const typeinfo *typei) {
+static adns_status qdparselabel(adns_state ads,
+                               const char **p_io, const char *pe,
+                               char label_r[], int *ll_io,
+                               adns_queryflags flags) {
   int ll, c;
   const char *p;
 
@@ -102,13 +101,6 @@ adns_status adns__qdpl_normal(adns_state ads,
        return adns_s_querydomaininvalid;
       }
     }
-    if (!(flags & adns_qf_quoteok_query)) {
-      if (c == '-') {
-       if (!ll) return adns_s_querydomaininvalid;
-      } else if (!ctype_alpha(c) && !ctype_digit(c)) {
-       return adns_s_querydomaininvalid;
-      }
-    }
     if (ll == *ll_io) return adns_s_querydomaininvalid;
     label_r[ll++]= c;
   }
@@ -122,7 +114,7 @@ adns_status adns__mkquery(adns_state ads, vbuf *vb, int *id_r,
                          const char *owner, int ol,
                          const typeinfo *typei, adns_rrtype type,
                          adns_queryflags flags) {
-  int labelnum, ll, nbytes;
+  int ll, nbytes;
   byte label[255];
   byte *rqp;
   const char *p, *pe;
@@ -134,10 +126,9 @@ adns_status adns__mkquery(adns_state ads, vbuf *vb, int *id_r,
 
   p= owner; pe= owner+ol;
   nbytes= 0;
-  labelnum= 0;
   while (p!=pe) {
     ll= sizeof(label);
-    st= typei->qdparselabel(ads, &p,pe, labelnum++, label, &ll, flags, typei);
+    st= qdparselabel(ads, &p,pe, label, &ll, flags);
     if (st) return st;
     if (!ll) return adns_s_querydomaininvalid;
     if (ll > DNS_MAXLABEL) return adns_s_querydomaintoolong;
index 3765abb..675442c 100644 (file)
  * _mailbox                   (pap,csp +pap_mailbox822)
  * _rp                        (pa,cs)
  * _soa                       (pa,mf,cs)
- * _srv*                      (qdpl,(pap),pa*2,mf*2,di,(csp),cs*2,postsort)
+ * _srv*                      (ckl,(pap),pa*2,mf*2,di,(csp),cs*2,postsort)
  * _byteblock                 (mf)
  * _opaque                    (pa,cs)
  * _flat                      (mf)
  *
  * within each section:
- *    qdpl_*
+ *    ckl_*
  *    pap_*
  *    pa_*
  *    dip_*
@@ -1010,36 +1010,17 @@ static adns_status cs_soa(vbuf *vb, const void *datap) {
 }
 
 /*
- * _srv*  (qdpl,(pap),pa*2,mf*2,di,(csp),cs*2,postsort)
+ * _srv*  (ckl,(pap),pa*2,mf*2,di,(csp),cs*2,postsort)
  */
 
-static adns_status qdpl_srv(adns_state ads,
-                           const char **p_io, const char *pe, int labelnum,
-                           char label_r[DNS_MAXDOMAIN], int *ll_io,
-                           adns_queryflags flags,
-                           const typeinfo *typei) {
-  int useflags;
-  const char *p_orig;
-  adns_status st;
-
-  if (labelnum < 2 && !(flags & adns_qf_quoteok_query)) {
-    useflags= adns_qf_quoteok_query;
-    p_orig= *p_io;
-  } else {
-    useflags= flags;
-    p_orig= 0;
+static adns_status ckl_srv(adns_state ads, adns_queryflags flags,
+                          union checklabel_state *cls, qcontext *ctx,
+                          int labnum, const char *label, int lablen) {
+  if (labnum < 2 && !(flags & adns_qf_quoteok_query)) {
+    if (!lablen || label[0] != '_') return adns_s_querydomaininvalid;
+    return adns_s_ok;
   }
-  st= adns__qdpl_normal(ads, p_io,pe, labelnum,label_r, ll_io, useflags,typei);
-  if (st) return st;
-
-  if (p_orig) {
-    int ll= *ll_io;
-    if (!ll || label_r[0]!='_')
-      return adns_s_querydomaininvalid;
-    if (memchr(p_orig+1, '\\', pe - (p_orig+1)))
-      return adns_s_querydomaininvalid;
-  }
-  return adns_s_ok;
+  return adns__ckl_hostname(ads, flags, cls, ctx, labnum, label, lablen);
 }
 
 static adns_status pap_srv_begin(const parseinfo *pai, int *cbyte_io, int max,
@@ -1253,11 +1234,11 @@ static void mf_flat(adns_query qu, void *data) { }
 #define DEEP_TYPE(code,rrt,fmt,memb,parser,comparer,/*printer*/...)    \
  { adns_r_##code, rrt,fmt,TYPESZ_M(memb), mf_##memb,                   \
      GLUE(cs_, CAR(__VA_ARGS__)),pa_##parser,di_##comparer,            \
-     adns__qdpl_normal, CDR(__VA_ARGS__) }
+     adns__ckl_hostname, CDR(__VA_ARGS__) }
 #define FLAT_TYPE(code,rrt,fmt,memb,parser,comparer,/*printer*/...)    \
  { adns_r_##code, rrt,fmt,TYPESZ_M(memb), mf_flat,                     \
      GLUE(cs_, CAR(__VA_ARGS__)),pa_##parser,di_##comparer,            \
-     adns__qdpl_normal, CDR(__VA_ARGS__) }
+     adns__ckl_hostname, CDR(__VA_ARGS__) }
 
 #define di_0 0
 
@@ -1275,14 +1256,14 @@ DEEP_TYPE(mx_raw, "MX",   "raw",intstr,    mx_raw,  mx_raw,inthost         ),
 DEEP_TYPE(txt,    "TXT",   0,   manyistr,  txt,     0,     txt             ),
 DEEP_TYPE(rp_raw, "RP",   "raw",strpair,   rp,      0,     rp              ),
 DEEP_TYPE(srv_raw,"SRV",  "raw",srvraw ,   srvraw,  srv,   srvraw,
-                          .qdparselabel= qdpl_srv, .postsort= postsort_srv),
+                             .checklabel= ckl_srv, .postsort= postsort_srv),
 
 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(mx,     "MX", "+addr",inthostaddr,mx,     mx,    inthostaddr,    ),
 DEEP_TYPE(srv,    "SRV","+addr",srvha,     srvha,   srv,   srvha,
-                          .qdparselabel= qdpl_srv, .postsort= postsort_srv),
+                             .checklabel= ckl_srv, .postsort= postsort_srv),
 
 DEEP_TYPE(soa,    "SOA","822",  soa,       soa,     0,     soa             ),
 DEEP_TYPE(rp,     "RP", "822",  strpair,   rp,      0,     rp              ),