[master] Replaced SERVER_ID_CHECK compile switch with runtime config parameter

    Merges in rt37551.

diff --git a/RELNOTES b/RELNOTES
index 342e851f58c5e350b4ebefee9d9c893f4f01091f..4a4b9fb4407a3570b3cffcc251f491737b43cc2c 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -170,6 +170,15 @@ by Eric Young (eay@cryptsoft.com).
   components.
   [ISC-Bugs #20558]
 
+- Added the server-id-check parameter to the server configuration.
+  This parameter allows run-time control over whether or not a server,
+  participating in failover, verifies the dhcp-server-identifier option in
+  DHCP REQUESTs against the server’s id before processing the request.
+  Formerly, enabling this behavior was done at compilation time through
+  the use of the #define, SERVER_ID_CHECK, which has been removed from site.h
+  The functionality is now only available through the new runtime paramater.
+  [ISC-Bugs #37551]
+
                        Changes since 4.3.1b1
 
 - Modify the linux and openwrt dhclient scripts to process information

diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index 625a0bd64f676603c0bbdaab65354dd575281112..f5ac1f4c9c930a90e74cf34f5be6e1e39820e63b 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -740,6 +740,7 @@ struct lease_state {
 #define SV_LOG_THRESHOLD_LOW           83
 #define SV_LOG_THRESHOLD_HIGH          84
 #define SV_ECHO_CLIENT_ID              85
+#define SV_SERVER_ID_CHECK             86
 
 #if !defined (DEFAULT_PING_TIMEOUT)
 # define DEFAULT_PING_TIMEOUT 1
@@ -1950,6 +1951,7 @@ extern struct timeval cur_tv;
 
 extern int ddns_update_style;
 extern int dont_use_fsync;
+extern int server_id_check;
 
 extern const char *path_dhcpd_conf;
 extern const char *path_dhcpd_db;

diff --git a/includes/site.h b/includes/site.h
index 19a2e11faad3f040df649c40cb01ef535c463192..bbc0c91849791832c83e8e332494a3bf08c71381 100644 (file)
--- a/includes/site.h
+++ b/includes/site.h
@@ -246,32 +246,9 @@
 
 #define SERVER_ID_FOR_NAK
 
-/* When processing a request do a simple check to compare the
-   server id the client sent with the one the server would send.
-   In order to minimize the complexity of the code the server
-   only checks for a server id option in the global and subnet
-   scopes.  Complicated configurations may result in differnet
-   server ids for this check and when the server id for a reply
-   packet is determined, which would prohibit the server from
-   responding.
-
-   The primary use for this option is when a client broadcasts
-   a request but requires the response to come from one of the
-   failover peers.  An example of this would be when a client
-   reboots while its lease is still active - in this case both
-   servers will normally respond.  Most of the time the client
-   won't check the server id and can use either of the responses.
-   However if the client does check the server id it may reject
-   the response if it came from the wrong peer.  If the timing
-   is such that the "wrong" peer responds first most of the time
-   the client may not get an address for some time.
-
-   Currently this option is only available when failover is in
-   use.
-
-   Care should be taken before enabling this option. */
-
-/* #define SERVER_ID_CHECK */
+/* NOTE:  SERVER_ID_CHECK switch has been removed. Enabling server id
+ * checking is now done via the server-id-check statement. Please refer
+ * to the dhcpd manpage (server/dhcpd.conf.5) */
 
 /* Include code to do a slow transition of DDNS records
    from the interim to the standard version, or backwards.

diff --git a/server/dhcp.c b/server/dhcp.c
index 770dce60b991f4f9000b54b91456296af8e5ce5a..552955c817916755e942a87d45b901ae272e25cd 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -553,31 +553,28 @@ void dhcprequest (packet, ms_nulltp, ip_lease)
                        goto out;
                }
 
-#if defined(SERVER_ID_CHECK)
-               /* Do a quick check on the server source address to see if
-                  it is ours.  sip is the incoming servrer id.  To avoid
-                  problems with confused clients we do some sanity checks
-                  to verify sip's length and that it isn't all zeros.
-                  We then get the server id we would likely use for this
-                  packet and compare them.  If they don't match it we assume
-                  we didn't send the offer and so we don't process the request.
-               */
-
-               if ((sip.len == 4) &&
+               /* If server-id-check is enabled, verify that the client's
+                * server source address (sip from incoming packet) is ours.
+                * To avoid problems with confused clients we do some sanity
+                * checks to verify sip's length and that it isn't all zeros.
+                * We then get the server id we would likely use for this
+                * packet and compare them.  If they don't match it we assume
+                * we didn't send the offer and so we don't process the
+                * request. */
+               if ((server_id_check == 1) && (sip.len == 4) &&
                    (memcmp(sip.iabuf, "\0\0\0\0", sip.len) != 0)) {
                        struct in_addr from;
                        struct option_state *eval_options = NULL;
 
                        eval_network_statements(&eval_options, packet, NULL);
-                       get_server_source_address(&from, eval_options, NULL,
-                                                 packet);
+                       get_server_source_address(&from, eval_options,
+                                                 NULL, packet);
                        option_state_dereference (&eval_options, MDL);
                        if (memcmp(sip.iabuf, &from, sip.len) != 0) {
                                log_debug("%s: not our server id", msgbuf);
                                goto out;
                        }
                }
-#endif /* if defined(SERVER_ID_CHECK) */
 
                /* At this point it's possible that we will get a broadcast
                   DHCPREQUEST for a lease that we didn't offer, because

diff --git a/server/dhcpd.c b/server/dhcpd.c
index 6747c209e0088b96594a5adb6db796f8008bd9e9..ff5968c5293cc0f88062c980a1ac89ef9749ce40 100644 (file)
--- a/server/dhcpd.c
+++ b/server/dhcpd.c
@@ -72,6 +72,7 @@ option server.ddns-rev-domainname = \"in-addr.arpa.\";";
 #endif /* NSUPDATE */
 int ddns_update_style;
 int dont_use_fsync = 0; /* 0 = default, use fsync, 1 = don't use fsync */
+int server_id_check = 0; /* 0 = default, don't check server id, 1 = do check */
 
 const char *path_dhcpd_conf = _PATH_DHCPD_CONF;
 const char *path_dhcpd_db = _PATH_DHCPD_DB;
@@ -1078,6 +1079,14 @@ void postconf_initialization (int quiet)
                log_error("Not using fsync() to flush lease writes");
        }
 
+       oc = lookup_option(&server_universe, options, SV_SERVER_ID_CHECK);
+       if ((oc != NULL) &&
+          evaluate_boolean_option_cache(NULL, NULL, NULL, NULL, options, NULL,
+                                        &global_scope, oc, MDL)) {
+               log_info("Setting server-id-check true");
+               server_id_check = 1;
+       }
+
        /* Don't need the options anymore. */
        option_state_dereference(&options, MDL);
 }

diff --git a/server/dhcpd.conf.5 b/server/dhcpd.conf.5
index 10878704ebf4210f887057f602b49ce99085d6b5..e437ce3cfe7c5c2e46778931c051dc4d42138ae3 100644 (file)
--- a/server/dhcpd.conf.5
+++ b/server/dhcpd.conf.5
@@ -2792,6 +2792,44 @@ to using the server-identifier statement.
 .RE
 .PP
 The
+.I server-id-check
+statement
+.RS 0.25i
+.PP
+.B server-id-check \fIflag\fR\fB;\fR
+.PP
+The server-id-check statement is used to control whether or not a server,
+participating in failover, verifies that the value of the
+dhcp-server-identifier option in received DHCP REQUESTs match the server's
+id before processing the request. Server id checking is disabled by default.
+Setting this flag enables id checking and thereafter the server will only
+process requests that match.  Note the flag setting should be consistent
+between failover partners.
+.PP
+Unless overridden by use of the server-identifier statement, the value the
+server uses as its id will be the first IP address associated with the
+physical network interface on which the request arrived.
+.PP
+In order to reduce runtime overhead the server only checks for a server id
+option in the global and subnet scopes.  Complicated configurations
+may result in differnet server ids for this check and when the server id for
+a reply packet is determined, which would prohibit the server from responding.
+.PP
+The primary use for this option is when a client broadcasts a request
+but requires that the response come from a specific failover peer.
+An example of this would be when a client reboots while its lease is still
+active - in this case both servers will normally respond.  Most of the
+time the client won't check the server id and can use either of the responses.
+However if the client does check the server id it may reject the response
+if it came from the wrong peer.  If the timing is such that the "wrong"
+peer responds first most of the time the client may not get an address for
+some time.
+.PP
+Care should be taken before enabling this option.
+.PP
+.RE
+.PP
+The
 .I server-duid
 statement
 .RS 0.25i

diff --git a/server/stables.c b/server/stables.c
index ef3d74c36462ee431326913ad291dae6d9b8bcdb..436883510e023ce35b3f93740fc4f4df6b93195b 100644 (file)
--- a/server/stables.c
+++ b/server/stables.c
@@ -268,6 +268,7 @@ static struct option server_options[] = {
        { "log-threshold-low", "B",             &server_universe,  83, 1 },
        { "log-threshold-high", "B",            &server_universe,  84, 1 },
        { "echo-client-id", "f",                &server_universe,  SV_ECHO_CLIENT_ID, 1 },
+       { "server-id-check", "f",               &server_universe,  SV_SERVER_ID_CHECK, 1 },
        { NULL, NULL, NULL, 0, 0 }
 };