trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sji...@apache.org
Subject svn commit: r989934 - in /trafficserver/traffic/trunk/iocore/dns: DNS.cc DNSConnection.cc P_DNSConnection.h P_DNSProcessor.h
Date Thu, 26 Aug 2010 21:28:33 GMT
Author: sjiang
Date: Thu Aug 26 21:28:32 2010
New Revision: 989934

URL: http://svn.apache.org/viewvc?rev=989934&view=rev
Log:
TS-425 randomize DNS query ids and randomized single source port

Reviewers: Leif, Vijay

Enhance protection against DNS cache poisoning by randomizing query ids and 
use a less predictable method of picking the (static) DNS source port [CVE-2010-2952]


Modified:
    trafficserver/traffic/trunk/iocore/dns/DNS.cc
    trafficserver/traffic/trunk/iocore/dns/DNSConnection.cc
    trafficserver/traffic/trunk/iocore/dns/P_DNSConnection.h
    trafficserver/traffic/trunk/iocore/dns/P_DNSProcessor.h

Modified: trafficserver/traffic/trunk/iocore/dns/DNS.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/dns/DNS.cc?rev=989934&r1=989933&r2=989934&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/dns/DNS.cc (original)
+++ trafficserver/traffic/trunk/iocore/dns/DNS.cc Thu Aug 26 21:28:32 2010
@@ -45,7 +45,6 @@ int dns_failover_period = DEFAULT_FAILOV
 int dns_failover_try_period = DEFAULT_FAILOVER_TRY_PERIOD;
 int dns_max_dns_in_flight = MAX_DNS_IN_FLIGHT;
 int dns_validate_qname = 0;
-unsigned int dns_sequence_number = 0;
 unsigned int dns_handler_initialized = 0;
 int dns_ns_rr = 0;
 int dns_ns_rr_init_down = 1;
