[Swan-dev] Matt's changes to informational message handling

[Matt Rogers]

----- Original Message -----
> From: "Paul Wouters" <paul at nohats.ca>
> To: "D. Hugh Redelmeier" <hugh at mimosa.com>, "Matt Rogers" <mrogers at redhat.com>
> Cc: "Libreswan Develpment List" <swan-dev at lists.libreswan.org>
> Sent: Tuesday, February 4, 2014 1:52:26 AM
> Subject: Re: [Swan-dev] Matt's changes to informational message handling
> 
> On Tue, 4 Feb 2014, D. Hugh Redelmeier wrote:
> 
> > I don't really understand this change.  I imagine it is correct.

The problem was with the informational exchange functions where they
didn't make a distinction between the SA initiator/responder and the 
current info-exchange initiator/responder (since an exchange can be
started by the responder). This caused the SA responder to always use a
bad msgid, and the SA initiator expected differently.

While the SA initiator could send the proper exchanges, if the responder
has dpd enabled as well, things would go haywire and eventually both
hit the timeout.

For this purpose the responder also needed to keep track of a nexuse
value when the counters are updated.

> >
> > Would it be possible to get a test case that works correctly because
> > of the change?
> >
> > I know it is asking a lot: only a few know how to make new tests.
> > Maybe Paul could generate a test if you explained what should be
> > tested.
> 
> I had already asked Matt for a test too. Especially since the change
> seemed to hit a passert for me:
> 
>  	} else if (type == EVENT_v2_LIVENESS) {
>  		passert(st->st_liveness_event == ev);
> 
> I am also looking now at why the connection came up with DPD=none.

I'll take a look at the test cases today (up to this point I've just been 
testing this manually). Not sure yet about the passert, I've not run into
that myself.

The main scenario to test would be liveness enabled on east and west, and
their informational exchanges can overlap properly: then block traffic and
check that they each ran the dpd action after the timeout.

> 
> It seems like we have st->st_dpd_local that will tell us whether we
> should/will send out DPD/liveness probes. But it is hardly used,
> and only set in ikev1. And the only consumer of this variable is
> the DPD=xxxx line, so it might as well check it directly using:
> 
>  	if (st->st_connection->dpd_delay && st->st_connection->dpd_timeout)
> 
> which is how st->st_dpd_local gets set.
> 
> iekv2 has st->st_liveness that I still need to look into.
> 
> Paul
>

Other than the configuration keywords, I've tried to keep the liveness and dpd
variables seperate for clarity. In this situation, st_liveness serves the same
purpose as st_dpd_local does for ikev1 (set during complete_vX_state_transition).

Thank you both for looking! 

Matt