[Swan-dev] revive connection's false positive found. It's a bug in whack delete vs down
Andrew Cagney
andrew.cagney at gmail.com
Thu Mar 14 14:41:58 UTC 2019
BTW, per IRC, another code path is whack --deletestate #N
On Thu, 14 Mar 2019 at 06:09, Paul Wouters <paul at nohats.ca> wrote:
>
>
> I found out why linux-audit-01 falsely triggered the new "connection must remain up" messagem
> and false trigered the revive connection code.
What's the expectation here?
> ipsec auto --down ends up calling terminate_connection()
Gently shut down the SAs?
Correctly implemented this requires a number of state transitions as,
first, the CHILD SAs are deleted and then the IKE SA is deleted.
delete_state() doesn't cut it, but this is long term.
> ipsec auto --delete ends up calling delete_connection()
Wipe the connections off the map (which isn't as friendly but seems to
be what happens), or first gently shutdown the SAs?
This probably explains the strangeness I encountered when hacking
libreswan-up-down.sh - even though I was being told that --down was
redundant in --down --delete, I left it in as I found --delete behaved
a little strange.
> However, one would expect delete_connection() to do everything
> of terminate_connection() plus the actual delete. But it does not.
>
> terminate_connection() does:
> - clears POLICY_UP which explains the above bug later in delete_state()
> - calls flush_pending_by_connection();
> - if IKE SA is shared, delete_state with serial from c->newest_ipsec_sa
> - if IKE SA not shared, call delete_states_by_connection()
>
> delete_connection() does:
> - if connection is an instance,
> - free lease
> - call release_connection()
> - if a CK_GROUP, delete group
> - remove connection from connections list
> - cleanup hostpair
> - free up memory of connection
> - handle c->spd routing stuff
>
>
> This kind of assumes the user will do a --down before a --delete, as otherwise the
> tasks in terminate_connection are skipped. I havent yet did archeology
> to see if / when this has happened.
>
> We can fix this in 3 ways:
>
> 1) change rcv_whack() for the --delete action to call both terminate_connection() and delete_connection()
> 2) modify delete_connection to first call terminate_connection
Both of these are the gentle options.
If shutting down SAs is ever fixed, the code will get more complicated.
> 3) modify delete_connection to remove POLICY_UP and call flush_pending_by_connection()
>
> I've done 1) on my laptop for testing and it seems okay, see below. But I think 3 is probably more
> connect? 2 seems to be doing some duplicate work
Where as this restores existing behaviour.
BTW, I 'm pretty sure flush_pending_by_connection() needs to zap
connection from the revive list. For instance, when a connection on
the revive list gets deleted then added it will spontaneously come up.
> diff --git a/programs/pluto/rcv_whack.c b/programs/pluto/rcv_whack.c
> index 0b1943eb89..40a20e7d0f 100644
> --- a/programs/pluto/rcv_whack.c
> +++ b/programs/pluto/rcv_whack.c
> @@ -383,8 +383,11 @@ void whack_process(fd_t whackfd, const struct whack_message *const m)
> * To make this more useful, in only this combination,
> * delete will silently ignore the lack of the connection.
> */
> - if (m->whack_delete)
> + if (m->whack_delete) {
> + passert(m->name != NULL);
> + terminate_connection(m->name);
> delete_connections_by_name(m->name, !m->whack_connection);
> + }
>
> if (m->whack_deleteuser) {
> log_to_log("received whack to delete connection by user %s",
>
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev
More information about the Swan-dev
mailing list