DHCPD stopping, my fix... for 4.1.0

David W. Hankins dhankins at isc.org
Thu Jan 14 23:59:07 UTC 2010


On Tue, Jan 12, 2010 at 12:56:02PM -0800, Friesen, Don SSBC:EX wrote:
> --- server/dhcp.c.ORIGINAL	Thu Jan  7 15:01:33 2010
> +++ server/dhcp.c	Tue Jan 12 11:46:13 2010
> @@ -1381,6 +1381,7 @@
>  	if (packet->interface->address_count)
>  		raw.siaddr = packet->interface->addresses[0];
>  	raw.giaddr = packet -> raw -> giaddr;
> +	raw.ciaddr = packet -> raw -> ciaddr;
>  	memcpy (raw.chaddr, packet -> raw -> chaddr, sizeof raw.chaddr);
>  	raw.hlen = packet -> raw -> hlen;
>  	raw.htype = packet -> raw -> htype;

This non-confirms with RFC 2131 section 4.3.1 table 3, which specifies
a zero value for the 'ciaddr' field.

> @@ -1387,7 +1388,7 @@
>  
>  	raw.xid = packet -> raw -> xid;
>  	raw.secs = packet -> raw -> secs;
> -	raw.flags = packet -> raw -> flags | htons (BOOTP_BROADCAST);
> +	raw.flags = packet -> raw -> flags;
>  	raw.hops = packet -> raw -> hops;
>  	raw.op = BOOTREPLY;
>  
> @@ -1428,10 +1429,10 @@
>       if (outgoing.packet_length < BOOTP_MIN_LEN)
>               outgoing.packet_length = BOOTP_MIN_LEN;
>  
> -     /* If this was gatewayed, send it back to the gateway.
> -        Otherwise, broadcast it on the local network. */
> +     /* If this was gatewayed, send it back to the gateway. */
>       if (raw.giaddr.s_addr) {
>               to.sin_addr = raw.giaddr;
> +             raw.flags |= htons (BOOTP_BROADCAST);
>               if (raw.giaddr.s_addr != htonl (INADDR_LOOPBACK))
>                       to.sin_port = local_port;
>               else

This conforms to RFC 2131 section 4.3.1 table 3, which specifies the
use of the flags field from the client without change.

RFC 2131 could be considered inconsistent on this point however,
because it often directs, as quoted in section 3.2 and 4.1 for example
that;

   ... In
   all cases, when 'giaddr' is zero, the server broadcasts any DHCPNAK
   messages to 0xffffffff.

And similar text.

Meanwhile, a conforming BOOTP relay agent that forwards a reply to the
client with a flags field not containing the broadcast flag may very
well unicast the DHCPNAK directly to the client, which would be
different behaviour than if the client is directly connected to the
server.

The difference in behaviour when a DHCP server is coresident on a
broadcast domain (where it can broadcast a reply no matter the flags
fields), and operating with a BOOTP relay agent inbetween is a curious
inconsistency in the DHCP protocol specification, but applying the
broadcast flag resolves the disparity in a general way that does not
seem to be harmful.

> @@ -1444,13 +1445,27 @@
>  					      from, &to, &hto);
>  			return;
>  		}
> +
> +	   /* If it comes from a client that already knows its address
> +	      and is not requesting a broadcast response, and we can
> +	      unicast to a client without using the ARP protocol, sent
> it
> +	      directly to that client. */
> +	} else if (raw.ciaddr.s_addr &&
> +         		can_unicast_without_arp (packet -> interface)) {
> +	      to.sin_addr = raw.ciaddr;
> +	      to.sin_port = remote_port;
> +
> +	/* Otherwise, broadcast it on the local network. */
>  	} else {
> +	        raw.flags |= htons (BOOTP_BROADCAST);
>  		to.sin_addr = limited_broadcast;
>  		to.sin_port = remote_port;
>  	}
>  
>  	errno = 0;
> -	result = send_packet (packet -> interface,
> +	result = send_packet ((fallback_interface
> +		      		? fallback_interface : packet ->
> interface),
>  			      packet, &raw, outgoing.packet_length,
>  			      from, &to, (struct hardware *)0);
>  }

This non-conforms to various wording in RFC 2131 sections 3.2 and
4.1 as I've quoted above.  Further, it is provably broken when
processing directly attached clients in the INIT-REBOOT or REBINDING
states, as it may (will in INIT-REBOOT's case) attempt a routed
unicast to a client that is not on the network.

Further, by using the fallback interface in the zeroed ciaddr case,
it depends upon local system implementation but may either attempt to
unicast a reply to the all-ones limited broadcast address (a unicast
to 255.255.255.255 via its default gateway), or at worst on better
performing platforms it will send a broadcast response to this
address; but may very well send it on the wrong interface (not the one
the client is on).

I do not recommend applying this patch.

>     My supervisor hopes that the unicast NAK can make it into production
> code, so we can avoid having to work this fix in with each release we
> upgrade to.  The DHCPRequest packet this services looks like:

Unfortunately, by gazing into a DHCP packet header and contents, there
is no way to discriminate between a RENEWING or REBINDING client, and
consequently there is no way to know if the client is directly or
remotely connected.  To do this reliably we have to investigate the
IP destination address the client transmitted the packet to, in
addition to knowing if that address is or is not an address that
corresponds to the local server (as it is got via a raw socket, it
may e.g. be a client in RENEWING state trying to unicast to a
different server, but either finding the local server's interface
either due to matching MAC addresses, or ethernet flooding while the
promiscuous bit is set, or similar).

It gets complicated.  We have the capability to perform this with
some of our interface API's, but not all of them reliably, last I
checked, and anyway new code to perform the detection would need to
be written.

The worst case, presently, is that remote clients in the RENEWING
state will not receive either DHCPNAKs or DHCPACKs, and will move
instead to the REBINDING state as timers allow, which will get to the
DHCP server via a relay agent, facilitating the reply.


Curiously, RFC 2131 makes no mention in any of its bulk of how to
appropriately respond to a DHCPREQUEST in the RENEWING state with a
DHCPNAK.  This is left entirely out of RFC 2131's consciousness.  But
we have seen servers that can perform this action, and we have seen
clients that respond appropriately to its reception.

So it is worth considering instrumenting our socket handling layers
so that we can discriminate between RENEWING and REBINDING clients and
provide unicast DHCPNAKs to RENEWING clients specifically.

-- 
David W. Hankins	BIND 10 needs more DHCP voices.
Software Engineer		There just aren't enough in our heads.
Internet Systems Consortium, Inc.		http://bind10.isc.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <https://lists.isc.org/pipermail/dhcp-users/attachments/20100114/71c8a44c/attachment.bin>


More information about the dhcp-users mailing list