[master]

+- Add support for a simple check that the server id in a request message
+  to a failover peer matches the server id of the server.  This support
+  is enabled by editing the file includes/site.h and uncommenting the
+  definition for SERVER_ID_CHECK.  The option has several restrictions
+  and issues - please read the comment in the site.h file before
+  enabling it.
+  [ISC-Bugs #31463]

5 files changed, 206 insertions, 114 deletions

diff --git a/RELNOTES b/RELNOTES
index aa86a9dd..5115b7a8 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -161,6 +161,14 @@ work on other platforms. Please report any problems and suggested fixes to
   variables.
   [ISC-Bugs #29068]
 
+- Add support for a simple check that the server id in a request message
+  to a failover peer matches the server id of the server.  This support
+  is enabled by editing the file includes/site.h and uncommenting the
+  definition for SERVER_ID_CHECK.  The option has several restrictions
+  and issues - please read the comment in the site.h file before
+  enabling it.
+  [ISC-Bugs #31463]
+
 			Changes since 4.2.3
 
 ! Add a check for a null pointer before calling the regexec function.
diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index 9945df04..3d513dda 100644
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -2163,7 +2163,11 @@ unsigned cons_agent_information_options (struct option_state *,
 					 unsigned, unsigned);
 void get_server_source_address(struct in_addr *from,
 			       struct option_state *options,
+			       struct option_state *out_options,
 			       struct packet *packet);
+void setup_server_source_address(struct in_addr *from,
+				 struct option_state *options,
+				 struct packet *packet);
 
 /* dhcpleasequery.c */
 void dhcpleasequery (struct packet *, int);
diff --git a/includes/site.h b/includes/site.h
index f424129d..76be056c 100644
--- a/includes/site.h
+++ b/includes/site.h
@@ -248,3 +248,30 @@
    computed for a NAK may not match that computed for an ACK. */
 
 #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 */
diff --git a/server/dhcp.c b/server/dhcp.c
index 6eb71feb..1e15a229 100644
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -472,8 +472,10 @@ void dhcprequest (packet, ms_nulltp, ip_lease)
 		 * safe.
 		 */
 		sprintf (smbuf, " (%s)", piaddr (sip));
-	} else
+	} else {
 		smbuf [0] = 0;
+		sip.len = 0;
+	}
 
 	/* %Audit% This is log output. %2004.06.17,Safe%
 	 * If we truncate we hope the user can get a hint from the log.
@@ -554,6 +556,27 @@ 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) &&
+		    (memcmp(sip.iabuf, "\0\0\0\0", sip.len) != 0)) {
+			struct in_addr from;
+			setup_server_source_address(&from, NULL, packet);
+			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
 		   both we and the peer are in a position to offer it.
@@ -1139,7 +1162,7 @@ void dhcpinform (packet, ms_nulltp)
 		option_cache_dereference (&oc, MDL);
 	}
 
-	get_server_source_address(&from, options, packet);
+	get_server_source_address(&from, options, options, packet);
 
 	/* Use the subnet mask from the subnet declaration if no other
 	   mask has been provided. */
@@ -1336,7 +1359,6 @@ void nak_lease (packet, cip)
 	struct sockaddr_in to;
 	struct in_addr from;
 	int result;
-	int got_source = 0;
 	struct dhcp_packet raw;
 	unsigned char nak = DHCPNAK;
 	struct packet outgoing;
@@ -1388,95 +1410,22 @@ void nak_lease (packet, cip)
 	save_option (&dhcp_universe, options, oc);
 	option_cache_dereference (&oc, MDL);
 
-#if defined(SERVER_ID_FOR_NAK)
 	/*
-	 * Check to see if there is a server id we should use for the NAK.
-	 * In order to minimize the effort involved we only check the
-	 * global, shared_network and first subnet and pool on the
-	 * shared_network (if they exist).  We skip the other subnets
-	 * and pools and don't check on the host declarations.
-	 *
-	 * We get the shared subnet from the packet and execute the statements
-	 * then check for a server id.  As we only want the server ID we
-	 * execute the statements into a separate options area and then
-	 * free that area when we finish
+	 * If we are configured to do so we try to find a server id
+	 * option even for NAKS by calling setup_server_source_address().
+	 * This function will set up an options list from the global
+	 * and subnet scopes before trying to get the source address.
+	 * 
+	 * Otherwise we simply call get_server_source_address()
+	 * directly, without a server options list, this means
+	 * we'll get the source address from the interface address.
 	 */
-	if (packet->shared_network != NULL) {
-		struct option_state *sid_options = NULL;
-		struct data_string d;
-		struct option_cache *soc = NULL;
-
-		option_state_allocate (&sid_options, MDL);
-		/*
-		 * If we have a subnet and group start with that else start
-		 * with the shared network group.  The first will recurse and
-		 * include the second.
-		 */
-		if ((packet->shared_network->subnets != NULL) &&
-		    (packet->shared_network->subnets->group != NULL)) {
-			execute_statements_in_scope(NULL, packet, NULL, NULL,
-					packet->options, sid_options,
-					&global_scope,
-					packet->shared_network->subnets->group,
-					NULL);
-		} else {
-			execute_statements_in_scope(NULL, packet, NULL, NULL,
-					packet->options, sid_options,
-					&global_scope,
-					packet->shared_network->group,
-					NULL);
-		}
-
-		/* do the pool if there is one */
-		if (packet->shared_network->pools != NULL) {
-			execute_statements_in_scope(NULL, packet, NULL, NULL,
-					packet->options, sid_options,
-					&global_scope,
-					packet->shared_network->pools->group,
-					packet->shared_network->group);
-		}
-
-		memset(&d, 0, sizeof(d));
-
-		i = DHO_DHCP_SERVER_IDENTIFIER;
-		oc = lookup_option(&dhcp_universe, sid_options, i);
-		if ((oc != NULL) &&
-		    (evaluate_option_cache(&d, packet, NULL, NULL,
-					   packet->options, sid_options, 
-					   &global_scope, oc, MDL))) {
-			if (d.len == sizeof(from)) {
-				/* We have a server id and it's the proper length
-				 * for an address save the address and try to add
-				 * it to the options list we are building for the
-				 * response packet.
-				 */
-				memcpy(&from, d.data, sizeof(from));
-				got_source = 1;
-
-				if (option_cache_allocate(&soc, MDL) &&
-				    (make_const_data(&soc->expression,
-						     (unsigned char *)&from,
-						     sizeof(from),
-						     0, 1, MDL))) {
-					option_code_hash_lookup(&soc->option,
-							dhcp_universe.code_hash,
-							&i, 0, MDL);
-					save_option(&dhcp_universe, options,
-						    soc);
-				}
-				if (soc != NULL)
-					option_cache_dereference(&soc, MDL);
-			}
-			data_string_forget(&d, MDL);
-		}
-		oc = NULL;
-		option_state_dereference (&sid_options, MDL);
-	}
+#if defined(SERVER_ID_FOR_NAK)
+	setup_server_source_address(&from, options, packet);
+#else
+	get_server_source_address(&from, NULL, options, packet);
 #endif /* if defined(SERVER_ID_FOR_NAK) */
 
-	if (got_source == 0) {
-		get_server_source_address(&from, options, packet);
-	}
 
 	/* If there were agent options in the incoming packet, return
 	 * them.  We do not check giaddr to detect the presence of a
@@ -2634,7 +2583,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
 			enqueue = ISC_FALSE;
 
 		/* Install the new information on 'lt' onto the lease at
-		 * 'lease'.  We will not 'commit' this information to disk
+		 * 'lease'.  We will not 'commit' this information to disk
 		 * yet (fsync()), we will 'propogate' the information if
 		 * this is BOOTP or a DHCPACK, but we will not 'pimmediate'ly
 		 * transmit failover binding updates (this is delayed until
@@ -2742,7 +2691,8 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
 			option_cache_dereference (&oc, MDL);
 		}
 
-		get_server_source_address(&from, state->options, packet);
+		get_server_source_address(&from, state->options,
+					  state->options, packet);
 		memcpy(state->from.iabuf, &from, sizeof(from));
 		state->from.len = sizeof(from);
 
@@ -4510,23 +4460,47 @@ int locate_network (packet)
 /*
  * Try to figure out the source address to send packets from.
  *
- * If the packet we received specified the server address, then we
- * will use that.
+ * from is the address structure we use to return any address
+ * we find.
+ *
+ * options is the option cache to search.  This may include
+ * options from the incoming packet and configuration information.
  *
- * Otherwise, use the first address from the interface. If we do
- * this, we also save this into the option cache as the server
- * address.
+ * out_options is the outgoing option cache.  This cache
+ * may be the same as options.  If send_options isn't NULL
+ * we may save the server address option into it.  We do so
+ * if send_options is different than options or if the option
+ * wasn't in options and we needed to find the address elsewhere.
+ *
+ * packet is the state structure for the incoming packet
+ *
+ * When finding the address we first check to see if it is
+ * in the options list.  If it isn't we use the first address
+ * from the interface.
+ *
+ * While this is slightly more complicated than I'd like it allows
+ * us to use the same code in several different places.  ack,
+ * inform and lease query use it to find the address and fill
+ * in the options if we get the address from the interface.
+ * nack uses it to find the address and copy it to the outgoing
+ * cache.  dhcprequest uses it to find the address for comparison
+ * and doesn't need to add it to an outgoing list.
  */
+
 void
 get_server_source_address(struct in_addr *from,
 			  struct option_state *options,
+			  struct option_state *out_options,
 			  struct packet *packet) {
 	unsigned option_num;
-	struct option_cache *oc;
+	struct option_cache *oc = NULL;
 	struct data_string d;
-	struct in_addr *a;
+	struct in_addr *a = NULL;
+	isc_boolean_t found = ISC_FALSE;
+	int allocate = 0;
 
 	memset(&d, 0, sizeof(d));
+	memset(from, 0, sizeof(*from));
 
        	option_num = DHO_DHCP_SERVER_IDENTIFIER;
        	oc = lookup_option(&dhcp_universe, options, option_num);
@@ -4535,32 +4509,111 @@ get_server_source_address(struct in_addr *from,
 					  packet->options, options, 
 					  &global_scope, oc, MDL)) {
 			if (d.len == sizeof(*from)) {
+				found = ISC_TRUE;
 				memcpy(from, d.data, sizeof(*from));
-				data_string_forget(&d, MDL);
-				return;
+
+				/*
+				 * Arrange to save a copy of the data
+				 * to the outgoing list.
+				 */
+				if ((out_options != NULL) &&
+				    (options != out_options)) {
+					a = from;
+					allocate = 1;
+				}
 			}
 			data_string_forget(&d, MDL);
 		}
 		oc = NULL;
 	}
 
