udhcpd: untangle incredibly messy handling of DHCPREQUEST

Also fixes attacks possible via DHCPDECLINE / DHCPRELEASE

function                                             old     new   delta
udhcpd_main                                         1846    1949    +103
send_renew                                           105     142     +37
send_NAK                                              61       -     -61
send_ACK                                             180       -    -180
------------------------------------------------------------------------------
(add/remove: 0/2 grow/shrink: 2/0 up/down: 140/-241)         Total: -101 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

9 files changed, 267 insertions, 184 deletions

diff --git a/networking/udhcp/clientpacket.c b/networking/udhcp/clientpacket.c
index a255d6e84..a0be42885 100644
--- a/networking/udhcp/clientpacket.c
+++ b/networking/udhcp/clientpacket.c
@@ -109,24 +109,6 @@ static int raw_bcast_from_client_config_ifindex(struct dhcp_packet *packet)
 }
 
 
-#if ENABLE_FEATURE_UDHCPC_ARPING
-/* Broadcast a DHCP decline message */
-int FAST_FUNC send_decline(uint32_t xid, uint32_t server, uint32_t requested)
-{
-	struct dhcp_packet packet;
-
-	init_packet(&packet, DHCPDECLINE);
-	packet.xid = xid;
-	add_simple_option(packet.options, DHCP_REQUESTED_IP, requested);
-	add_simple_option(packet.options, DHCP_SERVER_ID, server);
-
-	bb_info_msg("Sending decline...");
-
-	return raw_bcast_from_client_config_ifindex(&packet);
-}
-#endif
-
-
 /* Broadcast a DHCP discover packet to the network, with an optionally requested IP */
 int FAST_FUNC send_discover(uint32_t xid, uint32_t requested)
 {
@@ -136,11 +118,9 @@ int FAST_FUNC send_discover(uint32_t xid, uint32_t requested)
 	packet.xid = xid;
 	if (requested)
 		add_simple_option(packet.options, DHCP_REQUESTED_IP, requested);
-
 	/* Explicitly saying that we want RFC-compliant packets helps
 	 * some buggy DHCP servers to NOT send bigger packets */
 	add_simple_option(packet.options, DHCP_MAX_SIZE, htons(576));
-
 	add_param_req_option(&packet);
 
 	bb_info_msg("Sending discover...");
@@ -159,7 +139,6 @@ int FAST_FUNC send_select(uint32_t xid, uint32_t server, uint32_t requested)
 
 	init_packet(&packet, DHCPREQUEST);
 	packet.xid = xid;
-
 	add_simple_option(packet.options, DHCP_REQUESTED_IP, requested);
 	add_simple_option(packet.options, DHCP_SERVER_ID, server);
 	add_param_req_option(&packet);
@@ -177,17 +156,46 @@ int FAST_FUNC send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
 
 	init_packet(&packet, DHCPREQUEST);
 	packet.xid = xid;
+	/* RFC 2131:
+	 * "3.2 Client-server interaction - reusing a previously
+	 * allocated network address"...
+	 * The client broadcasts a DHCPREQUEST message on its local subnet.
+	 * The message includes the client's network address in the
+	 * REQUESTED_IP option. As the client has not received its
+	 * network address, it MUST NOT fill in the 'ciaddr' field."
+	 *
+	 * FIXME: we seem to not follow this, we do set ciaddr.
+	 */
 	packet.ciaddr = ciaddr;
-
+	add_simple_option(packet.options, DHCP_REQUESTED_IP, ciaddr);
+	if (server)
+		add_simple_option(packet.options, DHCP_SERVER_ID, server);
 	add_param_req_option(&packet);
+
 	bb_info_msg("Sending renew...");
 	if (server)
 		return udhcp_send_kernel_packet(&packet,
 			ciaddr, CLIENT_PORT,
 			server, SERVER_PORT);
+	return raw_bcast_from_client_config_ifindex(&packet);
+}
+
+
+#if ENABLE_FEATURE_UDHCPC_ARPING
+/* Broadcast a DHCP decline message */
+int FAST_FUNC send_decline(uint32_t xid, uint32_t server, uint32_t requested)
+{
+	struct dhcp_packet packet;
+
+	init_packet(&packet, DHCPDECLINE);
+	packet.xid = xid;
+	add_simple_option(packet.options, DHCP_REQUESTED_IP, requested);
+	add_simple_option(packet.options, DHCP_SERVER_ID, server);
 
+	bb_info_msg("Sending decline...");
 	return raw_bcast_from_client_config_ifindex(&packet);
 }
