From 7116a34fc9b1fb307bcdca22e6963254289ecb80 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 8 Dec 2014 14:56:40 -0500 Subject: [PATCH] [master] Replaced SERVER_ID_CHECK compile switch with runtime config parameter Merges in rt37551. --- RELNOTES | 9 +++++++++ includes/dhcpd.h | 2 ++ includes/site.h | 29 +++-------------------------- server/dhcp.c | 25 +++++++++++-------------- server/dhcpd.c | 9 +++++++++ server/dhcpd.conf.5 | 38 ++++++++++++++++++++++++++++++++++++++ server/stables.c | 1 + 7 files changed, 73 insertions(+), 40 deletions(-) diff --git a/RELNOTES b/RELNOTES index 342e851f..4a4b9fb4 100644 --- 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 625a0bd6..f5ac1f4c 100644 --- 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 19a2e11f..bbc0c918 100644 --- 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 770dce60..552955c8 100644 --- 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 6747c209..ff5968c5 100644 --- 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 10878704..e437ce3c 100644 --- 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 ef3d74c3..43688351 100644 --- 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 } };