@@ -62,7 +61,7 @@ ClassAllocator<HostEnt> dnsBufAllocator(
 // Function Prototypes
 //
 static bool dns_process(DNSHandler * h, HostEnt * ent, int len);
-static DNSEntry *get_dns(DNSHandler * h, u_short id);
+static DNSEntry *get_dns(DNSHandler * h, uint16 id);
 // returns true when e is done
 static void dns_result(DNSHandler * h, DNSEntry * e, HostEnt * ent, bool retry);
 static void write_dns(DNSHandler * h);
@@ -154,17 +153,6 @@ DNSProcessor::open(unsigned int aip, int
 void
 DNSProcessor::dns_init()
 {
-  int64 sval, cval = 0;
-
-  DNS_READ_DYN_STAT(dns_sequence_number_stat, sval, cval);
-
-  if (cval > 0) {
-    dns_sequence_number = (unsigned int)
-      (cval + DNS_SEQUENCE_NUMBER_RESTART_OFFSET);
-  } else {                      // select a sequence number at random
-    dns_sequence_number = (unsigned int) (ink_get_hrtime() / HRTIME_MSECOND);
-  }
-  Debug("dns", "initial dns_sequence_number = %d\n", (u_short) dns_sequence_number);
   gethostname(try_server_names[0], 255);
   Debug("dns", "localhost=%s\n", try_server_names[0]);
   Debug("dns", "Round-robin nameservers = %d\n", dns_ns_rr);
@@ -367,7 +355,7 @@ DNSHandler::open_con(unsigned int aip, i
   } else {
     ns_down[icon] = 0;
     if (con[icon].eio.start(pd, &con[icon], EVENTIO_READ) < 0) {
-      Error("iocore_dns", "open_con: Failed to add %d server to epoll list\n", icon);
+      Error("[iocore_dns] open_con: Failed to add %d server to epoll list\n", icon);
     } else {
       con[icon].num = icon;
       Debug("dns", "opening connection %d.%d.%d.%d:%d SUCCEEDED for %d", DOT_SEPARATED(aip),
aport, icon);
@@ -769,7 +757,7 @@ DNSHandler::mainEvent(int event, Event *
 
 /** Find a DNSEntry by id. */
 inline static DNSEntry *
-get_dns(DNSHandler * h, u_short id)
+get_dns(DNSHandler * h, uint16 id)
 {
   for (DNSEntry * e = h->entries.head; e; e = (DNSEntry *) e->link.next) {
     if (e->once_written_flag)
@@ -783,6 +771,7 @@ get_dns(DNSHandler * h, u_short id)
   return NULL;
 }
 
+/** Find a DNSEntry by query name and type. */
 inline static DNSEntry *
 get_entry(DNSHandler * h, char *qname, int qtype)
 {
@@ -834,6 +823,39 @@ write_dns(DNSHandler * h)
   h->in_write_dns = false;
 }
 
+uint16
+DNSHandler::get_query_id()
+{
+  uint16 q1, q2;
+  q2 = q1 = (uint16)(generator.random() & 0xFFFF);
+  if (query_id_in_use(q2)) {
+    uint16 i = q2>>6;
+    while (qid_in_flight[i] == INTU64_MAX) {
+      if (++i ==  sizeof(qid_in_flight)/sizeof(uint64)) {
+        i = 0;
+      }
+      if (i == q1>>6) {
+        Error("[iocore_dns] get_query_id: Exhausted all DNS query ids");
+        return q1;
+      }
+    }
+    i <<= 6;
+    q2 &= 0x3F;
+    while (query_id_in_use(i+q2)) {
+      ++q2;
+      q2 &= 0x3F;
+      if (q2 == (q1 & 0x3F)) {
+        Error("[iocore_dns] get_query_id: Exhausted all DNS query ids");
+        return q1;
+      }
+    }
+    q2 += i;
+  }
+
+  set_query_id_in_use(q2);
+  return q2;
+}
+
 /**
   Construct and Write the request for a single entry (using send(3N)).
 
@@ -859,19 +881,15 @@ write_dns_event(DNSHandler * h, DNSEntry
   }
 #endif
 
-  int id = dns_retries - e->retries;
-  if (id >= MAX_DNS_RETRIES)
-    id = MAX_DNS_RETRIES - 1;   // limit id history
-
-  ++dns_sequence_number;
-
-  DNS_SET_DYN_COUNT(dns_sequence_number_stat, dns_sequence_number);
-
 #ifdef DNS_PROXY
   if (!e->proxy) {
 #endif
-    u_short i = (u_short) dns_sequence_number;
+    uint16 i = h->get_query_id();
     ((HEADER *) (buffer))->id = htons(i);
+    if(e->id[dns_retries - e->retries] >= 0) {
+      //clear previous id in case named was switched or domain was expanded
+      h->release_query_id(e->id[dns_retries - e->retries]);
+    }
     e->id[dns_retries - e->retries] = i;
 #ifdef DNS_PROXY
   } else {
@@ -1165,6 +1183,17 @@ DNSEntry::post(DNSHandler * h, HostEnt *
     }
     return 0;
   }
+#ifdef DNS_PROXY
+  if (!proxy) {
+#endif
+    for (int i = 0; i < MAX_DNS_RETRIES; i++) {
+      if (id[i] < 0)
+        break;
+      h->release_query_id(id[i]);      
+    }
+#ifdef DNS_PROXY
+  }
+#endif
   action.mutex = NULL;
   mutex = NULL;
   dnsEntryAllocator.free(this);
@@ -1183,6 +1212,18 @@ DNSEntry::postEvent(int event, Event * e
   if (result_ent)
     if (ink_atomic_increment(&result_ent->ref_count, -1) == 1)
       dnsProcessor.free_hostent(result_ent);
+
+#ifdef DNS_PROXY
+  if (!proxy) {
+#endif
+    for (int i = 0; i < MAX_DNS_RETRIES; i++) {
+      if (id[i] < 0)
+        break;
+      dnsH->release_query_id(id[i]);      
+    }
+#ifdef DNS_PROXY
+  }
+#endif
   action.mutex = NULL;
   mutex = NULL;
   dnsEntryAllocator.free(this);
@@ -1195,7 +1236,7 @@ dns_process(DNSHandler * handler, HostEn
 {
   ProxyMutex *mutex = handler->mutex;
   HEADER *h = (HEADER *) (buf->buf);
-  DNSEntry *e = get_dns(handler, (u_short) ntohs(h->id));
+  DNSEntry *e = get_dns(handler, (uint16) ntohs(h->id));
   bool retry = false;
   bool server_ok = true;
   uint32 temp_ttl = 0;
@@ -1204,7 +1245,7 @@ dns_process(DNSHandler * handler, HostEn
   // Do we have an entry for this id?
   //
   if (!e || !e->written_flag) {
-    Debug("dns", "unknown DNS id = %u", (u_short) ntohs(h->id));
+    Debug("dns", "unknown DNS id = %u", (uint16) ntohs(h->id));
     if (!handler->hostent_cache)
       handler->hostent_cache = buf;
     else
@@ -1572,9 +1613,6 @@ ink_dns_init(ModuleVersion v)
                      "proxy.process.dns.in_flight",
                      RECD_INT, RECP_NON_PERSISTENT, (int) dns_in_flight_stat, RecRawStatSyncSum);
 
-  RecRegisterRawStat(dns_rsb, RECT_PROCESS,
-                     "proxy.process.dns.sequence_number",
-                     RECD_INT, RECP_NULL, (int) dns_sequence_number_stat, RecRawStatSyncCount);
 }
 
 

Modified: trafficserver/traffic/trunk/iocore/dns/DNSConnection.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/dns/DNSConnection.cc?rev=989934&r1=989933&r2=989934&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/dns/DNSConnection.cc (original)
+++ trafficserver/traffic/trunk/iocore/dns/DNSConnection.cc Thu Aug 26 21:28:32 2010
@@ -37,8 +37,8 @@
 // set in the OS
 // #define RECV_BUF_SIZE            (1024*64)
 // #define SEND_BUF_SIZE            (1024*64)
-#define FIRST_RANDOM_PORT        16000
-#define LAST_RANDOM_PORT         32000
+#define FIRST_RANDOM_PORT        (16000)
+#define LAST_RANDOM_PORT         (60000)
 
 #define ROUNDUP(x, y) ((((x)+((y)-1))/(y))*(y))
 
@@ -47,7 +47,7 @@
 //
 
 DNSConnection::DNSConnection():
-fd(NO_FD), num(0)
+  fd(NO_FD), num(0), generator(time(NULL) ^ (long) this)
 {
   memset(&sa, 0, sizeof(struct sockaddr_in));
 }
@@ -94,18 +94,16 @@ DNSConnection::connect(unsigned int ip, 
 
   if (bind_random_port) {
     int retries = 0;
-    int offset = 0;
     while (retries++ < 10000) {
       struct sockaddr_in bind_sa;
       memset(&sa, 0, sizeof(bind_sa));
       bind_sa.sin_family = AF_INET;
       bind_sa.sin_addr.s_addr = INADDR_ANY;
-      int p = time(NULL) + offset;
-      p = (p % (LAST_RANDOM_PORT - FIRST_RANDOM_PORT)) + FIRST_RANDOM_PORT;
+      uint32 p = generator.random();
+      p = (uint16)((p % (LAST_RANDOM_PORT - FIRST_RANDOM_PORT)) + FIRST_RANDOM_PORT);
       bind_sa.sin_port = htons(p);
-      Debug("dns", "random port = %d\n", p);
+      Debug("dns", "random port = %u\n", p);
       if ((res = socketManager.ink_bind(fd, (struct sockaddr *) &bind_sa, sizeof(bind_sa),
Proto)) < 0) {
-        offset += 101;
         continue;
       }
       goto Lok;

Modified: trafficserver/traffic/trunk/iocore/dns/P_DNSConnection.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/dns/P_DNSConnection.h?rev=989934&r1=989933&r2=989934&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/dns/P_DNSConnection.h (original)
+++ trafficserver/traffic/trunk/iocore/dns/P_DNSConnection.h Thu Aug 26 21:28:32 2010
@@ -63,6 +63,7 @@ struct DNSConnection
   int num;
   LINK(DNSConnection, link);
   EventIO eio;
+  InkRand generator;
 
   int connect(unsigned int ip, int port,
               bool non_blocking_connect = NON_BLOCKING_CONNECT,

Modified: trafficserver/traffic/trunk/iocore/dns/P_DNSProcessor.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/dns/P_DNSProcessor.h?rev=989934&r1=989933&r2=989934&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/dns/P_DNSProcessor.h (original)
+++ trafficserver/traffic/trunk/iocore/dns/P_DNSProcessor.h Thu Aug 26 21:28:32 2010
@@ -247,6 +247,11 @@ struct DNSHandler: public Continuation
   ink_res_state m_res;
   int txn_lookup_timeout;
 
+  InkRand generator;
+  // bitmap of query ids in use
+  uint64 qid_in_flight[(USHRT_MAX+1)/64];
+
+
   void received_one(int i)
   {
     failover_number[i] = failover_soon_number[i] = crossed_failover_number[i] = 0;
@@ -289,6 +294,19 @@ struct DNSHandler: public Continuation
   void retry_named(int ndx, ink_hrtime t, bool reopen = true);
   void try_primary_named(bool reopen = true);
   void switch_named(int ndx);
+  uint16 get_query_id();
+
+  void release_query_id(uint16 qid) {
+    qid_in_flight[qid >> 6] &= (uint64)~(0x1ULL << (qid & 0x3F));
+  };
+
+  void set_query_id_in_use(uint16 qid) {
+    qid_in_flight[qid >> 6] |= (uint64)(0x1ULL << (qid & 0x3F));
+  };
+
+  bool query_id_in_use(uint16 qid) {
+    return (qid_in_flight[(uint16)(qid) >> 6] & (uint64)(0x1ULL << ((uint16)(qid)
& 0x3F))) != 0;
+  };
 
   DNSHandler();
 };
@@ -301,7 +319,7 @@ TS_INLINE DNSHandler::DNSHandler()
   n_con(0),
   options(0),
   in_flight(0), name_server(0), in_write_dns(0), hostent_cache(0), last_primary_retry(0),
last_primary_reopen(0),
-  m_res(0), txn_lookup_timeout(0)
+  m_res(0), txn_lookup_timeout(0), generator(time(NULL) ^ (long) this)
 {
   for (int i = 0; i < MAX_NAMED; i++) {
     ifd[i] = -1;
@@ -310,6 +328,7 @@ TS_INLINE DNSHandler::DNSHandler()
     crossed_failover_number[i] = 0;
     ns_down[i] = 1;
   }
+  memset(&qid_in_flight, 0, sizeof(qid_in_flight));  
   SET_HANDLER(&DNSHandler::startEvent);
   Debug("net_epoll", "inline DNSHandler::DNSHandler()");
 }



Mime
View raw message