On Mon, 16 Nov 2015, D. Hugh Redelmeier wrote:

I don't really understand this.  So I added passerts to find out when the
list had more than one entry.  Apparently never in our test suite.  (If I
remember correctly, which is questionable.)

But Paul removed one of the passerts, surely because it was firing in
the Real World.  See ec90950ab9044e71e9a44c22f2443ace6a940fb6

Paul: when did this arise?  The commit comment isn't explicit enough for
me ("xauth: remove passert that is false for conns going up -> down ->
up").

When using xauth + modeconfig with split tunnel. Since we do not support
the server side split tunnel, there is no test case for this.

This passert was in modecfg_inR1(), within handling ISAKMP_CFG_REPLY,
handling CISCO_SPLIT_INC.
        passert(last_spd->spd_next == NULL);

The thing is, if the passert would fail, later code is suspect:

        tmp_spd->spd_next = NULL;
        last_spd->spd_next = tmp_spd;
        last_spd = tmp_spd;

It sure looks to me as if whatever was in last_spd->spd_next is lost.
At best, this is a leak.  But if the value had a meaning, that meaning
has been lost.

The IPsec SA should be killed when the --down is issued. I am not sure
why the last_spd is not set to NULL.

We need to be able to duplicate this case in the test suite.

Other people would like us to support split tunnel server side too :)
It is a matter of checking if rightsubnet is not 0/0, and then adding
the SPLIT_INC payload, and then adding those extra spd entries :P

Paul
_______________________________________________
Swan-dev mailing list
[email protected]
https://lists.libreswan.org/mailman/listinfo/swan-dev

Reply via email to