[strongSwan-dev] IKEv1 rekeying / uniqueness honoring

[Timo Teras]

Please disregard the below.

Racoon does support IKE_SA deletion. It seems there was somehow a
mismatch on the CHILD_SAs the racoon side initiated, and what
strongSwan initiated. It's slightly curious how that resulted in total
disconnect, but it might've been related to other scripts I use.

Also that patch posted, does not probably work correctly unless
additional REKEYED state is introduced to IKE_SA and marked as such
when childs have been adopted - otherwise the IKE_SA rekeyed by remote,
will be rekeyed again by us.

I'll investigate more.

On Fri, 8 May 2015 09:00:29 +0300
Timo Teras <timo.teras at iki.fi> wrote:

> 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
>  				{