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]
2025-09-01 14:55:30 +00:00 · 2011-04-21 13:24:24 +00:00
parent 5ce8d6e749
commit 5d082abd92
9 changed files with 219 additions and 71 deletions
--- a/32
+++ b/32
@@ -58,6 +58,38 @@ work on other platforms. Please report any problems and suggested fixes to
  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. The goal of this change 
+  is to start up faster.
+  
+- 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.
+
+- 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.
+
+- 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.
+
+- 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.
+
+- 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).
+  
 			Changes since 4.2.0

 - Documentation cleanup covering multiple tickets
--- 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;
@@ -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;
 }
-
--- a/client/dhclient.c
+++ b/client/dhclient.c
@@ -91,6 +91,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);
@@ -608,13 +609,28 @@ main(int argc, char **argv) {
 					do_release(client);
 				else {
 					client->state = S_INIT;
-					/* Set up a timeout to start the
-					 * initialization process.
+
+					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
 						 */
-					tv.tv_sec = cur_time + random() % 5;
-					tv.tv_usec = 0;
 						add_timeout(&tv, state_reboot,
 						            client, 0, 0);
+					} else {
+						state_reboot(client);
+					}
 				}
 			}
 		}
@@ -1064,21 +1080,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);
@@ -1189,8 +1218,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)
@@ -1199,9 +1230,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),
@@ -1565,15 +1597,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
@@ -1873,9 +1905,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
@@ -1923,9 +1961,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.");
@@ -1981,11 +2022,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 ();
 }

@@ -2140,9 +2181,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)
@@ -2624,6 +2666,9 @@ void rewrite_client_leases ()
 				write_client6_lease(client,
 						    client->active_lease,
 						    1, 0);
+
+			/* Reset last_write after rewrites. */
+			client->last_write = 0;
 		}
 	}

@@ -2642,6 +2687,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);
@@ -2985,12 +3033,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;
 }

@@ -3552,7 +3603,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
@@ -3590,13 +3640,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;
@@ -3741,9 +3787,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;
 }
@@ -3861,8 +3907,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;
@@ -4030,8 +4076,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 {
@@ -4199,5 +4245,30 @@ 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.");
+}
--- a/client/dhclient.conf.5
+++ b/client/dhclient.conf.5
@@ -1,6 +1,6 @@
-.\"	$Id: dhclient.conf.5,v 1.31 2010/09/14 23:05:39 sar Exp $
+.\"	$Id: dhclient.conf.5,v 1.32 2011/04/21 13:24:24 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
--- 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"))
--- 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. */
--- a/includes/dhctoken.h
+++ b/includes/dhctoken.h
@@ -3,7 +3,8 @@
   Tokens for config file lexer and parser. */

 /*
- * Copyright (c) 2004,2007-2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004,2007-2009,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 +358,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 &&	\
--- 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);
--- 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);