From ddbf3bf35ffd70e7105f4cf8fe8aa81b3c7667e2 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Sun, 12 Apr 2009 04:09:09 +0000 Subject: [PATCH] dnsd: fix a number of bugs. Ideas by Ming-Ching Tiew (mctiew AT yahoo.com) function old new delta undot - 42 +42 dnsd_main 1394 1213 -181 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 0/1 up/down: 42/-181) Total: -139 bytes --- networking/dnsd.c | 488 ++++++++++++++++++++++++++++++---------------- 1 file changed, 325 insertions(+), 163 deletions(-) diff --git a/networking/dnsd.c b/networking/dnsd.c index f95ba6b40..4f980dccd 100644 --- a/networking/dnsd.c +++ b/networking/dnsd.c @@ -24,73 +24,63 @@ #define DEBUG 0 enum { - MAX_HOST_LEN = 16, // longest host name allowed is 15 - IP_STRING_LEN = 18, // .xxx.xxx.xxx.xxx\0 - -//must be strlen('.in-addr.arpa') larger than IP_STRING_LEN - MAX_NAME_LEN = (IP_STRING_LEN + 13), - -/* Cannot get bigger packets than 512 per RFC1035 - In practice this can be set considerably smaller: - Length of response packet is header (12B) + 2*type(4B) + 2*class(4B) + - ttl(4B) + rlen(2B) + r (MAX_NAME_LEN =21B) + - 2*querystring (2 MAX_NAME_LEN= 42B), all together 90 Byte -*/ + /* Can tweak this */ + DEFAULT_TTL = 120, + +/* Cannot get bigger packets than 512 per RFC1035. + * In practice this can be set considerably smaller: + * Length of response packet is header (12B) + 2*type(4B) + 2*class(4B) + + * ttl(4B) + rlen(2B) + r (MAX_NAME_LEN = 21B) + + * 2*querystring (2 MAX_NAME_LEN = 42B), all together 90 Bytes + */ MAX_PACK_LEN = 512, - - DEFAULT_TTL = 30, // increase this when not testing? - + IP_STRING_LEN = sizeof(".xxx.xxx.xxx.xxx"), + MAX_NAME_LEN = IP_STRING_LEN - 1 + sizeof(".in-addr.arpa"), REQ_A = 1, - REQ_PTR = 12 + REQ_PTR = 12, }; -struct dns_head { // the message from client and first part of response mag +/* the message from client and first part of response msg */ +struct dns_head { uint16_t id; uint16_t flags; - uint16_t nquer; // accepts 0 - uint16_t nansw; // 1 in response - uint16_t nauth; // 0 - uint16_t nadd; // 0 + uint16_t nquer; + uint16_t nansw; + uint16_t nauth; + uint16_t nadd; }; struct dns_prop { uint16_t type; uint16_t class; }; -struct dns_entry { // element of known name, ip address and reversed ip address +/* element of known name, ip address and reversed ip address */ +struct dns_entry { struct dns_entry *next; - char ip[IP_STRING_LEN]; // dotted decimal IP - char rip[IP_STRING_LEN]; // length decimal reversed IP - char name[MAX_HOST_LEN]; + uint32_t ip; + char rip[IP_STRING_LEN]; /* length decimal reversed IP */ + char name[1]; }; #define OPT_verbose (option_mask32) -/* - * Convert host name from C-string to dns length/string. - */ -static void convname(char *a, uint8_t *q) -{ - int i = (q[0] == '.') ? 0 : 1; - for (; i < MAX_HOST_LEN-1 && *q; i++, q++) - a[i] = tolower(*q); - a[0] = i - 1; - a[i] = 0; -} - /* * Insert length of substrings instead of dots */ -static void undot(uint8_t *rip) +static void undot(char *rip) { - int i = 0, s = 0; + int i = 0; + int s = 0; + while (rip[i]) i++; for (--i; i >= 0; i--) { if (rip[i] == '.') { rip[i] = s; s = 0; - } else s++; + } else { + s++; + } } } @@ -101,199 +91,369 @@ static struct dns_entry *parse_conf_file(const char *fileconf) { char *token[2]; parser_t *parser; - struct dns_entry *m, *prev, *conf_data; + struct dns_entry *m, *conf_data; + struct dns_entry **nextp; + + conf_data = NULL; + nextp = &conf_data; - prev = conf_data = NULL; parser = config_open(fileconf); while (config_read(parser, token, 2, 2, "# \t", PARSE_NORMAL)) { - unsigned a, b, c, d; - /* - * Assumes all host names are lower case only - * Hostnames with more than one label are not handled correctly. - * Presently the dot is copied into name without - * converting to a length/string substring for that label. - */ -// if (!token[1] || sscanf(token[1], ".%u.%u.%u.%u"+1, &a, &b, &c, &d) != 4) - if (sscanf(token[1], ".%u.%u.%u.%u"+1, &a, &b, &c, &d) != 4) - continue; + struct in_addr ip; + uint32_t v32; - m = xzalloc(sizeof(*m)); - /*m->next = NULL;*/ - sprintf(m->ip, ".%u.%u.%u.%u"+1, a, b, c, d); - sprintf(m->rip, ".%u.%u.%u.%u", d, c, b, a); - undot((uint8_t*)m->rip); - convname(m->name, (uint8_t*)token[0]); + if (inet_aton(token[1], &ip) == 0) { + bb_error_msg("error at line %u, skipping", parser->lineno); + continue; + } if (OPT_verbose) - bb_error_msg("name:%s, ip:%s", &(m->name[1]), m->ip); + bb_error_msg("name:%s, ip:%s", token[0], token[1]); - if (prev == NULL) - conf_data = m; - else - prev->next = m; - prev = m; + /* sizeof(*m) includes 1 byte for m->name[0] */ + m = xzalloc(sizeof(*m) + strlen(token[0]) + 1); + /*m->next = NULL;*/ + *nextp = m; + nextp = &m->next; + + m->name[0] = '.'; + strcpy(m->name + 1, token[0]); + undot(m->name); + m->ip = ip.s_addr; /* in network order */ + v32 = ntohl(m->ip); + /* inverted order */ + sprintf(m->rip, ".%u.%u.%u.%u", + (uint8_t)(v32), + (uint8_t)(v32 >> 8), + (uint8_t)(v32 >> 16), + (v32 >> 24) + ); + undot(m->rip); } config_close(parser); return conf_data; } /* - * Look query up in dns records and return answer if found - * qs is the query string, first byte the string length + * Look query up in dns records and return answer if found. + * qs is the query string. */ -static int table_lookup(struct dns_entry *d, uint16_t type, uint8_t *as, uint8_t *qs) +static int table_lookup(uint8_t *as, struct dns_entry *d, uint16_t type, uint8_t *qs) { - int i; - - do { + while (d) { + unsigned len = d->name[0]; + /* d->name[len] is the last (non NUL) char */ #if DEBUG char *p, *q; q = (char *)&(qs[1]); p = &(d->name[1]); - fprintf(stderr, "\n%s: %d/%d p:%s q:%s %d", - __FUNCTION__, (int)strlen(p), (int)(d->name[0]), - p, q, (int)strlen(q)); + fprintf(stderr, "%d/%d p:%s q:%s %d\n", + (int)strlen(p), len, + p, q, (int)strlen(q) + ); #endif - if (type == REQ_A) { + if (type == htons(REQ_A)) { /* search by host name */ - for (i = 1; i <= (int)(d->name[0]); i++) - if (tolower(qs[i]) != d->name[i]) - break; - if (i > (int)(d->name[0]) - || (d->name[0] == 1 && d->name[1] == '*') - ) { - strcpy((char *)as, d->ip); + if (len != 1 || d->name[1] != '*') { + if (strcasecmp(d->name, (char*)qs) != 0) + goto next; + } + move_to_unaligned32((uint32_t *)as, d->ip); #if DEBUG - fprintf(stderr, " OK as:%s\n", as); + fprintf(stderr, "OK as:%x\n", (int)d->ip); #endif - return 0; - } - } else if (type == REQ_PTR) { - /* search by IP-address */ - if ((d->name[0] != 1 || d->name[1] != '*') - && !strncmp(d->rip + 1, (char*)qs + 1, strlen(d->rip)-1) - ) { - strcpy((char *)as, d->name); - return 0; - } + return 0; } + /* search by IP-address */ + if ((len != 1 || d->name[1] != '*') + /* assume (do not check) that qs ends in ".in-addr.arpa" */ + && strncmp(d->rip, (char*)qs, strlen(d->rip)) == 0 + ) { + strcpy((char *)as, d->name); +#if DEBUG + fprintf(stderr, "OK as:%s\n", as); +#endif + return 0; + } + next: d = d->next; - } while (d); + } + return -1; } /* * Decode message and generate answer */ +/* RFC 1035 +... +Whenever an octet represents a numeric quantity, the left most bit +in the diagram is the high order or most significant bit. +That is, the bit labeled 0 is the most significant bit. +... + +4.1.1. Header section format + 1 1 1 1 1 1 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | ID | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + |QR| OPCODE |AA|TC|RD|RA| Z | RCODE | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | QDCOUNT | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | ANCOUNT | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | NSCOUNT | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | ARCOUNT | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ +ID 16 bit random identifier assigned by query. + Used to match query/response. +QR message is a query (0), or a response (1). +OPCODE 0 standard query (QUERY) + 1 inverse query (IQUERY) + 2 server status request (STATUS) +AA Authoritative Answer - this bit is valid in responses, + and specifies that the responding name server is an + authority for the domain name in question section. + Note that the contents of the answer section may have + multiple owner names because of aliases. The AA bit + corresponds to the name which matches the query name, or + the first owner name in the answer section. +TC TrunCation - specifies that this message was truncated. +RD Recursion Desired - this bit may be set in a query and + is copied into the response. If RD is set, it directs + the name server to pursue the query recursively. + Recursive query support is optional. +RA Recursion Available - this be is set or cleared in a + response, and denotes whether recursive query support is + available in the name server. +Z Reserved for future use. Must be zero. +RCODE Response code. + 0 No error condition + 1 Format error + 2 Server failure - The name server was + unable to process this query due to a + problem with the name server. + 3 Name Error - Meaningful only for + responses from an authoritative name + server, this code signifies that the + domain name referenced in the query does + not exist. + 4 Not Implemented. + 5 Refused. +QDCOUNT number of entries in the question section. +ANCOUNT number of resource records in the answer section. +NSCOUNT number of name server resource records in the authority records section. +ARCOUNT number of resource records in the additional records section. + +4.1.2. Question section format + +The section contains QDCOUNT (usually 1) entries, each of the following format: + 1 1 1 1 1 1 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + / QNAME / + / / + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | QTYPE | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | QCLASS | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ +QNAME a domain name represented as a sequence of labels, where + each label consists of a length octet followed by that + number of octets. The domain name terminates with the + zero length octet for the null label of the root. Note + that this field may be an odd number of octets; no + padding is used. +QTYPE a two octet type of the query. + 1 a host address [REQ_A const] + 2 an authoritative name server + 3 a mail destination (Obsolete - use MX) + 4 a mail forwarder (Obsolete - use MX) + 5 the canonical name for an alias + 6 marks the start of a zone of authority + 7 a mailbox domain name (EXPERIMENTAL) + 8 a mail group member (EXPERIMENTAL) + 9 a mail rename domain name (EXPERIMENTAL) + 10 a null RR (EXPERIMENTAL) + 11 a well known service description + 12 a domain name pointer [REQ_PTR const] + 13 host information + 14 mailbox or mail list information + 15 mail exchange + 16 text strings + 0x1c IPv6? + 252 a request for a transfer of an entire zone + 253 a request for mailbox-related records (MB, MG or MR) + 254 a request for mail agent RRs (Obsolete - see MX) + 255 a request for all records +QCLASS a two octet code that specifies the class of the query. + 1 the Internet + (others are historic only) + 255 any class + +4.1.3. Resource record format + +The answer, authority, and additional sections all share the same +format: a variable number of resource records, where the number of +records is specified in the corresponding count field in the header. +Each resource record has the following format: + 1 1 1 1 1 1 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + / / + / NAME / + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | TYPE | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | CLASS | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | TTL | + | | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | RDLENGTH | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--| + / RDATA / + / / + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ +NAME a domain name to which this resource record pertains. +TYPE two octets containing one of the RR type codes. This + field specifies the meaning of the data in the RDATA + field. +CLASS two octets which specify the class of the data in the + RDATA field. +TTL a 32 bit unsigned integer that specifies the time + interval (in seconds) that the resource record may be + cached before it should be discarded. Zero values are + interpreted to mean that the RR can only be used for the + transaction in progress, and should not be cached. +RDLENGTH an unsigned 16 bit integer that specifies the length in + octets of the RDATA field. +RDATA a variable length string of octets that describes the + resource. The format of this information varies + according to the TYPE and CLASS of the resource record. + For example, if the TYPE is A and the CLASS is IN, + the RDATA field is a 4 octet ARPA Internet address. + +4.1.4. Message compression + +In order to reduce the size of messages, the domain system utilizes a +compression scheme which eliminates the repetition of domain names in a +message. In this scheme, an entire domain name or a list of labels at +the end of a domain name is replaced with a pointer to a prior occurance +of the same name. + +The pointer takes the form of a two octet sequence: + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ + | 1 1| OFFSET | + +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ +The first two bits are ones. This allows a pointer to be distinguished +from a label, since the label must begin with two zero bits because +labels are restricted to 63 octets or less. The OFFSET field specifies +an offset from the start of the message (i.e., the first octet +of the ID field in the domain header). +A zero offset specifies the first byte of the ID field, etc. + +The compression scheme allows a domain name in a message to be +represented as either: + - a sequence of labels ending in a zero octet + - a pointer + - a sequence of labels ending with a pointer + */ static int process_packet(struct dns_entry *conf_data, uint32_t conf_ttl, uint8_t *buf) { uint8_t answstr[MAX_NAME_LEN + 1]; struct dns_head *head; - struct dns_prop *qprop; + struct dns_prop *unaligned_qprop; uint8_t *from, *answb; uint16_t outr_rlen; uint16_t outr_flags; - uint16_t flags; - int lookup_result, type, packet_len; + uint16_t type; + uint16_t class; int querystr_len; answstr[0] = '\0'; head = (struct dns_head *)buf; if (head->nquer == 0) { - bb_error_msg("no queries"); + bb_error_msg("packet has 0 queries, ignored"); return -1; } - if (head->flags & 0x8000) { - bb_error_msg("ignoring response packet"); + if (head->flags & htons(0x8000)) { /* QR bit */ + bb_error_msg("response packet, ignored"); return -1; } - from = (void *)&head[1]; // start of query string -//FIXME: strlen of untrusted data??! - querystr_len = strlen((char *)from) + 1 + sizeof(struct dns_prop); - answb = from + querystr_len; // where to append answer block + /* start of query string */ + from = (void *)(head + 1); + /* caller guarantees strlen is <= MAX_PACK_LEN */ + querystr_len = strlen((char *)from) + 1; + /* may be unaligned! */ + unaligned_qprop = (void *)(from + querystr_len); + /* where to append answer block */ + answb = (void *)(unaligned_qprop + 1); outr_rlen = 0; - outr_flags = 0; - - qprop = (struct dns_prop *)(answb - 4); - type = ntohs(qprop->type); + /* QR = 1 "response", RCODE = 4 "Not Implemented" */ + outr_flags = htons(0x8000 | 4); - // only let REQ_A and REQ_PTR pass - if (!(type == REQ_A || type == REQ_PTR)) { - goto empty_packet; /* we can't handle the query type */ + move_from_unaligned16(type, &unaligned_qprop->type); + if (type != htons(REQ_A) && type != htons(REQ_PTR)) { + /* we can't handle the query type */ + goto empty_packet; } - - if (ntohs(qprop->class) != 1 /* class INET */ ) { - outr_flags = 4; /* not supported */ + move_from_unaligned16(class, &unaligned_qprop->class); + if (class != htons(1)) { /* not class INET? */ goto empty_packet; } - /* we only support standard queries */ - - if ((ntohs(head->flags) & 0x7800) != 0) + /* OPCODE != 0 "standard query" ? */ + if ((head->flags & htons(0x7800)) != 0) { goto empty_packet; + } - // We have a standard query bb_info_msg("%s", (char *)from); - lookup_result = table_lookup(conf_data, type, answstr, from); - if (lookup_result != 0) { - outr_flags = 3 | 0x0400; // name do not exist and auth + if (table_lookup(answstr, conf_data, type, from) != 0) { + /* QR = 1 "response" + * AA = 1 "Authoritative Answer" + * RCODE = 3 "Name Error" */ + outr_flags = htons(0x8000 | 0x0400 | 3); goto empty_packet; } - if (type == REQ_A) { // return an address - struct in_addr a; // NB! its "struct { unsigned __long__ s_addr; }" - uint32_t v32; - if (!inet_aton((char*)answstr, &a)) { //dotted dec to long conv - outr_flags = 1; /* Frmt err */ - goto empty_packet; - } - v32 = a.s_addr; /* in case long != int */ - move_to_unaligned32(answstr, v32); - outr_rlen = 4; // uint32_t IP - } else - outr_rlen = strlen((char *)answstr) + 1; // a host name - outr_flags |= 0x0400; /* authority-bit */ - // we have an answer + /* return an address */ + outr_rlen = 4; + if (type == htons(REQ_PTR)) { + /* return a host name */ + outr_rlen = strlen((char *)answstr) + 1; + } + /* QR = 1 "response", + * AA = 1 "Authoritative Answer", + * RCODE = 0 "success" */ + outr_flags = htons(0x8000 | 0x0400 | 0); + /* we have one answer */ head->nansw = htons(1); - - // copy query block to answer block + /* copy query block to answer block */ + querystr_len += sizeof(unaligned_qprop); memcpy(answb, from, querystr_len); answb += querystr_len; - - // and append answer rr -// FIXME: unaligned accesses?? - *(uint32_t *) answb = htonl(conf_ttl); + /* append answer Resource Record */ + move_to_unaligned32((uint32_t *)answb, htonl(conf_ttl)); answb += 4; - *(uint16_t *) answb = htons(outr_rlen); + move_to_unaligned32((uint16_t *)answb, htons(outr_rlen)); answb += 2; memcpy(answb, answstr, outr_rlen); answb += outr_rlen; empty_packet: - - flags = ntohs(head->flags); - // clear rcode and RA, set responsebit and our new flags - flags |= (outr_flags & 0xff80) | 0x8000; - head->flags = htons(flags); + head->flags |= outr_flags; head->nauth = head->nadd = 0; - head->nquer = htons(1); + head->nquer = htons(1); // why??? - packet_len = answb - buf; - return packet_len; + return answb - buf; } -/* - * Exit on signal - */ -//static void interrupt(int sig) -//{ -// /* unlink("/var/run/dnsd.lock"); */ -// bb_error_msg("interrupt, exiting\n"); -// kill_myself_with_sig(sig); -//} - int dnsd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int dnsd_main(int argc UNUSED_PARAM, char **argv) { @@ -328,7 +488,6 @@ int dnsd_main(int argc UNUSED_PARAM, char **argv) conf_data = parse_conf_file(fileconf); -// signal(SIGINT, interrupt); - just for one message? bb_signals(0 /* why? + (1 << SIGPIPE) */ + (1 << SIGHUP) @@ -348,8 +507,11 @@ int dnsd_main(int argc UNUSED_PARAM, char **argv) from = xzalloc(lsa_size); to = xzalloc(lsa_size); - bb_info_msg("Accepting UDP packets on %s", - xmalloc_sockaddr2dotted(&lsa->u.sa)); + { + char *p = xmalloc_sockaddr2dotted(&lsa->u.sa); + bb_info_msg("Accepting UDP packets on %s", p); + free(p); + } while (1) { int r; @@ -361,7 +523,7 @@ int dnsd_main(int argc UNUSED_PARAM, char **argv) memcpy(to, lsa, lsa_size); r = recv_from_to(udps, buf, MAX_PACK_LEN + 1, 0, &from->u.sa, &to->u.sa, lsa->len); if (r < 12 || r > MAX_PACK_LEN) { - bb_error_msg("invalid packet size"); + bb_error_msg("packet size %d, ignored", r); continue; } if (OPT_verbose) -- 2.25.1