[Swan-dev] analyses of regression in test ikev2-ike-rekey-03

[Paul Wouters]

On Sat, 23 Jun 2018, Antony Antony wrote:

> I will bring up my concern again of handling uniqueids again.
>
> uniqueid related logic (ISAKMP_SA_established) called during ike rekey seems wrong to me.

You are right, it should not do anything during rekey. I had not
realised that this function was also called during rekey. I checked,
and IKEv1 does _not_ call this function during rekey. (eg two --up's)

> I am also considering to set st_seen_initialc=false after
> duplicating state for IKE rekey.

Please don't. The st_seen_*s are indicators of what we got from the
peer. It does not mean we got it in some certain part of the
negotiation. So unsetting it would create a "lie" on the state.

Instead, as you said, any conn/state cleanup based on initial contact
should only be done during IKE_AUTH when we receive the initial contact
payload and not during rekey. Since we cannot check the current state,
since for both rekey and initial response the state is STATE_PARENT_R2,
I replaced the IKEv2 ISAKMP_SA_established(pst) call with:

         if (isa_xchg == ISAKMP_v2_CREATE_CHILD_SA) {
                 /* skip check for rekey */
                 pst->st_connection->newest_isakmp_sa = pst->st_serialno;
         } else {
                 ISAKMP_SA_established(pst);
         }

I tested with ikev2-ike-rekey-03 which confirmed the ISAKMP_SA_established()
call only happened once on the responder.

Unfortunately though, Microsoft Windows does not send Initial Contact.
This is confirmed at: https://msdn.microsoft.com/en-us/library/cc233476.aspx

 	"INITIAL_CONTACT: It MAY be sent when an IKE_SA is established after a
 	crash,…"


 	Section 3.10.11


 	Supported in Windows 2000 Professional through Windows Vista and Windows
 	2000 Server through Windows Server 2008.

Although this text comes from IKEv1 (https://tools.ietf.org/html/rfc4306#section-3.10)

So I am currently testing without checking for the initial contact (but
did not commit that change yet)

So, perhaps we should go back to ignoring Initial Contact and always do
this, and depend on our uniqueids= and authby=secret override ?

Paul