Fix for case-sensitivity problems in DNS.

Fix a case sensitivity problem which has been lurking for a long while.
When we get example.com and Example.com and combine them, we send whichever
query arrives first upstream and then later answer it, and we also
answer the second with the same answer. That means that if example.com
arrives first, it will get the answer example.com - good - but Example.com
will _also_ get the answer example.com - not so good.

In theory, fixing this is simple without having to keep seperate
copies of all the queries: Just use the bit-vector representation
of case flipping that we have for 0x20-encoding to keep the
differences in case. The complication comes from the fact that
the existing bit-vector code only holds data on the first 32 alpha
letters, because we only flip that up to many for 0x20 encoding.

In practise, the delta between combined queries can almost always
be represented with that data, since almost all queries are
all lower case and we only purturb the first 32 letters with
0x20 encoding. It's therefore worth keeping the existing,
efficient data structure for the 99.9% of the time it works.
For the 0.1% it doesn't, however, one needs an arbitrary-length data
structure with the resource implications of that.

Thanks to Peter Tirsek for the well researched bug report which set me
on to these problems.

CHANGELOG

@@ -116,6 +116,14 @@ version 2.91
 	will be logged. If this coincides with DNS not functioning, it
 	is necessary to disable bit 0x20 encoding with --no-0x20-encode.
 
+	Fix a long-standing problem when two queries which are identical
+	in every repect _except_ case, get combined by dnsmasq. If
+	dnsmasq gets eg, two queries for example.com and Example.com
+	in quick succession it will get the answer for example.com from
+	upstream and send that answer to both requestors. This means that
+	the query for Example.com will get an answer for example.com, and
+	in the modern DNS, that answer may not be accepted.
+
 	
 version 2.90
 	Fix reversion in --rev-server introduced in 2.88 which

src/dnsmasq.h

