- A failover bug that was allowing leases that EXPIRED or were RELEASED

  where tsfp and tstp are identical timestamps to languish in these
  transitional states has been repaired.  As a side effect, lease
  databases should be kept more consistent overall, not just for these
  transitional states. [ISC-Bugs #15429] [ISC-Support #694]

6 files changed, 85 insertions, 38 deletions

diff --git a/server/confpars.c b/server/confpars.c
index 603ff2ca..205f4d6b 100644
--- a/server/confpars.c
+++ b/server/confpars.c
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: confpars.c,v 1.143.2.25 2005/08/26 22:45:49 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: confpars.c,v 1.143.2.26 2005/09/22 16:19:58 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -2595,6 +2595,11 @@ int parse_lease_declaration (struct lease **lp, struct parse *cfile)
 				seenbit = 131072;
 				lease -> tsfp = t;
 				break;
+
+			      case ATSFP:
+				seenbit = 262144;
+				lease->atsfp = t;
+				break;
 				
 			      case CLTT:
 				seenbit = 524288;
diff --git a/server/db.c b/server/db.c
index 614d6a01..785c7efa 100644
--- a/server/db.c
+++ b/server/db.c
@@ -3,7 +3,7 @@
    Persistent database management routines for DHCPD... */
 
 /*
- * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2005 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1995-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: db.c,v 1.63.2.11 2004/06/17 20:54:40 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: db.c,v 1.63.2.12 2005/09/22 16:19:59 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -133,6 +133,14 @@ int write_lease (lease)
 			++errors;
 		}
 	}
+	if (lease->atsfp) {
+		t = gmtime(&lease->atsfp);
+		if (fprintf(db_file,
+			    "\n  atsfp %d %d/%02d/%02d %02d:%02d:%02d;",
+			    t->tm_wday, t->tm_year + 1900, t->tm_mon + 1,
+			    t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec) <= 0)
+			++errors;
+	}
 	if (lease -> cltt) {
 		t = gmtime (&lease -> cltt);
 		errno = 0;
diff --git a/server/dhcpd.8 b/server/dhcpd.8
index 94491893..533a608a 100644
--- a/server/dhcpd.8
+++ b/server/dhcpd.8
@@ -28,7 +28,7 @@
 .\" see ``http://www.vix.com''.   To learn more about Nominum, Inc., see
 .\" ``http://www.nominum.com''.
 .\"
-.\" $Id: dhcpd.8,v 1.18.2.11 2005/09/01 22:03:26 dhankins Exp $
+.\" $Id: dhcpd.8,v 1.18.2.12 2005/09/22 16:19:59 dhankins Exp $
 .\"
 .TH dhcpd 8
 .SH NAME
@@ -469,9 +469,16 @@ server.
 .RE
 .B tsfp \fItime\fR examine
 .RS 0.5i
-the time when the lease's current state ends, as understood by the
-failover peer (if there is no failover peer, this value is
-undefined).
+the adjusted time when the lease's current state ends, as understood by
+the failover peer (if there is no failover peer, this value is
+undefined).  Generally this value is only adjusted for expired, released,
+or reset leases while the server is operating in partner-down state, and
+otherwise is simply the value supplied by the peer.
+.RE
+.B atsfp \fItime\fR examine
+.RS 0.5i
+the actual tsfp value sent from the peer.  This value is forgotten when a
+lease binding state change is made, to facillitate retransmission logic.
 .RE
 .PP
 .B cltt \fItime\fR examine
diff --git a/server/failover.c b/server/failover.c
index 477eddab..49a36d81 100644
--- a/server/failover.c
+++ b/server/failover.c
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: failover.c,v 1.53.2.39 2005/08/26 22:45:50 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: failover.c,v 1.53.2.40 2005/09/22 16:19:59 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -4570,8 +4570,8 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
 	if (msg -> options_present & FTB_CLTT) {
 		lt -> cltt = msg -> client_ltt;
 	}
-	if (msg -> options_present & FTB_POTENTIAL_EXPIRY) {
-		lt -> tsfp = msg -> potential_expiry;
+	if (msg->options_present & FTB_POTENTIAL_EXPIRY) {
+		lt->atsfp = lt->tsfp = msg->potential_expiry;
 	}
 
 	if (msg -> options_present & FTB_BINDING_STATUS) {
@@ -4679,20 +4679,20 @@ isc_result_t dhcp_failover_process_bind_ack (dhcp_failover_state_t *state,
 	if (msg -> options_present & FTB_POTENTIAL_EXPIRY) {
 		/* XXX it could be a problem to do this directly if the
 		   XXX lease is sorted by tsfp. */
+		lease->atsfp = lease->tsfp = msg->potential_expiry;
+
 		if ((lease -> binding_state == FTS_EXPIRED ||
 		     lease -> binding_state == FTS_RESET ||
 		     lease -> binding_state == FTS_RELEASED) &&
 		    (msg -> options_present & FTB_BINDING_STATUS) &&
 		    msg -> binding_status == FTS_FREE)
 		{
-			lease -> tsfp = msg -> potential_expiry;
 			lease -> next_binding_state = FTS_FREE;
 			supersede_lease (lease, (struct lease *)0, 0, 0, 0);
 			write_lease (lease);
 			if (state -> me.state == normal)
 				commit_leases ();
 		} else {
-			lease -> tsfp = msg -> potential_expiry;
 			if ((lease -> desired_binding_state !=
 			     lease -> binding_state) &&
 			    (msg -> options_present & FTB_BINDING_STATUS) &&
@@ -4810,6 +4810,9 @@ isc_result_t dhcp_failover_generate_update_queue (dhcp_failover_state_t *state,
 	   expiry routine on the pool. */
 	for (s = shared_networks; s; s = s -> next) {
 	    for (p = s -> pools; p; p = p -> next) {
+		if (p->failover_peer != state)
+			continue;
+
 		lptr [FREE_LEASES] = &p -> free;
 		lptr [ACTIVE_LEASES] = &p -> active;
 		lptr [EXPIRED_LEASES] = &p -> expired;
@@ -4818,11 +4821,11 @@ isc_result_t dhcp_failover_generate_update_queue (dhcp_failover_state_t *state,
 
 		for (i = FREE_LEASES; i <= BACKUP_LEASES; i++) {
 		    for (l = *(lptr [i]); l; l = l -> next) {
-			if (p -> failover_peer == state &&
-			    ((everythingp &&
-			      (l -> starts != MIN_TIME ||
-			       l -> ends != MIN_TIME)) ||
-			     l -> tstp > l -> tsfp)) {
+			if ((everythingp &&
+			     (l->starts != MIN_TIME ||
+			      l->ends != MIN_TIME)) ||
+			    (l->tstp > l->atsfp) ||
+			    (i == EXPIRED_LEASES)) {
 				l -> desired_binding_state = l -> binding_state;
 				dhcp_failover_queue_update (l, 0);
 			}
@@ -5353,6 +5356,12 @@ int lease_mine_to_reallocate (struct lease *lease)
 		      case FTS_RESET:
 		      case FTS_RELEASED:
 		      case FTS_EXPIRED:
+			/* XXX - upon entering partner down state, the server
+			 * sets tsfp to stos + mclt.  this checks that tsfp
+			 * + mclt expire before continuing.  pick one?  i
+			 * think abandoned should be treated this way, but
+			 * reset/released/expired should check cur_time only.
+			 */
 			if (peer -> service_state == service_partner_down &&
 			    (lease -> tsfp < peer -> me.stos
 			     ? peer -> me.stos + peer -> mclt < cur_time
diff --git a/server/mdb.c b/server/mdb.c
index 4cf76c05..4b97c15a 100644
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: mdb.c,v 1.67.2.22 2005/03/03 16:55:25 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: mdb.c,v 1.67.2.23 2005/09/22 16:19:59 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -851,7 +851,7 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 		comp -> uid = (unsigned char *)0;
 	} else
 		enter_uid = 1;
-	
+
 	if (comp -> hardware_addr.hlen &&
 	    ((comp -> hardware_addr.hlen !=
 	      lease -> hardware_addr.hlen) ||
@@ -862,7 +862,7 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 		enter_hwaddr = 1;
 	} else if (!comp -> hardware_addr.hlen)
 		enter_hwaddr = 1;
-	
+
 	/* If the lease has been billed to a class, remove the billing. */
 	if (comp -> billing_class != lease -> billing_class) {
 		if (comp -> billing_class)
@@ -949,24 +949,29 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 		executable_statement_reference (&comp -> on_release,
 						lease -> on_release, MDL);
 	}
-	
+
 	/* Record the lease in the uid hash if necessary. */
 	if (enter_uid && comp -> uid) {
 		uid_hash_add (comp);
 	}
-	
+
 	/* Record it in the hardware address hash if necessary. */
 	if (enter_hwaddr && lease -> hardware_addr.hlen) {
 		hw_hash_add (comp);
 	}
-	
+
 #if defined (FAILOVER_PROTOCOL)
-	comp -> cltt = lease -> cltt;
-	comp -> tstp = lease -> tstp;
-	comp -> tsfp = lease -> tsfp;
+	comp->cltt = lease->cltt;
+	comp->tstp = lease->tstp;
+	comp->tsfp = lease->tsfp;
+	/* If this lease update is transmitted, reduce atsfp to zero. */
+	if (propogate)
+		comp->atsfp = 0;
+	else
+		comp->atsfp = lease->atsfp;
 #endif /* FAILOVER_PROTOCOL */
-	comp -> ends = lease -> ends;
-	comp -> next_binding_state = lease -> next_binding_state;
+	comp->ends = lease->ends;
+	comp->next_binding_state = lease->next_binding_state;
 
       just_move_it:
 	if (!comp -> pool) {
@@ -1012,6 +1017,7 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 
 	/* Remove the lease from its current place in its current
 	   timer sequence. */
+	/* XXX this is horrid. */
 	prev = (struct lease *)0;
 	for (lp = *lq; lp; lp = lp -> next) {
 		if (lp == comp)
@@ -1328,14 +1334,15 @@ int lease_copy (struct lease **lp,
 		executable_statement_reference (&lt -> on_release,
 						lease -> on_release,
 						file, line);
-	lt -> flags = lease -> flags;
-	lt -> tstp = lease -> tstp;
-	lt -> tsfp = lease -> tsfp;
-	lt -> cltt = lease -> cltt;
-	lt -> binding_state = lease -> binding_state;
-	lt -> next_binding_state = lease -> next_binding_state;
-	status = lease_reference (lp, lt, file, line);
-	lease_dereference (&lt, MDL);
+	lt->flags = lease->flags;
+	lt->tstp = lease->tstp;
+	lt->tsfp = lease->tsfp;
+	lt->atsfp = lease->atsfp;
+	lt->cltt = lease -> cltt;
+	lt->binding_state = lease->binding_state;
+	lt->next_binding_state = lease->next_binding_state;
+	status = lease_reference(lp, lt, file, line);
+	lease_dereference(&lt, MDL);
 	return status == ISC_R_SUCCESS;
 }
 
@@ -2001,7 +2008,7 @@ void expire_all_pools ()
 			}
 #if defined (FAILOVER_PROTOCOL)
 			if (p -> failover_peer &&
-			    l -> tstp > l -> tsfp &&
+			    l -> tstp > l -> atsfp &&
 			    !(l -> flags & ON_UPDATE_QUEUE)) {
 				l -> desired_binding_state = l -> binding_state;
 				dhcp_failover_queue_update (l, 1);
diff --git a/server/omapi.c b/server/omapi.c
index 5dbd77fd..60e1675f 100644
--- a/server/omapi.c
+++ b/server/omapi.c
@@ -41,7 +41,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: omapi.c,v 1.46.2.20 2005/08/26 22:45:50 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: omapi.c,v 1.46.2.21 2005/09/22 16:20:00 dhankins Exp $ Copyright (c) 2004-2005 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -648,6 +648,17 @@ isc_result_t dhcp_lease_stuff_values (omapi_object_t *c,
 	if (status != ISC_R_SUCCESS)
 		return status;
 
+	status = omapi_connection_put_name(c, "atsfp");
+	if (status != ISC_R_SUCCESS)
+		return status;
+	status = omapi_connection_put_uint32(c, sizeof(TIME));
+	if (status != ISC_R_SUCCESS)
+		return status;
+	status = omapi_connection_copyin(c,
+			(const unsigned char *)&(lease->atsfp), sizeof(TIME));
+	if (status != ISC_R_SUCCESS)
+		return status;
+
 	status = omapi_connection_put_name (c, "cltt");
 	if (status != ISC_R_SUCCESS)
 		return status;