[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