[Swan-dev] I think 229e2d24a4 needs to be reverted

Paul Wouters paul at nohats.ca
Wed May 8 01:22:19 UTC 2019


-#define IS_IKE_SA(st) ( ((st)->st_clonedfrom == SOS_NOBODY) && \
-       (IS_PHASE1((st)->st_state) || IS_PHASE15((st)->st_state) ||
         IS_PARENT_SA(st)) )
+#define IS_IKE_SA(st)   ((st)->st_clonedfrom == SOS_NOBODY)



The idea here is that the normal case for checking IS_IKE_SA() depends
only on the state it is in.

Because we had that stupid thing of IKEv2 using STATE_PARENT_I3 and
STATE_PARENT_R2 for both IKE and CHILD SA, we had to work around a
lot of things, and the clonedfrom check was added. But this is _wrong_
long term, because we want to clone states for IKE too. Cloning _should_
not be the actual check for if a state is parent or child.

So you made a workaround bandaid the actual sole solution, instead of
something that can later be removed again.

Also, I am very concerned this has unknown side effects, because you
reduced the check to no longer care about any specific phase 1 states.
Especially so close to 3.28, I am really concerned about this change.

I do not think your change is required for anything, other than maybe
doing a tiny optimalization for now, so I see no important gains from
this commit.

This commit really should be reverted at this time.

Paul


More information about the Swan-dev mailing list