Partial backport of DNS address/name checking (r16621), and backport of 0x20 hack (r17171).

svn:r17636

6 files changed, 109 insertions, 8 deletions

diff --git a/ChangeLog b/ChangeLog
index 250053f129..5d4574492f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -32,7 +32,6 @@ Changes in version 0.2.0.33 - 200?-??-??
     - Use 64 bits instead of 32 bits for connection identifiers used with
       the controller protocol, to greatly reduce risk of identifier reuse.
 
-
   o Minor features:
     - Report the case where all signatures in a detached set are rejected
       differently than the case where there is an error handling the detached
@@ -40,6 +39,15 @@ Changes in version 0.2.0.33 - 200?-??-??
     - When we realize that another process has modified our cached
       descriptors, print out a more useful error message rather than
       triggering an assertion. Fixes bug 885.  Patch from Karsten.
+    - Implement the 0x20 hack to better resist DNS poisoning: set the
+      case on outgoing DNS requests randomly, and reject responses that do
+      not match the case correctly. This logic can be disabled with the
+      ServerDNSRamdomizeCase setting, if you are using one of the 0.3%
+      of servers that do not reliably preserve case in replies. See
+      "Increased DNS Forgery Resistance through 0x20-Bit Encoding"
+      for more info.
+    - Check DNS replies for more matching fields to better resist DNS
+      poisoning.
 
 
 Changes in version 0.2.0.32 - 2008-11-20
diff --git a/doc/TODO.020 b/doc/TODO.020
index abda5d74b5..bc0855f22f 100644
--- a/doc/TODO.020
+++ b/doc/TODO.020
@@ -8,8 +8,10 @@ Backport for 0.2.0:
 Backport for 0.2.0 once better tested:
   o r16136: prevent circid collision.  [Also backport to 0.1.2.x??]
   o r16558: Avoid mis-routing CREATED cells.