+#endif
 
 
 /* Unicast a DHCP release message */
diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h
index d7c874e5b..cf3e0cd39 100644
--- a/networking/udhcp/common.h
+++ b/networking/udhcp/common.h
@@ -21,11 +21,17 @@ extern const uint8_t MAC_BCAST_ADDR[6]; /* six all-ones */
 
 /*** packet.h ***/
 
-#define DHCP_OPTIONS_BUFSIZE  308
+/* DHCP protocol. See RFC 2131 */
+#define DHCP_MAGIC              0x63825363
+
+#define DHCP_OPTIONS_BUFSIZE    308
+
+#define BOOTREQUEST             1
+#define BOOTREPLY               2
 
 //TODO: rename ciaddr/yiaddr/chaddr
 struct dhcp_packet {
-	uint8_t op;      /* 1 = BOOTREQUEST, 2 = BOOTREPLY */
+	uint8_t op;      /* BOOTREQUEST or BOOTREPLY */
 	uint8_t htype;   /* hardware address type. 1 = 10mb ethernet */
 	uint8_t hlen;    /* hardware address length */
 	uint8_t hops;    /* used by relay agents only */
@@ -61,6 +67,71 @@ struct BUG_bad_sizeof_struct_ip_udp_dhcp_packet {
 		[(sizeof(struct ip_udp_dhcp_packet) != 576 + CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS) ? -1 : 1];
 };
 
+// RFC 2131  Table 5: Fields and options used by DHCP clients
+//
+// Field      DHCPDISCOVER          DHCPREQUEST           DHCPDECLINE,
+//            DHCPINFORM                                  DHCPRELEASE
+// -----      ------------          -----------           -----------
+// 'op'       BOOTREQUEST           BOOTREQUEST           BOOTREQUEST
+// 'hops'     0                     0                     0
+// 'xid'      selected by client    'xid' from server     selected by
+//                                  DHCPOFFER message     client
+// 'secs'     0 or seconds since    0 or seconds since    0
+//            DHCP process started  DHCP process started
+// 'flags'    Set 'BROADCAST'       Set 'BROADCAST'       0
+//            flag if client        flag if client
+//            requires broadcast    requires broadcast
+//            reply                 reply
+// 'ciaddr'   0 (DHCPDISCOVER)      0 or client's         0 (DHCPDECLINE)
+//            client's              network address       client's network
+//            network address       (BOUND/RENEW/REBIND)  address
+//            (DHCPINFORM)                                (DHCPRELEASE)
+// 'yiaddr'   0                     0                     0
+// 'siaddr'   0                     0                     0
+// 'giaddr'   0                     0                     0
+// 'chaddr'   client's hardware     client's hardware     client's hardware
+//            address               address               address
+// 'sname'    options, if           options, if           (unused)
+//            indicated in          indicated in
+//            'sname/file'          'sname/file'
+//            option; otherwise     option; otherwise
+//            unused                unused
+// 'file'     options, if           options, if           (unused)
+//            indicated in          indicated in
+//            'sname/file'          'sname/file'
+//            option; otherwise     option; otherwise
+//            unused                unused
+// 'options'  options               options               (unused)
+//
+// Option                     DHCPDISCOVER  DHCPREQUEST      DHCPDECLINE,
+//                            DHCPINFORM                     DHCPRELEASE
+// ------                     ------------  -----------      -----------
+// Requested IP address       MAY           MUST (in         MUST
+//                            (DISCOVER)    SELECTING or     (DHCPDECLINE),
+//                            MUST NOT      INIT-REBOOT)     MUST NOT
+//                            (INFORM)      MUST NOT (in     (DHCPRELEASE)
+//                                          BOUND or
+//                                          RENEWING)
+// IP address lease time      MAY           MAY              MUST NOT
+//                            (DISCOVER)
+//                            MUST NOT
+//                            (INFORM)
+// Use 'file'/'sname' fields  MAY           MAY              MAY
+// Client identifier          MAY           MAY              MAY
+// Vendor class identifier    MAY           MAY              MUST NOT
+// Server identifier          MUST NOT      MUST (after      MUST
+//                                          SELECTING)
+//                                          MUST NOT (after
+//                                          INIT-REBOOT,
+//                                          BOUND, RENEWING
+//                                          or REBINDING)
+// Parameter request list     MAY           MAY              MUST NOT
+// Maximum message size       MAY           MAY              MUST NOT
+// Message                    SHOULD NOT    SHOULD NOT       SHOULD
+// Site-specific              MAY           MAY              MUST NOT
+// All others                 MAY           MAY              MUST NOT
+
+
 uint16_t udhcp_checksum(void *addr, int count) FAST_FUNC;
 
 void udhcp_init_header(struct dhcp_packet *packet, char type) FAST_FUNC;
diff --git a/networking/udhcp/dhcpc.h b/networking/udhcp/dhcpc.h
index dfab2368c..c9827b6e7 100644
--- a/networking/udhcp/dhcpc.h
+++ b/networking/udhcp/dhcpc.h
@@ -40,7 +40,6 @@ int send_select(uint32_t xid, uint32_t server, uint32_t requested) FAST_FUNC;
 int send_decline(uint32_t xid, uint32_t server, uint32_t requested) FAST_FUNC;
 #endif
 int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) FAST_FUNC;
