From 3540caec6116f7c66d43cabec178d5e622a10f76 Mon Sep 17 00:00:00 2001 Message-Id: <3540caec6116f7c66d43cabec178d5e622a10f76.1716254009.git.mdw@distorted.org.uk> From: Mark Wooding Date: Wed, 21 May 2014 01:21:58 +0100 Subject: [PATCH] Turn query domain parsing upside-down. Organization: Straylight/Edgeware From: Mark Wooding 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. --- src/internal.h | 76 ++++++++++++++++++++++++++------------------------ src/query.c | 53 +++++++++++++++++++++++++++++++++-- src/transmit.c | 71 +++++++++++++++------------------------------- src/types.c | 56 +++++++++++++------------------------ 4 files changed, 132 insertions(+), 124 deletions(-) diff --git a/src/internal.h b/src/internal.h index f5f185e..b602c92 100644 --- a/src/internal.h +++ b/src/internal.h @@ -129,6 +129,10 @@ union gen_addr { struct in6_addr v6; }; +union checklabel_state { + int dummy; +}; + typedef struct { int af; int width; @@ -150,6 +154,24 @@ typedef struct { struct afinfo_addr { const afinfo *ai; union gen_addr addr; }; +typedef struct { + void *ext; + void (*callback)(adns_query parent, adns_query child); + + union { + struct afinfo_addr ptr_parent_addr; + adns_rr_hostaddr *hostaddr; + } pinfo; /* state for use by parent's callback function */ + + union { + struct { + unsigned want, have; + } addr; + } tinfo; /* type-specific state for the query itself: zero-init if you + * don't know better. */ + +} qcontext; + typedef struct typeinfo { adns_rrtype typekey; const char *rrtname; @@ -185,16 +207,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 *css, 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 query has not yet been constructed, + * and this hook can refuse its submission by returning a nonzero status. + * State can be stored in *css 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); @@ -215,13 +238,12 @@ 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 *css, + 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 */ typedef struct allocnode { struct allocnode *next, *back; @@ -237,24 +259,6 @@ union maxalign { union maxalign *up; } data; -typedef struct { - void *ext; - void (*callback)(adns_query parent, adns_query child); - - union { - struct afinfo_addr ptr_parent_addr; - adns_rr_hostaddr *hostaddr; - } pinfo; /* state for use by parent's callback function */ - - union { - struct { - unsigned want, have; - } addr; - } tinfo; /* type-specific state for the query itself: zero-init if you - * don't know better. */ - -} qcontext; - struct adns__query { adns_state ads; enum { query_tosend, query_tcpw, query_childw, query_done } state; @@ -507,7 +511,7 @@ adns_status adns__internal_submit(adns_state ads, adns_query *query_r, const typeinfo *typei, adns_rrtype, vbuf *qumsg_vb, int id, adns_queryflags flags, struct timeval now, - const qcontext *ctx); + qcontext *ctx); /* Submits a query (for internal use, called during external submits). * * The new query is returned in *query_r, or we return adns_s_nomemory. diff --git a/src/query.c b/src/query.c index 71d635d..66682b4 100644 --- a/src/query.c +++ b/src/query.c @@ -109,21 +109,67 @@ static void query_submit(adns_state ads, adns_query qu, else adns__query_send(qu, now); } +adns_status adns__ckl_hostname(adns_state ads, adns_queryflags flags, + union checklabel_state *css, + 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= 0); + err= typei->checklabel(ads,flags, &css,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, adns_rrtype type, vbuf *qumsg_vb, int id, adns_queryflags flags, struct timeval now, - const qcontext *ctx) { + 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,type,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, @@ -146,6 +192,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); diff --git a/src/transmit.c b/src/transmit.c index 3e5ef5f..212c6ec 100644 --- a/src/transmit.c +++ b/src/transmit.c @@ -74,55 +74,11 @@ 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) { - int ll, c; - const char *p; - - ll= 0; - p= *p_io; - - while (p!=pe && (c= *p++)!='.') { - if (c=='\\') { - if (!(flags & adns_qf_quoteok_query)) return adns_s_querydomaininvalid; - if (ctype_digit(p[0])) { - if (p+1==pe || p+2==pe) return adns_s_querydomaininvalid; - if (ctype_digit(p[1]) && ctype_digit(p[2])) { - c= (*p++ - '0')*100; - c += (*p++ - '0')*10; - c += (*p++ - '0'); - if (c >= 256) return adns_s_querydomaininvalid; - } else { - return adns_s_querydomaininvalid; - } - } else if (!(c= *p++)) { - 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; - } - - *p_io= p; - *ll_io= ll; - return adns_s_ok; -} - 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 labelnum, ll, c, nbytes; byte label[255]; byte *rqp; const char *p, *pe; @@ -136,11 +92,28 @@ adns_status adns__mkquery(adns_state ads, vbuf *vb, int *id_r, nbytes= 0; labelnum= 0; while (p!=pe) { - ll= sizeof(label); - st= typei->qdparselabel(ads, &p,pe, labelnum++, label, &ll, flags, typei); - if (st) return st; + + ll= 0; + while (p!=pe && (c= *p++)!='.') { + if (c=='\\') { + if (ctype_digit(p[0])) { + if (p+1==pe || p+2==pe) return adns_s_querydomaininvalid; + if (ctype_digit(p[1]) && ctype_digit(p[2])) { + c= (*p++ - '0')*100; + c += (*p++ - '0')*10; + c += (*p++ - '0'); + if (c >= 256) return adns_s_querydomaininvalid; + } else { + return adns_s_querydomaininvalid; + } + } else if (!(c= *p++)) { + return adns_s_querydomaininvalid; + } + } + if (ll >= DNS_MAXLABEL) return adns_s_querydomaintoolong; + label[ll++]= c; + } if (!ll) return adns_s_querydomaininvalid; - if (ll > DNS_MAXLABEL) return adns_s_querydomaintoolong; nbytes+= ll+1; if (nbytes >= DNS_MAXDOMAIN) return adns_s_querydomaintoolong; MKQUERY_ADDB(ll); diff --git a/src/types.c b/src/types.c index 3bed29e..4264754 100644 --- a/src/types.c +++ b/src/types.c @@ -66,13 +66,13 @@ * _mailbox (pap +pap_mailbox822) * _rp (pa) * _soa (pa,mf,cs) - * _srv* (qdpl,(pap),pa,mf,di,(csp),cs,postsort) + * _srv* (ckl,(pap),pa,mf,di,(csp),cs,postsort) * _byteblock (mf) * _opaque (pa,cs) * _flat (mf) * * within each section: - * qdpl_* + * ckl_* * pap_* * pa_* * dip_* @@ -1459,36 +1459,18 @@ static adns_status cs_soa(vbuf *vb, const void *datap) { } /* - * _srv* (pa*2,di,cs*2,qdpl,postsort) + * _srv* (pa*2,di,cs*2,ckl,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; - } - 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; +static adns_status ckl_srv(adns_state ads, adns_queryflags flags, + union checklabel_state *css, 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; + label++; lablen--; } - return adns_s_ok; + return adns__ckl_hostname(ads, flags, css, ctx, labnum, label, lablen); } static adns_status pap_srv_begin(const parseinfo *pai, int *cbyte_io, int max, @@ -1704,14 +1686,14 @@ static void mf_flat(adns_query qu, void *data) { } #define DEEP_TYPE(code,rrt,fmt,memb,parser,comparer,printer) \ { adns_r_##code & adns_rrt_reprmask, rrt,fmt,TYPESZ_M(memb), \ - mf_##memb, printer,parser,comparer, adns__qdpl_normal,0,0,0 } + mf_##memb, printer,parser,comparer, adns__ckl_hostname,0,0,0 } #define FLAT_TYPE(code,rrt,fmt,memb,parser,comparer,printer) \ { adns_r_##code & adns_rrt_reprmask, rrt,fmt,TYPESZ_M(memb), \ - mf_flat, printer,parser,comparer, adns__qdpl_normal,0,0,0 } + mf_flat, printer,parser,comparer, adns__ckl_hostname,0,0,0 } #define XTRA_TYPE(code,rrt,fmt,memb,parser,comparer,printer, \ - makefinal,qdpl,postsort,getrrsz,sender) \ + makefinal,ckl,postsort,getrrsz,sender) \ { adns_r_##code & adns_rrt_reprmask, rrt,fmt,TYPESZ_M(memb), makefinal, \ - printer,parser,comparer,qdpl,postsort,getrrsz,sender } + printer,parser,comparer,ckl,postsort,getrrsz,sender } static const typeinfo typeinfos[] = { /* Must be in ascending order of rrtype ! */ @@ -1728,15 +1710,15 @@ DEEP_TYPE(txt, "TXT", 0, manyistr,pa_txt, 0, cs_txt ), DEEP_TYPE(rp_raw, "RP", "raw",strpair, pa_rp, 0, cs_rp ), FLAT_TYPE(aaaa, "AAAA", 0, in6addr, pa_in6addr, di_in6addr,cs_in6addr ), XTRA_TYPE(srv_raw,"SRV", "raw",srvraw , pa_srvraw, di_srv, cs_srvraw, - mf_srvraw, qdpl_srv, postsort_srv, 0, 0), + mf_srvraw, ckl_srv, postsort_srv, 0, 0), XTRA_TYPE(addr, "A", "addr", addr, pa_addr, di_addr, cs_addr, - mf_flat, adns__qdpl_normal, 0, gsz_addr, qs_addr), + mf_flat, adns__ckl_hostname, 0, gsz_addr, qs_addr), DEEP_TYPE(ns, "NS", "+addr",hostaddr,pa_hostaddr,di_hostaddr,cs_hostaddr ), DEEP_TYPE(ptr, "PTR","checked",str, pa_ptr, 0, cs_domain ), DEEP_TYPE(mx, "MX", "+addr",inthostaddr,pa_mx, di_mx, cs_inthostaddr), XTRA_TYPE(srv, "SRV","+addr",srvha, pa_srvha, di_srv, cs_srvha, - mf_srvha, qdpl_srv, postsort_srv, 0, 0), + mf_srvha, ckl_srv, postsort_srv, 0, 0), DEEP_TYPE(soa, "SOA","822", soa, pa_soa, 0, cs_soa ), DEEP_TYPE(rp, "RP", "822", strpair, pa_rp, 0, cs_rp ), @@ -1744,7 +1726,7 @@ DEEP_TYPE(rp, "RP", "822", strpair, pa_rp, 0, cs_rp ), static const typeinfo tinfo_addrsub = XTRA_TYPE(none, "","sub",addr, pa_addr, 0, cs_addr, - mf_flat, adns__qdpl_normal, 0, gsz_addr, 0); + mf_flat, adns__ckl_hostname, 0, gsz_addr, 0); static const typeinfo typeinfo_unknown= DEEP_TYPE(unknown,0, "unknown",byteblock,pa_opaque, 0, cs_opaque ); -- [mdw]