From 7c1212e3d1b88359d2c28fa2fe42fee2fcc39732 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 9 Feb 2025 22:03:22 +0000 Subject: [PATCH] Fix query-combining for queries with class other than IN. Along the way, use of extract_request() and extract_name() got further refined. --- src/dnsmasq.c | 5 +- src/dnsmasq.h | 8 ++-- src/forward.c | 129 ++++++++++++++++++++++++++------------------------ src/rfc1035.c | 6 ++- 4 files changed, 77 insertions(+), 71 deletions(-) diff --git a/src/dnsmasq.c b/src/dnsmasq.c index 34845e4..41a62cd 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -2141,7 +2141,7 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot) cache_recv_insert() calls pop_and_retry_query() after the result arrives via the pipe to the parent. */ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header *header, - ssize_t *plen, int class, struct server *server, int *keycount, int *validatecount) + ssize_t *plen, char *name, int class, struct server *server, int *keycount, int *validatecount) { struct server *s; @@ -2215,8 +2215,7 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header } } - status = tcp_from_udp(now, status, header, plen, class, daemon->namebuff, daemon->keyname, - server, keycount, validatecount); + status = tcp_from_udp(now, status, header, plen, class, name, server, keycount, validatecount); /* close upstream connections. */ for (s = daemon->servers; s; s = s->next) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 59d1dfa..86f0eaf 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1393,8 +1393,8 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp, unsigned char *skip_name(unsigned char *ansp, struct dns_header *header, size_t plen, int extrabytes); unsigned char *skip_questions(struct dns_header *header, size_t plen); unsigned char *skip_section(unsigned char *ansp, int count, struct dns_header *header, size_t plen); -unsigned int extract_request(struct dns_header *header, size_t qlen, - char *name, unsigned short *typep); +unsigned int extract_request(struct dns_header *header, size_t qlen, char *name, + unsigned short *typep, unsigned short *classp); void setup_reply(struct dns_header *header, unsigned int flags, int ede); int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t now, struct ipsets *ipsets, struct ipsets *nftsets, int is_sign, @@ -1530,7 +1530,7 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s #ifdef HAVE_DNSSEC void pop_and_retry_query(struct frec *forward, int status, time_t now); int tcp_from_udp(time_t now, int status, struct dns_header *header, ssize_t *n, - int class, char *name, char *keyname, struct server *server, + int class, char *name, struct server *server, int *keycount, int *validatecount); #endif unsigned char *tcp_request(int confd, time_t now, @@ -1654,7 +1654,7 @@ void send_event(int fd, int event, int data, char *msg); void clear_cache_and_reload(time_t now); #ifdef HAVE_DNSSEC int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header *header, - ssize_t *plen, int class, struct server *server, int *keycount, int *validatecount); + ssize_t *plen, char *name, int class, struct server *server, int *keycount, int *validatecount); #endif /* netlink.c */ diff --git a/src/forward.c b/src/forward.c index 030f9b6..afb7dfd 100644 --- a/src/forward.c +++ b/src/forward.c @@ -176,9 +176,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, int first, last, start = 0; int forwarded = 0; int ede = EDE_UNSET; - unsigned short rrtype; + unsigned short rrtype, rrclass; - gotname = extract_request(header, plen, daemon->namebuff, &rrtype); + gotname = extract_request(header, plen, daemon->namebuff, &rrtype, &rrclass); /* Check for retry on existing query. FREC_DNSKEY and FREC_DS_QUERY are never set in flags, so the test below @@ -192,7 +192,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, old_reply = 1; fwd_flags = forward->flags; } - else if (gotname && (forward = lookup_frec(daemon->namebuff, C_IN, (int)rrtype, -1, fwd_flags, + else if (gotname && (forward = lookup_frec(daemon->namebuff, (int)rrclass, (int)rrtype, -1, fwd_flags, FREC_CHECKING_DISABLED | FREC_AD_QUESTION | FREC_DO_QUESTION | FREC_HAS_PHEADER | FREC_DNSKEY_QUERY | FREC_DS_QUERY | FREC_NO_CACHE))) { @@ -264,7 +264,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, The original query we sent is now in packet buffer and the query name in the new instance is on daemon->namebuff. */ - if (extract_request(header, forward->stash_len, daemon->workspacename, NULL)) + if (extract_name(header, forward->stash_len, NULL, daemon->workspacename, EXTR_NAME_EXTRACT, 0)) { unsigned int i, gobig = 0; char *s1, *s2; @@ -456,7 +456,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, blockdata_retrieve(forward->stash, forward->stash_len, (void *)header); plen = forward->stash_len; /* get query for logging. */ - gotname = extract_request(header, plen, daemon->namebuff, NULL); + gotname = extract_request(header, plen, daemon->namebuff, NULL, NULL); /* Find suitable servers: should never fail. */ if (!filter_servers(forward->sentto->arrayposn, F_DNSSECOK, &first, &last)) @@ -705,12 +705,12 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server (void)do_bit; #ifdef HAVE_IPSET - if (daemon->ipsets && extract_request(header, n, daemon->namebuff, NULL)) + if (daemon->ipsets && extract_name(header, n, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) ipsets = domain_find_sets(daemon->ipsets, daemon->namebuff); #endif #ifdef HAVE_NFTSET - if (daemon->nftsets && extract_request(header, n, daemon->namebuff, NULL)) + if (daemon->nftsets && extract_name(header, n, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) nftsets = domain_find_sets(daemon->nftsets, daemon->namebuff); #endif @@ -787,7 +787,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server log_query(F_UPSTREAM, NULL, NULL, "truncated", 0); else if (!bogusanswer || (header->hb4 & HB4_CD)) { - if (rcode == NXDOMAIN && extract_request(header, n, daemon->namebuff, NULL) && + if (rcode == NXDOMAIN && extract_name(header, n, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0) && (check_for_local_domain(daemon->namebuff, now) || lookup_domain(daemon->namebuff, F_CONFIG, NULL, NULL))) { /* if we forwarded a query for a locally known name (because it was for @@ -919,22 +919,26 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, /* Get the query we sent by UDP */ blockdata_retrieve(forward->stash, forward->stash_len, (void *)header); - if (extract_request(header, forward->stash_len, daemon->namebuff, NULL)) - log_query(F_UPSTREAM | F_NOEXTRA, daemon->namebuff, NULL, "truncated", 0); - - /* Don't count failed UDP attempt AND TCP */ - if (status != STAT_OK) - orig->work_counter++; - - /* NOTE: Can't move connection marks from UDP to TCP */ - plen = forward->stash_len; - status = swap_to_tcp(forward, now, status, header, &plen, forward->class, forward->sentto, &orig->work_counter, &orig->validate_counter); - - /* We forked a new process. pop_and_retry_query() will be called when is completes. */ - if (STAT_ISEQUAL(status, STAT_ASYNC)) + if (!extract_name(header, forward->stash_len, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) + status = STAT_ABANDONED; + else { - forward->flags |= FREC_GONE_TO_TCP; - return; + log_query(F_UPSTREAM | F_NOEXTRA, daemon->namebuff, NULL, "truncated", 0); + + /* Don't count failed UDP attempt AND TCP */ + if (status != STAT_OK) + orig->work_counter++; + + /* NOTE: Can't move connection marks from UDP to TCP */ + plen = forward->stash_len; + status = swap_to_tcp(forward, now, status, header, &plen, daemon->namebuff, forward->class, forward->sentto, &orig->work_counter, &orig->validate_counter); + + /* We forked a new process. pop_and_retry_query() will be called when is completes. */ + if (STAT_ISEQUAL(status, STAT_ASYNC)) + { + forward->flags |= FREC_GONE_TO_TCP; + return; + } } } else @@ -1090,7 +1094,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, if (log_resource) { /* Log the actual validation that made us barf. */ - if (extract_request(header, plen, daemon->namebuff, NULL)) + if (extract_name(header, plen, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), daemon->namebuff[0] ? daemon->namebuff : "."); } @@ -1384,7 +1388,7 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s no_cache_dnssec = 1; bogusanswer = 1; - if (extract_request(header, n, daemon->namebuff, NULL)) + if (extract_name(header, n, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) domain = daemon->namebuff; } @@ -1813,7 +1817,7 @@ void receive_query(struct listener *listen, time_t now) if (OPCODE(header) != QUERY) log_query_mysockaddr(F_QUERY | F_FORWARD, "opcode", &source_addr, "non-query", 0); - else if (extract_request(header, (size_t)n, daemon->namebuff, &type)) + else if (extract_request(header, (size_t)n, daemon->namebuff, &type, NULL)) { #ifdef HAVE_AUTH struct auth_zone *zone; @@ -2167,14 +2171,13 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet, returned truncated. (Which type held in status). Resend the query (in header) via TCP */ int tcp_from_udp(time_t now, int status, struct dns_header *header, ssize_t *plenp, - int class, char *name, char *keyname, struct server *server, + int class, char *name, struct server *server, int *keycount, int *validatecount) { unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16)); struct dns_header *new_header = (struct dns_header *)&packet[2]; int start, first, last, new_status; ssize_t n = *plenp; - int have_req = extract_request(header, n, keyname, NULL); int log_save = daemon->log_display_id; *plenp = 0; @@ -2191,48 +2194,48 @@ int tcp_from_udp(time_t now, int status, struct dns_header *header, ssize_t *ple first = start = server->arrayposn; last = first + 1; - if (!STAT_ISEQUAL(status, STAT_OK) && (!have_req || (start = dnssec_server(server, keyname, &first, &last)) == -1)) - new_status = STAT_ABANDONED; - else if ((n = tcp_talk(first, last, start, packet, n, 0, 0, &server)) == 0) + if (!STAT_ISEQUAL(status, STAT_OK) && (start = dnssec_server(server, name, &first, &last)) == -1) new_status = STAT_ABANDONED; else { - if (have_req) - { - if (STAT_ISEQUAL(status, STAT_OK)) - log_query_mysockaddr(F_SERVER | F_FORWARD, keyname, &server->addr, NULL, 0); - else - log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, keyname, &server->addr, - STAT_ISEQUAL(status, STAT_NEED_KEY) ? "dnssec-query[DNSKEY]" : "dnssec-query[DS]", 0); - } - - new_status = tcp_key_recurse(now, status, new_header, n, class, name, keyname, server, 0, 0, keycount, validatecount); - if (STAT_ISEQUAL(status, STAT_OK)) - { - /* downstream query: strip DNSSSEC RRs and see if it will - fit in a UDP reply. */ - rrfilter(new_header, (size_t *)&n, RRFILTER_DNSSEC); + log_query_mysockaddr(F_SERVER | F_FORWARD, name, &server->addr, NULL, 0); + else + log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, name, &server->addr, + STAT_ISEQUAL(status, STAT_NEED_KEY) ? "dnssec-query[DNSKEY]" : "dnssec-query[DS]", 0); - if (n >= daemon->edns_pktsz) + if ((n = tcp_talk(first, last, start, packet, n, 0, 0, &server)) == 0) + new_status = STAT_ABANDONED; + else + { + new_status = tcp_key_recurse(now, status, new_header, n, class, daemon->namebuff, daemon->keyname, server, 0, 0, keycount, validatecount); + + if (STAT_ISEQUAL(status, STAT_OK)) { - /* still too bIg, strip optional sections and try again. */ - new_header->nscount = htons(0); - new_header->arcount = htons(0); - n = resize_packet(new_header, n, NULL, 0); + /* downstream query: strip DNSSSEC RRs and see if it will + fit in a UDP reply. */ + rrfilter(new_header, (size_t *)&n, RRFILTER_DNSSEC); + if (n >= daemon->edns_pktsz) { - /* truncating the packet will break the answers, so remove them too - and mark the reply as truncated. */ - new_header->ancount = htons(0); + /* still too bIg, strip optional sections and try again. */ + new_header->nscount = htons(0); + new_header->arcount = htons(0); n = resize_packet(new_header, n, NULL, 0); - new_status = STAT_TRUNCATED; + if (n >= daemon->edns_pktsz) + { + /* truncating the packet will break the answers, so remove them too + and mark the reply as truncated. */ + new_header->ancount = htons(0); + n = resize_packet(new_header, n, NULL, 0); + new_status = STAT_TRUNCATED; + } } + + /* return the stripped or truncated reply. */ + memcpy(header, new_header, n); + *plenp = n; } - - /* return the stripped or truncated reply. */ - memcpy(header, new_header, n); - *plenp = n; } } @@ -2276,7 +2279,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si if (STAT_ISEQUAL(new_status, STAT_ABANDONED)) { /* Log the actual validation that made us barf. */ - if (extract_request(header, n, daemon->namebuff, NULL)) + if (extract_name(header, n, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), daemon->namebuff[0] ? daemon->namebuff : "."); break; @@ -2460,7 +2463,7 @@ unsigned char *tcp_request(int confd, time_t now, gotname = 0; flags = F_RCODE; } - else if (!(gotname = extract_request(header, (unsigned int)size, daemon->namebuff, &qtype))) + else if (!(gotname = extract_request(header, (unsigned int)size, daemon->namebuff, &qtype, NULL))) ede = EDE_INVALID_DATA; else { @@ -2605,7 +2608,7 @@ unsigned char *tcp_request(int confd, time_t now, else { /* get query name again for logging - may have been overwritten */ - if (!(gotname = extract_request(header, (unsigned int)size, daemon->namebuff, &qtype))) + if (!extract_name(header, (unsigned int)size, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) strcpy(daemon->namebuff, "query"); log_query_mysockaddr(F_SERVER | F_FORWARD, daemon->namebuff, &serv->addr, NULL, 0); @@ -2647,7 +2650,7 @@ unsigned char *tcp_request(int confd, time_t now, no_cache_dnssec = 1; bogusanswer = 1; - if (extract_request(header, m, daemon->namebuff, NULL)) + if (extract_name(header, m, NULL, daemon->namebuff, EXTR_NAME_EXTRACT, 0)) domain = daemon->namebuff; } diff --git a/src/rfc1035.c b/src/rfc1035.c index f7afe1b..cd18618 100644 --- a/src/rfc1035.c +++ b/src/rfc1035.c @@ -1238,7 +1238,8 @@ void report_addresses(struct dns_header *header, size_t len, u32 mark) /* If the packet holds exactly one query return F_IPV4 or F_IPV6 and leave the name from the query in name */ -unsigned int extract_request(struct dns_header *header, size_t qlen, char *name, unsigned short *typep) +unsigned int extract_request(struct dns_header *header, size_t qlen, char *name, + unsigned short *typep, unsigned short *classp) { unsigned char *p = (unsigned char *)(header+1); int qtype, qclass; @@ -1263,6 +1264,9 @@ unsigned int extract_request(struct dns_header *header, size_t qlen, char *name, if (typep) *typep = qtype; + if (classp) + *classp = qclass; + if (qclass == C_IN) { if (qtype == T_A)