[or-cvs] r16933: {tor} Oops; we need to make sure that DNS request names are matche (in tor/trunk: . src/or)

[nickm at seul.org]

Author: nickm
Date: 2008-09-22 16:06:25 -0400 (Mon, 22 Sep 2008)
New Revision: 16933

Modified:
   tor/trunk/ChangeLog
   tor/trunk/src/or/eventdns.c
Log:
Oops; we need to make sure that DNS request names are matched in the _questions_ section of the replies.  Rejecting answers whether the _answers_ section did not match made us reject A records waiting at the end of a CNAME record.  Bug 823.

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2008-09-21 17:39:09 UTC (rev 16932)
+++ tor/trunk/ChangeLog	2008-09-22 20:06:25 UTC (rev 16933)
@@ -12,6 +12,9 @@
       descriptor from a hidden service directory for which the router
       descriptor has not yet been downloaded. Fixes bug 767. Bugfix
       on 0.2.0.10-alpha.
+    - DNS replies need to have names matching their requests, but these names
+      should be in the questions section, not necessarily in the answers
+      section.  Fixes bug 823.  Bugfix on 0.2.1.5-alpha.
 
   o Minor bugfixes:
     - Fix compile on OpenBSD 4.4-current. Bugfix on 0.2.1.5-alpha.

Modified: tor/trunk/src/or/eventdns.c
===================================================================
--- tor/trunk/src/or/eventdns.c	2008-09-21 17:39:09 UTC (rev 16932)
+++ tor/trunk/src/or/eventdns.c	2008-09-22 20:06:25 UTC (rev 16933)
@@ -873,6 +873,7 @@
 	struct reply reply;
 	struct request *req = NULL;
 	unsigned int i;
+	int name_matches = 0;
 
 	GET16(trans_id);
 	GET16(flags);
@@ -913,16 +914,20 @@
 		 */
 		GET_NAME;
 		j += 4;
+		if (!strcasecmp(req->name, tmp_name))
+		    name_matches = 1;
 		if (j >= length) goto err;
 	}
 
+	if (!name_matches)
+		goto err;
+
 	/* now we have the answer section which looks like
 	 * <label:name><u16:type><u16:class><u32:ttl><u16:len><data...>
 	 */
 
 	for (i = 0; i < answers; ++i) {
 		u16 type, class;
-		int name_matches;
 
 		GET_NAME;
 		GET16(type);
@@ -930,8 +935,6 @@
 		GET32(ttl);
 		GET16(datalength);
 
-		name_matches = !strcasecmp(req->name, tmp_name);
-
 		if (type == TYPE_A && class == CLASS_INET) {
 			int addrcount, addrtocopy;
 			if (req->request_type != TYPE_A) {
@@ -945,25 +948,21 @@
 			ttl_r = MIN(ttl_r, ttl);
 			/* we only bother with the first four addresses. */
 			if (j + 4*addrtocopy > length) goto err;
-			if (name_matches) {
-				memcpy(&reply.data.a.addresses[reply.data.a.addrcount],
-					   packet + j, 4*addrtocopy);
-				reply.data.a.addrcount += addrtocopy;
-				reply.have_answer = 1;
-				if (reply.data.a.addrcount == MAX_ADDRS) break;
-			}
+			memcpy(&reply.data.a.addresses[reply.data.a.addrcount],
+				   packet + j, 4*addrtocopy);
+			reply.data.a.addrcount += addrtocopy;
+			reply.have_answer = 1;
+			if (reply.data.a.addrcount == MAX_ADDRS) break;
 			j += 4*addrtocopy;
 		} else if (type == TYPE_PTR && class == CLASS_INET) {
 			if (req->request_type != TYPE_PTR) {
 				j += datalength; continue;
 			}
 			GET_NAME;
-			if (name_matches) {
-				strlcpy(reply.data.ptr.name, tmp_name,
-						sizeof(reply.data.ptr.name));
-				ttl_r = MIN(ttl_r, ttl);
-				reply.have_answer = 1;
-			}
+			strlcpy(reply.data.ptr.name, tmp_name,
+					sizeof(reply.data.ptr.name));
+			ttl_r = MIN(ttl_r, ttl);
+			reply.have_answer = 1;
 			break;
 		} else if (type == TYPE_AAAA && class == CLASS_INET) {
 			int addrcount, addrtocopy;
@@ -978,13 +977,11 @@
 
 			/* we only bother with the first four addresses. */
 			if (j + 16*addrtocopy > length) goto err;
-			if (name_matches) {
-				memcpy(&reply.data.aaaa.addresses[reply.data.aaaa.addrcount],
-					   packet + j, 16*addrtocopy);
-				reply.data.aaaa.addrcount += addrtocopy;
-				reply.have_answer = 1;
-				if (reply.data.aaaa.addrcount == MAX_ADDRS) break;
-			}
+			memcpy(&reply.data.aaaa.addresses[reply.data.aaaa.addrcount],
+				   packet + j, 16*addrtocopy);
+			reply.data.aaaa.addrcount += addrtocopy;
+			reply.have_answer = 1;
+			if (reply.data.aaaa.addrcount == MAX_ADDRS) break;
 			j += 16*addrtocopy;
 		} else {
 			/* skip over any other type of resource */