[Swan-dev] the problems with Record 'n' Send and delete_state()

Andrew Cagney andrew.cagney at gmail.com
Mon Feb 4 16:04:31 UTC 2019


In IKEv2, where pluto's window size aka N==1:

   An IKE endpoint MUST NOT exceed the peer's stated window size for
   transmitted IKE requests.  In other words, if the responder stated
   its window size is N, then when the initiator needs to make a request
   X, it MUST wait until it has received responses to all requests up
   through request X-N.

and our code is designed to handle this:

- the state transition records any outgoing message (adding it to the
send queue)
- complete_v2_state_transition() -> success_v2_state_transition() ->
which sends the next message in the queue
(I suspect this logic can be simplified - I can't 100% convince myself
that the current code won't re-order messages - but that is in the
noise)

however, we've also code doing what I call Record 'n' Send (name comes
from a function I deleted) where the recorded message is sent
immediately.  This is then paired with the all singing all dancing all
knowing function that everyone but me seems to love - delete_state().
For instance, mid transition we find:

for each or a CHILD SA:
..delete-state(CHILD SA) which does:
....record delete notify for child in CHILD SA (route response to IKE SA)
....send delete notify found in CHILD SA
....delete child containing notify (why response is routed to IKE SA)
possibly followed by:
..delete_state(IKE SA) which does:
....record delete notify for IKE SA in IKE SA (route response to IKE SA)
....send delete notify found in IKE SA
....(?) delete IKE SA containing notify

which is a lot of outgoing packets with no ability to re-transmit and
no ability to process responses (as an aside, I think this may help
explain why a 'clean' shutdown isn't very clean).  Further because the
messages jumps the outgoing message queue we've got the potential for
re-ordering outgoing messages.

So how do I think it should work?  I think the existing delete_state()
needs to be broken up.  Any logic dealing with sending notifications
turned into state transitions leaving a very simple discard_state()
function vis:

For the initiator, something like:

- the states record their delete request notification, schedule a
discard, and then transition ESTABLISHED -> DEL
- the response gets routed through the correct state which can then
discard itself
- the discard even triggers then the state discards itself

For the responder, something like:

- record the delete response, schedule a discard, and then transition
ESTABLISHED -> DEL
- since the state lingers re-transmits are processed properly
- the discard event then cleans things up

There are other tweaks vis:

- code initiating and responding to notification payloads using just
Record 'n' Send, that hopefully just requires a tweak to route things
through success_v2_state_transition()
- the IKE SA should be made solely responsible for managing the
message window and transmits

Andrew


More information about the Swan-dev mailing list