[strongSwan-dev] [PATCH] starter: cleanup SAs when deleting a connection

[Emeric POUPON]

Ok, thanks for this explanation.

Martin, do you plan to fix this? Do you want me to fill in a bug report?

Best regards,

Emeric

----- Mail original -----
De: "Christophe Gouault" <christophe.gouault at 6wind.com>
À: "Emeric POUPON" <emeric.poupon at stormshield.eu>
Cc: "Martin Willi" <martin at strongswan.org>, dev at lists.strongswan.org
Envoyé: Mercredi 25 Février 2015 12:00:32
Objet: Re: [strongSwan-dev] [PATCH] starter: cleanup SAs when deleting a connection

2015-02-25 11:24 GMT+01:00 Emeric POUPON <emeric.poupon at stormshield.eu>:
> Hello,
>
> Thanks for your support.
>
> I noticed the terminate_XX methods of the controller are synchronous when a callback is provided:
>
>          * Terminate an IKE_SA and all of its CHILD_SAs.
>          *
>          * If a callback is provided the function is synchronous and thus blocks
>          * until the IKE_SA is properly deleted, or the call timed out.
>
> Therefore, doing things like this in the src/libcharon/plugins/stroke/stroke_control.c file seems to correct the problem:
>
> @@ -390,7 +390,7 @@ METHOD(stroke_control_t, terminate, void,
>         while (enumerator->enumerate(enumerator, &del))
>         {
>                 status = charon->controller->terminate_ike(charon->controller, del,
> -                                                       (controller_cb_t)stroke_log, &info, this->timeout);
> +                                                       msg->output_verbosity == -1 ? NULL : (controller_cb_t)stroke_log, &info, this->timeout);
>                 report_terminate_status(this, status, out, del, FALSE);
>         }
>         enumerator->destroy(enumerator);
>
> I really don't know if it is the right fix, since it may raise other problems...
>
> What do you think?

Well spotted!

It seems that there was a will to support non blocking connection
initiation/termination in 2013 (stroke up-nb and down-nb commands were
added in commit 4182c86aed84933b3efa0367 "stroke: Add non-blocking
versions of up and down"; they precisely use the output_verbosity),
but only stroke up-nb works as expected: terminate does not take
output_verbosity in account...

Christophe

> Emeric
>
> ----- Mail original -----
> De: "Christophe Gouault" <christophe.gouault at 6wind.com>
> À: "Emeric POUPON" <emeric.poupon at stormshield.eu>
> Cc: "Martin Willi" <martin at strongswan.org>, dev at lists.strongswan.org
> Envoyé: Mercredi 25 Février 2015 10:57:58
> Objet: Re: [strongSwan-dev] [PATCH] starter: cleanup SAs when deleting a connection
>
> 2015-02-23 18:23 GMT+01:00 Christophe Gouault <christophe.gouault at 6wind.com>:
>> Hello Emeric,
>>
>> 2015-02-20 18:10 GMT+01:00 Emeric POUPON <emeric.poupon at stormshield.eu>:
>>> Hello,
>>>
>>> Unfortunately, I am facing an issue with this patch.
>>> For example, we may want to update the configuration file since the remote host's IP address has changed.
>>> When charon receives the terminate stroke message, it sends the DELETE message but it may take minutes before giving up if the remote host is down!
>>
>> Indeed, if the peer does not respond, the actual tear down of the
>> connection will last until the timeout is reached, but as far as I
>> know, this does not prevent from completing the cleanup and applying
>> the new configuration.
>>
>>> Therefore the new configuration may be applied several minutes later, which is quite unexpected.
>>>
>>> What do you think?
>>
>> Well, I think the new conf can be used immediately (the old connection
>> will just survive for a while until the timeout is reached). I'll try
>> to do a little test.
>
> Hello Emeric,
>
> After testing, I confirm the problem you describe: the unsuccessful
> sending of a delete message delays the cleanup and applying of the new
> conf.
>
> This patch obviously needs some rework. Thanks for raising the issue.
>
> Best Regards,
> Christophe