]> git.ipfire.org Git - thirdparty/dhcp.git/commitdiff
Several time related improvements:
authorTomek Mrugalski <tomek@isc.org>
Thu, 21 Apr 2011 13:24:24 +0000 (13:24 +0000)
committerTomek Mrugalski <tomek@isc.org>
Thu, 21 Apr 2011 13:24:24 +0000 (13:24 +0000)
- 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]

RELNOTES
client/clparse.c
client/dhclient.c
client/dhclient.conf.5
common/conflex.c
includes/dhcpd.h
includes/dhctoken.h
server/dhcp.c
server/dhcpd.c

index 690c933c1e79c4727cf3126d0ae2bec3278161d0..e1dd9968db83b61010469ea6a0f5f6948afae71a 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -57,6 +57,38 @@ 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. 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
 
index c4245ce789da7dedad71baf52fea42a54206ba62..9de4ce26a135305fbe55e3ba4e24d34538312b3f 100644 (file)
@@ -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;
 }
-
index e4b533dc64735f7da62fcf12c173501581c3bca4..30ecb15c33089fa458c14381c7cd408f1a7fec51 100644 (file)
@@ -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.
-                                        */
-                                       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);
+                                       }
                                }
                        }
                }
@@ -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 {
@@ -4198,6 +4244,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.");
+}
index 23000883856447741b4381429bc25f0dbd20e150..65faa42cd51ff377a03ff8166601fa81c06de2ac 100644 (file)
@@ -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
index 27a96e1d515ed0e52f97f95ef02de57efca51263..81515bf9746cd191ca4f53dd16a3010faf000133 100644 (file)
@@ -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"))
index a096f1250afa64d1ac5398ab657b9417dc68ff31..f2e0ae96e66d81feb39a2d7081c944cd30047279 100644 (file)
@@ -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. */
index 6cfa04b0dd68517917490bb49197dcde0d56de66..766cc593679cacac1a4a06ccbc4144a09e2659f4 100644 (file)
@@ -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 &&  \
index e391ea6462f8f940e8cccb7e9c80a17a0f2ec021..57f39d5fe5397516909b3ac3405cb009b428f600 100644 (file)
@@ -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);
index b5577cd54aaee5ed321513603fd7bd58644dee7c..3239459f8fafc7f0c0c53f2d94aa2fb1b182a50b 100644 (file)
@@ -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);