sd-dhcp: equally verify udp and raw dhcp messages

Also be more explicit about why packages are ignored.

3 files changed, 68 insertions, 50 deletions

diff --git a/src/libsystemd-dhcp/dhcp-internal.h b/src/libsystemd-dhcp/dhcp-internal.h
index efb8ea5411..9a997c10c4 100644
--- a/src/libsystemd-dhcp/dhcp-internal.h
+++ b/src/libsystemd-dhcp/dhcp-internal.h
@@ -48,8 +48,8 @@ int dhcp_option_parse(DHCPMessage *message, size_t len,
 int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid, uint8_t type,
                       uint16_t secs, uint8_t **opt, size_t *optlen);
 
-void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint8_t op, uint16_t len);
+void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint16_t len);
 
-int dhcp_packet_verify_headers(DHCPPacket *packet, uint8_t op, size_t len);
+int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len);
 
 #define log_dhcp_client(client, fmt, ...) log_meta(LOG_DEBUG, __FILE__, __LINE__, __func__, "DHCP CLIENT: " fmt, ##__VA_ARGS__)
diff --git a/src/libsystemd-dhcp/dhcp-packet.c b/src/libsystemd-dhcp/dhcp-packet.c
index 4501d27571..7c209fa658 100644
--- a/src/libsystemd-dhcp/dhcp-packet.c
+++ b/src/libsystemd-dhcp/dhcp-packet.c
@@ -94,10 +94,7 @@ static uint16_t dhcp_checksum(void *buf, int len) {
         return ~sum;
 }
 
-void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint8_t op,
-                                   uint16_t len) {
-        assert(op == BOOTREQUEST || op == BOOTREPLY);
-
+void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint16_t len) {
         packet->ip.version = IPVERSION;
         packet->ip.ihl = DHCP_IP_SIZE / 4;
         packet->ip.tot_len = htobe16(len);
@@ -106,16 +103,8 @@ void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint8_t op,
         packet->ip.saddr = INADDR_ANY;
         packet->ip.daddr = INADDR_BROADCAST;
 
-        switch (op) {
-                case BOOTREQUEST:
-                        packet->udp.source = htobe16(DHCP_PORT_CLIENT);
-                        packet->udp.dest = htobe16(DHCP_PORT_SERVER);
-                        break;
-                case BOOTREPLY:
-                        packet->udp.source = htobe16(DHCP_PORT_SERVER);
-                        packet->udp.dest = htobe16(DHCP_PORT_CLIENT);
-                        break;
-        }
+        packet->udp.source = htobe16(DHCP_PORT_CLIENT);
+        packet->udp.dest = htobe16(DHCP_PORT_SERVER);
 
         packet->udp.len = htobe16(len - DHCP_IP_SIZE);
 
@@ -127,30 +116,56 @@ void dhcp_packet_append_ip_headers(DHCPPacket *packet, uint8_t op,
         packet->ip.check = dhcp_checksum(&packet->ip, DHCP_IP_SIZE);
 }
 
-int dhcp_packet_verify_headers(DHCPPacket *packet, uint8_t op, size_t len) {
+int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len) {
         size_t hdrlen;
 
-        assert(op == BOOTREQUEST || op == BOOTREPLY);
+        /* IP */
 
-        if (len < (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE)) {
-                log_dhcp_client(client, "ignoring packet: packet too small");
+        if (len < DHCP_IP_SIZE) {
+                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
+                                " smaller than IP header (%u bytes)", len,
+                                DHCP_IP_SIZE);
+                return -EINVAL;
+        }
+
+        if (packet->ip.ihl < 5) {
+                log_dhcp_client(client, "ignoring packet: IPv4 IHL (%u words) invalid",
+                                packet->ip.ihl);
                 return -EINVAL;
         }
 
         hdrlen = packet->ip.ihl * 4;
-        if (hdrlen < 20 || hdrlen > len) {
-                log_dhcp_client(client, "ignoring packet: header with wrong size");
+        if (hdrlen < 20) {
+                log_dhcp_client(client, "ignoring packet: IPv4 IHL (%zu bytes) "
+                                "smaller than minimum (20 bytes)", hdrlen);
+                return -EINVAL;
+        }
+
+        if (len < hdrlen) {
+                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
+                                "smaller than expected (%zu) by IP header", len,
+                                hdrlen);
                 return -EINVAL;
         }
 
         if (dhcp_checksum(&packet->ip, hdrlen)) {
-                log_dhcp_client(client, "ignoring packet: invalid ip checksum");
+                log_dhcp_client(client, "ignoring packet: invalid IP checksum");
+                return -EINVAL;
+        }
+
+        /* UDP */
+
+        if (len < DHCP_IP_UDP_SIZE) {
+                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
+                                " smaller than IP+UDP header (%u bytes)", len,
+                                DHCP_IP_UDP_SIZE);
                 return -EINVAL;
         }
 
         if (hdrlen + be16toh(packet->udp.len) > len) {
-                log_dhcp_client(client, "ignoring packet: packet too small (udp.len=%u)",
-                                be16toh(packet->udp.len));
+                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
+                                "smaller than expected (%zu) by UDP header", len,
+                                hdrlen + be16toh(packet->udp.len));
                 return -EINVAL;
         }
 