@@ -781,7 +781,7 @@ struct frec {
   struct frec_src {
     union mysockaddr source;
     union all_addr dest;
-    unsigned int iface, log_id;
+    unsigned int iface, log_id, encode_bitmap, *encode_bigmap;
     int fd;
     unsigned short orig_id, udp_pkt_size;
     struct frec_src *next;
@@ -792,7 +792,6 @@ struct frec {
   int forwardall, flags;
   time_t time;
   u32 forward_timestamp;
-  unsigned int encode_bitmap;
   int forward_delay;
   struct blockdata *stash; /* saved query or saved reply, whilst we validate */
   size_t stash_len;

src/forward.c

@@ -197,6 +197,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 					     FREC_HAS_PHEADER | FREC_DNSKEY_QUERY | FREC_DS_QUERY | FREC_NO_CACHE)))
     {
       struct frec_src *src;
+      struct dns_header *saved_header;
+      unsigned int casediff = 0;
+      unsigned int *bitvector = NULL;
       
       for (src = &forward->frec_src; src; src = src->next)
 	if (src->orig_id == ntohs(header->id) && 
@@ -221,6 +224,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 	    {
 	      daemon->frec_src_count++;
 	      daemon->free_frec_src->next = NULL;
+	      daemon->free_frec_src->encode_bigmap = NULL;
 	    }
 	  
 	  /* If we've been spammed with many duplicates, return REFUSED. */
@@ -237,6 +241,54 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 		free_frec(forward);
 	      goto reply;
 	    }
+
+	  /* Find a bitmap of case differences between the query send upstream and this one,
+	     so we can reply to each query with the correct case pattern.
+	     Since we need this to get back the exact case pattern of each query when doing
+	     query combining, we have to handle the (rare) case that there are case differences
+	     beyond the first 32 letters.
+	     If that happens we have to allocate memory to save it, and the casediff variable
+	     holds the length of that array.
+	     Mismatches beyond 32 letters are rare because most queries are all lowercase and
+	     we only scramble the first 32 letters for security reasons.
+
+	     Note the two names are guaranteed to be the same length and differ only in the case
+	     of letters. */
+	  if ((saved_header = blockdata_retrieve(forward->stash, forward->stash_len, NULL)) &&
+	      extract_request(saved_header, forward->stash_len, daemon->workspacename, NULL))
+	    {
+	      unsigned int i, gobig = 0;
+	      char *s1, *s2;
+
+#define BITS_IN_INT (sizeof(unsigned int) * 8)
+	      
+	    big_redo:
+	      for (s1 = daemon->namebuff, s2 = daemon->workspacename, i = 0; *s1; s1++, s2++)
+		{
+		  char c1 = *s1, c2 = *s2;
+		  
+		  if ((c1 >= 'a' && c1 <= 'z') || (c1 >= 'A' && c1 <= 'Z'))
+		    {
+		      if ((c1 & 0x20) ^ (c2 & 0x20))
+			{
+			  if (bitvector)
+			    bitvector[i/BITS_IN_INT] |= 1<<(i%BITS_IN_INT);
+			  else if (i >= BITS_IN_INT)
+			    gobig = 1; /* More than 32 */
+			  else
+			    casediff |= 1<<i;
+			}
+		      i++;
+		    }
+		}
+
+	      if (gobig && !bitvector)
+		{
+		  casediff = (i/BITS_IN_INT) + 1; /* length of array */
+		  if ((bitvector = whine_malloc(casediff)))
+		    goto big_redo;
+		}
+	    }
 	  
 	  src = daemon->free_frec_src;
 	  daemon->free_frec_src = src->next;
@@ -248,6 +300,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 	  src->log_id = daemon->log_id;
 	  src->iface = dst_iface;
 	  src->fd = udpfd;
+	  src->encode_bitmap = casediff;
+	  src->encode_bigmap = bitvector;
+	  
 	  src->udp_pkt_size = (unsigned short)replylimit;
 
 	  /* closely spaced identical queries cannot be a try and a retry, so
@@ -326,9 +381,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
       forward->new_id = get_id();
       header->id = ntohs(forward->new_id);
       
-      forward->encode_bitmap = option_bool(OPT_NO_0x20) ? 0 : rand32();
+      forward->frec_src.encode_bitmap = option_bool(OPT_NO_0x20) ? 0 : rand32();
       p = (unsigned char *)(header+1);
-      if (!extract_name(header, plen, &p, NULL, EXTR_NAME_FLIP, forward->encode_bitmap))
+      if (!extract_name(header, plen, &p, (char *)&forward->frec_src.encode_bitmap, EXTR_NAME_FLIP, 1))
 	goto reply;
       
       /* Keep copy of query for retries and move to TCP */
@@ -975,7 +1030,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
 		  new->flags &= ~(FREC_DNSKEY_QUERY | FREC_DS_QUERY);
 		  new->flags |= flags;
 		  new->forwardall = 0;
-		  new->encode_bitmap = 0;
+		  new->frec_src.encode_bitmap = 0;
 
 		  forward->next_dependent = NULL;
 		  new->dependent = forward; /* to find query awaiting new one. */
@@ -1205,7 +1260,7 @@ void reply_query(int fd, time_t now)
   
   /* Flip the bits back in the query name. */
   p = (unsigned char *)(header+1);
-  if (!extract_name(header, n, &p, NULL, EXTR_NAME_FLIP, forward->encode_bitmap))
+  if (!extract_name(header, n, &p, (char *)&forward->frec_src.encode_bitmap, EXTR_NAME_FLIP, 1))
     return;
       
 #ifdef HAVE_DNSSEC
@@ -1227,6 +1282,51 @@ void reply_query(int fd, time_t now)
     return_reply(now, forward, header, n, STAT_OK); 
 }
 
