[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