-  - r16621: Make some DNS code more robust (partial; see also libevent
+  Xo r16621: Make some DNS code more robust (partial; see also libevent
             approach). (Also maybe r16674)
+    [Partially backported.  Instead of the basic name checking, I backported
+     r17171 instead, to be even more resistant to poisoning.]
   - r17091: distinguish "no routers support pending circuits" from
             "no circuits are pending." See also r17181 and r17184.
   - r17137: send END cell in response to connect to nonexistent hidserv port.
diff --git a/src/or/config.c b/src/or/config.c
index 9e9bccc88b..d69254b30d 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -274,6 +274,7 @@ static config_var_t _option_vars[] = {
   V(ServerDNSAllowBrokenResolvConf, BOOL,  "1"),
   V(ServerDNSAllowNonRFC953Hostnames, BOOL,"0"),
   V(ServerDNSDetectHijacking,    BOOL,     "1"),
+  V(ServerDNSRandomizeCase,      BOOL,     "1"),
   V(ServerDNSResolvConfFile,     STRING,   NULL),
   V(ServerDNSSearchDomains,      BOOL,     "0"),
   V(ServerDNSTestAddresses,      CSV,
diff --git a/src/or/dns.c b/src/or/dns.c
index db52651ce3..c8da9969d6 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -198,6 +198,10 @@ dns_init(void)
 {
   init_cache_map();
   evdns_set_transaction_id_fn(dns_get_transaction_id);
+  if (get_options()->ServerDNSRandomizeCase)
+    evdns_set_option("randomize-case", "1", DNS_OPTIONS_ALL);
+  else
+    evdns_set_option("randomize-case", "0", DNS_OPTIONS_ALL);
   if (server_mode(get_options()))
     return configure_nameservers(1);
   return 0;
diff --git a/src/or/eventdns.c b/src/or/eventdns.c
index 34e622b6e6..b442b5cbc2 100644
--- a/src/or/eventdns.c
+++ b/src/or/eventdns.c
@@ -307,6 +307,9 @@ static int global_max_retransmits = 3;	/* number of times we'll retransmit a req
 /* number of timeouts in a row before we consider this server to be down */
 static int global_max_nameserver_timeout = 3;
 
+/* DOCDOC */
+static int global_randomize_case = 1;
+
 /* These are the timeout values for nameservers. If we find a nameserver is down */
 /* we try to probe it at intervals as given below. Values are in seconds. */
 static const struct timeval global_nameserver_timeouts[] = {{10, 0}, {60, 0}, {300, 0}, {900, 0}, {3600, 0}};
@@ -377,6 +380,9 @@ inet_aton(const char *c, struct in_addr *addr)
 
 #define ISSPACE(c) isspace((int)(unsigned char)(c))
 #define ISDIGIT(c) isdigit((int)(unsigned char)(c))
+#define ISALPHA(c) isalpha((int)(unsigned char)(c))
+#define TOLOWER(c) (char)tolower((int)(unsigned char)(c))
+#define TOUPPER(c) (char)toupper((int)(unsigned char)(c))
 
 #ifndef NDEBUG
 static const char *
@@ -813,9 +819,10 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, size_t name_out_len
 static int
 reply_parse(u8 *packet, int length) {
 	int j = 0;	/* index into packet */
+	int k;
 	u16 _t;	 /* used by the macros */
 	u32 _t32;  /* used by the macros */
-	char tmp_name[256]; /* used by the macros */
+	char tmp_name[256], cmp_name[256]; /* used by the macros */
 
 	u16 trans_id, questions, answers, authority, additional, datalength;
 	u16 flags = 0;
@@ -823,6 +830,7 @@ reply_parse(u8 *packet, int length) {
 	struct reply reply;
 	struct request *req = NULL;
 	unsigned int i;
+	int name_matches = 0;
 
 	GET16(trans_id);
 	GET16(flags);
@@ -848,11 +856,28 @@ reply_parse(u8 *packet, int length) {
 	/* if (!answers) return; */  /* must have an answer of some form */
 
 	/* This macro skips a name in the DNS reply. */
-#define SKIP_NAME														\
+#define GET_NAME														\
 	do { tmp_name[0] = '\0';											\
 		if (name_parse(packet, length, &j, tmp_name, sizeof(tmp_name))<0) \
 			goto err;													\
 	} while(0);
+#define TEST_NAME														\
+	do { tmp_name[0] = '\0';											\
+		cmp_name[0] = '\0';												\
+		k = j;															\
+		if (name_parse(packet, length, &j, tmp_name, sizeof(tmp_name))<0) \
+			goto err;													\
+		if (name_parse(req->request, req->request_len, &k, cmp_name, sizeof(cmp_name))<0) \
+			goto err;													\
+		if (global_randomize_case) {									\
+			if (strcmp(tmp_name, cmp_name) == 0)						\
+				name_matches = 1; /* we ignore mismatching names */		\
+		} else {														\
+			if (strcasecmp(tmp_name, cmp_name) == 0)					\
+				name_matches = 1;										\
+		}																\
+	} while(0)
+
 
 	reply.type = req->request_type;
 
@@ -861,11 +886,14 @@ reply_parse(u8 *packet, int length) {
 		/* the question looks like
 		 * <label:name><u16:type><u16:class>
 		 */
-		SKIP_NAME;
+	    TEST_NAME;
 		j += 4;
 		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...>
 	 */
@@ -875,7 +903,7 @@ reply_parse(u8 *packet, int length) {
 
 		/* XXX I'd be more comfortable if we actually checked the name */
 		/* here. -NM */
-		SKIP_NAME;
+		GET_NAME;
 		GET16(type);
 		GET16(class);
 		GET32(ttl);
@@ -1082,6 +1110,22 @@ evdns_set_transaction_id_fn(uint16_t (*fn)(void))
 		trans_id_function = default_transaction_id_fn;
 }
 
+
+static void
+get_random_bytes(char *buf, size_t n)
+{
+	unsigned i;
+	for (i = 0; i < n-1; i += 2) {
+		u16 tid = trans_id_function();
+		buf[i] = (tid >> 8) & 0xff;
+		buf[i+1] = tid & 0xff;
+	}
+	if (i < n) {
+		u16 tid = trans_id_function();
+		buf[i] = tid & 0xff;
+	}
+}
+
 /* Try to choose a strong transaction id which isn't already in flight */
 static u16
 transaction_id_pick(void) {
@@ -1143,17 +1187,34 @@ nameserver_pick(void) {
 /* this is called when a namesever socket is ready for reading */
 static void
 nameserver_read(struct nameserver *ns) {
+	struct sockaddr_storage ss;
+	struct sockaddr *sa = (struct sockaddr *)&ss;
+	struct sockaddr_in *sin;
+	socklen_t addrlen = sizeof(ss);
 	u8 packet[1500];
 
 	for (;;) {
 		const int r =
-            (int)recv(ns->socket, packet,(socklen_t)sizeof(packet), 0);
+            (int)recvfrom(ns->socket, packet,(socklen_t)sizeof(packet), 0,
+						  sa, &addrlen);
 		if (r < 0) {
 			int err = last_error(ns->socket);
 			if (error_is_eagain(err)) return;
 			nameserver_failed(ns, strerror(err));
 			return;
 		}
+		if (sa->sa_family != AF_INET) {
+			log(EVDNS_LOG_WARN,
+				"Address family mismatch on received DNS packet.");
+			return;
+		}
+		sin = (struct sockaddr_in *)sa;
+		if (sin->sin_addr.s_addr != ns->address) {
+			log(EVDNS_LOG_WARN,
+				"Address mismatch on received DNS packet.  Address was %s.",
+				debug_ntoa(sin->sin_addr.s_addr));
+			return;
+		}
 		ns->timedout = 0;
 		reply_parse(packet, r);
 	}
@@ -2243,12 +2304,29 @@ request_new(int type, const char *name, int flags,
 	/* the request data is alloced in a single block with the header */
 	struct request *const req =
 		(struct request *) malloc(sizeof(struct request) + request_max_len);
+	char namebuf[256];
 	int rlen;
 	(void) flags;
 
 	if (!req) return NULL;
 	memset(req, 0, sizeof(struct request));
 
+	if (global_randomize_case) {
+		unsigned i;
+		char randbits[32];
+		strlcpy(namebuf, name, sizeof(namebuf));
+		get_random_bytes(randbits, (name_len+7)/8);
+		for (i = 0; i < name_len; ++i) {
+			if (ISALPHA(namebuf[i])) {
+				if ((randbits[i >> 3] & (1<<(i%7))))
+					namebuf[i] = TOLOWER(namebuf[i]);
+				else
+					namebuf[i] = TOUPPER(namebuf[i]);
+			}
+		}
+		name = namebuf;
+	}
+
 	/* request data lives just after the header */
 	req->request = ((u8 *) req) + sizeof(struct request);
 	/* denotes that the request data shouldn't be free()ed */
@@ -2690,7 +2768,13 @@ evdns_set_option(const char *option, const char *val, int flags)
 		if (!(flags & DNS_OPTION_MISC)) return 0;
 		log(EVDNS_LOG_DEBUG, "Setting retries to %d", retries);
 		global_max_retransmits = retries;
+	} else if (!strncmp(option, "randomize-case:", 15)) {
+		int randcase = strtoint(val);
+		if (!(flags & DNS_OPTION_MISC)) return 0;
+		log(EVDNS_LOG_DEBUG, "Setting randomize_case to %d", randcase);
+		global_randomize_case = randcase;
 	}
+
 	return 0;
 }
 
@@ -3127,7 +3211,7 @@ evdns_server_callback(struct evdns_server_request *req, void *data)
 		}
 	}
 
-	r = evdns_request_respond(req, 0);
+	r = evdns_server_request_respond(req, 0);
 	if (r<0)
 		printf("eeek, couldn't send reply.\n");
 }
diff --git a/src/or/or.h b/src/or/or.h
index 48bc3375d1..a0a6a5e376 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2297,6 +2297,8 @@ typedef struct {
                       * the local domains. */
   int ServerDNSDetectHijacking; /**< Boolean: If true, check for DNS failure
                                  * hijacking. */
+  int ServerDNSRandomizeCase; /**< Boolean: Use the 0x20-hack to prevent
+                               * DNS poisoning attacks. */
   char *ServerDNSResolvConfFile; /**< If provided, we configure our internal
                      * resolver from the file here rather than from
                      * /etc/resolv.conf (Unix) or the registry (Windows). */