<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 15:23, Paul Wouters <<a href="mailto:paul@nohats.ca">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"><div dir="auto">On Jul 5, 2021, at 15:03, Andrew Cagney <<a href="mailto:andrew.cagney@gmail.com" target="_blank">andrew.cagney@gmail.com</a>> wrote:<br><div dir="ltr"><blockquote type="cite"><br></blockquote></div><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Was the test run with?</div></div></div></div></blockquote><div><br></div>As you did without my patch…</div></blockquote><div><br></div><div>But with:  ikev2: only return STF_{OK,FAIL,FATAL} from CREATE_CHILD_SA processor</div><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"><div dir="auto"><div><br></div><div><br><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div dir="ltr">With "pluto: When Child state fails, don't tear down IKE SA" reverted I see:</div><div class="gmail_quote"><div><br>+003 "road/0x2" #3: dropping unexpected CREATE_CHILD_SA message containing TS_UNACCEPTABLE notification; message payloads: SK; encrypted payloads: N; missing payloads: SA,Ni,TSi,TSr<br>+036 "road/0x2" #3: encountered fatal error in state STATE_V2_NEW_CHILD_I1</div><div><br></div><div>which is a missing state transition.  Likely fallout from the above.<br></div></div></div></div></blockquote><div><br></div>In your terms, this should be “fail”, not as it shows “fatal”.</div></div></blockquote></div><div class="gmail_quote"><br></div><div class="gmail_quote">That isn't the problem here.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Because of a bug, the code encountered a protected packet with no matching state transition.  The response, which I think is reasonable, was to kill the IKE family.<br><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"><div dir="auto"><div><br><blockquote type="cite"><div dir="ltr"><div dir="ltr"><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">And for this I see:</blockquote><div><br>+003 "road/0x1" #2: IKE_AUTH response contained the error notification TS_UNACCEPTABLE<br>+002 "road/0x4" #1: deleting other state #2 connection (STATE_V2_IKE_AUTH_CHILD_I0) "road/0x1" and NOT sending notification<br></div><div><br></div><div>which is from this code:</div><div><br></div><div style="margin-left:40px">  v2_notification_t n = process_v2_IKE_AUTH_response_child_sa_payloads(ike, md);<br>        if (v2_notification_fatal(n)) {<br></div><div style="margin-left:40px"><div style="margin-left:40px">               /* already logged */<br>          /*<br>             * XXX: should be sending the fatal notification using<br>                 * a new exchange.<br>             */<br>           return STF_FATAL;<br></div></div><div style="margin-left:40px">       } else if (n != v2N_NOTHING_WRONG) {<br></div><div style="margin-left:40px"><div style="margin-left:40px">          /* already logged */<br>          /*<br>             * XXX: should be sending the child failure<br>            * notification using an additional exchange and then<br>          * leaving the IKE SA up.<br>              *<br>             * Instead just wipe out the IKE family :-(<br>            */<br>           return STF_V2_DELETE_EXCHANGE_INITIATOR_IKE_SA;<br></div></div><div style="margin-left:40px"> }</div></div><div class="gmail_quote"><br></div><div>Like the comments point out, this is a hack.</div></div></div></blockquote><div><br></div>It’s really bad. I’d rather that this subnet never tried again and keeps the house intact.</div></div></blockquote><div><br></div><div>No argument.  It's there to keep existing behaviour.<br></div><div><br></div><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"><div dir="auto"><div>It is common for ends to have mismatched subnets. Right now this means global outage. Can we just return STF_FAIL?</div><div><br></div></div></blockquote></div><div class="gmail_quote"><br></div><div class="gmail_quote">Sure, try it.  It's the first step.</div><div class="gmail_quote"><br></div><div>I'm just not sure what delete_state(child) will do.  Will it put the connection back onto the pending queue?</div><div><br></div><div>Alternatively modify the code in process_v2_IKE_AUTH_response_child_sa_payloads() following the comment:</div><div><br></div><div>    * Was there a child error notification?  The RFC says this<br>   * list isn't definitive.<br>  *<br>     * XXX: can this code assume that the response contains only<br>   * one notify and that is for the child?  Given notifies are<br>  * used to communicate compression I've my doubt.</div><div><br></div><div>to zap the state and return v2N_NOTHING_WRONG.<br></div><div><br></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"><div dir="auto"><div><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div>  The fix means queueing up a new exchange to send a delete notify with the SPI received from the responder?<br></div></div></div></blockquote><div><br></div>There is no need to send a notify when you received TS_UNACCEPTABLE.</div><div><br></div><div>Yes queueing a pending task with delay would be needed but i am happy to do that after the 4.5 release.</div><div><br></div><div>Paul</div></div></blockquote></div></div></div>