+static void xor_array(unsigned int *arg1, unsigned int *arg2, unsigned int len)
+{
+  unsigned int i;
+
+  for (i = 0; i < len; i++)
+    arg1[i] ^= arg2[i];
+}
+
+/* Call extract_name() to flip case of query in packet according to the XOR of the bit maps help in arg1 and arg2 */
+static void flip_queryname(struct dns_header *header, ssize_t len, struct frec_src *arg1, struct frec_src *arg2)
+{
+  unsigned char *p = (unsigned char *)(header+1);
+  unsigned int *arg1p, *arg2p, arg1len, arg2len, *swapp, swap;
+
+   /* Two cases: bitmap is single 32 bit int, or it's arbitrary-length array of 32bit ints.
+      The two args may be different and of different lengths.
+      The shorter gets notionally extended with zeros. */
+    
+  if (arg1->encode_bigmap)
+    arg1p = arg1->encode_bigmap, arg1len = arg1->encode_bitmap;
+  else
+    arg1p = &arg1->encode_bitmap, arg1len = 1;
+
+  if (arg2->encode_bigmap)
+    arg2p = arg2->encode_bigmap, arg2len = arg2->encode_bitmap;
+  else
+    arg2p = &arg2->encode_bitmap, arg2len = 1;
+
+  /* make arg1 the longer, if they differ. */
+  if (arg2len > arg1len)
+    {
+      swap = arg1len;
+      swapp = arg1p;
+      arg1len = arg2len;
+      arg1p = arg2p;
+      arg2len = swap;
+      arg2p = swapp;
+    }
+
+  /* XOR on shorter length, flip on longer, operate on longer */
+  xor_array(arg1p, arg2p, arg2len);
+  extract_name(header, len, &p, (char *)arg1p, EXTR_NAME_FLIP, arg1len);
+  xor_array(arg1p, arg2p, arg2len); /* restore */
+}
+
 void return_reply(time_t now, struct frec *forward, struct dns_header *header, ssize_t n, int status)
 {
   int check_rebind = 0, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
@@ -1316,10 +1416,10 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s
 			  !(forward->flags & FREC_HAS_PHEADER), &forward->frec_src.source,
 			  ((unsigned char *)header) + daemon->edns_pktsz, ede)))
     {
-      struct frec_src *src;
+      struct frec_src *src, *prev;
       int do_trunc;
-      
+            
-      for (do_trunc = 0, src = &forward->frec_src; src; src = src->next)
+      for (do_trunc = 0, prev = NULL, src = &forward->frec_src; src; prev = src, src = src->next)
 	{
 	  header->id = htons(src->orig_id);
 	  
@@ -1333,6 +1433,13 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s
 	    }
 #endif
 	  
+	  /* You will have to draw diagrams and scratch your head to convince yourself
+	     that this works. Bear in mind that the flip to upstream state has already been undone,
+	     for the original query so nothing needs to be done, but subsequent queries' flips
+	     were recorded relative to the flipped name sent upstream. */
+	  if (prev)
+	    flip_queryname(header, nn, prev, src);
+	  
 	  if (src->fd != -1)
 	    {
 	      /* Only send packets that fit what the requestor allows.
@@ -1356,7 +1463,7 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s
 		}
 	    }
 	}
-      
+
       /* The packet is too big for one or more requestors, send them a truncated answer. */
       if (do_trunc)
 	{
@@ -1368,22 +1475,31 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s
 	  header->arcount = htons(0);
 	  header->hb3 |= HB3_TC;
 	  new = resize_packet(header, nn, pheader, hlen);
-
+	  
 	  daemon->log_display_id = forward->frec_src.log_id;
 	  daemon->log_source_addr = &forward->frec_src.source;
 	  log_query(F_UPSTREAM, NULL, NULL, "truncated", 0);
 
-	  for (src = &forward->frec_src; src; src = src->next)
+	  /* This gets the name back to the state it was in when we started. */
-	    if (src->fd != -1 && nn > src->udp_pkt_size)
+	  flip_queryname(header, nn, prev, &forward->frec_src);
-	      {
+	  
-		header->id = htons(src->orig_id);
+	  for (src = &forward->frec_src, prev = NULL; src; prev = src, src = src->next)
-		send_from(src->fd, option_bool(OPT_NOWILD) || option_bool (OPT_CLEVERBIND), daemon->packet, new, 
+	    {
-			  &src->source, &src->dest, src->iface);
+	      /* If you didn't undertand this above, you won't understand it here either. */
-		
+	      if (prev)
+		flip_queryname(header, nn, prev, src);
+	      
+	      if (src->fd != -1 && nn > src->udp_pkt_size)
+		{
+		  header->id = htons(src->orig_id);
+		  send_from(src->fd, option_bool(OPT_NOWILD) || option_bool (OPT_CLEVERBIND), daemon->packet, new, 
+			    &src->source, &src->dest, src->iface);
+		  
 #ifdef HAVE_DUMPFILE
-		dump_packet_udp(DUMP_REPLY, daemon->packet, (size_t)new, NULL, &src->source, src->fd);
+		  dump_packet_udp(DUMP_REPLY, daemon->packet, (size_t)new, NULL, &src->source, src->fd);
 #endif
-	      }
+		}
+	    }
 	}
     }
       
@@ -2916,8 +3032,15 @@ static void free_frec(struct frec *f)
 {
   struct frec_src *last;
   
-  /* add back to freelist if not the record builtin to every frec. */
+  /* add back to freelist if not the record builtin to every frec,
-  for (last = f->frec_src.next; last && last->next; last = last->next) ;
+     also free any bigmaps they's been decorated with. */
+  for (last = f->frec_src.next; last && last->next; last = last->next)
+    if (last->encode_bigmap)
+      {
+	free(last->encode_bigmap);
+	last->encode_bigmap = NULL;
+      }
+  
   if (last)
     {
       last->next = daemon->free_frec_src;
@@ -2934,7 +3057,7 @@ static void free_frec(struct frec *f)
       blockdata_free(f->stash);
       f->stash = NULL;
     }
-  
+
 #ifdef HAVE_DNSSEC
   /* Anything we're waiting on is pointless now, too */
   if (f->blocking_query)
@@ -3059,8 +3182,8 @@ static struct frec *lookup_frec(char *target, int class, int rrtype, int id, int
   struct dns_header *header;
   int compare_mode = EXTR_NAME_COMPARE;
 
-  /* Only compare case-sensitive when matching frec to a an recieved anwer,
+  /* Only compare case-sensitive when matching frec to a recieved answer,
-     not when looking for a duplicated question. */
+     NOT when looking for a duplicated question. */
   if (flags & FREC_ANSWER)
     {
       flags &= ~FREC_ANSWER;

src/rfc1035.c

@@ -19,7 +19,11 @@
 /* EXTR_NAME_EXTRACT -> extract name
    EXTR_NAME_COMPARE -> compare name, case insensitive
    EXTR_NAME_NOCASE -> compare name, case sensitive
-   EXTR_NAME_FLIP -> flip 0x20 bits in packet, controlled by bitmap in parm. name may be NULL 
+   EXTR_NAME_FLIP -> flip 0x20 bits in packet.
+
+   For flip, name is  an array of ints, whose size
+   is given in parm, which forms the bitmap. Bits beyond the size
+   are assumed to be zero.
    
    return = 0 -> error
    return = 1 -> extract OK, compare OK, flip OK
@@ -31,14 +35,21 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
 {
   unsigned char *cp = (unsigned char *)name, *p = *pp, *p1 = NULL;
   unsigned int j, l, namelen = 0, hops = 0;
+  unsigned int bigmap_counter = 0, bigmap_posn = 0, bigmap_size, bitmap;
   int retvalue = 1, case_insens = 1, isExtract = 0, flip = 0, extrabytes = (int)parm;
-  
+  unsigned int *bigmap;
+
   if (func == EXTR_NAME_EXTRACT)
     isExtract = 1, *cp = 0;
   else if (func == EXTR_NAME_NOCASE)
     case_insens = 0;
   else if (func == EXTR_NAME_FLIP)
-    flip = 1, extrabytes = 0;
+    {
+      flip = 1, extrabytes = 0;
+      bigmap = (unsigned int *)name;
+      name = NULL;
+      bigmap_size = parm;
+    }
   
   while (1)
     { 
@@ -116,12 +127,18 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
 	      {
 		unsigned char c = *p;
 
-		/* parm is unsigned. We only flip up to the first 32 alpha-chars. */
 		if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'))
 		  {
-		    if (parm & 1)
+		    /* Get the next int of the bitmap */
+		    if (bigmap_posn < bigmap_size && bigmap_counter-- == 0)
+		      {
+			bitmap = bigmap[bigmap_posn++];
+			bigmap_counter = (sizeof(unsigned int) * 8) - 1;
+		      }
+		    
+		    if (bitmap & 1)
 		      *p ^= 0x20;
-		    parm >>= 1;
+		    bitmap >>= 1;
 		  }
 	      }
 	    else