From: hno <>
Date: Mon, 9 May 2005 08:32:09 +0000 (+0000)
Subject: Reshuffling of DNS code to make room for more secure query ID
X-Git-Tag: SQUID_3_0_PRE4~762
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=108d67a0f4213df042e28b39cacdef36f147709b;p=thirdparty%2Fsquid.git

Reshuffling of DNS code to make room for more secure query ID
assignments.
---

diff --git a/include/rfc1035.h b/include/rfc1035.h
index b35637e384..2a93ef7b18 100644
--- a/include/rfc1035.h
+++ b/include/rfc1035.h
@@ -1,5 +1,5 @@
 /*
- * $Id: rfc1035.h,v 1.11 2005/04/18 21:52:40 hno Exp $
+ * $Id: rfc1035.h,v 1.12 2005/05/09 02:32:09 hno Exp $
  *
  * AUTHOR: Duane Wessels
  *
@@ -56,13 +56,15 @@ struct _rfc1035_rr {
     unsigned short rdlength;
     char *rdata;
 };
-SQUIDCEXTERN unsigned short rfc1035BuildAQuery(const char *hostname,
+SQUIDCEXTERN ssize_t rfc1035BuildAQuery(const char *hostname,
     char *buf,
-    size_t * szp);
-SQUIDCEXTERN unsigned short rfc1035BuildPTRQuery(const struct IN_ADDR,
+    size_t sz,
+    unsigned short qid);
+SQUIDCEXTERN ssize_t rfc1035BuildPTRQuery(const struct IN_ADDR,
     char *buf,
-    size_t * szp);
-SQUIDCEXTERN unsigned short rfc1035RetryQuery(char *);
+    size_t sz,
+    unsigned short qid);
+SQUIDCEXTERN void rfc1035SetQueryID(char *, unsigned short qid);
 SQUIDCEXTERN int rfc1035AnswersUnpack(const char *buf,
     size_t sz,
     rfc1035_rr ** records,
diff --git a/lib/rfc1035.c b/lib/rfc1035.c
index b75dad74f2..9dcc231028 100644
--- a/lib/rfc1035.c
+++ b/lib/rfc1035.c
@@ -1,6 +1,6 @@
 
 /*
- * $Id: rfc1035.c,v 1.40 2005/05/09 02:09:16 hno Exp $
+ * $Id: rfc1035.c,v 1.41 2005/05/09 02:32:09 hno Exp $
  *
  * Low level DNS protocol routines
  * AUTHOR: Duane Wessels
@@ -433,13 +433,6 @@ rfc1035RRUnpack(const char *buf, size_t sz, off_t * off, rfc1035_rr * RR)
     return 0;
 }
 
-static unsigned short
-rfc1035Qid(void)
-{
-    unsigned short qid = squid_random() & 0xFFFF;
-    return qid;
-}
-
 static void
 rfc1035SetErrno(int n)
 {
@@ -587,16 +580,15 @@ rfc1035AnswersUnpack(const char *buf,
  * probably be at least 512 octets.  The 'szp' initially
  * specifies the size of the buffer, on return it contains
  * the size of the message (i.e. how much to write).
- * Return value is the query ID.
+ * Returns the size of the query
  */
-unsigned short
-rfc1035BuildAQuery(const char *hostname, char *buf, size_t * szp)
+ssize_t
+rfc1035BuildAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid)
 {
     static rfc1035_header h;
     size_t offset = 0;
-    size_t sz = *szp;
     memset(&h, '\0', sizeof(h));
-    h.id = rfc1035Qid();
+    h.id = qid;
     h.qr = 0;
     h.rd = 1;
     h.opcode = 0;		/* QUERY */
@@ -608,8 +600,7 @@ rfc1035BuildAQuery(const char *hostname, char *buf, size_t * szp)
 	RFC1035_TYPE_A,
 	RFC1035_CLASS_IN);
     assert(offset <= sz);
-    *szp = (size_t) offset;
-    return h.id;
+    return offset;
 }
 
 /*
@@ -622,12 +613,11 @@ rfc1035BuildAQuery(const char *hostname, char *buf, size_t * szp)
  * the size of the message (i.e. how much to write).
  * Return value is the query ID.
  */
