Fix DHCPv6 address allocation for some pathalogical cases.

CHANGELOG

@@ -10,6 +10,9 @@ version 2.64
 	    also a match. This allows multiple addresses for a name in 
 	    /etc/hosts with one of them assigned via DHCP.
 
+	    Fix broken vendor-option processing for BOOTP. Thanks to
+	    Hans-Joachim Baader for the bug report.
+
 
 version 2.63
             Do duplicate dhcp-host address check in --test mode.

debian/changelog

@@ -2,7 +2,13 @@ dnsmasq (2.64-1) unstable; urgency=low
 
    * New upstream.
 
- -- Simon Kelley <simon@thekelleys.org.uk>  Tue, 28 Aug 2012 16:19:15 +0000
+ -- Simon Kelley <simon@thekelleys.org.uk>  Tue, 4 Sep 2012 21:50:22 +0000
+
+dnsmasq (2.63-3) unstable; urgency=low
+
+   * Move adduser dependency to dnsmasq-base. (closes: #686694)
+
+ -- Simon Kelley <simon@thekelleys.org.uk>  Tue, 4 Sep 2012 21:44:15 +0000
 
 dnsmasq (2.63-2) unstable; urgency=low
 

debian/control

@@ -7,7 +7,7 @@ Standards-Version: 3.9.3
 
 Package: dnsmasq
 Architecture: all
-Depends: netbase, adduser, dnsmasq-base(>= ${source:Version})
+Depends: netbase, dnsmasq-base(>= ${source:Version})
 Suggests: resolvconf
 Conflicts: resolvconf (<<1.15)
 Description: Small caching DNS proxy and DHCP/TFTP server
@@ -22,7 +22,7 @@ Description: Small caching DNS proxy and DHCP/TFTP server
 
 Package: dnsmasq-base
 Architecture: any
-Depends: ${shlibs:Depends}
+Depends: adduser, ${shlibs:Depends}
 Breaks: dnsmasq (<< 2.63-1~)
 Replaces: dnsmasq (<< 2.63-1~)
 Description: Small caching DNS proxy and DHCP/TFTP server

src/dnsmasq.c

@@ -383,15 +383,48 @@ int main (int argc, char **argv)
       /* write pidfile _after_ forking ! */
       if (daemon->runfile)
 	{
-	  FILE *pidfile;
+	  int fd, err = 0;
 
-	  /* only complain if started as root */
+	  sprintf(daemon->namebuff, "%d\n", (int) getpid());
-	  if ((pidfile = fopen(daemon->runfile, "w")))
+
+	  /* Explanation: Some installations of dnsmasq (eg Debian/Ubuntu) locate the pid-file
+	     in a directory which is writable by the non-privileged user that dnsmasq runs as. This
+	     allows the daemon to delete the file as part of its shutdown. This is a security hole to the 
+	     extent that an attacker running as the unprivileged  user could replace the pidfile with a 
+	     symlink, and have the target of that symlink overwritten as root next time dnsmasq starts. 
+
+	     The folowing code first deletes any existing file, and then opens it with the O_EXCL flag,
+	     ensuring that the open() fails should there be any existing file (because the unlink() failed, 
+	     or an attacker exploited the race between unlink() and open()). This ensures that no symlink
+	     attack can succeed. 
+
+	     Any compromise of the non-privileged user still theoretically allows the pid-file to be
+	     replaced whilst dnsmasq is running. The worst that could allow is that the usual 
+	     "shutdown dnsmasq" shell command could be tricked into stopping any other process.
+
+	     Note that if dnsmasq is started as non-root (eg for testing) it silently ignores 
+	     failure to write the pid-file.
+	  */
+
+	  unlink(daemon->runfile); 
+	  
+	  if ((fd = open(daemon->runfile, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL, S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH)) == -1)
 	    {
-	      fprintf(pidfile, "%d\n", (int) getpid());
+	      /* only complain if started as root */
-	      fclose(pidfile);
+	      if (getuid() == 0)
+		err = 1;
 	    }
-	  else if (getuid() == 0)
+	  else
+	    {
+	      if (!read_write(fd, (unsigned char *)daemon->namebuff, strlen(daemon->namebuff), 0))
+		err = 1;
+	      
+	      while (!err && close(fd) == -1)
+		if (!retry_send())
+		  err = 1;
+	    }
+
+	  if (err)
 	    {
 	      send_event(err_pipe[1], EVENT_PIDFILE, errno, daemon->runfile);
 	      _exit(0);
@@ -607,7 +640,7 @@ int main (int argc, char **argv)
 	      end = &dhcp_tmp->end6;
 	      struct in6_addr subnet = dhcp_tmp->start6;
 	      setaddr6part(&subnet, 0);
-	      inet_ntop(AF_INET6, &subnet, daemon->addrbuff, 256);
+	      inet_ntop(AF_INET6, &subnet, daemon->dhcp_buff2, 256);
 	    }
 #endif
 	  
@@ -620,9 +653,7 @@ int main (int argc, char **argv)
 	      prettyprint_time(p, dhcp_tmp->lease_time);
 	    }
 	  
-	  if (daemon->dhcp_buff)
 	  inet_ntop(family, start, daemon->dhcp_buff, 256);
-	  if (daemon->dhcp_buff3)
 	  inet_ntop(family, end, daemon->dhcp_buff3, 256);
 	  if ((dhcp_tmp->flags & CONTEXT_DHCP) || family == AF_INET) 
 	    my_syslog(MS_DHCP | LOG_INFO, 
@@ -637,7 +668,7 @@ int main (int argc, char **argv)
 
 	  if (dhcp_tmp->flags & CONTEXT_RA_NAME)
 	    my_syslog(MS_DHCP | LOG_INFO, _("DHCPv4-derived IPv6 names on %s"), 
-		      daemon->addrbuff);
+		      daemon->dhcp_buff2);
 	  if (dhcp_tmp->flags & (CONTEXT_RA_ONLY | CONTEXT_RA_NAME | CONTEXT_RA_STATELESS))
 	    {
 	      if (!(dhcp_tmp->flags & CONTEXT_DEPRECATE))
@@ -647,7 +678,7 @@ int main (int argc, char **argv)
 		  prettyprint_time(p, dhcp_tmp->lease_time > 7200 ? dhcp_tmp->lease_time : 7200);
 		}
 	      my_syslog(MS_DHCP | LOG_INFO, _("SLAAC on %s %s"), 
-			daemon->addrbuff, daemon->namebuff);
+			daemon->dhcp_buff2, daemon->namebuff);
 	    }
 	}
       

