[Swan-dev] revive connection's false positive found. It's a bug in whack delete vs down
Paul Wouters
paul at nohats.ca
Thu Mar 14 10:09:49 UTC 2019
I found out why linux-audit-01 falsely triggered the new "connection must remain up" messagem
and false trigered the revive connection code.
ipsec auto --down ends up calling terminate_connection()
ipsec auto --delete ends up calling delete_connection()
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
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
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",
More information about the Swan-dev
mailing list