[45364] Replaced --enable-secs-byteorder with runtime check-secs-byte-order

configure.ac
   removed --enable-secs-byteorder support

includes/dhcpd.h
    added SV_CHECK_SECS_BYTE_ORDER

includes/failover.h
    added extern int check_secs_byte_order

server/dhcpd.c
    postconf_initialization() - added logic to
    set check_secs_byte_order

server/failover.c
    load_balance_mine() - replaced conditional comp
    directives with runtime test of check_secs_byte_order

Added release note and updated man page.

RELNOTES

@@ -203,6 +203,16 @@ dhcp-users@lists.isc.org.
   even valid values to fail as invalid on some environments.
   [ISC-Bugs #46535]
 
+- Replaced compilation option, enable-secs-byteorder, with a run-time, server
+  configuration parameter, check-secs-byte-order.  When enabled, the
+  server will check for clients that do the byte ordering on the secs field
+  incorrectly.  This field should be in network byte order but some clients
+  get it wrong.  When this parameter is enabled the server will examine the
+  secs field and if it looks wrong (high byte non zero and low byte zero) swap
+  the bytes.  The default is disabled.  This parameter is only useful when
+  doing load balancing within failover.
+  [ISC-Bugs #45364]
+
 			Changes since 4.3.0 (bug fixes)
 
 - Tidy up several small tickets.

configure.ac

@@ -216,16 +216,6 @@ if test "$enable_use_sockets" = "yes"; then
 		[Define to 1 to use the standard BSD socket API.])
 fi
 
-# Try to hnadle incorrect byte order for secs field
-# This is off by default
-AC_ARG_ENABLE(secs_byteorder,
-	AS_HELP_STRING([--enable-secs-byteorder],[Correct bad byteorders in the secs field (default is no).]))
-
-if test "$enable_secs_byteorder" = "yes" ; then
-	AC_DEFINE([SECS_BYTEORDER], [1],
-		[Define to correct bad byteorders in secs field.])
-fi
-
 # Include the PID in the log messages.  This is useful when there may
 # be multiple instances of a program.
 # This is off by default

includes/dhcpd.h

@@ -802,6 +802,9 @@ struct lease_state {
 #ifdef EUI_64
 #define SV_USE_EUI_64			90
 #endif
+#if defined (FAILOVER_PROTOCOL)
+#define SV_CHECK_SECS_BYTE_ORDER	91
+#endif
 
 #if !defined (DEFAULT_PING_TIMEOUT)
 # define DEFAULT_PING_TIMEOUT 1

includes/failover.h

@@ -392,5 +392,7 @@ typedef struct _dhcp_failover_state {
 	u_int32_t updxid;		/* XID of UPDREQ* message in action. */
 } dhcp_failover_state_t;
 
+extern int check_secs_byte_order; /* check byte order of secs field when true */
+
 #define DHCP_FAILOVER_VERSION		1
 #endif /* FAILOVER_PROTOCOL */

server/dhcpd.c

@@ -1378,6 +1378,15 @@ void postconf_initialization (int quiet)
 		data_string_forget (&db, MDL);
         }
 
+#if defined (FAILOVER_PROTOCOL)
+       oc = lookup_option(&server_universe, options, SV_CHECK_SECS_BYTE_ORDER);
+       if ((oc != NULL) &&
+	   evaluate_boolean_option_cache(NULL, NULL, NULL, NULL, options, NULL,
+					 &global_scope, oc, MDL)) {
+		check_secs_byte_order = 1;
+	}
+#endif
+
 #if defined (BINARY_LEASES)
 	if (local_family == AF_INET) {
 		log_info("Source compiled to use binary-leases");

server/dhcpd.conf.5

@@ -2021,6 +2021,20 @@ addresses, as long as those addresses are not restricted by
 and \fIdeny\fR statements within their \fIpool\fR declarations.
 .RE
 .PP
+The \fIcheck-secs-byte-order\fR statement
+.RS 0.25i
+.PP
+.B check-secs-byte-order \fIflag\fB;\fR
+.PP
+When \fIcheck-secs-byte-order\fR is enabled, the server will check for DHPCv4
+clients that do the byte ordering on the secs field incorrectly. This field
+should be in network byte order but some clients get it wrong. When this
+parameter is enabled the server will examine the secs field and if it looks
+wrong (high byte non zero and low byte zero) swap the bytes.  The default
+is disabled. This parameter is only useful when doing load balancing within
+failover. (Formerly, this behavior had to be enabled during compilation
+configuration via --enable-secs-byteorder).
+.PP
 The \fIdb-time-format\fR statement
 .RS 0.25i
 .PP

server/failover.c

@@ -50,6 +50,7 @@ static inline int secondary_not_hoarding(dhcp_failover_state_t *state,
 					 struct pool *p);
 static void scrub_lease(struct lease* lease, const char *file, int line);
 
+int check_secs_byte_order = 0; /* enables byte order check of secs field if 1 */
 
 /*!
  * \brief Performs a "pre-flight" sanity check of failover configuration
@@ -5964,17 +5965,16 @@ int load_balance_mine (struct packet *packet, dhcp_failover_state_t *state)
 
 	ec = ntohs(packet->raw->secs);
 
-#if defined(SECS_BYTEORDER)
 	/*
 	 * If desired check to see if the secs field may have been byte
 	 * swapped.  We assume it has if the high order byte isn't cleared
 	 * while the low order byte is cleared.  In this case we swap the
 	 * bytes and continue processing.
 	 */
-	if ((ec > 255) && ((ec & 0xff) == 0)) {
+	if ((check_secs_byte_order == 1) &&
+	    ((ec > 255) && ((ec & 0xff) == 0))) {
 		ec = (ec >> 8) | (ec << 8);
 	}
-#endif
 
 	if ((state->load_balance_max_secs == 0) ||
 	    (state->load_balance_max_secs < ec)) {

server/stables.c

@@ -279,6 +279,9 @@ static struct option server_options[] = {
 	{ "abandon-lease-time", "T",		&server_universe,  SV_ABANDON_LEASE_TIME, 1 },
 #ifdef EUI_64
 	{ "use-eui-64", "f",		&server_universe,  SV_USE_EUI_64, 1 },
+#endif
+#if defined (FAILOVER_PROTOCOL)
+	{ "check-secs-byte-order", "f", &server_universe, SV_CHECK_SECS_BYTE_ORDER, 1 },
 #endif
 	{ NULL, NULL, NULL, 0, 0 }
 };

server/tests/load_bal_unittest.c

@@ -134,12 +134,13 @@ ATF_TC_HEAD(load_balance_swap, tc)
 ATF_TC_BODY(load_balance_swap, tc)
 {
 #if defined(FAILOVER_PROTOCOL) 
-#if defined(SECS_BYTEORDER)
 	struct packet packet;
 	struct dhcp_packet raw;
 	dhcp_failover_state_t pstate, sstate;
 	u_int8_t hba[256];
 
+    check_secs_byte_order = 1;
+
 	memset(&packet, 0, sizeof(struct packet));
 	memset(&raw, 0, sizeof(struct dhcp_packet));
 	packet.raw = &raw;
@@ -179,10 +180,6 @@ ATF_TC_BODY(load_balance_swap, tc)
 	if (load_balance_mine(&packet, &sstate) != 1) {
 		atf_tc_fail("ERROR: secondary not accepted %s:%d", MDL);
 	}
-	
-#else
-	atf_tc_skip("SECS_BYTEORDER not defined");
-#endif
 #else
 	atf_tc_skip("failover is disabled");
 #endif