]> git.ipfire.org Git - thirdparty/dhcp.git/commitdiff
Several time related improvements:
authorTomek Mrugalski <tomek@isc.org>
Thu, 21 Apr 2011 14:08:15 +0000 (14:08 +0000)
committerTomek Mrugalski <tomek@isc.org>
Thu, 21 Apr 2011 14:08:15 +0000 (14:08 +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 a1759758e331dcfb030503d843c3a666febca4a6..bf460ad820e231e9733c7ee06f9fb653d0278385 100644 (file)
--- 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
 
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 f9248f4171f6247f3d5cbbba860b0ef6901f03a0..3ddf3bb5ae08b64f7b0d7a520082bede3825168f 100644 (file)
@@ -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.");
+}
index fb82efe1bc66fc9f7f690565c013f733bc5c7f9f..ba74672604c09b24faf1370c4e5a03ec100c3bc1 100644 (file)
@@ -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
index 27a96e1d515ed0e52f97f95ef02de57efca51263..4a84ee6c5f97ae9b7a7211ea195d0f01c3caf247 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 3d2253f11fed791e05d51602ff850cd0a5a07d5d..7c2ed41876858381e403419cb638b25acf45c193 100644 (file)
@@ -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 &&  \
index 9d42a7aca0b35b39970d007bc0f5293506914d60..f398bfc2cd6875de86248510fa305fa0c94b6b09 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 7e15e3a4b1835fe8576d3f24097a4c72aa0b7523..448e16407a0b5ea60c1071812d34cf597ecd6254 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);