From 08a3ee0cce1b6f09da22d213c0cc91f552094e79 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 27 Oct 2020 17:02:21 +0100 Subject: [PATCH] bus: Change ike_update() signature and only call it once This avoids multiple events when both addresses change (e.g. switching address families). --- src/libcharon/bus/bus.c | 7 ++-- src/libcharon/bus/bus.h | 9 ++--- src/libcharon/bus/listeners/listener.h | 10 +++--- .../plugins/connmark/connmark_listener.c | 17 ++------- .../plugins/forecast/forecast_listener.c | 15 ++------ src/libcharon/sa/ike_sa.c | 35 ++++++++++++------- 6 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/libcharon/bus/bus.c b/src/libcharon/bus/bus.c index b7348f0f9..f5b0651f7 100644 --- a/src/libcharon/bus/bus.c +++ b/src/libcharon/bus/bus.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2016 Tobias Brunner + * Copyright (C) 2011-2020 Tobias Brunner * Copyright (C) 2006 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -866,7 +866,7 @@ METHOD(bus_t, ike_rekey, void, } METHOD(bus_t, ike_update, void, - private_bus_t *this, ike_sa_t *ike_sa, bool local, host_t *new) + private_bus_t *this, ike_sa_t *ike_sa, host_t *local, host_t *remote) { enumerator_t *enumerator; entry_t *entry; @@ -881,7 +881,8 @@ METHOD(bus_t, ike_update, void, continue; } entry->calling++; - keep = entry->listener->ike_update(entry->listener, ike_sa, local, new); + keep = entry->listener->ike_update(entry->listener, ike_sa, local, + remote); entry->calling--; if (!keep) { diff --git a/src/libcharon/bus/bus.h b/src/libcharon/bus/bus.h index abf95a4c9..22248f7a0 100644 --- a/src/libcharon/bus/bus.h +++ b/src/libcharon/bus/bus.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012-2016 Tobias Brunner + * Copyright (C) 2012-2020 Tobias Brunner * Copyright (C) 2006-2009 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -417,10 +417,11 @@ struct bus_t { * IKE_SA peer endpoint update hook. * * @param ike_sa updated IKE_SA, having old endpoints set - * @param local TRUE if local endpoint gets updated, FALSE for remote - * @param new new endpoint address and port + * @param local new/current local endpoint address and port + * @param remote new/current remote endpoint address and port */ - void (*ike_update)(bus_t *this, ike_sa_t *ike_sa, bool local, host_t *new); + void (*ike_update)(bus_t *this, ike_sa_t *ike_sa, host_t *local, + host_t *remote); /** * IKE_SA reestablishing hook (before resolving hosts). diff --git a/src/libcharon/bus/listeners/listener.h b/src/libcharon/bus/listeners/listener.h index 0f3b8578a..b24785a62 100644 --- a/src/libcharon/bus/listeners/listener.h +++ b/src/libcharon/bus/listeners/listener.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2016 Tobias Brunner + * Copyright (C) 2011-2020 Tobias Brunner * Copyright (C) 2009 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -160,13 +160,15 @@ struct listener_t { /** * Hook called for IKE_SA peer endpoint updates. * + * At least one endpoint has changed when this is invoked. + * * @param ike_sa updated IKE_SA, having old endpoints set - * @param local TRUE if local endpoint gets updated, FALSE for remote - * @param new new endpoint address and port + * @param local new/current local endpoint address and port + * @param remote new/current remote endpoint address and port * @return TRUE to stay registered, FALSE to unregister */ bool (*ike_update)(listener_t *this, ike_sa_t *ike_sa, - bool local, host_t *new); + host_t *local, host_t *remote); /** * Hook called when an initiator reestablishes an IKE_SA. diff --git a/src/libcharon/plugins/connmark/connmark_listener.c b/src/libcharon/plugins/connmark/connmark_listener.c index 7d23f1a23..824f7a199 100644 --- a/src/libcharon/plugins/connmark/connmark_listener.c +++ b/src/libcharon/plugins/connmark/connmark_listener.c @@ -494,24 +494,13 @@ METHOD(listener_t, child_rekey, bool, METHOD(listener_t, ike_update, bool, private_connmark_listener_t *this, ike_sa_t *ike_sa, - bool local, host_t *new) + host_t *local, host_t *remote) { struct iptc_handle *ipth; enumerator_t *enumerator; child_sa_t *child_sa; - host_t *dst, *src; bool oldencap, newencap; - if (local) - { - dst = new; - src = ike_sa->get_other_host(ike_sa); - } - else - { - dst = ike_sa->get_my_host(ike_sa); - src = new; - } /* during ike_update(), has_encap() on the CHILD_SA has not yet been * updated, but shows the old state. */ newencap = ike_sa->has_condition(ike_sa, COND_NAT_ANY); @@ -525,9 +514,9 @@ METHOD(listener_t, ike_update, bool, ipth = init_handle(); if (ipth) { - if (manage_policies(this, ipth, dst, src, oldencap, + if (manage_policies(this, ipth, local, remote, oldencap, child_sa, FALSE) && - manage_policies(this, ipth, dst, src, newencap, + manage_policies(this, ipth, local, remote, newencap, child_sa, TRUE)) { commit_handle(ipth); diff --git a/src/libcharon/plugins/forecast/forecast_listener.c b/src/libcharon/plugins/forecast/forecast_listener.c index b928cad35..f4d4081e3 100644 --- a/src/libcharon/plugins/forecast/forecast_listener.c +++ b/src/libcharon/plugins/forecast/forecast_listener.c @@ -569,24 +569,13 @@ METHOD(listener_t, child_rekey, bool, METHOD(listener_t, ike_update, bool, private_forecast_listener_t *this, ike_sa_t *ike_sa, - bool local, host_t *new) + host_t *local, host_t *remote) { struct iptc_handle *ipth; enumerator_t *enumerator; child_sa_t *child_sa; - host_t *lhost, *rhost; bool encap; - if (local) - { - lhost = new; - rhost = ike_sa->get_other_host(ike_sa); - } - else - { - lhost = ike_sa->get_my_host(ike_sa); - rhost = new; - } /* during ike_update(), has_encap() on the CHILD_SA has not yet been * updated, but shows the old state. */ encap = ike_sa->has_condition(ike_sa, COND_NAT_ANY); @@ -600,7 +589,7 @@ METHOD(listener_t, ike_update, bool, if (ipth) { if (remove_entry(this, ipth, child_sa) && - add_entry(this, ipth, lhost, rhost, child_sa, encap)) + add_entry(this, ipth, local, remote, child_sa, encap)) { commit_handle(ipth); } diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index f1e3e8fe5..b8fdcbb07 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -1116,7 +1116,8 @@ METHOD(ike_sa_t, float_ports, void, METHOD(ike_sa_t, update_hosts, void, private_ike_sa_t *this, host_t *me, host_t *other, bool force) { - bool update = FALSE; + host_t *new_me = NULL, *new_other = NULL; + bool silent = FALSE; if (me == NULL) { @@ -1131,18 +1132,16 @@ METHOD(ike_sa_t, update_hosts, void, if (this->my_host->is_anyaddr(this->my_host) || this->other_host->is_anyaddr(this->other_host)) { - set_my_host(this, me->clone(me)); - set_other_host(this, other->clone(other)); - update = TRUE; + new_me = me; + new_other = other; + silent = TRUE; } else { /* update our address in any case */ if (force && !me->equals(me, this->my_host)) { - charon->bus->ike_update(charon->bus, &this->public, TRUE, me); - set_my_host(this, me->clone(me)); - update = TRUE; + new_me = me; } if (!other->equals(other, this->other_host) && @@ -1154,20 +1153,32 @@ METHOD(ike_sa_t, update_hosts, void, (!has_condition(this, COND_NAT_HERE) || !has_condition(this, COND_ORIGINAL_INITIATOR))) { - charon->bus->ike_update(charon->bus, &this->public, FALSE, other); - set_other_host(this, other->clone(other)); - update = TRUE; + new_other = other; } } } - /* update all associated CHILD_SAs, if required */ - if (update) + if (new_me || new_other) { enumerator_t *enumerator; child_sa_t *child_sa; linked_list_t *vips; + if (!silent) + { + charon->bus->ike_update(charon->bus, &this->public, + new_me ?: this->my_host, + new_other ?: this->other_host); + } + if (new_me) + { + set_my_host(this, new_me->clone(new_me)); + } + if (new_other) + { + set_other_host(this, new_other->clone(new_other)); + } + vips = linked_list_create_from_enumerator( array_create_enumerator(this->my_vips));