[master] Replaced SERVER_ID_CHECK compile switch with runtime config parameter

Merges in rt37551.

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

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;

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.

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

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);
 }

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

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 }
 };