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

[Christophe Gouault]

2015-03-04 14:45 GMT+01:00 Emeric POUPON <emeric.poupon at stormshield.eu>:
> Hello,
>
>>> @@ -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
>>
>
> Unfortunately, I think this is not enough to solve this issue.
> 1- change the remote GW's IP address (from 172.16.0.3 to 172.16.2.3), keep the same left/right subnets
> 2- kill the remote host
> 3- SIGHUP the daemon.
>
> The old connection is being destroyed in the background (since the DELETE are not acked), and the new connection is loaded.
> Since the new connection has the same traffic selectors, the generated SP are the same.
> Therefore I get things like that:
>
> Feb 28 03:30:14 11[CFG] added configuration 'new_connection'
> ...
> Feb 28 03:30:14 14[CFG] received stroke: route 'new_connection'
> Feb 28 03:30:14 14[CFG] proposing traffic selectors for us:
> Feb 28 03:30:14 14[CFG]  192.168.3.0/24
> Feb 28 03:30:14 14[CFG] proposing traffic selectors for other:
> Feb 28 03:30:14 14[CFG]  192.168.13.0/24
> Feb 28 03:30:14 14[CFG] configured proposals: ESP:DES_CBC/HMAC_SHA1_96/NO_EXT_SEQ, ESP:DES_CBC/HMAC_MD5_96/NO_EXT_SEQ
> Feb 28 03:30:14 14[KNL] policy 192.168.3.0/24 === 192.168.13.0/24 out already exists, increasing refcount
> Feb 28 03:30:14 14[KNL] policy 192.168.13.0/24 === 192.168.3.0/24 in already exists, increasing refcount
>
> The SP in the kernel still references the IP of the previous GW. The tunnel can never goes up again...
>
> # setkey -DP
> 192.168.13.0/24[any] 192.168.3.0/24[any] 255
>         in ipsec
>         esp/tunnel/172.16.1.13-172.16.0.3/unique:1
>         spid=1638 seq=2 pid=92527
>         refcnt=1
> 192.168.3.0/24[any] 192.168.13.0/24[any] 255
>         out ipsec
>         esp/tunnel/172.16.0.3-172.16.1.13/unique:1
>         spid=1637 seq=0 pid=92527
>         refcnt=1
>
> Maybe we would need to flush the SP first if the last connection that uses it is being deleted?
>
> What do you think?
>
> Emeric

Hi Emeric,

Right, the SP should probably be removed first (as is done in the
original code).

The order of operations must be carefully checked. Probably first
deleting connection (starter_stroke_del_conn), to avoid immediate
renegotiation, then delete SPs (starter_stroke_unroute_conn), then
asynchronously terminate connections.

The main problem when terminating connections is to terminate the right IKE_SAs.

Identifying CHILD_SAs to delete is rather easy (their name is the same
as the connection name), but that is not the case of IKE_SAs: several
"connections" may have the same left/right pair, so share the same
IKE_SA, so:

- the name of an IKE_SA used by a connection may be the name of
another connection (the IKE_SA name is the name of the first
connection that established an IKE negotiation between the same
left/right pair).
- if you find an IKE_SA named after the connection that you are
deleting, you must be sure that it is not used by other CHILD SAs
derived from another (not deleted) connection.

That's why starter_stroke_terminate_conn then starter_stroke_purge_ike
are called in the patch (the first one terminates CHILD_SAs of deleted
connections, the second one terminates IKE_SAs with no CHILD_SA). But
in the asyncrhonous deletion case, the IKE_SAs will not be deleted
since deletion of CHILD_SAs is pending.

Best Regards,
Christophe