From 27c9e2c5533333fcabad463d5907bb6a85161d49 Mon Sep 17 00:00:00 2001 From: "Nicholas J. Kain" Date: Thu, 12 Feb 2015 23:28:54 -0500 Subject: [PATCH] Improve fingerprinting to support DHCP relay agents. Mostly reverts the previous commit and instead teaches ndhc to properly handle the case when it is communicating with a DHCP relay agent on its local segment rather than directly with a DHCP server. --- src/arp.c | 49 ++++++++++++++----------------------------------- src/dhcp.c | 10 +++++++--- src/ndhc.h | 3 +-- src/state.c | 24 ++++++++++++++---------- src/state.h | 2 +- 5 files changed, 37 insertions(+), 51 deletions(-) diff --git a/src/arp.c b/src/arp.c index 7b04fa8..2c11cf3 100644 --- a/src/arp.c +++ b/src/arp.c @@ -49,10 +49,6 @@ #define ARP_MSG_SIZE 0x2a #define ARP_RETRANS_DELAY 5000 // ms -// Maximum number of retries when finding the DHCP server ethernet address -// via ARP queries before assuming it is on a different segment. -#define MAX_DHCP_SERVER_HWADDR_QUERIES 1 - // From RFC5227 int arp_probe_wait = 1000; // initial random delay (ms) int arp_probe_num = 3; // number of probe packets @@ -351,7 +347,7 @@ int arp_gw_check(struct client_state_t *cs) return 0; garp.gw_check_initpings = garp.send_stats[ASEND_GW_PING].count; garp.server_replied = false; - if (arp_ping(cs, cs->serverAddr) < 0) + if (arp_ping(cs, cs->srcAddr) < 0) return -1; if (cs->routerAddr) { garp.router_replied = false; @@ -380,7 +376,7 @@ static int arp_get_gw_hwaddr(struct client_state_t *cs) log_line("%s: arp: Searching for dhcp server address...", client_config.interface); cs->got_server_arp = 0; - if (arp_ping(cs, cs->serverAddr) < 0) + if (arp_ping(cs, cs->srcAddr) < 0) return -1; if (cs->routerAddr) { cs->got_router_arp = 0; @@ -413,13 +409,13 @@ static int act_if_arp_gw_failed(struct client_state_t *cs) { if (garp.send_stats[ASEND_GW_PING].count >= garp.gw_check_initpings + 6) { if (garp.router_replied && !garp.server_replied) - log_line("%s: arp: DHCP server didn't reply. Getting new lease.", + log_line("%s: arp: DHCP agent didn't reply. Getting new lease.", client_config.interface); else if (!garp.router_replied && garp.server_replied) log_line("%s: arp: Gateway didn't reply. Getting new lease.", client_config.interface); else - log_line("%s: arp: DHCP server and gateway didn't reply. Getting new lease.", + log_line("%s: arp: DHCP agent and gateway didn't reply. Getting new lease.", client_config.interface); arp_gw_failed(cs); return 1; @@ -562,9 +558,9 @@ static void arp_gw_check_timeout(struct client_state_t *cs, long long nowts) client_config.interface); } if (!garp.server_replied) { - log_line("%s: arp: Still waiting for DHCP server to reply to arp ping...", + log_line("%s: arp: Still waiting for DHCP agent to reply to arp ping...", client_config.interface); - if (arp_ping(cs, cs->serverAddr) < 0) + if (arp_ping(cs, cs->srcAddr) < 0) log_warning("%s: arp: Failed to send ARP ping in retransmission.", client_config.interface); } @@ -595,20 +591,10 @@ static void arp_gw_query_timeout(struct client_state_t *cs, long long nowts) log_warning("%s: arp: Failed to send ARP ping in retransmission.", client_config.interface); } - // Here it can be a bit tricky, since DHCP proxies do exist and can - // mean that the DHCP server will not be on the local segment and thus - // will not respond to ARP. Therefore, the serverAddr can only be - // treated as extra information that may or may not exist. if (!cs->got_server_arp) { - if (++cs->server_arp_tries > MAX_DHCP_SERVER_HWADDR_QUERIES) { - log_line("%s: arp: No ARP response from DHCP server after %d tries. Proceeding.", - client_config.interface, cs->server_arp_tries); - arp_do_gw_query_done(cs); - return; - } - log_line("%s: arp: Still looking for DHCP server hardware address...", + log_line("%s: arp: Still looking for DHCP agent hardware address...", client_config.interface); - if (arp_ping(cs, cs->serverAddr) < 0) + if (arp_ping(cs, cs->srcAddr) < 0) log_warning("%s: arp: Failed to send ARP ping in retransmission.", client_config.interface); } @@ -680,16 +666,16 @@ static void arp_do_gw_query(struct client_state_t *cs) cs->routerArp[2], cs->routerArp[3], cs->routerArp[4], cs->routerArp[5]); cs->got_router_arp = 1; - if (cs->routerAddr == cs->serverAddr) + if (cs->routerAddr == cs->srcAddr) goto server_is_router; if (cs->got_server_arp) arp_do_gw_query_done(cs); return; } - if (!memcmp(garp.reply.sip4, &cs->serverAddr, 4)) { + if (!memcmp(garp.reply.sip4, &cs->srcAddr, 4)) { server_is_router: memcpy(cs->serverArp, garp.reply.smac, 6); - log_line("%s: arp: DHCP Server hardware address %02x:%02x:%02x:%02x:%02x:%02x", + log_line("%s: arp: DHCP agent hardware address %02x:%02x:%02x:%02x:%02x:%02x", client_config.interface, cs->serverArp[0], cs->serverArp[1], cs->serverArp[2], cs->serverArp[3], cs->serverArp[4], cs->serverArp[5]); @@ -723,14 +709,7 @@ static void arp_do_gw_check(struct client_state_t *cs) // Success only if the router/gw MAC matches stored value if (!memcmp(cs->routerArp, garp.reply.smac, 6)) { garp.router_replied = true; - // Handle the case where the DHCP server is proxed from - // a different segment and will never reply. - if (!cs->got_server_arp && - cs->server_arp_tries > MAX_DHCP_SERVER_HWADDR_QUERIES) { - arp_gw_success(cs); - return; - } - if (cs->routerAddr == cs->serverAddr) + if (cs->routerAddr == cs->srcAddr) goto server_is_router; if (garp.server_replied) arp_gw_success(cs); @@ -741,7 +720,7 @@ static void arp_do_gw_check(struct client_state_t *cs) } return; } - if (!memcmp(garp.reply.sip4, &cs->serverAddr, 4)) { + if (!memcmp(garp.reply.sip4, &cs->srcAddr, 4)) { server_is_router: // Success only if the server MAC matches stored value if (!memcmp(cs->serverArp, garp.reply.smac, 6)) { @@ -749,7 +728,7 @@ server_is_router: if (garp.router_replied) arp_gw_success(cs); } else { - log_line("%s: arp: DHCP server is different. Getting a new lease.", + log_line("%s: arp: DHCP agent is different. Getting a new lease.", client_config.interface); arp_gw_failed(cs); } diff --git a/src/dhcp.c b/src/dhcp.c index 8528265..66036e2 100644 --- a/src/dhcp.c +++ b/src/dhcp.c @@ -174,7 +174,8 @@ static int get_raw_packet_validate_bpf(struct ip_udp_dhcp_packet *packet) // Read a packet from a raw socket. Returns -1 on fatal error, -2 on // transient error. static ssize_t get_raw_packet(struct client_state_t *cs, - struct dhcpmsg *payload) + struct dhcpmsg *payload, + uint32_t *srcaddr) { struct ip_udp_dhcp_packet packet; memset(&packet, 0, sizeof packet); @@ -217,6 +218,8 @@ static ssize_t get_raw_packet(struct client_state_t *cs, client_config.interface); return -2; } + if (srcaddr) + *srcaddr = packet.ip.saddr; memcpy(payload, &packet.data, l); return l; } @@ -370,7 +373,8 @@ void handle_packet(struct client_state_t *cs) if (cs->listenFd < 0) return; struct dhcpmsg packet; - ssize_t r = get_raw_packet(cs, &packet); + uint32_t srcaddr; + ssize_t r = get_raw_packet(cs, &packet, &srcaddr); if (r < 0) { // Not a transient issue handled by packet collection functions. if (r != -2) { @@ -384,7 +388,7 @@ void handle_packet(struct client_state_t *cs) uint8_t msgtype; if (!validate_dhcp_packet(cs, (size_t)r, &packet, &msgtype)) return; - packet_action(cs, &packet, msgtype); + packet_action(cs, &packet, msgtype, srcaddr); } // Initialize a DHCP client packet that will be sent to a server diff --git a/src/ndhc.h b/src/ndhc.h index c3d4903..660978f 100644 --- a/src/ndhc.h +++ b/src/ndhc.h @@ -42,8 +42,7 @@ struct client_state_t { int ifDeconfig; // Set if the interface has already been deconfigured. int epollFd, signalFd, listenFd, arpFd, nlFd; int nlPortId; - int server_arp_tries; - uint32_t clientAddr, serverAddr, routerAddr; + uint32_t clientAddr, serverAddr, srcAddr, routerAddr; uint32_t lease, renewTime, rebindTime, xid; struct nk_random_state_u32 rnd32_state; uint8_t routerArp[6], serverArp[6]; diff --git a/src/state.c b/src/state.c index 31db685..43129d5 100644 --- a/src/state.c +++ b/src/state.c @@ -42,9 +42,9 @@ #include "sys.h" static void selecting_packet(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype); + uint8_t msgtype, uint32_t srcaddr); static void an_packet(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype); + uint8_t msgtype, uint32_t srcaddr); static void selecting_timeout(struct client_state_t *cs, long long nowts); static void requesting_timeout(struct client_state_t *cs, long long nowts); static void bound_timeout(struct client_state_t *cs, long long nowts); @@ -57,7 +57,7 @@ static void frenew(struct client_state_t *cs); typedef struct { void (*packet_fn)(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype); + uint8_t msgtype, uint32_t srcaddr); void (*timeout_fn)(struct client_state_t *cs, long long nowts); void (*force_renew_fn)(struct client_state_t *cs); void (*force_release_fn)(struct client_state_t *cs); @@ -96,7 +96,6 @@ static void reinit_shared_deconfig(struct client_state_t *cs) num_dhcp_requests = 0; cs->got_router_arp = 0; cs->got_server_arp = 0; - cs->server_arp_tries = 0; memset(&cs->routerArp, 0, sizeof cs->routerArp); memset(&cs->serverArp, 0, sizeof cs->serverArp); arp_reset_send_stats(); @@ -225,8 +224,9 @@ static int validate_serverid(struct client_state_t *cs, struct dhcpmsg *packet, // Can transition to DS_BOUND or DS_SELECTING. static void an_packet(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype) + uint8_t msgtype, uint32_t srcaddr) { + (void)srcaddr; if (msgtype == DHCPACK) { if (!validate_serverid(cs, packet, "a DHCP ACK")) return; @@ -281,7 +281,7 @@ static void an_packet(struct client_state_t *cs, struct dhcpmsg *packet, } static void selecting_packet(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype) + uint8_t msgtype, uint32_t srcaddr) { if (msgtype == DHCPOFFER) { int found; @@ -289,9 +289,11 @@ static void selecting_packet(struct client_state_t *cs, struct dhcpmsg *packet, if (found) { char clibuf[INET_ADDRSTRLEN]; char svrbuf[INET_ADDRSTRLEN]; + char srcbuf[INET_ADDRSTRLEN]; cs->serverAddr = sid; cs->xid = packet->xid; cs->clientAddr = packet->yiaddr; + cs->srcAddr = srcaddr; cs->dhcpState = DS_REQUESTING; dhcp_wake_ts = curms(); num_dhcp_requests = 0; @@ -299,8 +301,10 @@ static void selecting_packet(struct client_state_t *cs, struct dhcpmsg *packet, clibuf, sizeof clibuf); inet_ntop(AF_INET, &(struct in_addr){.s_addr=cs->serverAddr}, svrbuf, sizeof svrbuf); - log_line("%s: Received an offer of %s from server %s.", - client_config.interface, clibuf, svrbuf); + inet_ntop(AF_INET, &(struct in_addr){.s_addr=cs->srcAddr}, + srcbuf, sizeof srcbuf); + log_line("%s: Received IP offer: %s from server %s via %s.", + client_config.interface, clibuf, svrbuf, srcbuf); } else { log_line("%s: Invalid offer received: it didn't have a server id.", client_config.interface); @@ -402,10 +406,10 @@ void ifnocarrier_action(struct client_state_t *cs) } void packet_action(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype) + uint8_t msgtype, uint32_t srcaddr) { if (dhcp_states[cs->dhcpState].packet_fn) - dhcp_states[cs->dhcpState].packet_fn(cs, packet, msgtype); + dhcp_states[cs->dhcpState].packet_fn(cs, packet, msgtype, srcaddr); } void timeout_action(struct client_state_t *cs, long long nowts) diff --git a/src/state.h b/src/state.h index fc35478..c16247f 100644 --- a/src/state.h +++ b/src/state.h @@ -46,7 +46,7 @@ typedef enum { void reinit_selecting(struct client_state_t *cs, int timeout); void packet_action(struct client_state_t *cs, struct dhcpmsg *packet, - uint8_t msgtype); + uint8_t msgtype, uint32_t srcaddr); void timeout_action(struct client_state_t *cs, long long nowts); void force_renew_action(struct client_state_t *cs); void force_release_action(struct client_state_t *cs);