[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