From: Amos Jeffries <amosjeffries@squid-cache.org>
Date: Sun, 29 Aug 2010 00:49:56 +0000 (-0600)
Subject: Author: Henrik Nordstrom <henrik@henriknordstrom.net>
X-Git-Tag: SQUID_3_1_8~22
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f50d23e29bb3f95cb48566d6d35fbd6da88753e1;p=thirdparty%2Fsquid.git

Author: Henrik Nordstrom <henrik@henriknordstrom.net>
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.
---

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);
         }
     }