bin/128605: dhclient - security issue

[Raffaele De Lorenzo]

>Number:         128605
>Category:       bin
>Synopsis:       dhclient - security issue
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Nov 05 12:10:03 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Raffaele De Lorenzo
>Release:        FreeBSD 7.0
>Organization:
>Environment:
FreeBSD noel 7.0-RELEASE-p3 FreeBSD 7.0-RELEASE-p3 #15: Tue Sep  2 11:16:54 CEST 2008     root at noel:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
This is a security problem that I founded in the dhclient program. If you configure a dhcp-server (I tested it with "isc-dhcp30-server" from ports) with a particular Server Id ("option dhcp-server-identifier" in "dhcpd.conf") as "localhost" (127.0.0.1) the DHCP response (ack) packet will be sent from server to client identified by the localhost ip address. In this situation the client doesn't check the server identifier and always accepts the "ack" messages. 

RFC2131 needs a valid IP address as Server Identifier: (from RFC2131: "The 'server identifier' field is used both to identify a DHCP server in a DHCP message and as a destination address from clients to servers.  A server with multiple network addresses MUST be prepared to to accept any of its network addresses as identifying that server in a DHCP message.  To accommodate potentially incomplete network connectivity, a server MUST choose an address as a 'server identifier' that, to the best of the server's knowledge, is reachablefrom the client.")

I saw the dhcp client made by Darwin (Leopard), Windows Vista and Darwin Iphone OS respect the RFC2131 "server-identifier" and they rejected all "ack" messages identified by "127.0.0.1"; in reply FreeBSD 7, Linux and Windows XP/2000 doesn't respect it.  I couldn't test it with other OSs.
>How-To-Repeat:
If you use the "isc-dhcp30-server" from port:

dhcp server (A):
1) insert inside "dhcpd.conf" a server identifier like this:
    option dhcp-server-identifier foo

    you must insert this line in your /etc/hosts
    127.0.0.1    foo

client (B)
2) run dhclient <interface to "A">

>Fix:
I patched the dhclient to solve this problem. 

Patch attached with submission follows:

--- /usr/src/sbin/dhclient/dhclient.c	2007-02-09 18:50:26.000000000 +0100
+++ ./dhclient.c	2008-11-04 17:15:04.000000000 +0100
@@ -629,13 +629,15 @@
 {
 	struct interface_info *ip = packet->interface;
 	struct client_lease *lease;
+	struct in_addr addr;
 
 	/* If we're not receptive to an offer right now, or if the offer
 	   has an unrecognizable transaction id, then just drop it. */
 	if (packet->interface->client->xid != packet->raw->xid ||
 	    (packet->interface->hw_address.hlen != packet->raw->hlen) ||
 	    (memcmp(packet->interface->hw_address.haddr,
-	    packet->raw->chaddr, packet->raw->hlen)))
+	    packet->raw->chaddr, packet->raw->hlen))
+	   )
 		return;
 
 	if (ip->client->state != S_REBOOTING &&
@@ -654,6 +656,26 @@
 
 	ip->client->new = lease;
 
+	/* 
+	 * October 30 2008 - Raffaele De Lorenzo (raffaele.delorenzo at libero.it)
+	 * Security check - Check if the DHCP server identifier (ip address) match 
+	 * with localhost (127.0.0.0/8). RFC 2131 required a valid server identifier 
+	 * ip address.
+	 *
+	 */
+	bzero (&addr, sizeof (struct in_addr));
+	if (inet_aton (piaddr(packet->client_addr), &addr) != 1){
+		fprintf (stderr, "ERROR - dhcpack - Server address unreadable\n");
+		return;
+	}
+	/* apply mask at address */
+	addr.s_addr &= 0x000000FF;
+	if (addr.s_addr == 0x00007F){
+		fprintf (stderr, "ERROR - dhcpack - Server ID [%s] not valid (localhost)\n",
+			 piaddr(packet->client_addr));
+		return;
+	}
+
 	/* Stop resending DHCPREQUEST. */
 	cancel_timeout(send_request, ip);
 
@@ -838,12 +860,16 @@
 	    "DHCPOFFER" : "BOOTREPLY";
 
 	/* If we're not receptive to an offer right now, or if the offer
-	   has an unrecognizable transaction id, then just drop it. */
+	   has an unrecognizable transaction id, then just drop it. 
+	*/
+	
+
 	if (ip->client->state != S_SELECTING ||
 	    packet->interface->client->xid != packet->raw->xid ||
 	    (packet->interface->hw_address.hlen != packet->raw->hlen) ||
 	    (memcmp(packet->interface->hw_address.haddr,
-	    packet->raw->chaddr, packet->raw->hlen)))
+	    packet->raw->chaddr, packet->raw->hlen))
+	  )
 		return;
 
 	note("%s from %s", name, piaddr(packet->client_addr));


>Release-Note:
>Audit-Trail:
>Unformatted: