From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 7 Sep 2023 15:33:00 +0000 (+0200)
Subject: ike-sa: Destroy CHILD_SAs in order
X-Git-Tag: 5.9.12dr1~2
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=80e27fe9fd179b679d195b30a3694d5fe815e6e2;p=thirdparty%2Fstrongswan.git

ike-sa: Destroy CHILD_SAs in order

This works around an issue that occurs when recreating an IKE_SA with
multiple CHILD_SAs that use dynamically allocated reqids.

We currently try to preserve the reqid when reestablishing, so the
create-child task gets the reqid of the previous CHILD_SA and will try
to reallocate that once the CHILD_SA is installed.  Before that, the old
CHILD_SA is destroyed and the reqid is released and added to the array
of reqids to get reused.  However, because of the reverse order used here,
the first reqid in the array is the one of the last CHILD_SA.

So it can happen that a newly created CHILD_SA gets a reqid reassigned
that a later CHILD_SA will then also claim for itself and get assigned
because an entry with that reqid exists.  So multiple CHILD_SAs with
different traffic selectors could then share the same reqid.

References strongswan/strongswan#1855
---

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index a46a9754cc..7d432c570e 100644
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -3021,7 +3021,7 @@ METHOD(ike_sa_t, destroy, void,
 	}
 	/* uninstall CHILD_SAs before virtual IPs, otherwise we might kill
 	 * routes that the CHILD_SA tries to uninstall. */
-	while (array_remove(this->child_sas, ARRAY_TAIL, &child_sa))
+	while (array_remove(this->child_sas, ARRAY_HEAD, &child_sa))
 	{
 		charon->child_sa_manager->remove(charon->child_sa_manager, child_sa);
 		child_sa->destroy(child_sa);