-	if (packet->interface->address_count > 0) {
-		if (option_cache_allocate(&oc, MDL)) {
+	if ((found == ISC_FALSE) &&
+	    (packet->interface->address_count > 0)) {
+		*from = packet->interface->addresses[0];
+
+		if (out_options != NULL) {
 			a = &packet->interface->addresses[0];
-			if (make_const_data(&oc->expression,
-					    (unsigned char *)a, sizeof(*a),
-					    0, 0, MDL)) {
-				option_code_hash_lookup(&oc->option, 
-							dhcp_universe.code_hash,
-							&option_num, 0, MDL);
-				save_option(&dhcp_universe, options, oc);
-			}
-			option_cache_dereference(&oc, MDL);
 		}
-		*from = packet->interface->addresses[0];
-	} else {
-       		memset(from, 0, sizeof(*from));
 	}
+
+	if ((a != NULL) &&
+	    (option_cache_allocate(&oc, MDL))) {
+		if (make_const_data(&oc->expression,
+				    (unsigned char *)a, sizeof(*a),
+				    0, allocate, MDL)) {
+			option_code_hash_lookup(&oc->option, 
+						dhcp_universe.code_hash,
+						&option_num, 0, MDL);
+			save_option(&dhcp_universe, out_options, oc);
+		}
+		option_cache_dereference(&oc, MDL);
+	}
+
+	return;
+}
+
+/*
+ * Set up an option state list to try and find a server option.
+ * We don't go through all possible options - in particualr we
+ * skip the hosts and we don't include the lease to avoid 
+ * making changes to it.  This means that we won't get the
+ * correct server id if the admin puts them on hosts or
+ * builds the server id with information from the lease.
+ *
+ * As this is a fallback function (used to handle NAKs or
+ * sort out server id mismatch in failover) and requires
+ * configuration by the admin, it should be okay.
+ */
+ 
+void
+setup_server_source_address(struct in_addr *from,
+			    struct option_state *options,
+			    struct packet *packet) {
+
+	struct option_state *sid_options = NULL;
+
+	if (packet->shared_network != NULL) {
+		option_state_allocate (&sid_options, MDL);
+
+		/*
+		 * If we have a subnet and group start with that else start
+		 * with the shared network group.  The first will recurse and
+		 * include the second.
+		 */
+		if ((packet->shared_network->subnets != NULL) &&
+		    (packet->shared_network->subnets->group != NULL)) {
+			execute_statements_in_scope(NULL, packet, NULL, NULL,
+					packet->options, sid_options,
+					&global_scope,
+					packet->shared_network->subnets->group,
+					NULL);
+		} else {
+			execute_statements_in_scope(NULL, packet, NULL, NULL,
+					packet->options, sid_options,
+					&global_scope,
+					packet->shared_network->group,
+					NULL);
+		}
+
+		/* do the pool if there is one */
+		if (packet->shared_network->pools != NULL) {
+			execute_statements_in_scope(NULL, packet, NULL, NULL,
+					packet->options, sid_options,
+					&global_scope,
+					packet->shared_network->pools->group,
+					packet->shared_network->group);
+		}
+
+		/* currently we don't bother with classes or hosts as
+		 * neither seems to be useful in this case */
+	}
+
+	/* Make the call to get the server address */
+	get_server_source_address(from, sid_options, options, packet);
+
+	/* get rid of the option cache */
+	if (sid_options != NULL)
+		option_state_dereference(&sid_options, MDL);
 }
 
 /*
diff --git a/server/dhcpleasequery.c b/server/dhcpleasequery.c
index 09913c24..848611b9 100644
--- a/server/dhcpleasequery.c
+++ b/server/dhcpleasequery.c
@@ -616,7 +616,7 @@ dhcpleasequery(struct packet *packet, int ms_nulltp) {
 	/*
 	 * Figure out which address to use to send from.
 	 */
-	get_server_source_address(&siaddr, options, packet);
+	get_server_source_address(&siaddr, options, options, packet);
 
 	/* 
 	 * Set up the option buffer.