isakmpd phase 1 rekey issue(s)

[Jean-Francois Dive <jef@linuxbe.org>]

Hello Akan, all,

isakmpd have some issues for phase 1 rekeying:

in exchange.c:exchange_finalize(), the initiator and responder id's are copied from
the exchange struct to the isakmp_sa struct for further use. However, this is done too
late in the function (all of this in phase 1 rekey context): afterward, a lookup on
previous SA (done on ID's) fail to find the previous SA and so fail to set the REPLACED
flag which means that when it expire, another phase 1 SA will be negociated ( a third one ).

Another issue was pointed to me (ALWAYSON flag get lost from first phase 1 SA to the rekeyed one
wich means that it wont rekey more that once), but i haven't investigated this further.

Please find attach a patch fixing the first issue.

<----- CUT ME ----->

Index: exchange.c
===================================================================
RCS file: /cvs/src/sbin/isakmpd/exchange.c,v
retrieving revision 1.82
diff -u -r1.82 exchange.c
--- exchange.c	2003/06/10 16:41:29	1.82
+++ exchange.c	2003/06/13 14:34:40
@@ -1396,6 +1396,25 @@
   exchange_dump ("exchange_finalize", exchange);
 #endif
 
+  /* Copy the ID from phase 1 to exchange or phase 2 SA.  */
+  if (msg->isakmp_sa)
+    {
+      if (exchange->id_i && exchange->id_r)
+	{
+	  ipsec_clone_id (&msg->isakmp_sa->id_i, &msg->isakmp_sa->id_i_len,
+			  exchange->id_i, exchange->id_i_len);
+	  ipsec_clone_id (&msg->isakmp_sa->id_r, &msg->isakmp_sa->id_r_len,
+			  exchange->id_r, exchange->id_r_len);
+	}
+      else if (msg->isakmp_sa->id_i && msg->isakmp_sa->id_r)
+	{
+	  ipsec_clone_id (&exchange->id_i, &exchange->id_i_len,
+			  msg->isakmp_sa->id_i, msg->isakmp_sa->id_i_len);
+	  ipsec_clone_id (&exchange->id_r, &exchange->id_r_len,
+			  msg->isakmp_sa->id_r, msg->isakmp_sa->id_r_len);
+	}
+    }
+
   /*
    * Walk over all the SAs and noting them as ready.  If we set the
    * COMMIT bit, tell the peer each SA is connected.
@@ -1499,25 +1518,6 @@
 		? "<no transport>"
 		: msg->isakmp_sa->transport->vtbl->decode_ids (msg->isakmp_sa
 							       ->transport)));
-    }
-
-  /* Copy the ID from phase 1 to exchange or phase 2 SA.  */
-  if (msg->isakmp_sa)
-    {
-      if (exchange->id_i && exchange->id_r)
-	{
-	  ipsec_clone_id (&msg->isakmp_sa->id_i, &msg->isakmp_sa->id_i_len,
-			  exchange->id_i, exchange->id_i_len);
-	  ipsec_clone_id (&msg->isakmp_sa->id_r, &msg->isakmp_sa->id_r_len,
-			  exchange->id_r, exchange->id_r_len);
-	}
-      else if (msg->isakmp_sa->id_i && msg->isakmp_sa->id_r)
-	{
-	  ipsec_clone_id (&exchange->id_i, &exchange->id_i_len,
-			  msg->isakmp_sa->id_i, msg->isakmp_sa->id_i_len);
-	  ipsec_clone_id (&exchange->id_r, &exchange->id_r_len,
-			  msg->isakmp_sa->id_r, msg->isakmp_sa->id_r_len);
-	}
     }
 
   exchange->doi->finalize_exchange (msg);