src/dnsmasq.h

@@ -992,6 +992,7 @@ struct dhcp_lease *lease4_allocate(struct in_addr addr);
 struct dhcp_lease *lease6_allocate(struct in6_addr *addrp, int lease_type);
 struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len, 
 			       int lease_type, int iaid, struct in6_addr *addr);
+void lease6_filter(int lease_type, int iaid, struct dhcp_context *context);
 struct dhcp_lease *lease6_find_by_addr(struct in6_addr *net, int prefix, u64 addr);
 u64 lease_find_max_addr6(struct dhcp_context *context);
 void lease_ping_reply(struct in6_addr *sender, unsigned char *packet, char *interface);

src/lease.c

@@ -562,18 +562,31 @@ struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len,
 	   memcmp(clid, lease->clid, clid_len) != 0))
 	continue;
       
-      if (clid || addr)
-	{
       lease->flags |= LEASE_USED;
       return lease;
     }
-      else 
-	lease->flags &= ~LEASE_USED;
-    }
   
   return NULL;
 }
 
+void lease6_filter(int lease_type, int iaid, struct dhcp_context *context)
+{
+  struct dhcp_lease *lease;
+  
+  for (lease = leases; lease; lease = lease->next)
+    {
+      /* reset "USED flag */
+      lease->flags &= ~LEASE_USED;
+      
+      if (!(lease->flags & lease_type) || lease->hwaddr_type != iaid)
+	continue;
+      
+      /* leases on the wrong interface get filtered out here */
+      if (!address6_available(context, (struct in6_addr *)&lease->hwaddr, NULL))
+	lease->flags |= LEASE_USED;
+    }
+}
+
 struct dhcp_lease *lease6_find_by_addr(struct in6_addr *net, int prefix, u64 addr)
 {
   struct dhcp_lease *lease;

src/rfc2131.c

@@ -493,8 +493,9 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	      lease_set_interface(lease, int_index, now);
 	      
 	      clear_packet(mess, end);
+	      match_vendor_opts(NULL, daemon->dhcp_opts); /* clear flags */
 	      do_options(context, mess, end, NULL, hostname, get_domain(mess->yiaddr), 
-			 netid, subnet_addr, 0, 0, 0, NULL, 0, now);
+			 netid, subnet_addr, 0, 0, -1, NULL, 0, now);
 	    }
 	}
       

