From: Tomek Mrugalski Date: Thu, 21 Apr 2011 14:08:15 +0000 (+0000) Subject: Several time related improvements: X-Git-Tag: v4_2_2b1~30 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3a24baf5fcb2a326a357df0d12e3e7f653a3ec60;p=thirdparty%2Fdhcp.git Several time related improvements: - set initial delay to 0 to speed up client start - added 'initial-delay' parameter to possibly revert to old behavior - better handling of very short (1 or 2s) leases - client lease records are recorded at most once every 15 seconds - ICMP ping-check is now timed more precisely - Servers that don't offer lease-time are now black-listed [ISC-Bugs #19660] --- diff --git a/RELNOTES b/RELNOTES index a1759758e..bf460ad82 100644 --- a/RELNOTES +++ b/RELNOTES @@ -57,6 +57,39 @@ work on other platforms. Please report any problems and suggested fixes to to allow the user to supply the pidfile name for the relay in v6 mode at configure time. [ISC-Bugs #23351] [ISC-Bugs #17541] + +- 'dhclient' no longer waits a random interval after first starting up to + begin in the INIT state. This conforms to RFC 2131, but elects not to + implement a 'SHOULD' direction in section 4.1. [ISC-Bugs #19660] + +- Added 'initial-delay' parameter that specifies maximum amount of time + before client goes to the INIT state. The default value is 0. In previous + versions of the code client could wait up to 5 seconds. The old behavior + may be restored by using 'initial-delay 5;' in the client config file. + [ISC-Bugs #19660] + +- ICMP ping-check should now sit closer to precisely the number of seconds + configured (or default 1), due to making use of the new microsecond + scale timer internally to dhcpd. This corrects a bug where the server + may immediately timeout an ICMP ping-check if it was made late in the + current second. [ISC-Bugs #19660] + +- The DHCP client will schedule renewal and rebinding events in + microseconds if the DHCP server provided a lease-time that would result + in sub-1-second timers. This corrects a bug where a 2-second or lower + lease-time would cause the DHCP client to enter an infinite loop by + scheduling renewal at zero seconds. [ISC-Bugs #19660] + +- Client lease records are recorded at most once every 15 seconds. This + keeps the client from filling the lease database disk quickly on very small + lease times. [ISC-Bugs #19660] + +- To defend against RFC 2131 non-compliant DHCP servers which fail to + advertise a lease-time (either mangled, or zero in value) the DHCP + client now adds the server to the reject list ACL and returns to INIT + state to hopefully find an RFC 2131 compliant server (or retry in INIT + forever). [ISC-Bugs #19660] + Changes since 4.2.1rc1 diff --git a/client/clparse.c b/client/clparse.c index c4245ce78..9de4ce26a 100644 --- a/client/clparse.c +++ b/client/clparse.c @@ -3,7 +3,7 @@ Parser for dhclient config and lease files... */ /* - * Copyright (c) 2004-2010 by Internet Systems Consortium, Inc. ("ISC") + * Copyright (c) 2004-2011 by Internet Systems Consortium, Inc. ("ISC") * Copyright (c) 1996-2003 by Internet Software Consortium * * Permission to use, copy, modify, and distribute this software for any @@ -128,6 +128,16 @@ isc_result_t read_client_conf () top_level_config.retry_interval = 300; top_level_config.backoff_cutoff = 15; top_level_config.initial_interval = 3; + + /* + * RFC 2131, section 4.4.1 specifies that the client SHOULD wait a + * random time between 1 and 10 seconds. However, we choose to not + * implement this default. If user is inclined to really have that + * delay, he is welcome to do so, using 'initial-delay X;' parameter + * in config file. + */ + top_level_config.initial_delay = 0; + top_level_config.bootp_policy = P_ACCEPT; top_level_config.script_name = path_dhclient_script; top_level_config.requested_options = default_requested_options; @@ -209,7 +219,7 @@ int read_client_conf_file (const char *name, struct interface_info *ip, const char *val; int token; isc_result_t status; - + if ((file = open (name, O_RDONLY)) < 0) return uerr2isc (errno); @@ -645,6 +655,11 @@ void parse_client_statement (cfile, ip, config) parse_lease_time (cfile, &config -> initial_interval); return; + case INITIAL_DELAY: + token = next_token (&val, (unsigned *)0, cfile); + parse_lease_time (cfile, &config -> initial_delay); + return; + case SCRIPT: token = next_token (&val, (unsigned *)0, cfile); parse_string (cfile, &config -> script_name, (unsigned *)0); @@ -2218,4 +2233,3 @@ int parse_allow_deny (oc, cfile, flag) skip_to_semi (cfile); return 0; } - diff --git a/client/dhclient.c b/client/dhclient.c index f9248f417..3ddf3bb5a 100644 --- a/client/dhclient.c +++ b/client/dhclient.c @@ -93,6 +93,7 @@ void run_stateless(int exit_mode); static void usage(void); static isc_result_t write_duid(struct data_string *duid); +static void add_reject(struct packet *packet); static int check_domain_name(const char *ptr, size_t len, int dots); static int check_domain_name_list(const char *ptr, size_t len, int dots); @@ -610,13 +611,28 @@ main(int argc, char **argv) { do_release(client); else { client->state = S_INIT; - /* Set up a timeout to start the - * initialization process. - */ - tv.tv_sec = cur_time + random() % 5; - tv.tv_usec = 0; - add_timeout(&tv, state_reboot, - client, 0, 0); + + if (top_level_config.initial_delay>0) + { + tv.tv_sec = 0; + if (top_level_config. + initial_delay>1) + tv.tv_sec = cur_time + + random() + % (top_level_config. + initial_delay-1); + tv.tv_usec = random() + % 1000000; + /* + * this gives better + * distribution than just + *whole seconds + */ + add_timeout(&tv, state_reboot, + client, 0, 0); + } else { + state_reboot(client); + } } } } @@ -1066,21 +1082,34 @@ void dhcpack (packet) } else client -> new -> expiry = 0; - if (!client -> new -> expiry) { + if (client->new->expiry == 0) { + struct timeval tv; + log_error ("no expiry time on offered lease."); - /* XXX this is going to be bad - if this _does_ - XXX happen, we should probably dynamically - XXX disqualify the DHCP server that gave us the - XXX bad packet from future selections and - XXX then go back into the init state. */ - state_init (client); + + /* Quench this (broken) server. Return to INIT to reselect. */ + add_reject(packet); + + /* 1/2 second delay to restart at INIT. */ + tv.tv_sec = cur_tv.tv_sec; + tv.tv_usec = cur_tv.tv_usec + 500000; + + if (tv.tv_usec >= 1000000) { + tv.tv_sec++; + tv.tv_usec -= 1000000; + } + + add_timeout(&tv, state_init, client, 0, 0); return; } - /* A number that looks negative here is really just very large, - because the lease expiry offset is unsigned. */ - if (client -> new -> expiry < 0) - client -> new -> expiry = TIME_MAX; + /* + * A number that looks negative here is really just very large, + * because the lease expiry offset is unsigned. + */ + if (client->new->expiry < 0) + client->new->expiry = TIME_MAX; + /* Take the server-provided renewal time if there is one. */ oc = lookup_option (&dhcp_universe, client -> new -> options, DHO_DHCP_RENEWAL_TIME); @@ -1191,8 +1220,10 @@ void bind_lease (client) return; } - /* Write out the new lease. */ - write_client_lease (client, client -> new, 0, 0); + /* Write out the new lease if it has been long enough. */ + if (!client->last_write || + (cur_time - client->last_write) >= MIN_LEASE_WRITE) + write_client_lease(client, client->new, 0, 0); /* Replace the old active lease with the new one. */ if (client -> active) @@ -1201,9 +1232,10 @@ void bind_lease (client) client -> new = (struct client_lease *)0; /* Set up a timeout to start the renewal process. */ - tv . tv_sec = client -> active -> renewal; - tv . tv_usec = 0; - add_timeout (&tv, state_bound, client, 0, 0); + tv.tv_sec = client->active->renewal; + tv.tv_usec = ((client->active->renewal - cur_tv.tv_sec) > 1) ? + random() % 1000000 : cur_tv.tv_usec; + add_timeout(&tv, state_bound, client, 0, 0); log_info ("bound to %s -- renewal in %ld seconds.", piaddr (client -> active -> address), @@ -1567,15 +1599,15 @@ void dhcpoffer (packet) /* If the selecting interval has expired, go immediately to state_selecting(). Otherwise, time out into state_selecting at the select interval. */ - if (stop_selecting <= 0) + if (stop_selecting <= cur_tv.tv_sec) state_selecting (client); else { - tv . tv_sec = stop_selecting; - tv . tv_usec = 0; - add_timeout (&tv, state_selecting, client, 0, 0); - cancel_timeout (send_discover, client); + tv.tv_sec = stop_selecting; + tv.tv_usec = cur_tv.tv_usec; + add_timeout(&tv, state_selecting, client, 0, 0); + cancel_timeout(send_discover, client); } - log_info ("%s", obuf); + log_info("%s", obuf); } /* Allocate a client_lease structure and initialize it from the parameters @@ -1875,9 +1907,15 @@ void send_discover (cpp) inaddr_any, &sockaddr_broadcast, (struct hardware *)0); - tv . tv_sec = cur_time + client -> interval; - tv . tv_usec = 0; - add_timeout (&tv, send_discover, client, 0, 0); + /* + * If we used 0 microseconds here, and there were other clients on the + * same network with a synchronized local clock (ntp), and a similar + * zero-microsecond-scheduler behavior, then we could be participating + * in a sub-second DOS ttck. + */ + tv.tv_sec = cur_tv.tv_sec + client->interval; + tv.tv_usec = client->interval > 1 ? random() % 1000000 : cur_tv.tv_usec; + add_timeout(&tv, send_discover, client, 0, 0); } /* state_panic gets called if we haven't received any offers in a preset @@ -1925,9 +1963,12 @@ void state_panic (cpp) log_info ("bound: renewal in %ld %s.", (long)(client -> active -> renewal - cur_time), "seconds"); - tv . tv_sec = client -> active -> renewal; - tv . tv_usec = 0; - add_timeout (&tv, state_bound, client, 0, 0); + tv.tv_sec = client->active->renewal; + tv.tv_usec = ((client->active->renewal - + cur_time) > 1) ? + random() % 1000000 : + cur_tv.tv_usec; + add_timeout(&tv, state_bound, client, 0, 0); } else { client -> state = S_BOUND; log_info ("bound: immediate renewal."); @@ -1983,11 +2024,11 @@ void state_panic (cpp) script_write_params (client, "alias_", client -> alias); script_go (client); client -> state = S_INIT; - tv . tv_sec = cur_time + - ((client -> config -> retry_interval + 1) / 2 + - (random () % client -> config -> retry_interval)); - tv . tv_usec = 0; - add_timeout (&tv, state_init, client, 0, 0); + tv.tv_sec = cur_tv.tv_sec + ((client->config->retry_interval + 1) / 2 + + (random() % client->config->retry_interval)); + tv.tv_usec = ((tv.tv_sec - cur_tv.tv_sec) > 1) ? + random() % 1000000 : cur_tv.tv_usec; + add_timeout(&tv, state_init, client, 0, 0); go_daemon (); } @@ -2142,9 +2183,10 @@ void send_request (cpp) from, &destination, (struct hardware *)0); - tv . tv_sec = cur_time + client -> interval; - tv . tv_usec = 0; - add_timeout (&tv, send_request, client, 0, 0); + tv.tv_sec = cur_tv.tv_sec + client->interval; + tv.tv_usec = ((tv.tv_sec - cur_tv.tv_sec) > 1) ? + random() % 1000000 : cur_tv.tv_usec; + add_timeout(&tv, send_request, client, 0, 0); } void send_decline (cpp) @@ -2626,6 +2668,9 @@ void rewrite_client_leases () write_client6_lease(client, client->active_lease, 1, 0); + + /* Reset last_write after rewrites. */ + client->last_write = 0; } } @@ -2644,6 +2689,9 @@ void rewrite_client_leases () write_client6_lease(client, client->active_lease, 1, 0); + + /* Reset last_write after rewrites. */ + client->last_write = 0; } } fflush (leaseFile); @@ -2987,12 +3035,15 @@ int write_client_lease (client, lease, rewrite, makesure) if (fflush(leaseFile) != 0) errors++; + client->last_write = cur_time; + if (!errors && makesure) { if (fsync (fileno (leaseFile)) < 0) { log_info ("write_client_lease: %m"); return 0; } } + return errors ? 0 : 1; } @@ -3554,7 +3605,6 @@ isc_result_t dhclient_interface_startup_hook (struct interface_info *interface) { struct interface_info *ip; struct client_state *client; - struct timeval tv; /* This code needs some rethinking. It doesn't test against a signal name, and it just kind of bulls into doing something @@ -3592,13 +3642,9 @@ isc_result_t dhclient_interface_startup_hook (struct interface_info *interface) if (ip -> flags & INTERFACE_RUNNING) continue; ip -> flags |= INTERFACE_RUNNING; - for (client = ip -> client; client; client = client -> next) { - client -> state = S_INIT; - /* Set up a timeout to start the initialization - process. */ - tv . tv_sec = cur_time + random () % 5; - tv . tv_usec = 0; - add_timeout (&tv, state_reboot, client, 0, 0); + for (client = ip->client ; client ; client = client->next) { + client->state = S_INIT; + state_reboot(client); } } return ISC_R_SUCCESS; @@ -3743,9 +3789,9 @@ isc_result_t dhcp_set_control_state (control_object_state_t oldstate, } if (newstate == server_shutdown) { - tv . tv_sec = cur_tv . tv_sec + 1; - tv . tv_usec = cur_tv . tv_usec; - add_timeout (&tv, shutdown_exit, 0, 0, 0); + tv.tv_sec = cur_tv.tv_sec; + tv.tv_usec = cur_tv.tv_usec + 1; + add_timeout(&tv, shutdown_exit, 0, 0, 0); } return ISC_R_SUCCESS; } @@ -3863,8 +3909,8 @@ client_dns_update_action(dhcp_ddns_cb_t *ddns_cb, /* and update our timer */ if (ddns_cb->timeout < 3600) ddns_cb->timeout *= 10; - tv.tv_sec = cur_time + ddns_cb->timeout; - tv.tv_usec = 0; + tv.tv_sec = cur_tv.tv_sec + ddns_cb->timeout; + tv.tv_usec = cur_tv.tv_usec; add_timeout(&tv, client_dns_update_timeout, ddns_cb, NULL, NULL); return; @@ -4032,8 +4078,8 @@ dhclient_schedule_updates(struct client_state *client, client->ddns_cb = ddns_cb; - tv.tv_sec = cur_time + offset; - tv.tv_usec = 0; + tv.tv_sec = cur_tv.tv_sec + offset; + tv.tv_usec = cur_tv.tv_usec; add_timeout(&tv, client_dns_update_timeout, ddns_cb, NULL, NULL); } else { @@ -4200,6 +4246,31 @@ static int check_option_values(struct universe *universe, return(0); } - +static void +add_reject(struct packet *packet) { + struct iaddrmatchlist *list; + + list = dmalloc(sizeof(struct iaddrmatchlist), MDL); + if (!list) + log_fatal ("no memory for reject list!"); + /* + * client_addr is misleading - it is set to source address in common + * code. + */ + list->match.addr = packet->client_addr; + /* Set mask to indicate host address. */ + list->match.mask.len = list->match.addr.len; + memset(list->match.mask.iabuf, 0xff, sizeof(list->match.mask.iabuf)); + + /* Append to reject list for the source interface. */ + list->next = packet->interface->client->config->reject_list; + packet->interface->client->config->reject_list = list; + + /* + * We should inform user that we won't be accepting this server + * anymore. + */ + log_info("Server added to list of rejected servers."); +} diff --git a/client/dhclient.conf.5 b/client/dhclient.conf.5 index fb82efe1b..ba7467260 100644 --- a/client/dhclient.conf.5 +++ b/client/dhclient.conf.5 @@ -1,6 +1,6 @@ -.\" $Id: dhclient.conf.5,v 1.25.24.6 2010/09/14 23:03:56 sar Exp $ +.\" $Id: dhclient.conf.5,v 1.25.24.7 2011/04/21 14:08:14 tomasz Exp $ .\" -.\" Copyright (c) 2009-2010 by Internet Systems Consortium, Inc. ("ISC") +.\" Copyright (c) 2009-2011 by Internet Systems Consortium, Inc. ("ISC") .\" Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC") .\" Copyright (c) 1996-2003 by Internet Software Consortium .\" @@ -163,6 +163,21 @@ is sent, the interval between messages is incremented by twice the current interval multiplied by a random number between zero and one. If it is greater than the backoff-cutoff amount, it is set to that amount. It defaults to ten seconds. +.PP +.I The initial-delay +.I statement +.PP + \fBinitial-delay \fItime\fR\fB;\fR +.PP +.I initial-delay +parameter sets the maximum time client can wait after start before +commencing first transmission. +According to RFC2131 Section 4.4.1, client should wait a random time between +startup and the actual first transmission. Previous versions of ISC DHCP +client used to wait random time up to 5 seconds, but that was unwanted +due to impact on startup time. As such, new versions have the default +initial delay set to 0. To restore old behavior, please set initial-delay +to 5. .SH LEASE REQUIREMENTS AND REQUESTS The DHCP protocol allows the client to request that the server send it specific information, and not send it other information that it is not diff --git a/common/conflex.c b/common/conflex.c index 27a96e1d5..4a84ee6c5 100644 --- a/common/conflex.c +++ b/common/conflex.c @@ -1051,6 +1051,8 @@ intern(char *atom, enum dhcp_token dfv) { return IP6_ADDRESS; if (!strcasecmp (atom + 1, "nitial-interval")) return INITIAL_INTERVAL; + if (!strcasecmp (atom + 1, "nitial-delay")) + return INITIAL_DELAY; if (!strcasecmp (atom + 1, "nterface")) return INTERFACE; if (!strcasecmp (atom + 1, "dentifier")) diff --git a/includes/dhcpd.h b/includes/dhcpd.h index a096f1250..f2e0ae96e 100644 --- a/includes/dhcpd.h +++ b/includes/dhcpd.h @@ -754,6 +754,10 @@ struct lease_state { # define MAX_DEFAULT_DDNS_TTL 3600 #endif +#if !defined (MIN_LEASE_WRITE) +# define MIN_LEASE_WRITE 15 +#endif + /* Client option names */ #define CL_TIMEOUT 1 @@ -1082,6 +1086,8 @@ struct client_config { TIME timeout; /* Start to panic if we don't get a lease in this time period when SELECTING. */ + TIME initial_delay; /* Set initial delay before first + transmission. */ TIME initial_interval; /* All exponential backoff intervals start here. */ TIME retry_interval; /* If the protocol failed to produce @@ -1134,7 +1140,7 @@ struct client_state { int envc; /* Number of entries in environment. */ struct option_state *sent_options; /* Options we sent. */ enum dhcp_state state; /* Current state for this interface. */ - + TIME last_write; /* Last time this state was written. */ /* DHCPv4 values. */ struct client_lease *active; /* Currently active lease. */ diff --git a/includes/dhctoken.h b/includes/dhctoken.h index 3d2253f11..7c2ed4187 100644 --- a/includes/dhctoken.h +++ b/includes/dhctoken.h @@ -3,7 +3,7 @@ Tokens for config file lexer and parser. */ /* - * Copyright (c) 2004,2007-2010 by Internet Systems Consortium, Inc. ("ISC") + * Copyright (c) 2004,2007-2011 by Internet Systems Consortium, Inc. ("ISC") * Copyright (c) 1996-2003 by Internet Software Consortium * * Permission to use, copy, modify, and distribute this software for any @@ -357,7 +357,8 @@ enum dhcp_token { CONFLICT_DONE = 660, AUTO_PARTNER_DOWN = 661, GETHOSTNAME = 662, - REWIND = 663 + REWIND = 663, + INITIAL_DELAY = 664 }; #define is_identifier(x) ((x) >= FIRST_TOKEN && \ diff --git a/server/dhcp.c b/server/dhcp.c index 9d42a7aca..f398bfc2c 100644 --- a/server/dhcp.c +++ b/server/dhcp.c @@ -2836,8 +2836,14 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp) log_debug ("Ping timeout: %ld", (long)ping_timeout); #endif - tv . tv_sec = cur_time + ping_timeout; - tv . tv_usec = 0; + /* + * Set a timeout for 'ping-timeout' seconds from NOW, including + * current microseconds. As ping-timeout defaults to 1, the + * exclusion of current microseconds causes a value somewhere + * /between/ zero and one. + */ + tv.tv_sec = cur_tv.tv_sec + ping_timeout; + tv.tv_usec = cur_tv.tv_usec; add_timeout (&tv, lease_ping_timeout, lease, (tvref_t)lease_reference, (tvunref_t)lease_dereference); diff --git a/server/dhcpd.c b/server/dhcpd.c index 7e15e3a4b..448e16407 100644 --- a/server/dhcpd.c +++ b/server/dhcpd.c @@ -201,8 +201,8 @@ static void omapi_listener_start (void *foo) if (result != ISC_R_SUCCESS) { log_error ("Can't start OMAPI protocol: %s", isc_result_totext (result)); - tv.tv_sec = cur_time + 5; - tv.tv_usec = 0; + tv.tv_sec = cur_tv.tv_sec + 5; + tv.tv_usec = cur_tv.tv_usec; add_timeout (&tv, omapi_listener_start, 0, 0, 0); } omapi_object_dereference (&listener, MDL);