Don't rely on the packet buffer (client->packet) being preserved between

attempts to send DISCOVER or REQUEST packets. Some DHCP servers might
NAK the DISCOVER, or other nefarious packets arrive, between attempts
and overwrite the packet being sent. Create and use another buffer
for packets being sent.

Problem encountered by fgsch@, who noticed that once the DISCOVER was
NAK'ed our dhclient sent a bunch of NAK's back to the server. Like it
thought they were DISCOVER messages.

ok beck@

4 files changed, 72 insertions, 72 deletions

diff --git a/sbin/dhclient/bpf.c b/sbin/dhclient/bpf.c
index 50f5a7814b5..900039da52e 100644
--- a/sbin/dhclient/bpf.c
+++ b/sbin/dhclient/bpf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bpf.c,v 1.25 2013/02/03 21:04:19 krw Exp $	*/
+/*	$OpenBSD: bpf.c,v 1.26 2013/02/14 20:39:46 krw Exp $	*/
 
 /* BPF socket interface code, originally contributed by Archie Cobbs. */
 
@@ -274,13 +274,13 @@ send_packet(struct in_addr from, struct sockaddr_in *to,
 
 	assemble_udp_ip_header(buf, &bufp, from.s_addr,
 	    to->sin_addr.s_addr, to->sin_port,
-	    (unsigned char *)&client->packet,
-	    client->packet_length);
+	    (unsigned char *)&client->bootrequest_packet,
+	    client->bootrequest_packet_length);
 
 	iov[0].iov_base = (char *)buf;
 	iov[0].iov_len = bufp;