-int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) FAST_FUNC;
 int send_release(uint32_t server, uint32_t ciaddr) FAST_FUNC;
 
 int udhcp_recv_raw_packet(struct dhcp_packet *dhcp_pkt, int fd) FAST_FUNC;
diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c
index f594bad66..a529e1b58 100644
--- a/networking/udhcp/dhcpd.c
+++ b/networking/udhcp/dhcpd.c
@@ -28,18 +28,8 @@
 #include "options.h"
 
 
-/* send a packet to gateway_nip using the kernel ip stack */
-static int send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
-{
-	log1("Forwarding packet to relay");
-
-	return udhcp_send_kernel_packet(dhcp_pkt,
-			server_config.server_nip, SERVER_PORT,
-			dhcp_pkt->gateway_nip, SERVER_PORT);
-}
-
-/* send a packet to a specific mac address and ip address by creating our own ip packet */
-static int send_packet_to_client(struct dhcp_packet *dhcp_pkt, int force_broadcast)
+/* Send a packet to a specific mac address and ip address by creating our own ip packet */
+static void send_packet_to_client(struct dhcp_packet *dhcp_pkt, int force_broadcast)
 {
 	const uint8_t *chaddr;
 	uint32_t ciaddr;
@@ -69,25 +59,36 @@ static int send_packet_to_client(struct dhcp_packet *dhcp_pkt, int force_broadca
 		chaddr = dhcp_pkt->chaddr;
 	}
 
-	return udhcp_send_raw_packet(dhcp_pkt,
+	udhcp_send_raw_packet(dhcp_pkt,
 		/*src*/ server_config.server_nip, SERVER_PORT,
 		/*dst*/ ciaddr, CLIENT_PORT, chaddr,
 		server_config.ifindex);
 }
 
-/* Send the dhcp packet.
- * If force broadcast is set, the packet will be broadcast.
- */
-static int send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
+/* Send a packet to gateway_nip using the kernel ip stack */
+static void send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
+{
+	log1("Forwarding packet to relay");
+
+	udhcp_send_kernel_packet(dhcp_pkt,
+			server_config.server_nip, SERVER_PORT,
+			dhcp_pkt->gateway_nip, SERVER_PORT);
+}
+
+static void send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
 {
 	if (dhcp_pkt->gateway_nip)
-		return send_packet_to_relay(dhcp_pkt);
-	return send_packet_to_client(dhcp_pkt, force_broadcast);
+		send_packet_to_relay(dhcp_pkt);
+	else
+		send_packet_to_client(dhcp_pkt, force_broadcast);
 }
 
 static void init_packet(struct dhcp_packet *packet, struct dhcp_packet *oldpacket, char type)
 {
+	/* Sets op, htype, hlen, cookie fields
+	 * and adds DHCP_MESSAGE_TYPE option */
 	udhcp_init_header(packet, type);
+
 	packet->xid = oldpacket->xid;
 	memcpy(packet->chaddr, oldpacket->chaddr, sizeof(oldpacket->chaddr));
 	packet->flags = oldpacket->flags;
@@ -132,26 +133,32 @@ static uint32_t select_lease_time(struct dhcp_packet *packet)
 	return lease_time_sec;
 }
 
-/* send a DHCP OFFER to a DHCP DISCOVER */
-static int send_offer(struct dhcp_packet *oldpacket, uint32_t static_lease_nip, struct dyn_lease *lease)
+/* We got a DHCP DISCOVER. Send an OFFER. */
+static void send_offer(struct dhcp_packet *oldpacket, uint32_t static_lease_nip, struct dyn_lease *lease)
 {
 	struct dhcp_packet packet;
-	uint32_t lease_time_sec = server_config.max_lease_sec;
-	const char *p_host_name;
+	uint32_t lease_time_sec;
 	struct in_addr addr;
 
 	init_packet(&packet, oldpacket, DHCPOFFER);
 
-	/* ADDME: if static, short circuit */
+	/* If it is a static lease, use its IP */
+	packet.yiaddr = static_lease_nip;
+	/* Else: */
 	if (!static_lease_nip) {
+		/* We have no static lease for client's chaddr */
 		uint32_t req_nip;
 		uint8_t *req_ip_opt;
+		const char *p_host_name;
 
-		/* The client is in our lease/offered table */
 		if (lease) {
+			/* We have a dynamic lease for client's chaddr.
+			 * Reuse its IP (even if lease is expired).
+			 * Note that we ignore requested IP in this case.
+			 */
 			packet.yiaddr = lease->lease_nip;
 		}
-		/* Or the client has requested an IP */
+		/* Or: if client has requested an IP */
 		else if ((req_ip_opt = get_option(oldpacket, DHCP_REQUESTED_IP)) != NULL
 		 /* (read IP) */
 		 && (move_from_unaligned32(req_nip, req_ip_opt), 1)
@@ -165,50 +172,49 @@ static int send_offer(struct dhcp_packet *oldpacket, uint32_t static_lease_nip,
 		) {
 			packet.yiaddr = req_nip;
 		}
-		/* Otherwise, find a free IP */
 		else {
+			/* Otherwise, find a free IP */
 			packet.yiaddr = find_free_or_expired_nip(oldpacket->chaddr);
 		}
 
 		if (!packet.yiaddr) {
-			bb_error_msg("no IP addresses to give - OFFER abandoned");
-			return -1;
+			bb_error_msg("no free IP addresses. OFFER abandoned");
+			return;
 		}
+		/* Reserve the IP for a short time hoping to get DHCPREQUEST soon */
 		p_host_name = (const char*) get_option(oldpacket, DHCP_HOST_NAME);
-		if (add_lease(packet.chaddr, packet.yiaddr,
+		lease = add_lease(packet.chaddr, packet.yiaddr,
 				server_config.offer_time,
 				p_host_name,
 				p_host_name ? (unsigned char)p_host_name[OPT_LEN - OPT_DATA] : 0
-			) == 0
-		) {
-			bb_error_msg("lease pool is full - OFFER abandoned");
-			return -1;
+		);
+		if (!lease) {
+			bb_error_msg("no free IP addresses. OFFER abandoned");
+			return;
 		}
-		lease_time_sec = select_lease_time(oldpacket);
-	} else {
-		/* It is a static lease... use it */
-		packet.yiaddr = static_lease_nip;
 	}
 
+	lease_time_sec = select_lease_time(oldpacket);
 	add_simple_option(packet.options, DHCP_LEASE_TIME, htonl(lease_time_sec));
 	add_server_options(&packet);
 
 	addr.s_addr = packet.yiaddr;
 	bb_info_msg("Sending OFFER of %s", inet_ntoa(addr));
-	return send_packet(&packet, /*force_bcast:*/ 0);
+	/* send_packet emits error message itself if it detects failure */
+	send_packet(&packet, /*force_bcast:*/ 0);
 }
 
-static int send_NAK(struct dhcp_packet *oldpacket)
+static void send_NAK(struct dhcp_packet *oldpacket)
 {
 	struct dhcp_packet packet;
 
 	init_packet(&packet, oldpacket, DHCPNAK);
 
 	log1("Sending NAK");
-	return send_packet(&packet, /*force_bcast:*/ 1);
+	send_packet(&packet, /*force_bcast:*/ 1);
 }
 
-static int send_ACK(struct dhcp_packet *oldpacket, uint32_t yiaddr)
+static void send_ACK(struct dhcp_packet *oldpacket, uint32_t yiaddr)
 {
 	struct dhcp_packet packet;
 	uint32_t lease_time_sec;
@@ -219,15 +225,13 @@ static int send_ACK(struct dhcp_packet *oldpacket, uint32_t yiaddr)
 	packet.yiaddr = yiaddr;
 
 	lease_time_sec = select_lease_time(oldpacket);
-
 	add_simple_option(packet.options, DHCP_LEASE_TIME, htonl(lease_time_sec));
+
 	add_server_options(&packet);
 
-	addr.s_addr = packet.yiaddr;
+	addr.s_addr = yiaddr;
 	bb_info_msg("Sending ACK to %s", inet_ntoa(addr));
-
-	if (send_packet(&packet, /*force_bcast:*/ 0) < 0)
-		return -1;
+	send_packet(&packet, /*force_bcast:*/ 0);
 
 	p_host_name = (const char*) get_option(oldpacket, DHCP_HOST_NAME);
 	add_lease(packet.chaddr, packet.yiaddr,
@@ -239,18 +243,21 @@ static int send_ACK(struct dhcp_packet *oldpacket, uint32_t yiaddr)
 		/* rewrite the file with leases at every new acceptance */
 		write_leases();
 	}
-
-	return 0;
 }
 
-static int send_inform(struct dhcp_packet *oldpacket)
+static void send_inform(struct dhcp_packet *oldpacket)
 {
 	struct dhcp_packet packet;
 
+	/* "The server responds to a DHCPINFORM message by sending a DHCPACK
+	 * message directly to the address given in the 'ciaddr' field
+	 * of the DHCPINFORM message.  The server MUST NOT send a lease
+	 * expiration time to the client and SHOULD NOT fill in 'yiaddr'."
+	 */
 	init_packet(&packet, oldpacket, DHCPACK);
 	add_server_options(&packet);
 
-	return send_packet(&packet, /*force_bcast:*/ 0);
+	send_packet(&packet, /*force_bcast:*/ 0);
 }
 
 
@@ -352,6 +359,9 @@ int udhcpd_main(int argc UNUSED_PARAM, char **argv)
 	while (1) { /* loop until universe collapses */
 		int bytes;
 		struct timeval tv;
+		uint8_t *server_id_opt;
+		uint8_t *requested_opt;
+		uint32_t requested_nip = requested_nip; /* for compiler */
 
 		if (server_socket < 0) {
 			server_socket = udhcp_listen_socket(/*INADDR_ANY,*/ SERVER_PORT,
@@ -404,124 +414,126 @@ int udhcpd_main(int argc UNUSED_PARAM, char **argv)
 			}
 			continue;
 		}
-
 		if (packet.hlen != 6) {
 			bb_error_msg("MAC length != 6, ignoring packet");
 			continue;
 		}
-
+		if (packet.op != BOOTREQUEST) {
+			bb_error_msg("not a REQUEST, ignoring packet");
+			continue;
+		}
 		state = get_option(&packet, DHCP_MESSAGE_TYPE);
-		if (state == NULL) {
-			bb_error_msg("no message type option, ignoring packet");
+		if (state == NULL || state[0] < DHCP_MINTYPE || state[0] > DHCP_MAXTYPE) {
+			bb_error_msg("no or bad message type option, ignoring packet");
 			continue;
 		}
 
-		/* Look for a static lease */
+		/* Look for a static/dynamic lease */
 		static_lease_nip = get_static_nip_by_mac(server_config.static_leases, &packet.chaddr);
 		if (static_lease_nip) {
 			bb_info_msg("Found static lease: %x", static_lease_nip);
-
 			memcpy(&fake_lease.lease_mac, &packet.chaddr, 6);
 			fake_lease.lease_nip = static_lease_nip;
 			fake_lease.expires = 0;
-
 			lease = &fake_lease;
 		} else {
 			lease = find_lease_by_mac(packet.chaddr);
 		}
 
+		/* Get REQUESTED_IP and SERVER_ID if present */
+		server_id_opt = get_option(&packet, DHCP_SERVER_ID);
+		if (server_id_opt) {
+			uint32_t server_id_net;
+			move_from_unaligned32(server_id_net, server_id_opt);
+			if (server_id_net != server_config.server_nip) {
+				/* client talks to somebody else */
+				log1("server ID doesn't match, ignoring");
+				continue;
+			}
+		}
+		requested_opt = get_option(&packet, DHCP_REQUESTED_IP);
+		if (requested_opt) {
+			move_from_unaligned32(requested_nip, requested_opt);
+		}
+
 		switch (state[0]) {
+
 		case DHCPDISCOVER:
 			log1("Received DISCOVER");
 
-			if (send_offer(&packet, static_lease_nip, lease) < 0) {
-				bb_error_msg("send OFFER failed");
-			}
+			send_offer(&packet, static_lease_nip, lease);
 			break;
-		case DHCPREQUEST: {
-			uint8_t *server_id_opt, *requested_opt;
-			uint32_t server_id_net = server_id_net; /* for compiler */
-			uint32_t requested_nip = requested_nip; /* for compiler */
 
+		case DHCPREQUEST:
 			log1("Received REQUEST");
 
-			requested_opt = get_option(&packet, DHCP_REQUESTED_IP);
-			server_id_opt = get_option(&packet, DHCP_SERVER_ID);
-			if (requested_opt)
-				move_from_unaligned32(requested_nip, requested_opt);
-			if (server_id_opt)
-				move_from_unaligned32(server_id_net, server_id_opt);
-
-			if (lease) {
-				if (server_id_opt) {
-					/* SELECTING State */
-					if (server_id_net == server_config.server_nip
-					 && requested_opt
-					 && requested_nip == lease->lease_nip
-					) {
-						send_ACK(&packet, lease->lease_nip);
-					}
-				} else if (requested_opt) {
-					/* INIT-REBOOT State */
-					if (lease->lease_nip == requested_nip)
-						send_ACK(&packet, lease->lease_nip);
-					else
-						send_NAK(&packet);
-				} else if (lease->lease_nip == packet.ciaddr) {
-					/* RENEWING or REBINDING State */
-					send_ACK(&packet, lease->lease_nip);
-				} else { /* don't know what to do!!!! */
-					send_NAK(&packet);
-				}
-
-			/* what to do if we have no record of the client */
-			} else if (server_id_opt) {
-				/* SELECTING State */
-
-			} else if (requested_opt) {
-				/* INIT-REBOOT State */
-				lease = find_lease_by_nip(requested_nip);
-				if (lease) {
-					if (is_expired_lease(lease)) {
-						/* probably best if we drop this lease */
-						memset(lease->lease_mac, 0, sizeof(lease->lease_mac));
-					} else {
-						/* make some contention for this address */
-						send_NAK(&packet);
-					}
-				} else {
-					uint32_t r = ntohl(requested_nip);
-					if (r < server_config.start_ip
-				         || r > server_config.end_ip
-					) {
-						send_NAK(&packet);
-					}
-					/* else remain silent */
-				}
-
-			} else {
-				/* RENEWING or REBINDING State */
+			/* RFC 2131: "The REQUESTED_IP option MUST be set
+			 * to the value of 'yiaddr' in the DHCPOFFER message
+			 * from the server." */
+			if (!requested_opt) {
+				log1("no requested IP, ignoring");
+				break;
+			}
+			if (lease && requested_nip == lease->lease_nip) {
+				/* client requests IP which matches the lease.
+				 * ACK it, and bump lease expiration time. */
+				send_ACK(&packet, lease->lease_nip);
+				break;
+			}
+			if (server_id_opt) {
+				/* client was talking specifically to us.
+				 * "No, we don't have this IP for you". */
+				send_NAK(&packet);
 			}
 			break;
-		}
+
 		case DHCPDECLINE:
+			/* RFC 2131:
+			 * "If the server receives a DHCPDECLINE message,
+			 * the client has discovered through some other means
+			 * that the suggested network address is already
+			 * in use. The server MUST mark the network address
+			 * as not available and SHOULD notify the local
+			 * sysadmin of a possible configuration problem."
+			 *
+			 * SERVER_ID must be present,
+			 * REQUESTED_IP must be present,
+			 * chaddr must be filled in,
+			 * ciaddr must be 0 (we do not check this)
+			 */
 			log1("Received DECLINE");
-			if (lease) {
+			if (server_id_opt
+			 && requested_opt
+			 && lease  /* chaddr matches this lease */
+			 && requested_nip == lease->lease_nip
+			) {
 				memset(lease->lease_mac, 0, sizeof(lease->lease_mac));
 				lease->expires = time(NULL) + server_config.decline_time;
 			}
 			break;
+
 		case DHCPRELEASE:
+			/* "Upon receipt of a DHCPRELEASE message, the server
+			 * marks the network address as not allocated."
+			 *
+			 * SERVER_ID must be present,
+			 * REQUESTED_IP must not be present (we do not check this),
+			 * chaddr must be filled in,
+			 * ciaddr must be filled in
+			 */
 			log1("Received RELEASE");
-			if (lease)
+			if (server_id_opt
+			 && lease  /* chaddr matches this lease */
+			 && packet.ciaddr == lease->lease_nip
+			) {
 				lease->expires = time(NULL);
+			}
 			break;
+
 		case DHCPINFORM:
 			log1("Received INFORM");
 			send_inform(&packet);
 			break;
-		default:
-			bb_info_msg("Unsupported DHCP message (%02x) - ignoring", state[0]);
 		}
 	}
  ret0:
diff --git a/networking/udhcp/files.c b/networking/udhcp/files.c
index 40cfe9fd2..62f4a388f 100644
--- a/networking/udhcp/files.c
+++ b/networking/udhcp/files.c
@@ -99,7 +99,7 @@ static void attach_option(struct option_set **opt_list,
 		log2("Attaching option %02x to list", option->code);
 
 #if ENABLE_FEATURE_UDHCP_RFC3397
-		if ((option->flags & TYPE_MASK) == OPTION_STR1035)
+		if ((option->flags & OPTION_TYPE_MASK) == OPTION_STR1035)
 			/* reuse buffer and length for RFC1035-formatted string */
 			buffer = (char *)dname_enc(NULL, 0, buffer, &length);
 #endif
@@ -118,7 +118,7 @@ static void attach_option(struct option_set **opt_list,
 		new->next = *curr;
 		*curr = new;
 #if ENABLE_FEATURE_UDHCP_RFC3397
-		if ((option->flags & TYPE_MASK) == OPTION_STR1035 && buffer != NULL)
+		if ((option->flags & OPTION_TYPE_MASK) == OPTION_STR1035 && buffer != NULL)
 			free(buffer);
 #endif
 		return;
@@ -128,7 +128,7 @@ static void attach_option(struct option_set **opt_list,
 	log1("Attaching option %02x to existing member of list", option->code);
 	if (option->flags & OPTION_LIST) {
 #if ENABLE_FEATURE_UDHCP_RFC3397
-		if ((option->flags & TYPE_MASK) == OPTION_STR1035)
+		if ((option->flags & OPTION_TYPE_MASK) == OPTION_STR1035)
 			/* reuse buffer and length for RFC1035-formatted string */
 			buffer = (char *)dname_enc(existing->data + 2,
 					existing->data[OPT_LEN], buffer, &length);
@@ -136,7 +136,7 @@ static void attach_option(struct option_set **opt_list,
 		if (existing->data[OPT_LEN] + length <= 255) {
 			existing->data = xrealloc(existing->data,
 					existing->data[OPT_LEN] + length + 3);
-			if ((option->flags & TYPE_MASK) == OPTION_STRING) {
+			if ((option->flags & OPTION_TYPE_MASK) == OPTION_STRING) {
 				/* ' ' can bring us to 256 - bad */
 				if (existing->data[OPT_LEN] + length >= 255)
 					return;
@@ -148,7 +148,7 @@ static void attach_option(struct option_set **opt_list,
 			existing->data[OPT_LEN] += length;
 		} /* else, ignore the data, we could put this in a second option in the future */
 #if ENABLE_FEATURE_UDHCP_RFC3397
-		if ((option->flags & TYPE_MASK) == OPTION_STR1035 && buffer != NULL)
+		if ((option->flags & OPTION_TYPE_MASK) == OPTION_STR1035 && buffer != NULL)
 			free(buffer);
 #endif
 	} /* else, ignore the new data */
@@ -182,10 +182,10 @@ static int FAST_FUNC read_opt(const char *const_line, void *arg)
 	do {
 		val = strtok(NULL, ", \t");
 		if (!val) break;
-		length = dhcp_option_lengths[option->flags & TYPE_MASK];
+		length = dhcp_option_lengths[option->flags & OPTION_TYPE_MASK];
 		retval = 0;
 		opt = buffer; /* new meaning for variable opt */
-		switch (option->flags & TYPE_MASK) {
+		switch (option->flags & OPTION_TYPE_MASK) {
 		case OPTION_IP:
 			retval = read_nip(val, buffer);
 			break;
diff --git a/networking/udhcp/options.c b/networking/udhcp/options.c
index 5bef985bc..29a264047 100644
--- a/networking/udhcp/options.c
+++ b/networking/udhcp/options.c
@@ -252,7 +252,7 @@ void FAST_FUNC add_simple_option(uint8_t *optionptr, uint8_t code, uint32_t data
 			uint8_t option[6], len;
 
 			option[OPT_CODE] = code;
-			len = dhcp_option_lengths[dh->flags & TYPE_MASK];
+			len = dhcp_option_lengths[dh->flags & OPTION_TYPE_MASK];
 			option[OPT_LEN] = len;
 			if (BB_BIG_ENDIAN)
 				data <<= 8 * (4 - len);
diff --git a/networking/udhcp/options.h b/networking/udhcp/options.h
index 05090f12e..3ca4dc42d 100644
--- a/networking/udhcp/options.h
+++ b/networking/udhcp/options.h
@@ -5,7 +5,6 @@
 
 PUSH_AND_SET_FUNCTION_VISIBILITY_TO_HIDDEN
 
-#define TYPE_MASK       0x0F
 
 enum {
 	OPTION_IP = 1,
@@ -21,19 +20,13 @@ enum {
 	OPTION_U32,
 	OPTION_S32,
 	OPTION_STATIC_ROUTES,
-};
-
-/* Client requests this option by default */
-#define OPTION_REQ      0x10
-/* There can be a list of 1 or more of these */
-#define OPTION_LIST     0x20
 
-/*****************************************************************/
-/* Do not modify below here unless you know what you are doing!! */
-/*****************************************************************/
-
-/* DHCP protocol. See RFC 2131 */
-#define DHCP_MAGIC		0x63825363
+	OPTION_TYPE_MASK = 0x0f,
+	/* Client requests this option by default */
+	OPTION_REQ  = 0x10,
+	/* There can be a list of 1 or more of these */
+	OPTION_LIST = 0x20,
+};
 
 /* DHCP option codes (partial list). See RFC 2132 and
  * http://www.iana.org/assignments/bootp-dhcp-parameters/
@@ -81,7 +74,6 @@ enum {
 //#define DHCP_WPAD             0xfc /* MSIE's Web Proxy Autodiscovery Protocol */
 #define DHCP_END                0xff
 
-
 /* Offsets in option byte sequence */
 #define OPT_CODE                0
 #define OPT_LEN                 1
@@ -91,12 +83,7 @@ enum {
 #define FILE_FIELD              1
 #define SNAME_FIELD             2
 
-#define BOOTREQUEST             1
-#define BOOTREPLY               2
-
-#define ETH_10MB                1
-#define ETH_10MB_LEN            6
-
+/* DHCP_MESSAGE_TYPE values */
 #define DHCPDISCOVER            1 /* client -> server */
 #define DHCPOFFER               2 /* client <- server */
 #define DHCPREQUEST             3 /* client -> server */
@@ -105,6 +92,9 @@ enum {
 #define DHCPNAK                 6 /* client <- server */
 #define DHCPRELEASE             7 /* client -> server */
 #define DHCPINFORM              8 /* client -> server */
+#define DHCP_MINTYPE DHCPDISCOVER
+#define DHCP_MAXTYPE DHCPINFORM
+
 
 struct dhcp_option {
 	uint8_t flags;
diff --git a/networking/udhcp/packet.c b/networking/udhcp/packet.c
index 90410cbc0..84d83098c 100644
--- a/networking/udhcp/packet.c
+++ b/networking/udhcp/packet.c
@@ -21,7 +21,7 @@
 
 void FAST_FUNC udhcp_init_header(struct dhcp_packet *packet, char type)
 {
-	memset(packet, 0, sizeof(struct dhcp_packet));
+	memset(packet, 0, sizeof(*packet));
 	packet->op = BOOTREQUEST; /* if client to a server */
 	switch (type) {
 	case DHCPOFFER:
@@ -29,10 +29,11 @@ void FAST_FUNC udhcp_init_header(struct dhcp_packet *packet, char type)
 	case DHCPNAK:
 		packet->op = BOOTREPLY; /* if server to client */
 	}
-	packet->htype = ETH_10MB;
-	packet->hlen = ETH_10MB_LEN;
+	packet->htype = 1; /* ethernet */
+	packet->hlen = 6;
 	packet->cookie = htonl(DHCP_MAGIC);
-	packet->options[0] = DHCP_END;
+	if (DHCP_END != 0)
+		packet->options[0] = DHCP_END;
 	add_simple_option(packet->options, DHCP_MESSAGE_TYPE, type);
 }
 
@@ -228,6 +229,7 @@ int FAST_FUNC udhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
 	msg = "sendto";
  ret_close:
 	close(fd);
+	/* FIXME: and if result >= 0 but != IP_UPD_DHCP_SIZE? */
 	if (result < 0) {
  ret_msg:
 		bb_perror_msg(msg, "PACKET");
@@ -280,6 +282,7 @@ int FAST_FUNC udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
 	msg = "write";
  ret_close:
 	close(fd);
+	/* FIXME: and if result >= 0 but != DHCP_SIZE? */
 	if (result < 0) {
  ret_msg:
 		bb_perror_msg(msg, "UDP");
diff --git a/networking/udhcp/script.c b/networking/udhcp/script.c
index b4413fa16..400fd2b9b 100644
--- a/networking/udhcp/script.c
+++ b/networking/udhcp/script.c
@@ -64,7 +64,7 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
 
 	/* option points to OPT_DATA, need to go back and get OPT_LEN */
 	len = option[OPT_LEN - OPT_DATA];
-	type = type_p->flags & TYPE_MASK;
+	type = type_p->flags & OPTION_TYPE_MASK;
 	optlen = dhcp_option_lengths[type];
 	upper_length = len_of_option_as_string[type] * (len / optlen);