From 503c609149fcbde42e345116844fe55db59c233f Mon Sep 17 00:00:00 2001 From: Floris Bos Date: Sun, 9 Apr 2017 23:07:13 +0100 Subject: [PATCH] --dhcp-reply-delay option to workaround PXE client bugs. Adds option to delay replying to DHCP packets by one or more seconds. This provides a workaround for a PXE boot firmware implementation that has a bug causing it to fail if it receives a (proxy) DHCP reply instantly. On Linux it looks up the exact receive time of the UDP packet with the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets come in around the same time. --- CHANGELOG | 5 ++- man/dnsmasq.8 | 6 +++ src/dhcp.c | 9 ++++- src/dnsmasq.c | 101 +++++++++++++++++++++++++++++--------------------- src/dnsmasq.h | 10 ++++- src/option.c | 37 +++++++++++++++++- src/rfc2131.c | 32 ++++++++++++++-- src/tftp.c | 15 ++++---- 8 files changed, 158 insertions(+), 57 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3ad5e47..7309a63 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -82,6 +82,9 @@ version 2.77 Allow use of MAC addresses with --tftp-unique-root. Thanks to Floris Bos for the patch. + + Add --dhcp-reply-delay option. Thanks to Floris Bos + for the patch. version 2.76 @@ -90,7 +93,7 @@ version 2.76 least, 0.0.0.0 accesses the local host, so could be targets for DNS rebinding. See RFC 5735 section 3 for details. Thanks to Stephen Röttger for the bug report. - + Enhance --add-subnet to allow arbitrary subnet addresses. Thanks to Ed Barsley for the patch. diff --git a/man/dnsmasq.8 b/man/dnsmasq.8 index f7163e5..787c104 100644 --- a/man/dnsmasq.8 +++ b/man/dnsmasq.8 @@ -1790,6 +1790,12 @@ a router to advertise prefixes but not a route via itself. .B --ra-param=low,60,1200 The interface field may include a wildcard. .TP +.B --dhcp-reply-delay=[tag:,] +Delays sending DHCPOFFER and proxydhcp replies for at least the specified number of seconds. +This can be used as workaround for bugs in PXE boot firmware that does not function properly when +receiving an instant reply. +This option takes into account the time already spent waiting (e.g. performing ping check) if any. +.TP .B --enable-tftp[=[,]] Enable the TFTP server function. This is deliberately limited to that needed to net-boot a client. Only reading is allowed; the tsize and diff --git a/src/dhcp.c b/src/dhcp.c index 08952c8..ada1be8 100644 --- a/src/dhcp.c +++ b/src/dhcp.c @@ -149,8 +149,10 @@ void dhcp_packet(time_t now, int pxe_fd) int rcvd_iface_index; struct in_addr iface_addr; struct iface_param parm; + time_t recvtime = now; #ifdef HAVE_LINUX_NETWORK struct arpreq arp_req; + struct timeval tv; #endif union { @@ -177,6 +179,9 @@ void dhcp_packet(time_t now, int pxe_fd) return; #if defined (HAVE_LINUX_NETWORK) + if (ioctl(fd, SIOCGSTAMP, &tv) == 0) + recvtime = tv.tv_sec; + if (msg.msg_controllen >= sizeof(struct cmsghdr)) for (cmptr = CMSG_FIRSTHDR(&msg); cmptr; cmptr = CMSG_NXTHDR(&msg, cmptr)) if (cmptr->cmsg_level == IPPROTO_IP && cmptr->cmsg_type == IP_PKTINFO) @@ -335,14 +340,14 @@ void dhcp_packet(time_t now, int pxe_fd) lease_prune(NULL, now); /* lose any expired leases */ iov.iov_len = dhcp_reply(parm.current, ifr.ifr_name, iface_index, (size_t)sz, - now, unicast_dest, &is_inform, pxe_fd, iface_addr); + now, unicast_dest, &is_inform, pxe_fd, iface_addr, recvtime); lease_update_file(now); lease_update_dns(0); if (iov.iov_len == 0) return; } - + msg.msg_name = &dest; msg.msg_namelen = sizeof(dest); msg.msg_control = NULL; diff --git a/src/dnsmasq.c b/src/dnsmasq.c index d2cc7cc..70b84dc 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -1747,29 +1747,15 @@ int icmp_ping(struct in_addr addr) { /* Try and get an ICMP echo from a machine. */ - /* Note that whilst in the three second wait, we check for - (and service) events on the DNS and TFTP sockets, (so doing that - better not use any resources our caller has in use...) - but we remain deaf to signals or further DHCP packets. */ - - /* There can be a problem using dnsmasq_time() to end the loop, since - it's not monotonic, and can go backwards if the system clock is - tweaked, leading to the code getting stuck in this loop and - ignoring DHCP requests. To fix this, we check to see if select returned - as a result of a timeout rather than a socket becoming available. We - only allow this to happen as many times as it takes to get to the wait time - in quarter-second chunks. This provides a fallback way to end loop. */ - - int fd, rc; + int fd; struct sockaddr_in saddr; struct { struct ip ip; struct icmp icmp; } packet; unsigned short id = rand16(); - unsigned int i, j, timeout_count; + unsigned int i, j; int gotreply = 0; - time_t start, now; #if defined(HAVE_LINUX_NETWORK) || defined (HAVE_SOLARIS_NETWORK) if ((fd = make_icmp_sock()) == -1) @@ -1799,14 +1785,46 @@ int icmp_ping(struct in_addr addr) while (retry_send(sendto(fd, (char *)&packet.icmp, sizeof(struct icmp), 0, (struct sockaddr *)&saddr, sizeof(saddr)))); - for (now = start = dnsmasq_time(), timeout_count = 0; - (difftime(now, start) < (float)PING_WAIT) && (timeout_count < PING_WAIT * 4);) + gotreply = delay_dhcp(dnsmasq_time(), PING_WAIT, fd, addr.s_addr, id); + +#if defined(HAVE_LINUX_NETWORK) || defined(HAVE_SOLARIS_NETWORK) + while (retry_send(close(fd))); +#else + opt = 1; + setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &opt, sizeof(opt)); +#endif + + return gotreply; +} + +int delay_dhcp(time_t start, int sec, int fd, uint32_t addr, unsigned short id) +{ + /* Delay processing DHCP packets for "sec" seconds counting from "start". + If "fd" is not -1 it will stop waiting if an ICMP echo reply is received + from "addr" with ICMP ID "id" and return 1 */ + + /* Note that whilst waiting, we check for + (and service) events on the DNS and TFTP sockets, (so doing that + better not use any resources our caller has in use...) + but we remain deaf to signals or further DHCP packets. */ + + /* There can be a problem using dnsmasq_time() to end the loop, since + it's not monotonic, and can go backwards if the system clock is + tweaked, leading to the code getting stuck in this loop and + ignoring DHCP requests. To fix this, we check to see if select returned + as a result of a timeout rather than a socket becoming available. We + only allow this to happen as many times as it takes to get to the wait time + in quarter-second chunks. This provides a fallback way to end loop. */ + + int rc, timeout_count; + time_t now; + + for (now = dnsmasq_time(), timeout_count = 0; + (difftime(now, start) <= (float)sec) && (timeout_count < sec * 4);) { - struct sockaddr_in faddr; - socklen_t len = sizeof(faddr); - poll_reset(); - poll_listen(fd, POLLIN); + if (fd != -1) + poll_listen(fd, POLLIN); set_dns_listeners(now); set_log_writer(); @@ -1823,10 +1841,10 @@ int icmp_ping(struct in_addr addr) timeout_count++; now = dnsmasq_time(); - + check_log_writer(0); check_dns_listeners(now); - + #ifdef HAVE_DHCP6 if (daemon->doing_ra && poll_check(daemon->icmp6fd, POLLIN)) icmp6_packet(now); @@ -1836,27 +1854,26 @@ int icmp_ping(struct in_addr addr) check_tftp_listeners(now); #endif - if (poll_check(fd, POLLIN) && - recvfrom(fd, &packet, sizeof(packet), 0, - (struct sockaddr *)&faddr, &len) == sizeof(packet) && - saddr.sin_addr.s_addr == faddr.sin_addr.s_addr && - packet.icmp.icmp_type == ICMP_ECHOREPLY && - packet.icmp.icmp_seq == 0 && - packet.icmp.icmp_id == id) - { - gotreply = 1; - break; + if (fd != -1) + { + struct { + struct ip ip; + struct icmp icmp; + } packet; + struct sockaddr_in faddr; + socklen_t len = sizeof(faddr); + + if (poll_check(fd, POLLIN) && + recvfrom(fd, &packet, sizeof(packet), 0, (struct sockaddr *)&faddr, &len) == sizeof(packet) && + addr == faddr.sin_addr.s_addr && + packet.icmp.icmp_type == ICMP_ECHOREPLY && + packet.icmp.icmp_seq == 0 && + packet.icmp.icmp_id == id) + return 1; } } - -#if defined(HAVE_LINUX_NETWORK) || defined(HAVE_SOLARIS_NETWORK) - while (retry_send(close(fd))); -#else - opt = 1; - setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &opt, sizeof(opt)); -#endif - return gotreply; + return 0; } #endif diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 1fc3bec..25e4ad9 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -705,6 +705,12 @@ struct tag_if { struct tag_if *next; }; +struct delay_config { + int delay; + struct dhcp_netid *netid; + struct delay_config *next; +}; + struct hwaddr_config { int hwaddr_len, hwaddr_type; unsigned char hwaddr[DHCP_CHADDR_MAX]; @@ -975,6 +981,7 @@ extern struct daemon { struct tag_if *tag_if; struct addr_list *override_relays; struct dhcp_relay *relay4, *relay6; + struct delay_config *delay_conf; int override; int enable_pxe; int doing_ra, doing_dhcp6; @@ -1333,7 +1340,7 @@ void lease_add_extradata(struct dhcp_lease *lease, unsigned char *data, /* rfc2131.c */ #ifdef HAVE_DHCP size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, - size_t sz, time_t now, int unicast_dest, int *is_inform, int pxe_fd, struct in_addr fallback); + size_t sz, time_t now, int unicast_dest, int *is_inform, int pxe_fd, struct in_addr fallback, time_t recvtime); unsigned char *extended_hwaddr(int hwtype, int hwlen, unsigned char *hwaddr, int clid_len, unsigned char *clid, int *len_out); #endif @@ -1342,6 +1349,7 @@ unsigned char *extended_hwaddr(int hwtype, int hwlen, unsigned char *hwaddr, #ifdef HAVE_DHCP int make_icmp_sock(void); int icmp_ping(struct in_addr addr); +int delay_dhcp(time_t start, int sec, int fd, uint32_t addr, unsigned short id); #endif void queue_event(int event); void send_alarm(time_t event, time_t now); diff --git a/src/option.c b/src/option.c index a3ddec1..3112482 100644 --- a/src/option.c +++ b/src/option.c @@ -159,6 +159,7 @@ struct myoption { #define LOPT_SCRIPT_ARP 347 #define LOPT_DHCPTTL 348 #define LOPT_TFTP_MTU 349 +#define LOPT_REPLY_DELAY 350 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -323,6 +324,7 @@ static const struct myoption opts[] = { "dns-loop-detect", 0, 0, LOPT_LOOP_DETECT }, { "script-arp", 0, 0, LOPT_SCRIPT_ARP }, { "dhcp-ttl", 1, 0 , LOPT_DHCPTTL }, + { "dhcp-reply-delay", 1, 0, LOPT_REPLY_DELAY }, { NULL, 0, 0, 0 } }; @@ -494,6 +496,7 @@ static struct { { LOPT_LOOP_DETECT, OPT_LOOP_DETECT, NULL, gettext_noop("Detect and remove DNS forwarding loops."), NULL }, { LOPT_IGNORE_ADDR, ARG_DUP, "", gettext_noop("Ignore DNS responses containing ipaddr."), NULL }, { LOPT_DHCPTTL, ARG_ONE, "", gettext_noop("Set TTL in DNS responses with DHCP-derived addresses."), NULL }, + { LOPT_REPLY_DELAY, ARG_ONE, "", gettext_noop("Delay DHCP replies for at least number of seconds."), NULL }, { 0, 0, NULL, NULL, NULL } }; @@ -3317,11 +3320,43 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma break; } + case LOPT_REPLY_DELAY: /* --dhcp-reply-delay */ + { + struct dhcp_netid *id = NULL; + while (is_tag_prefix(arg)) + { + struct dhcp_netid *newid = opt_malloc(sizeof(struct dhcp_netid)); + newid->next = id; + id = newid; + comma = split(arg); + newid->net = opt_string_alloc(arg+4); + arg = comma; + }; + + if (!arg) + ret_err(gen_err); + else + { + struct delay_config *new; + int delay; + if (!atoi_check(arg, &delay)) + ret_err(gen_err); + + new = opt_malloc(sizeof(struct delay_config)); + new->delay = delay; + new->netid = id; + new->next = daemon->delay_conf; + daemon->delay_conf = new; + } + + break; + } + case LOPT_PXE_PROMT: /* --pxe-prompt */ { struct dhcp_opt *new = opt_malloc(sizeof(struct dhcp_opt)); int timeout; - + new->netid = NULL; new->opt = 10; /* PXE_MENU_PROMPT */ diff --git a/src/rfc2131.c b/src/rfc2131.c index 3e97402..6a8f0db 100644 --- a/src/rfc2131.c +++ b/src/rfc2131.c @@ -64,9 +64,10 @@ static int prune_vendor_opts(struct dhcp_netid *netid); static struct dhcp_opt *pxe_opts(int pxe_arch, struct dhcp_netid *netid, struct in_addr local, time_t now); struct dhcp_boot *find_boot(struct dhcp_netid *netid); static int pxe_uefi_workaround(int pxe_arch, struct dhcp_netid *netid, struct dhcp_packet *mess, struct in_addr local, time_t now, int pxe); - +static void apply_delay(u32 xid, time_t recvtime, struct dhcp_netid *netid); + size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, - size_t sz, time_t now, int unicast_dest, int *is_inform, int pxe, struct in_addr fallback) + size_t sz, time_t now, int unicast_dest, int *is_inform, int pxe, struct in_addr fallback, time_t recvtime) { unsigned char *opt, *clid = NULL; struct dhcp_lease *ltmp, *lease = NULL; @@ -918,6 +919,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, log_packet("PXE", NULL, emac, emac_len, iface_name, ignore ? "proxy-ignored" : "proxy", NULL, mess->xid); log_tags(tagif_netid, ntohl(mess->xid)); + if (!ignore) + apply_delay(mess->xid, recvtime, tagif_netid); return ignore ? 0 : dhcp_packet_size(mess, agent_id, real_end); } } @@ -1058,7 +1061,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, } log_tags(tagif_netid, ntohl(mess->xid)); - + apply_delay(mess->xid, recvtime, tagif_netid); log_packet("DHCPOFFER" , &mess->yiaddr, emac, emac_len, iface_name, NULL, NULL, mess->xid); time = calc_time(context, config, option_find(mess, sz, OPTION_LEASE_TIME, 4)); @@ -2626,6 +2629,29 @@ static void do_options(struct dhcp_context *context, } } +static void apply_delay(u32 xid, time_t recvtime, struct dhcp_netid *netid) +{ + struct delay_config *delay_conf; + + /* Decide which delay_config option we're using */ + for (delay_conf = daemon->delay_conf; delay_conf; delay_conf = delay_conf->next) + if (match_netid(delay_conf->netid, netid, 0)) + break; + + if (!delay_conf) + /* No match, look for one without a netid */ + for (delay_conf = daemon->delay_conf; delay_conf; delay_conf = delay_conf->next) + if (match_netid(delay_conf->netid, netid, 1)) + break; + + if (delay_conf) + { + if (!option_bool(OPT_QUIET_DHCP)) + my_syslog(MS_DHCP | LOG_INFO, _("%u reply delay: %d"), ntohl(xid), delay_conf->delay); + delay_dhcp(recvtime, delay_conf->delay, -1, 0, 0); + } +} + #endif diff --git a/src/tftp.c b/src/tftp.c index be0d474..fb0b658 100644 --- a/src/tftp.c +++ b/src/tftp.c @@ -394,11 +394,12 @@ void tftp_request(struct listener *listen, time_t now) if (stat(daemon->namebuff, &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)) daemon->namebuff[oldlen] = 0; } + if (option_bool(OPT_TFTP_APREF_MAC)) { unsigned char *macaddr = NULL; unsigned char macbuf[DHCP_CHADDR_MAX]; - + #ifdef HAVE_DHCP if (daemon->dhcp && peer.sa.sa_family == AF_INET) { @@ -408,11 +409,11 @@ void tftp_request(struct listener *listen, time_t now) macaddr = lease->hwaddr; } #endif - + /* If no luck, try to find in ARP table. This only works if client is in same (V)LAN */ if (!macaddr && find_mac(&peer, macbuf, 1, now) > 0) - macaddr = macbuf; - + macaddr = macbuf; + if (macaddr) { size_t oldlen = strlen(daemon->namebuff); @@ -420,13 +421,13 @@ void tftp_request(struct listener *listen, time_t now) snprintf(daemon->namebuff + oldlen, (MAXDNAME-1) - oldlen, "%.2x-%.2x-%.2x-%.2x-%.2x-%.2x/", macaddr[0], macaddr[1], macaddr[2], macaddr[3], macaddr[4], macaddr[5]); - + /* remove unique-directory if it doesn't exist */ if (stat(daemon->namebuff, &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)) daemon->namebuff[oldlen] = 0; } } - + /* Absolute pathnames OK if they match prefix */ if (filename[0] == '/') { @@ -439,7 +440,7 @@ void tftp_request(struct listener *listen, time_t now) else if (filename[0] == '/') daemon->namebuff[0] = 0; strncat(daemon->namebuff, filename, (MAXDNAME-1) - strlen(daemon->namebuff)); - + /* check permissions and open file */ if ((transfer->file = check_tftp_fileperm(&len, prefix))) {