<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 5 Jul 2021 at 09:26, Paul Wouters <<a href="mailto:paul@nohats.ca" target="_blank">paul@nohats.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, 5 Jul 2021, Andrew Cagney wrote:<br>
<br>
> commit 68fb298d059854253e8267680aeee1ee1f3158a3<br>
> Author: Paul Wouters <<a href="mailto:paul.wouters@aiven.io" target="_blank">paul.wouters@aiven.io</a>><br>
> Date: Sun Jul 4 22:15:51 2021 -0400<br>
><br>
> pluto: When Child state fails, don't tear down IKE SA<br>
><br>
> In complete_v2_state_transition() for a child SA state STF_FATAL<br>
> error, don't call delete_ike_family()<br>
> <br>
> <br>
> <br>
> A create child sa transaction can finish in one of three ways:<br>
> <br>
> - ok ...<br>
> - fail, the specific sa needs to be deleted but the ike sa remains<br>
> - fatal, something bad happened the entire family is dead; thing INVALID_SYNTAX<br>
> <br>
> <a href="https://github.com/libreswan/libreswan/commit/1f72ba5ce87a34bc3140e2e8fcaf843011f6a959" rel="noreferrer" target="_blank">https://github.com/libreswan/libreswan/commit/1f72ba5ce87a34bc3140e2e8fcaf843011f6a959</a><br>
> went through and eliminated remaining cases where fail+v2n was returned; it sounds like we've still got cases where FATAL is being<br>
> returned. <br>
> <br>
> so this is going in the wrong direction<br>
<br>
But that is changing the meaning of FAIL vs FATAL.<br>
<br></blockquote><div><br></div><div>(sarcasm) It's hard to change the meaning of something that was, well, meaningless.</div><div><br></div><div>This is what v3.23 had:<br></div><div><br></div><div>STF_INTERNAL_ERROR:<br></div><div>It just signals to the state machine to do nothing (as I discovered last week, it also signals that the state on the stack state may have been deleted)!</div><div>-> I think it should go; internal errors are either recoverable (just kill IKE family), or passert<br></div><div><br></div><div>STF_DROP:</div><div>delete current state; but "be vewy vewy quiet"</div><div>-> this is gone</div><div><br></div><div>STF_FATAL:</div><div>delete current state</div><div>(for a child it would delete sending a notify, then see the ike sa was childless and delete that sending a second notify; except when there were multiple children)<br></div><div>-> it now completes message exchange and deletes the IKE family; it is after all fatal</div><div><br></div><div>STF_FAIL:</div><div>do pretty much nothing</div><div>-> which is useless, the exchange failed so things are going nowhere</div><div>-> it now completes message exchange and deletes current state<br></div><div><br></div><div>STF_FAIL+v2N_....:</div><div>in the responder send v2N and then deleted a state (during IKE_SA_INIT that was IKE, else it was the child, but see alsofor the initiator this did nothing</div><div>-> this code path is gone<br></div><br><div>STF_IGNORE:</div><div>it's been pretty consistent; pretend the message never happened</div></div><div class="gmail_quote"><br></div><div class="gmail_quote">So STF_DROP, STF_FATAL and STF_FAIL+v2N could all potentially delete a state; and STF_FAIL on its own did nothing.</div><div class="gmail_quote">However, without looking at the code that returns the status, knowing what state was deleted was a struggle.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
FAIL used to mean "there is a non-fatal error, please keep trying this state"<br>
FATAL meant "there is no more hope, kill the state"<br>
<br></blockquote></div><div><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Note that this applies to the _states_ and not the connection. That is,<br>
starting a new keying attempt after a timeout and/or revival are not<br>
related to these state changes.<br></blockquote><div><br></div><div>If a state transition fails then it fails. There is no "please keep trying this state".</div><div>The only remaining question is how much damage should the failure inflict? Delete the state or delete the IKE family (and yes any exchange can trigger a delete family, a critical payload, bad payload lengths, ...).</div><div><br></div><div>However, what I have found and fixed is code trying to ignore failure (failure is not not an option :-). For instance, code would ignore unexpected notifications in an IKE_AUTH response, and instead let things retransmit. This is futile. The response was protected, so no number of retransmits will change its content.<br></div><br><div><div><br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
In that sense, a CREATE_CHILD_SA that received an answer that leads to<br>
an unfixable isse, like TS_UNACCEPTABLE, is a FATAL state change for<br>
the child sa. FAIL would mean we might try to retransmit, which is<br>
what is wrongly happening here. FAIL would also happen if we got an<br>
unauthenticated reply we didn't like, so we would still wait for a<br>
possible unauthenticated reply we like (to prevent attackers from<br>
locking out legitimate packets).<br></blockquote><div><br></div><div>If a packet fails protection, it's STF_IGNOREd. The exception is with IKE_SA_INIT bit it too is free to STF_IGNORE a message.<br></div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
The way IKEv2 works with serial MSGID, we really don't have STF_FAIL in<br>
that sense. We only have STF_OK or STF_FATAL. (well also STF_SUSPEND etc)<br>
The only STF_FAIL would be the if we get no reply, and we need to<br>
retransmit. I can't think of an STF_FAIL that would operate on the child<br>
SA state.<br></blockquote><div><br></div><div>We're somewhat on the same page. Any message can carry a notification that implies that the IKE family is dead. For instance, INVALID_SYNTAX. So the exchange outcome is either Ok, or fatal.<br></div></div><div class="gmail_quote"><br></div><div class="gmail_quote">However with a CREATE_CHILD_SA exchange, there's three outcomes:<br></div><div class="gmail_quote"><div><div>- exchange succeeds; child succeeds<br></div><div>- exchange succeeds; child fails (ike survives)<br></div><div>- exchange fails, implying that the ike family dies<br></div></div></div><div class="gmail_quote">hence the need for three status</div><div class="gmail_quote"><div><br></div><div><div><div>Taking a step back, and ignoring our current code, which would you consider more serious: FAIL or FATAL?</div><div>Personally, I'd treat something fatal as more serious and assume it's wiping things out.<br></div><div><br></div></div></div></div><div class="gmail_quote">The IKE_AUTH exchange is different. The message is passed to the IKE SA, and then as a nested operation, it tries to create the child. If the attempt to create the child proves fatal, the IKE SA can proceed accordingly.</div><div class="gmail_quote"><br></div><div class="gmail_quote">If CREATE_CHILD_SA followed this model (I call it nested states), then we could only need two codes.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
</blockquote></div></div></div>