From: Thomas Egerer <thomas.egerer@secunet.com>
Date: Wed, 4 Feb 2015 11:47:03 +0000 (+0100)
Subject: ha: Perform child rekeying outside of CHILD_SA enumerator
X-Git-Tag: 5.3.0dr1~83
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=875f7be5fcc85b863fa79edca529a88485aecd0f;p=thirdparty%2Fstrongswan.git

ha: Perform child rekeying outside of CHILD_SA enumerator

When rekey_child_sa is called while enumerating the children of an IKE_SA, and
the child to be rekeyed is redundant a QUICK_DELETE task is queued instead of a
QUICK_MODE task. This alters the IKE_SA's list of children (ike_sa_t::child_sas)
invalidating the current element of the child_sa_enumerator. The enumerate
function of linked_list_t will then advance to an element with unpredictable
contents most likely resulting in an segmentation violation. A similar behavior
should be observed when delete_child_sa is called.

This patch creates a list of protocol/spi values while holding the
child_sa_enumerator and performs the rekeying (deletion of redundant) chlidren
after releasing the enumerator.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---

diff --git a/src/libcharon/plugins/ha/ha_cache.c b/src/libcharon/plugins/ha/ha_cache.c
index 60e75fc7e8..6c1b3471dd 100644
--- a/src/libcharon/plugins/ha/ha_cache.c
+++ b/src/libcharon/plugins/ha/ha_cache.c
@@ -196,9 +196,26 @@ static status_t rekey_children(ike_sa_t *ike_sa)
 	enumerator_t *enumerator;
 	child_sa_t *child_sa;
 	status_t status = SUCCESS;
+	linked_list_t *children;
+	struct {
+		protocol_id_t protocol;
+		u_int32_t spi;
+	} *info;
 
+	children = linked_list_create();
 	enumerator = ike_sa->create_child_sa_enumerator(ike_sa);
-	while (enumerator->enumerate(enumerator, (void**)&child_sa))
+	while (enumerator->enumerate(enumerator, &child_sa))
+	{
+		INIT(info,
+			.protocol = child_sa->get_protocol(child_sa),
+			.spi = child_sa->get_spi(child_sa, TRUE),
+		);
+		children->insert_last(children, info);
+	}
+	enumerator->destroy(enumerator);
+
+	enumerator = children->create_enumerator(children);
+	while (enumerator->enumerate(enumerator, &info))
 	{
 		if (ike_sa->supports_extension(ike_sa, EXT_MS_WINDOWS) &&
 			ike_sa->has_condition(ike_sa, COND_NAT_THERE))
@@ -207,17 +224,13 @@ static status_t rekey_children(ike_sa_t *ike_sa)
 			 * with an "invalid situation" error. We just close the CHILD_SA,
 			 * Windows will reestablish it immediately if required. */
 			DBG1(DBG_CFG, "resyncing CHILD_SA using a delete");
-			status = ike_sa->delete_child_sa(ike_sa,
-											 child_sa->get_protocol(child_sa),
-											 child_sa->get_spi(child_sa, TRUE),
+			status = ike_sa->delete_child_sa(ike_sa, info->protocol, info->spi,
 											 FALSE);
 		}
 		else
 		{
 			DBG1(DBG_CFG, "resyncing CHILD_SA using a rekey");
-			status = ike_sa->rekey_child_sa(ike_sa,
-											child_sa->get_protocol(child_sa),
-											child_sa->get_spi(child_sa, TRUE));
+			status = ike_sa->rekey_child_sa(ike_sa, info->protocol, info->spi);
 		}
 		if (status == DESTROY_ME)
 		{
@@ -225,6 +238,8 @@ static status_t rekey_children(ike_sa_t *ike_sa)
 		}
 	}
 	enumerator->destroy(enumerator);
+	children->destroy_function(children, free);
+
 	return status;
 }