[Swan-dev] pluto: When Child state fails, don't tear down IKE SA

[Paul Wouters]

On Mon, 5 Jul 2021, Andrew Cagney wrote:

>       commit 68fb298d059854253e8267680aeee1ee1f3158a3
>       Author: Paul Wouters <paul.wouters at aiven.io>
>       Date:   Sun Jul 4 22:15:51 2021 -0400
>
>           pluto: When Child state fails, don't tear down IKE SA
>
>           In complete_v2_state_transition() for a child SA state STF_FATAL
>           error, don't call delete_ike_family()
> 
> 
> 
> A create child sa transaction can finish in one of three ways:
> 
> - ok ...
> - fail, the specific sa needs to be deleted but the ike sa remains
> - fatal, something bad happened the entire family is dead; thing INVALID_SYNTAX
>   
> https://github.com/libreswan/libreswan/commit/1f72ba5ce87a34bc3140e2e8fcaf843011f6a959
> went through and eliminated remaining cases where fail+v2n was returned; it sounds like we've still got cases where FATAL is being
> returned. 
> 
> so this is going in the wrong direction

But that is changing the meaning of FAIL vs FATAL.

FAIL used to mean "there is a non-fatal error, please keep trying this state"
FATAL meant "there is no more hope, kill the state"

Note that this applies to the _states_ and not the connection. That is,
starting a new keying attempt after a timeout and/or revival are not
related to these state changes.

In that sense, a CREATE_CHILD_SA that received an answer that leads to
an unfixable isse, like TS_UNACCEPTABLE, is a FATAL state change for
the child sa. FAIL would mean we might try to retransmit, which is
what is wrongly happening here. FAIL would also happen if we got an
unauthenticated reply we didn't like, so we would still wait for a
possible unauthenticated reply we like (to prevent attackers from
locking out legitimate packets).

The way IKEv2 works with serial MSGID, we really don't have STF_FAIL in
that sense. We only have STF_OK or STF_FATAL. (well also STF_SUSPEND etc)
The only STF_FAIL would be the if we get no reply, and we need to
retransmit. I can't think of an STF_FAIL that would operate on the child
SA state.

Paul