[Swan-dev] crash after pluto: Fix addresspool reference count

Antony Antony antony at phenome.org
Sat Oct 7 09:40:19 UTC 2017


Hi Wolfgang,
I couldn't stay away from this mystery since I already spend days on it.

One line summary, I can reproduce lsw299. And need to define some things 
before fixing it. There is a partial workaround to get connections 
established.

I applied patch to the test case and forked it to xauth-pluto-25-lsw299.
Now thinking what the fix should be.

>From a higher level the issue is triggered due to a double assignment of IP
address. The addresspool and password file assign the same address. I think 
this behavior is not handled, and lead to crash. Though pluto should not 
crash, this is arguably a a wrong configuration.

To test the theory assign IP address in the password file to one outside the 
pool. Connections come up. However, 'ipsec stop' would hit an assert, which 
might be an issue with the original patch.

-echo "xroad:xOzlFlqtwJIu2:east-any:192.0.2.100" >> /etc/ipsec.d/passwd
+echo "xroad:xOzlFlqtwJIu2:east-any:192.0.2.201" >> /etc/ipsec.d/passwd

Could you please test this? Atleast connections would come up. (no IPsec 
stop or --delete yet).

A simple ,in theory, possibly hard to get right, fix would be no support for 
double, or multiple 1. And fix "ipsec stop" by changing 
rel_lease_addr to handle an address from file properly.

Uniqueness of assigned address would be a nice thing. One could possibly 
argue Uniqueness is not necessary with netkey stack and a typical access vpn 
would work fine without uniqueness. However, debugging would get complicated, 
everything will be decided on SPI + IP, when IP addresses are not unique.

Another approach was possibly mentioned before. If the address from the file 
overlap with the addresspool, no matter it is assigned in the addresspool or 
not, do not again it. However, it would be hard to get this right in the 
entire life time of pluto. Address from the file could overlap with another 
pool, from another connection, and cause a duplicate assignment. One would 
think lets check all addresspools known to pluto at the time of assignment 
from file. This assume no addresspools are added later on:) A bit more 
compute intensive approach would be, also check all existing leases when 
adding new addresspool.

Pluto do not allow addresspool overlap. When adding a new addresspool, 
connection with a pool, there is a check for overlap with other addresspools 
and refuse the overlaping connection. We could extend the overlap check to 
to also check assigned address from files. Note pools can be shared between 
connections if they are exact size. One complexity is the xauth file may 
change on its own. I am not sure what this means.

Thanks again the patch. Crash is no more a mystery! And we have example how 
to use this feature.  

On Fri, Oct 06, 2017 at 09:29:33PM +0200, wolfgang at linogate.de wrote:
> I have added a patch for you for the xauth-pluto-22 test to reproduce 
> lsw299
> with v3.21 and it also triggers the rel_lease_addr crash with my actual patch.

I am think rel_lease_addr need fixing. Warning I didn't not look at the code 
only focused on re-producing.

> The actual problems seems to be when installing a new addresspool from
> ikev1_xauth.c.
> This code is initially from me and I think when I implemented it I overlooked
> that the pool is shared and not copied for the instance.
> I can look to rework it next week.

Added test with ipsec stop, xauth-pluto-26. 

I wonder ipsec stop , with password file, is yet to implemented. I need to 
double check.  xauth-pluto-26 has just one client, address assignment from 
the file, there is a pool defined. The tunnel come up fine, however, pluto 
seems to crash on "ipsec stop". I will stop here for now. 

ABORT: ASSERTION FAILED: pool->pool_refcount > 0 (in 
unreference_addresspool() at addresspool.c:441)

-antony


More information about the Swan-dev mailing list