From f50d23e29bb3f95cb48566d6d35fbd6da88753e1 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Sat, 28 Aug 2010 18:49:56 -0600 Subject: [PATCH] Author: Henrik Nordstrom Harden DNS client against packet queue attacks. Strengthen the internal DNS client somewhat by making sure to keep the receive queue drained. Also avoid parsing messages unless we have a pending query. --- src/dns_internal.cc | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/dns_internal.cc b/src/dns_internal.cc index 271dee0b9a..1713317394 100644 --- a/src/dns_internal.cc +++ b/src/dns_internal.cc @@ -828,7 +828,7 @@ idnsSendQuery(idns_query * q) } else { if (DnsSocketB >= 0 && nameservers[ns].S.IsIPv6()) y = comm_udp_sendto(DnsSocketB, nameservers[ns].S, q->buf, q->sz); - else + else if (DnsSocketA) x = comm_udp_sendto(DnsSocketA, nameservers[ns].S, q->buf, q->sz); } @@ -843,16 +843,11 @@ idnsSendQuery(idns_query * q) } while ( (x<0 && y<0) && q->nsends % nns != 0); - if (!q->need_vc) { - if (y >= 0) { - fd_bytes(DnsSocketB, y, FD_WRITE); - commSetSelect(DnsSocketB, COMM_SELECT_READ, idnsRead, NULL, 0); - } - - if (x >= 0) { - fd_bytes(DnsSocketA, x, FD_WRITE); - commSetSelect(DnsSocketA, COMM_SELECT_READ, idnsRead, NULL, 0); - } + if (y > 0) { + fd_bytes(DnsSocketB, y, FD_WRITE); + } + if (x > 0) { + fd_bytes(DnsSocketA, x, FD_WRITE); } nameservers[ns].nqueries++; @@ -1139,6 +1134,10 @@ idnsRead(int fd, void *data) debugs(78, 3, "idnsRead: starting with FD " << fd); + // Always keep reading. This stops (or at least makes harder) several + // attacks on the DNS client. + commSetSelect(fd, COMM_SELECT_READ, idnsRead, NULL, 0); + /* BUG (UNRESOLVED) * two code lines after returning from comm_udprecvfrom() * something overwrites the memory behind the from parameter. @@ -1185,7 +1184,14 @@ idnsRead(int fd, void *data) if (ns >= 0) { nameservers[ns].nreplies++; - } else if (Config.onoff.ignore_unknown_nameservers) { + } + + // Before unknown_nameservers check to avoid flooding cache.log on attacks, + // but after the ++ above to keep statistics right. + if (!lru_list.head) + continue; // Don't process replies if there is no pending query. + + if (ns < 0 && Config.onoff.ignore_unknown_nameservers) { static time_t last_warning = 0; if (squid_curtime - last_warning > 60) { @@ -1199,10 +1205,6 @@ idnsRead(int fd, void *data) idnsGrokReply(rbuf, len); } - - if (lru_list.head) { - commSetSelect(fd, COMM_SELECT_READ, idnsRead, NULL, 0); - } } static void @@ -1270,7 +1272,7 @@ idnsReadVC(int fd, char *buf, size_t len, comm_err_t flag, int xerrno, void *dat return; } - vc->msg->size += len; // XXX should not access -> size directly + vc->msg->size += len; // XXX should not access -> size directly if (vc->msg->contentSize() < vc->msglen) { comm_read(fd, buf + len, vc->msglen - vc->msg->contentSize(), idnsReadVC, vc); @@ -1389,10 +1391,12 @@ idnsInit(void) if (DnsSocketB >= 0) { port = comm_local_port(DnsSocketB); debugs(78, 1, "DNS Socket created at " << addrB << ", FD " << DnsSocketB); + commSetSelect(DnsSocketB, COMM_SELECT_READ, idnsRead, NULL, 0); } if (DnsSocketA >= 0) { port = comm_local_port(DnsSocketA); debugs(78, 1, "DNS Socket created at " << addrA << ", FD " << DnsSocketA); + commSetSelect(DnsSocketA, COMM_SELECT_READ, idnsRead, NULL, 0); } } -- 2.47.3