From 06b44be71db4033db8c855491a95f3cf0e7e4595 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Sun, 23 Feb 2014 14:15:05 +0100 Subject: [PATCH] sd-dhcp: equally verify udp and raw dhcp messages Also be more explicit about why packages are ignored. --- src/libsystemd-dhcp/dhcp-internal.h | 4 +- src/libsystemd-dhcp/dhcp-packet.c | 86 ++++++++++++++-------------- src/libsystemd-dhcp/sd-dhcp-client.c | 28 +++++++-- 3 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/libsystemd-dhcp/dhcp-internal.h b/src/libsystemd-dhcp/dhcp-internal.h index efb8ea541..9a997c10c 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 4501d2757..7c209fa65 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 7f86d92fb..54bc19511 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; -- 2.30.2