-unsigned short
-rfc1035BuildPTRQuery(const struct IN_ADDR addr, char *buf, size_t * szp)
+ssize_t
+rfc1035BuildPTRQuery(const struct IN_ADDR addr, char *buf, size_t sz, unsigned short qid)
 {
     static rfc1035_header h;
     size_t offset = 0;
-    size_t sz = *szp;
     static char rev[32];
     unsigned int i;
     memset(&h, '\0', sizeof(h));
@@ -637,7 +627,7 @@ rfc1035BuildPTRQuery(const struct IN_ADDR addr, char *buf, size_t * szp)
 	(i >> 8) & 255,
 	(i >> 16) & 255,
 	(i >> 24) & 255);
-    h.id = rfc1035Qid();
+    h.id = qid;
     h.qr = 0;
     h.rd = 1;
     h.opcode = 0;		/* QUERY */
@@ -649,8 +639,7 @@ rfc1035BuildPTRQuery(const struct IN_ADDR addr, char *buf, size_t * szp)
 	RFC1035_TYPE_PTR,
 	RFC1035_CLASS_IN);
     assert(offset <= sz);
-    *szp = offset;
-    return h.id;
+    return offset;
 }
 
 /*
@@ -658,13 +647,11 @@ rfc1035BuildPTRQuery(const struct IN_ADDR addr, char *buf, size_t * szp)
  * just need a new ID for it.  Lucky for us ID
  * is the first field in the message buffer.
  */
-unsigned short
-rfc1035RetryQuery(char *buf)
+void
+rfc1035SetQueryID(char *buf, unsigned short qid)
 {
-    unsigned short qid = rfc1035Qid();
     unsigned short s = htons(qid);
     memcpy(buf, &s, sizeof(s));
-    return qid;
 }
 
 #if DRIVER
diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index 0944105797..89813751e9 100644
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: dns_internal.cc,v 1.69 2005/05/09 01:58:34 hno Exp $
+ * $Id: dns_internal.cc,v 1.70 2005/05/09 02:32:09 hno Exp $
  *
  * DEBUG: section 78    DNS lookups; interacts with lib/rfc1035.c
  * AUTHOR: Duane Wessels
@@ -548,6 +548,25 @@ idnsFindQuery(unsigned short id)
     return NULL;
 }
 
+static unsigned short
+idnsQueryID(void)
+{
+    unsigned short id = squid_random() & 0xFFFF;
+    unsigned short first_id = id;
+
+    while(idnsFindQuery(id)) {
+        id++;
+
+        if (id > 0xFFFF)
+            id = 0;
+
+        if (id == first_id)
+            break;
+    }
+
+    return squid_random() & 0xFFFF;
+}
+
 static void
 idnsCallback(idns_query *q, rfc1035_rr *answers, int n, const char *error)
 {
@@ -578,6 +597,7 @@ idnsCallback(idns_query *q, rfc1035_rr *answers, int n, const char *error)
     }
 }
 
+/* FIXME: We should also verify that the response is to the correct query to eleminate overlaps */
 static void
 idnsGrokReply(const char *buf, size_t sz)
 {
@@ -623,7 +643,8 @@ idnsGrokReply(const char *buf, size_t sz)
              */
             assert(NULL == answers);
             q->start_t = current_time;
-            q->id = rfc1035RetryQuery(q->buf);
+            q->id = idnsQueryID();
+            rfc1035SetQueryID(q->buf, q->id);
             idnsSendQuery(q);
             return;
         }
@@ -910,9 +931,7 @@ idnsALookup(const char *name, IDNSCB * callback, void *data)
 
     q = (idns_query *)memAllocate(MEM_IDNS_QUERY);
 
-    q->sz = sizeof(q->buf);
-
-    q->id = rfc1035BuildAQuery(name, q->buf, &q->sz);
+    q->sz = rfc1035BuildAQuery(name, q->buf, sizeof(q->buf), idnsQueryID());
 
     debug(78, 3) ("idnsALookup: buf is %d bytes for %s, id = %#hx\n",
                   (int) q->sz, name, q->id);
@@ -941,9 +960,7 @@ idnsPTRLookup(const struct IN_ADDR addr, IDNSCB * callback, void *data)
 
     q = (idns_query *)memAllocate(MEM_IDNS_QUERY);
 
-    q->sz = sizeof(q->buf);
-
-    q->id = rfc1035BuildPTRQuery(addr, q->buf, &q->sz);
+    q->sz = rfc1035BuildPTRQuery(addr, q->buf, sizeof(q->buf), idnsQueryID());
 
     debug(78, 3) ("idnsPTRLookup: buf is %d bytes for %s, id = %#hx\n",
                   (int) q->sz, ip, q->id);