@@ -160,32 +175,17 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, uint8_t op, size_t len) {
 
                 if (dhcp_checksum(&packet->ip.ttl,
                                   be16toh(packet->udp.len) + 12)) {
-                        log_dhcp_client(client, "ignoring packet: invalid udp checksum");
+                        log_dhcp_client(client, "ignoring packet: invalid UDP checksum");
                         return -EINVAL;
                 }
         }
 
-        if (packet->dhcp.op != op) {
-                log_dhcp_client(client, "ignoring packet: wrong operation");
+        if (be16toh(packet->udp.source) != DHCP_PORT_SERVER ||
+            be16toh(packet->udp.dest) != DHCP_PORT_CLIENT) {
+                log_dhcp_client(client, "ignoring packet: wrong ports (source: %u, destination: %u)",
+                                be16toh(packet->udp.source), be16toh(packet->udp.dest));
                 return -EINVAL;
         }
 
-        switch (op) {
-                case BOOTREQUEST:
-                        if (be16toh(packet->udp.source) != DHCP_PORT_CLIENT ||
-                            be16toh(packet->udp.dest) != DHCP_PORT_SERVER) {
-                                log_dhcp_client(client, "ignoring packet: wrong ports");
-                                return -EINVAL;
-                        }
-                        break;
-                case BOOTREPLY:
-                        if (be16toh(packet->udp.source) != DHCP_PORT_SERVER ||
-                            be16toh(packet->udp.dest) != DHCP_PORT_CLIENT) {
-                                log_dhcp_client(client, "ignoring packet: wrong ports");
-                                return -EINVAL;
-                        }
-                        break;
-        }
-
         return 0;
 }
diff --git a/src/libsystemd-dhcp/sd-dhcp-client.c b/src/libsystemd-dhcp/sd-dhcp-client.c
index 7f86d92fb5..54bc19511d 100644
--- a/src/libsystemd-dhcp/sd-dhcp-client.c
+++ b/src/libsystemd-dhcp/sd-dhcp-client.c
@@ -279,7 +279,7 @@ static int client_send_discover(sd_dhcp_client *client, uint16_t secs) {
         if (err < 0)
                 return err;
 
-        dhcp_packet_append_ip_headers(discover, BOOTREQUEST, len);
+        dhcp_packet_append_ip_headers(discover, len);
 
         err = dhcp_network_send_raw_socket(client->fd, &client->link,
                                            discover, len);
@@ -332,7 +332,7 @@ static int client_send_request(sd_dhcp_client *client, uint16_t secs) {
                                                    &request->dhcp,
                                                    len - DHCP_IP_UDP_SIZE);
         } else {
-                dhcp_packet_append_ip_headers(request, BOOTREQUEST, len);
+                dhcp_packet_append_ip_headers(request, len);
 
                 err = dhcp_network_send_raw_socket(client->fd, &client->link,
                                                    request, len);
@@ -736,12 +736,30 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message,
         assert(client->event);
         assert(message);
 
-        if (be32toh(message->xid) != client->xid)
+        if (len < DHCP_MESSAGE_SIZE) {
+                log_dhcp_client(client, "message too small (%d bytes): "
+                                "ignoring", len);
                 return 0;
+        }
+
+        if (message->op != BOOTREPLY) {
+                log_dhcp_client(client, "not a BOOTREPLY message: ignoring");
+                return 0;
+        }
+
+        if (be32toh(message->xid) != client->xid) {
+                log_dhcp_client(client, "received xid (%u) does not match "
+                                "expected (%u): ignoring",
+                                be32toh(message->xid), client->xid);
+                return 0;
+        }
 
         if (memcmp(&message->chaddr[0], &client->mac_addr.ether_addr_octet,
-                   ETHER_ADDR_LEN))
+                   ETHER_ADDR_LEN)) {
+                log_dhcp_client(client, "received chaddr does not match "
+                                "expected: ignoring");
                 return 0;
+        }
 
         switch (client->state) {
         case DHCP_STATE_SELECTING:
@@ -869,7 +887,7 @@ static int client_receive_message_raw(sd_event_source *s, int fd,
 
         packet = (DHCPPacket *) buf;
 
-        r = dhcp_packet_verify_headers(packet, BOOTREPLY, len);
+        r = dhcp_packet_verify_headers(packet, len);
         if (r < 0)
                 return 0;