[strongSwan-dev] IKEv1 rekeying / uniqueness honoring

[Timo Teras]

On Thu, 7 May 2015 17:31:31 +0300
Timo Teras <timo.teras at iki.fi> wrote:

> I'm testing strongSwan against racoon for an upgrade path. But it
> seems the IKEv1 IKE_SA rekeying fails. What happens is:
>...
> 2) strongSwan accepts, and queues adopt_children_job
> 3) adopt_children_job moves all childs to the new IKE_SA just fine,
>    but then it unconditionally terminates the old IKE_SAs. even if my
>    config has "unique=no" for the peer. apparently it's bug in
>    adpot_children_job::execute?
>...
> 
> Would it be possible to fix adopt_children_job to honor "unique=no"
> and not delete the old IKE_SAs?

I'm trying to figure out from where IKEv1 replacement/rekey detection
happens, and it seems to be spread in many places. Probably because in
IKEv1 the IKE_SA - CHILD_SA relation ship is not well defined.

I found the following places:
1) ike_sa_manager.c: uses check_unique() when INITIAL-CONTACT is sent
2) ikev1/tasks/*.c: uses adopt_children_job() when IKE_SA
   authentication is complete
3) plugins/ha: uses adopt_children_job() also

And seems that the actual CHILD_SA moving / checking / IKE_SA
termination is duplicated code in check_unique() and
adopt_children_job().

Would the following patch be appropriate first aid?

Thanks,
Timo

diff --git a/src/libcharon/processing/jobs/adopt_children_job.c b/src/libcharon/processing/jobs/adopt_children_job.c
index c8a9c17..120897d 100644
--- a/src/libcharon/processing/jobs/adopt_children_job.c
+++ b/src/libcharon/processing/jobs/adopt_children_job.c
@@ -130,10 +130,20 @@ METHOD(job_t, execute, job_requeue_t,
 							 "adopting %d children and %d virtual IPs",
 							 children->get_count(children), vips->get_count(vips));
 					}
-					ike_sa->set_state(ike_sa, IKE_DELETING);
-					charon->bus->ike_updown(charon->bus, ike_sa, FALSE);
-					charon->ike_sa_manager->checkin_and_destroy(
-											charon->ike_sa_manager, ike_sa);
+					switch (cfg->get_unique_policy(cfg))
+					{
+					case UNIQUE_NEVER:
+					case UNIQUE_NO:
+						charon->ike_sa_manager->checkin(
+												charon->ike_sa_manager, ike_sa);
+						break;
+					default:
+						ike_sa->set_state(ike_sa, IKE_DELETING);
+						charon->bus->ike_updown(charon->bus, ike_sa, FALSE);
+						charon->ike_sa_manager->checkin_and_destroy(
+												charon->ike_sa_manager, ike_sa);
+						break;
+					}
 				}
 				else
 				{