-	iov[1].iov_base = (char *)&client->packet;
-	iov[1].iov_len = client->packet_length;
+	iov[1].iov_base = (char *)&client->bootrequest_packet;
+	iov[1].iov_len = client->bootrequest_packet_length;
 
 	if (to->sin_addr.s_addr == INADDR_BROADCAST) {
 		result = writev(ifi->wfdesc, iov, IOVCNT);
diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 41ec7375376..fdbd4b67e2f 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhclient.c,v 1.227 2013/02/13 19:32:52 krw Exp $	*/
+/*	$OpenBSD: dhclient.c,v 1.228 2013/02/14 20:39:46 krw Exp $	*/
 
 /*
  * Copyright 2004 Henning Brauer <henning@openbsd.org>
@@ -1310,12 +1310,13 @@ send_decline(void)
 void
 make_discover(struct client_lease *lease)
 {
-	unsigned char discover = DHCPDISCOVER;
 	struct option_data options[256];
+	struct dhcp_packet *packet = &client->bootrequest_packet;
+	unsigned char discover = DHCPDISCOVER;
 	int i;
 
 	memset(options, 0, sizeof(options));
-	memset(&client->packet, 0, sizeof(client->packet));
+	memset(packet, 0, sizeof(*packet));
 
 	/* Set DHCP_MESSAGE_TYPE to DHCPDISCOVER */
 	i = DHO_DHCP_MESSAGE_TYPE;
@@ -1346,37 +1347,37 @@ make_discover(struct client_lease *lease)
 
 	/* Set up the option buffer to fit in a minimal UDP packet. */
 	i = cons_options(options);
-	if (i == -1 || client->packet.options[i] != DHO_END)
+	if (i == -1 || packet->options[i] != DHO_END)
 		error("options do not fit in DHCPDISCOVER packet.");
-	client->packet_length = DHCP_FIXED_NON_UDP+i+1;
-	if (client->packet_length < BOOTP_MIN_LEN)
-		client->packet_length = BOOTP_MIN_LEN;
-
-	client->packet.op = BOOTREQUEST;
-	client->packet.htype = ifi->hw_address.htype;
-	client->packet.hlen = ifi->hw_address.hlen;
-	client->packet.hops = 0;
-	client->packet.xid = arc4random();
-	client->packet.secs = 0; /* filled in by send_discover. */
-	client->packet.flags = 0;
-
-	memset(&client->packet.ciaddr, 0, sizeof(client->packet.ciaddr));
-	memset(&client->packet.yiaddr, 0, sizeof(client->packet.yiaddr));
-	memset(&client->packet.siaddr, 0, sizeof(client->packet.siaddr));
-	memset(&client->packet.giaddr, 0, sizeof(client->packet.giaddr));
-	memcpy(client->packet.chaddr, ifi->hw_address.haddr,
-	    ifi->hw_address.hlen);
+	client->bootrequest_packet_length = DHCP_FIXED_NON_UDP+i+1;
+	if (client->bootrequest_packet_length < BOOTP_MIN_LEN)
+		client->bootrequest_packet_length = BOOTP_MIN_LEN;
+
+	packet->op = BOOTREQUEST;
+	packet->htype = ifi->hw_address.htype;
+	packet->hlen = ifi->hw_address.hlen;
+	packet->hops = 0;
+	packet->xid = arc4random();
+	packet->secs = 0; /* filled in by send_discover. */
+	packet->flags = 0;
+
+	memset(&packet->ciaddr, 0, sizeof(client->packet.ciaddr));
+	memset(&packet->yiaddr, 0, sizeof(client->packet.yiaddr));
+	memset(&packet->siaddr, 0, sizeof(client->packet.siaddr));
+	memset(&packet->giaddr, 0, sizeof(client->packet.giaddr));
+	memcpy(&packet->chaddr, ifi->hw_address.haddr, ifi->hw_address.hlen);
 }
 
 void
 make_request(struct client_lease * lease)
 {
-	unsigned char request = DHCPREQUEST;
 	struct option_data options[256];
+	struct dhcp_packet *packet = &client->bootrequest_packet;
+	unsigned char request = DHCPREQUEST;
 	int i;
 
 	memset(options, 0, sizeof(options));
-	memset(&client->packet, 0, sizeof(client->packet));
+	memset(packet, 0, sizeof(*packet));
 
 	/* Set DHCP_MESSAGE_TYPE to DHCPREQUEST */
 	i = DHO_DHCP_MESSAGE_TYPE;
@@ -1413,48 +1414,47 @@ make_request(struct client_lease * lease)
 
 	/* Set up the option buffer to fit in a minimal UDP packet. */
 	i = cons_options(options);
-	if (i == -1 || client->packet.options[i] != DHO_END)
+	if (i == -1 || packet->options[i] != DHO_END)
 		error("options do not fit in DHCPREQUEST packet.");
-	client->packet_length = DHCP_FIXED_NON_UDP+i+1;
-	if (client->packet_length < BOOTP_MIN_LEN)
-		client->packet_length = BOOTP_MIN_LEN;
-
-	client->packet.op = BOOTREQUEST;
-	client->packet.htype = ifi->hw_address.htype;
-	client->packet.hlen = ifi->hw_address.hlen;
-	client->packet.hops = 0;
-	client->packet.xid = client->xid;
-	client->packet.secs = 0; /* Filled in by send_request. */
-	client->packet.flags = 0;
+	client->bootrequest_packet_length = DHCP_FIXED_NON_UDP+i+1;
+	if (client->bootrequest_packet_length < BOOTP_MIN_LEN)
+		client->bootrequest_packet_length = BOOTP_MIN_LEN;
+
+	packet->op = BOOTREQUEST;
+	packet->htype = ifi->hw_address.htype;
+	packet->hlen = ifi->hw_address.hlen;
+	packet->hops = 0;
+	packet->xid = client->xid;
+	packet->secs = 0; /* Filled in by send_request. */
+	packet->flags = 0;
 
 	/* If we own the address we're requesting, put it in ciaddr;
 	   otherwise set ciaddr to zero. */
 	if (client->state == S_BOUND ||
 	    client->state == S_RENEWING ||
 	    client->state == S_REBINDING) {
-		memcpy(&client->packet.ciaddr, &lease->address.s_addr,
+		memcpy(&packet->ciaddr, &lease->address.s_addr,
 		    sizeof(in_addr_t));
 	} else {
-		memset(&client->packet.ciaddr, 0,
-		    sizeof(client->packet.ciaddr));
+		memset(&packet->ciaddr, 0, sizeof(packet->ciaddr));
 	}
 
-	memset(&client->packet.yiaddr, 0, sizeof(client->packet.yiaddr));
-	memset(&client->packet.siaddr, 0, sizeof(client->packet.siaddr));
-	memset(&client->packet.giaddr, 0, sizeof(client->packet.giaddr));
-	memcpy(client->packet.chaddr, ifi->hw_address.haddr,
-	    ifi->hw_address.hlen);
+	memset(&packet->yiaddr, 0, sizeof(packet->yiaddr));
+	memset(&packet->siaddr, 0, sizeof(packet->siaddr));
+	memset(&packet->giaddr, 0, sizeof(packet->giaddr));
+	memcpy(&packet->chaddr, ifi->hw_address.haddr, ifi->hw_address.hlen);
 }
 
 void
 make_decline(struct client_lease *lease)
 {
 	struct option_data options[256];
+	struct dhcp_packet *packet = &client->bootrequest_packet;
 	unsigned char decline = DHCPDECLINE;
 	int i;
 
 	memset(options, 0, sizeof(options));
-	memset(&client->packet, 0, sizeof(client->packet));
+	memset(packet, 0, sizeof(*packet));
 
 	/* Set DHCP_MESSAGE_TYPE to DHCPDECLINE */
 	i = DHO_DHCP_MESSAGE_TYPE;
@@ -1480,27 +1480,26 @@ make_decline(struct client_lease *lease)
 
 	/* Set up the option buffer to fit in a minimal UDP packet. */
 	i = cons_options(options);
-	if (i == -1 || client->packet.options[i] != DHO_END)
+	if (i == -1 || packet->options[i] != DHO_END)
 		error("options do not fit in DHCPDECLINE packet.");
-	client->packet_length = DHCP_FIXED_NON_UDP+i+1;
-	if (client->packet_length < BOOTP_MIN_LEN)
-		client->packet_length = BOOTP_MIN_LEN;
-
-	client->packet.op = BOOTREQUEST;
-	client->packet.htype = ifi->hw_address.htype;
-	client->packet.hlen = ifi->hw_address.hlen;
-	client->packet.hops = 0;
-	client->packet.xid = client->xid;
-	client->packet.secs = 0; /* Filled in by send_request. */
-	client->packet.flags = 0;
+	client->bootrequest_packet_length = DHCP_FIXED_NON_UDP+i+1;
+	if (client->bootrequest_packet_length < BOOTP_MIN_LEN)
+		client->bootrequest_packet_length = BOOTP_MIN_LEN;
+
+	packet->op = BOOTREQUEST;
+	packet->htype = ifi->hw_address.htype;
+	packet->hlen = ifi->hw_address.hlen;
+	packet->hops = 0;
+	packet->xid = client->xid;
+	packet->secs = 0; /* Filled in by send_request. */
+	packet->flags = 0;
 
 	/* ciaddr must always be zero. */
-	memset(&client->packet.ciaddr, 0, sizeof(client->packet.ciaddr));
-	memset(&client->packet.yiaddr, 0, sizeof(client->packet.yiaddr));
-	memset(&client->packet.siaddr, 0, sizeof(client->packet.siaddr));
-	memset(&client->packet.giaddr, 0, sizeof(client->packet.giaddr));
-	memcpy(client->packet.chaddr, ifi->hw_address.haddr,
-	    ifi->hw_address.hlen);
+	memset(&packet->ciaddr, 0, sizeof(packet->ciaddr));
+	memset(&packet->yiaddr, 0, sizeof(packet->yiaddr));
+	memset(&packet->siaddr, 0, sizeof(packet->siaddr));
+	memset(&packet->giaddr, 0, sizeof(packet->giaddr));
+	memcpy(&packet->chaddr, ifi->hw_address.haddr, ifi->hw_address.hlen);
 }
 
 void
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index 3b620a8679e..1dc85e63927 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhcpd.h,v 1.108 2013/02/03 21:04:19 krw Exp $	*/
+/*	$OpenBSD: dhcpd.h,v 1.109 2013/02/14 20:39:46 krw Exp $	*/
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -158,7 +158,8 @@ struct client_state {
 	time_t			 first_sending;
 	time_t			 interval;
 	struct dhcp_packet	 packet;
-	int			 packet_length;
+	struct dhcp_packet	 bootrequest_packet;
+	int			 bootrequest_packet_length;
 	struct in_addr		 requested_address;
 };
 
diff --git a/sbin/dhclient/options.c b/sbin/dhclient/options.c
index fd2a1e99833..46121450cea 100644
--- a/sbin/dhclient/options.c
+++ b/sbin/dhclient/options.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: options.c,v 1.50 2013/01/16 21:35:41 krw Exp $	*/
+/*	$OpenBSD: options.c,v 1.51 2013/02/14 20:39:46 krw Exp $	*/
 
 /* DHCP options parsing and reassembly. */
 
@@ -146,7 +146,7 @@ parse_option_buffer(struct option_data *options, unsigned char *buffer,
 int
 cons_options(struct option_data *options)
 {
-	unsigned char *buf = client->packet.options;
+	unsigned char *buf = client->bootrequest_packet.options;
 	int buflen = 576 - DHCP_FIXED_LEN;
 	int ix, incr, length, bufix, code, lastopt = -1;