src/rfc3315.c

@@ -497,7 +497,7 @@ static int dhcp6_no_relay(int msg_type, struct in6_addr *link_address, struct dh
 	    ia_option =  opt6_find(opt6_ptr(opt, ia_type == OPTION6_IA_NA ? 12 : 4), ia_end, OPTION6_IAADDR, 24);
 	    
 	    /* reset "USED" flags on leases */
-	    lease6_find(NULL, 0, ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, NULL);
+	    lease6_filter(ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, context);
 	    
 	    o = new_opt6(ia_type);
 	    put_opt6_long(iaid);
@@ -570,7 +570,12 @@ static int dhcp6_no_relay(int msg_type, struct in6_addr *link_address, struct dh
 			  my_syslog(MS_DHCP | LOG_WARNING, _("not using configured address %s because it was previously declined"), 
 				    daemon->addrbuff);
 			else
+			  {
 			    addrp = &config->addr6;
+			    /* may have existing lease for this address */
+			    lease = lease6_find(clid, clid_len, 
+						ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, addrp); 
+			  }
 		      }
 		    
 		    /* existing lease */
@@ -832,7 +837,7 @@ static int dhcp6_no_relay(int msg_type, struct in6_addr *link_address, struct dh
 	    iacntr = save_counter(-1); 
 	    
 	    /* reset "USED" flags on leases */
-	    lease6_find(NULL, 0, ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, NULL);
+	    lease6_filter(ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, context);
 	    
 	    ia_option = opt6_ptr(opt, ia_type == OPTION6_IA_NA ? 12 : 4);
 	    ia_end = opt6_ptr(opt, opt6_len(opt));
@@ -1036,7 +1041,7 @@ static int dhcp6_no_relay(int msg_type, struct in6_addr *link_address, struct dh
 	    ia_option = opt6_ptr(opt, ia_type == OPTION6_IA_NA ? 12 : 4);
 	    
 	    /* reset "USED" flags on leases */
-	    lease6_find(NULL, 0, ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, NULL);
+	    lease6_filter(ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, context);
 		
 	    for (ia_option = opt6_find(ia_option, ia_end, OPTION6_IAADDR, 24);
 		 ia_option;
@@ -1115,7 +1120,7 @@ static int dhcp6_no_relay(int msg_type, struct in6_addr *link_address, struct dh
 	    ia_option = opt6_ptr(opt, ia_type == OPTION6_IA_NA ? 12 : 4);
 	    
 	    /* reset "USED" flags on leases */
-	    lease6_find(NULL, 0, ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, NULL);
+	    lease6_filter(ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA, iaid, context);
 		
 	    for (ia_option = opt6_find(ia_option, ia_end, OPTION6_IAADDR, 24);
 		 ia_option;