[systemd-devel] [PATCH 24/28] dhcp: Process DHCP Ack/Nak message

2013-11-13 Thread Patrik Flykt 
Process a DHCP Ack/Nak in much the same way as an DHCP Offer. Factor
out header verification and process options sent. Add notification
functionality with discrete values for the outcome of the DHCP Ack/
Nak processing.
---
 src/dhcp/client.c |  145 +
 src/dhcp/client.h |4 ++
 2 files changed, 128 insertions(+), 21 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 45c62f3..669804a 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -151,6 +151,11 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
ether_addr *addr)
 return 0;
 }
 
+static int client_notify(DHCPClient *client, int event)
+{
+return 0;
+}
+
 static int client_stop(DHCPClient *client, int error)
 {
 if (!client)
@@ -178,6 +183,7 @@ static int client_stop(DHCPClient *client, int error)
 
 case DHCP_STATE_INIT:
 case DHCP_STATE_SELECTING:
+case DHCP_STATE_REQUESTING:
 
 client-start_time = 0;
 client-state = DHCP_STATE_INIT;
@@ -185,7 +191,6 @@ static int client_stop(DHCPClient *client, int error)
 
 case DHCP_STATE_INIT_REBOOT:
 case DHCP_STATE_REBOOTING:
-case DHCP_STATE_REQUESTING:
 case DHCP_STATE_BOUND:
 case DHCP_STATE_RENEWING:
 case DHCP_STATE_REBINDING:
@@ -499,41 +504,52 @@ static int client_parse_offer(uint8_t code, uint8_t len, 
uint8_t *option,
 return 0;
 }
 
-static int client_receive_offer(DHCPClient *client,
-DHCPPacket *offer, int len)
+static int client_verify_headers(DHCPClient *client,
+ DHCPPacket *message, int len)
 {
 int hdrlen;
-DHCPLease *lease;
 
 if (len  (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
 return -EINVAL;
 
-hdrlen = offer-ip.ihl * 4;
-if (hdrlen  20 || hdrlen  len || client_checksum(offer-ip,
+hdrlen = message-ip.ihl * 4;
+if (hdrlen  20 || hdrlen  len || client_checksum(message-ip,
hdrlen))
 return -EINVAL;
 
-offer-ip.check = offer-udp.len;
-offer-ip.ttl = 0;
+message-ip.check = message-udp.len;
+message-ip.ttl = 0;
 
-if (hdrlen + ntohs(offer-udp.len)  len ||
-client_checksum(offer-ip.ttl, ntohs(offer-udp.len) + 12))
+if (hdrlen + ntohs(message-udp.len)  len ||
+client_checksum(message-ip.ttl, ntohs(message-udp.len) + 12))
 return -EINVAL;
 
-if (ntohs(offer-udp.source) != DHCP_PORT_SERVER ||
-ntohs(offer-udp.dest) != DHCP_PORT_CLIENT)
+if (ntohs(message-udp.source) != DHCP_PORT_SERVER ||
+ntohs(message-udp.dest) != DHCP_PORT_CLIENT)
 return -EINVAL;
 
-if (offer-dhcp.op != BOOTREPLY)
+if (message-dhcp.op != BOOTREPLY)
 return -EINVAL;
 
-if (ntohl(offer-dhcp.xid) != client-xid)
+if (ntohl(message-dhcp.xid) != client-xid)
 return -EINVAL;
 
-if (memcmp(offer-dhcp.chaddr[0], client-mac_addr.ether_addr_octet,
+if (memcmp(message-dhcp.chaddr[0], 
client-mac_addr.ether_addr_octet,
 ETHER_ADDR_LEN))
 return -EINVAL;
 
+return 0;
+}
+
+static int client_receive_offer(DHCPClient *client, DHCPPacket *offer, int len)
+{
+int err;
+DHCPLease *lease;
+
+err = client_verify_headers(client, offer, len);
+if (err  0)
+return err;
+
 lease = new0(DHCPLease, 1);
 if (!lease)
 return -ENOBUFS;
@@ -561,6 +577,64 @@ error:
 return -ENOMSG;
 }
 
+static int client_receive_ack(DHCPClient *client, DHCPPacket *offer, int len)
+{
+int err = -ENOMSG;
+DHCPLease *lease;
+int type;
+
+err = client_verify_headers(client, offer, len);
+if (err  0)
+return err;
+
+lease = new0(DHCPLease, 1);
+if (!lease)
+return -ENOBUFS;
+
+len = len - DHCP_IP_UDP_SIZE;
+type = __dhcp_option_parse(offer-dhcp, len, client_parse_offer,
+   lease);
+
+if (type == DHCP_NAK) {
+err = DHCP_EVENT_NAK;
+goto error;
+}
+
+if (type != DHCP_ACK)
+goto error;
+
+lease-address.s_addr = offer-dhcp.yiaddr;
+
+if (lease-address.s_addr == INADDR_ANY ||
+lease-server_address.s_addr == INADDR_ANY ||
+lease-subnet_mask.s_addr == INADDR_ANY ||
+lease-lifetime == 0) {
+err = -ENOMSG;
+goto error;
+}
+
+err = 0;
+
+if (client-lease) {
+if (client-lease-address.s_addr != lease-address.s_addr ||
+client-lease-subnet_mask.s_addr !=
+

Re: [systemd-devel] [PATCH 24/28] dhcp: Process DHCP Ack/Nak message

2013-11-13 Thread Zbigniew Jędrzejewski-Szmek 
On Wed, Nov 13, 2013 at 11:22:52PM +0200, Patrik Flykt wrote:
 Process a DHCP Ack/Nak in much the same way as an DHCP Offer. Factor
 out header verification and process options sent. Add notification
 functionality with discrete values for the outcome of the DHCP Ack/
 Nak processing.
 ---
  src/dhcp/client.c |  145 
 +
  src/dhcp/client.h |4 ++
  2 files changed, 128 insertions(+), 21 deletions(-)
 
 diff --git a/src/dhcp/client.c b/src/dhcp/client.c
 index 45c62f3..669804a 100644
 --- a/src/dhcp/client.c
 +++ b/src/dhcp/client.c
 @@ -151,6 +151,11 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
 ether_addr *addr)
  return 0;
  }
  
 +static int client_notify(DHCPClient *client, int event)
 +{
 +return 0;
 +}
 +
  static int client_stop(DHCPClient *client, int error)
  {
  if (!client)
 @@ -178,6 +183,7 @@ static int client_stop(DHCPClient *client, int error)
  
  case DHCP_STATE_INIT:
  case DHCP_STATE_SELECTING:
 +case DHCP_STATE_REQUESTING:
  
  client-start_time = 0;
  client-state = DHCP_STATE_INIT;
 @@ -185,7 +191,6 @@ static int client_stop(DHCPClient *client, int error)
  
  case DHCP_STATE_INIT_REBOOT:
  case DHCP_STATE_REBOOTING:
 -case DHCP_STATE_REQUESTING:
  case DHCP_STATE_BOUND:
  case DHCP_STATE_RENEWING:
  case DHCP_STATE_REBINDING:
 @@ -499,41 +504,52 @@ static int client_parse_offer(uint8_t code, uint8_t 
 len, uint8_t *option,
  return 0;
  }
  
 -static int client_receive_offer(DHCPClient *client,
 -DHCPPacket *offer, int len)
 +static int client_verify_headers(DHCPClient *client,
 + DHCPPacket *message, int len)
  {
  int hdrlen;
 -DHCPLease *lease;
  
  if (len  (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
  return -EINVAL;
  
 -hdrlen = offer-ip.ihl * 4;
 -if (hdrlen  20 || hdrlen  len || client_checksum(offer-ip,
 +hdrlen = message-ip.ihl * 4;
 +if (hdrlen  20 || hdrlen  len || client_checksum(message-ip,
 hdrlen))
  return -EINVAL;
  
 -offer-ip.check = offer-udp.len;
 -offer-ip.ttl = 0;
 +message-ip.check = message-udp.len;
 +message-ip.ttl = 0;
  
 -if (hdrlen + ntohs(offer-udp.len)  len ||
 -client_checksum(offer-ip.ttl, ntohs(offer-udp.len) + 12))
 +if (hdrlen + ntohs(message-udp.len)  len ||
 +client_checksum(message-ip.ttl, ntohs(message-udp.len) + 12))
  return -EINVAL;
  
 -if (ntohs(offer-udp.source) != DHCP_PORT_SERVER ||
 -ntohs(offer-udp.dest) != DHCP_PORT_CLIENT)
 +if (ntohs(message-udp.source) != DHCP_PORT_SERVER ||
 +ntohs(message-udp.dest) != DHCP_PORT_CLIENT)
  return -EINVAL;
  
 -if (offer-dhcp.op != BOOTREPLY)
 +if (message-dhcp.op != BOOTREPLY)
  return -EINVAL;
  
 -if (ntohl(offer-dhcp.xid) != client-xid)
 +if (ntohl(message-dhcp.xid) != client-xid)
  return -EINVAL;
  
 -if (memcmp(offer-dhcp.chaddr[0], 
 client-mac_addr.ether_addr_octet,
 +if (memcmp(message-dhcp.chaddr[0], 
 client-mac_addr.ether_addr_octet,
  ETHER_ADDR_LEN))
  return -EINVAL;
  
 +return 0;
 +}
 +
 +static int client_receive_offer(DHCPClient *client, DHCPPacket *offer, int 
 len)
 +{
 +int err;
 +DHCPLease *lease;
 +
 +err = client_verify_headers(client, offer, len);
 +if (err  0)
 +return err;
 +
  lease = new0(DHCPLease, 1);
  if (!lease)
  return -ENOBUFS;
 @@ -561,6 +577,64 @@ error:
  return -ENOMSG;
  }
  
 +static int client_receive_ack(DHCPClient *client, DHCPPacket *offer, int len)
 +{
 +int err = -ENOMSG;
Since err is not an error always, but a sucess return value too,
it would be nicer to call it r. Also this value is override right below.

You might consider the following pattern:
  _cleanup_free_  DHCPLease *lease = NULL;

and ...

 +DHCPLease *lease;
 +int type;
 +
 +err = client_verify_headers(client, offer, len);
 +if (err  0)
 +return err;
 +
 +lease = new0(DHCPLease, 1);
 +if (!lease)
 +return -ENOBUFS;
 +
 +len = len - DHCP_IP_UDP_SIZE;
 +type = __dhcp_option_parse(offer-dhcp, len, client_parse_offer,
 +   lease);
 +
 +if (type == DHCP_NAK) {
 +err = DHCP_EVENT_NAK;
 +goto error;
... and
  return DHCP_EVENT_NAK;
and ...

 +}
 +
 +if (type != DHCP_ACK)
 +goto error;
Is err set correctly here?

 +
 +