[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Bug in DHCP server



I recently used my DHCP server via a relay agent
and discovered that the messages written to /var/log/daemon,
or to the screen in debug mode were incorrect

The message was:
DHCPOFFER on 192.168.1.100 to 00:10:5a:5d:73:0d via 192.168.1.100
and it should have been
DHCPOFFER on 192.168.1.100 to 00:10:5a:5d:73:0d via 192.168.0.31

This problem is only apparent when the DHCP request
comes in with the 'giaddr' non-zero.
The problem is caused by multiple uses of inet_ntoa()
and piaddr() (which internally uses inet_ntoa())
in the note() calls used to generate these messages.

Atttached are suggested fixes.

David Shifflett
*** bootp1.3.c.orig	Tue Apr  1 13:59:10 2003
--- bootp1.3.c	Tue Apr  1 14:14:29 2003
***************
*** 56,65 ****
--- 56,66 ----
  	struct tree_cache *options [256];
  	struct subnet *subnet;
  	struct lease *lease;
  	struct iaddr ip_address;
  	int i;
+ 	char *ipbuf;
  
  	if (packet -> raw -> op != BOOTREQUEST)
  		return;
  
  	note ("BOOTREQUEST from %s via %s%s",
***************
*** 317,328 ****
  	memcpy (hto.haddr, packet -> raw -> chaddr, hto.hlen);
  
  	from = packet -> interface -> primary_address;
  
  	/* Report what we're doing... */
  	note ("BOOTREPLY for %s to %s (%s) via %s",
! 	      piaddr (ip_address), hp -> name,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
--- 318,330 ----
  	memcpy (hto.haddr, packet -> raw -> chaddr, hto.hlen);
  
  	from = packet -> interface -> primary_address;
  
  	/* Report what we're doing... */
+ 	ipbuf = piaddr (ip_address);
  	note ("BOOTREPLY for %s to %s (%s) via %s",
! 	      ipbuf, hp -> name,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
*** dhcp1.8.c.orig	Tue Apr  1 13:59:30 2003
--- dhcp1.8.c	Tue Apr  1 14:21:54 2003
***************
*** 187,196 ****
--- 187,197 ----
  {
  	struct lease *lease;
  	struct iaddr cip;
  	struct subnet *subnet;
  	int ours = 0;
+ 	char *ipbuf;
  
  	if (packet -> options [DHO_DHCP_REQUESTED_ADDRESS].len) {
  		cip.len = 4;
  		memcpy (cip.iabuf,
  			packet -> options [DHO_DHCP_REQUESTED_ADDRESS].data,
***************
*** 207,218 ****
  	if (subnet)
  		lease = find_lease (packet, subnet -> shared_network, &ours);
  	else
  		lease = (struct lease *)0;
  
  	note ("DHCPREQUEST for %s from %s via %s",
! 	      piaddr (cip),
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
--- 208,220 ----
  	if (subnet)
  		lease = find_lease (packet, subnet -> shared_network, &ours);
  	else
  		lease = (struct lease *)0;
  
+ 	ipbuf = piaddr (cip);
  	note ("DHCPREQUEST for %s from %s via %s",
! 	      ipbuf,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 361,370 ****
--- 363,373 ----
  	struct packet *packet;
  {
  	struct lease *lease;
  	struct iaddr cip;
  	int i;
+ 	char ipbuf[32];
  
  	/* DHCPRELEASE must not specify address in requested-address
             option, but old protocol specs weren't explicit about this,
             so let it go. */
  	if (packet -> options [DHO_DHCP_REQUESTED_ADDRESS].len) {
***************
*** 394,405 ****
  		cip.len = 4;
  		memcpy (cip.iabuf, &packet -> raw -> ciaddr, 4);
  		lease = find_lease_by_ip_addr (cip);
  	}
  
  	note ("DHCPRELEASE of %s from %s via %s (%sfound)",
! 	      inet_ntoa (packet -> raw -> ciaddr),
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
--- 397,410 ----
  		cip.len = 4;
  		memcpy (cip.iabuf, &packet -> raw -> ciaddr, 4);
  		lease = find_lease_by_ip_addr (cip);
  	}
  
+ 	strlcpy (ipbuf, inet_ntoa(packet -> raw -> ciaddr),
+ 		sizeof(ipbuf));
  	note ("DHCPRELEASE of %s from %s via %s (%sfound)",
! 	      ipbuf,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 413,424 ****
  		 * this avoids the problem of spoofed releases
  		 * being used to liberate addresses from the
  		 * server.
  		 */
  		if (! lease->releasing) {
  			note ("DHCPRELEASE of %s from %s via %s (found)",
! 			      inet_ntoa (packet -> raw -> ciaddr),
  			      print_hw_addr (packet -> raw -> htype,
  					     packet -> raw -> hlen,
  					     packet -> raw -> chaddr),
  			      packet -> raw -> giaddr.s_addr
  			      ? inet_ntoa (packet -> raw -> giaddr)
--- 418,431 ----
  		 * this avoids the problem of spoofed releases
  		 * being used to liberate addresses from the
  		 * server.
  		 */
  		if (! lease->releasing) {
+ 			strlcpy (ipbuf, inet_ntoa(packet -> raw -> ciaddr),
+ 				sizeof(ipbuf));
  			note ("DHCPRELEASE of %s from %s via %s (found)",
! 			      ipbuf,
  			      print_hw_addr (packet -> raw -> htype,
  					     packet -> raw -> hlen,
  					     packet -> raw -> chaddr),
  			      packet -> raw -> giaddr.s_addr
  			      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 428,450 ****
  			add_timeout (cur_time + 1, lease_ping_timeout, lease);
  			icmp_echorequest (&(lease -> ip_addr));
  			++outstanding_pings;
  		}
  		else {
  			note ("DHCPRELEASE of %s from %s via %s ignored (release already pending)",
! 			      inet_ntoa (packet -> raw -> ciaddr),
  			      print_hw_addr (packet -> raw -> htype,
  					     packet -> raw -> hlen,
  					     packet -> raw -> chaddr),
  			      packet -> raw -> giaddr.s_addr
  			      ? inet_ntoa (packet -> raw -> giaddr)
  			      : packet -> interface -> name);
  		}
  	}
  	else {
  		note ("DHCPRELEASE of %s from %s via %s for nonexistent lease",
! 		      inet_ntoa (packet -> raw -> ciaddr),
  		      print_hw_addr (packet -> raw -> htype,
  				     packet -> raw -> hlen,
  				     packet -> raw -> chaddr),
  		      packet -> raw -> giaddr.s_addr
  		      ? inet_ntoa (packet -> raw -> giaddr)
--- 435,461 ----
  			add_timeout (cur_time + 1, lease_ping_timeout, lease);
  			icmp_echorequest (&(lease -> ip_addr));
  			++outstanding_pings;
  		}
  		else {
+ 			strlcpy (ipbuf, inet_ntoa(packet -> raw -> ciaddr),
+ 				sizeof(ipbuf));
  			note ("DHCPRELEASE of %s from %s via %s ignored (release already pending)",
! 			      ipbuf,
  			      print_hw_addr (packet -> raw -> htype,
  					     packet -> raw -> hlen,
  					     packet -> raw -> chaddr),
  			      packet -> raw -> giaddr.s_addr
  			      ? inet_ntoa (packet -> raw -> giaddr)
  			      : packet -> interface -> name);
  		}
  	}
  	else {
+ 		strlcpy (ipbuf, inet_ntoa(packet -> raw -> ciaddr),
+ 			sizeof(ipbuf));
  		note ("DHCPRELEASE of %s from %s via %s for nonexistent lease",
! 		      ipbuf,
  		      print_hw_addr (packet -> raw -> htype,
  				     packet -> raw -> hlen,
  				     packet -> raw -> chaddr),
  		      packet -> raw -> giaddr.s_addr
  		      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 455,464 ****
--- 466,476 ----
  void dhcpdecline (packet)
  	struct packet *packet;
  {
  	struct lease *lease;
  	struct iaddr cip;
+ 	char *ipbuf;
  
  	/* DHCPDECLINE must specify address. */
  	if (!packet -> options [DHO_DHCP_REQUESTED_ADDRESS].len) {
  		return;
  	}
***************
*** 466,477 ****
  	cip.len = 4;
  	memcpy (cip.iabuf,
  		packet -> options [DHO_DHCP_REQUESTED_ADDRESS].data, 4);
  	lease = find_lease_by_ip_addr (cip);
  
  	note ("DHCPDECLINE on %s from %s via %s",
! 	      piaddr (cip),
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
--- 478,490 ----
  	cip.len = 4;
  	memcpy (cip.iabuf,
  		packet -> options [DHO_DHCP_REQUESTED_ADDRESS].data, 4);
  	lease = find_lease_by_ip_addr (cip);
  
+ 	ipbuf = piaddr (cip);
  	note ("DHCPDECLINE on %s from %s via %s",
! 	      ipbuf,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 500,509 ****
--- 513,523 ----
  	struct dhcp_packet raw;
  	unsigned char nak = DHCPNAK;
  	struct packet outgoing;
  	struct hardware hto;
  	int i;
+ 	char *ipbuf;
  
  	struct tree_cache *options [256];
  	struct tree_cache dhcpnak_tree;
  	struct tree_cache dhcpmsg_tree;
  
***************
*** 553,564 ****
  	raw.flags = packet -> raw -> flags | htons (BOOTP_BROADCAST);
  	raw.hops = packet -> raw -> hops;
  	raw.op = BOOTREPLY;
  
  	/* Report what we're sending... */
  	note ("DHCPNAK on %s to %s via %s",
! 	      piaddr (*cip),
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
--- 567,579 ----
  	raw.flags = packet -> raw -> flags | htons (BOOTP_BROADCAST);
  	raw.hops = packet -> raw -> hops;
  	raw.op = BOOTREPLY;
  
  	/* Report what we're sending... */
+ 	ipbuf = piaddr (*cip);
  	note ("DHCPNAK on %s to %s via %s",
! 	      ipbuf,
  	      print_hw_addr (packet -> raw -> htype,
  			     packet -> raw -> hlen,
  			     packet -> raw -> chaddr),
  	      packet -> raw -> giaddr.s_addr
  	      ? inet_ntoa (packet -> raw -> giaddr)
***************
*** 1177,1186 ****
--- 1192,1202 ----
  	int i;
  	struct lease_state *state = lease -> state;
  	int nulltp, bootpp;
  	u_int8_t *prl;
  	int prl_len;
+ 	char *ipbuf;
  
  	if (!state)
  		error ("dhcp_reply was supplied lease with no state!");
  
  	/* Compose a response for the client... */
***************
*** 1268,1282 ****
  	raw.flags = state -> bootp_flags;
  	raw.hops = state -> hops;
  	raw.op = BOOTREPLY;
  
  	/* Say what we're doing... */
  	note ("%s on %s to %s via %s",
  	      (state -> offer
  	       ? (state -> offer == DHCPACK ? "DHCPACK" : "DHCPOFFER")
  	       : "BOOTREPLY"),
! 	      piaddr (lease -> ip_addr),
  	      print_hw_addr (lease -> hardware_addr.htype,
  			     lease -> hardware_addr.hlen,
  			     lease -> hardware_addr.haddr),
  	      state -> giaddr.s_addr
  	      ? inet_ntoa (state -> giaddr)
--- 1284,1299 ----
  	raw.flags = state -> bootp_flags;
  	raw.hops = state -> hops;
  	raw.op = BOOTREPLY;
  
  	/* Say what we're doing... */
+ 	ipbuf = piaddr (lease -> ip_addr);
  	note ("%s on %s to %s via %s",
  	      (state -> offer
  	       ? (state -> offer == DHCPACK ? "DHCPACK" : "DHCPOFFER")
  	       : "BOOTREPLY"),
! 	      ipbuf,
  	      print_hw_addr (lease -> hardware_addr.htype,
  			     lease -> hardware_addr.hlen,
  			     lease -> hardware_addr.haddr),
  	      state -> giaddr.s_addr
  	      ? inet_ntoa (state -> giaddr)



Visit your host, monkey.org