From: Tobias Brunner <tobias@strongswan.org>
Date: Mon, 30 Sep 2024 10:26:05 +0000 (+0200)
Subject: mem-pool: Fix issue with make-before-break reauth and multiple IKE_SAs
X-Git-Tag: 6.0.0rc1~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d4a0dd9f93d67648eed1ebb223c6cb853101a80a;p=thirdparty%2Fstrongswan.git

mem-pool: Fix issue with make-before-break reauth and multiple IKE_SAs

If uniqueness checks are disabled and multiple IKE_SAs with the same
identities are created, an offline lease could have gotten reassigned
during a make-before-break reauthentication if such an SA was closed
earlier.  Checking for an online lease for the same client (IP/port)
first ensures that the correct IP is reassigned during the
reauthentication.

References strongswan/strongswan#2472
---

diff --git a/src/libcharon/attributes/mem_pool.c b/src/libcharon/attributes/mem_pool.c
index 95e400353c..a7d0394480 100644
--- a/src/libcharon/attributes/mem_pool.c
+++ b/src/libcharon/attributes/mem_pool.c
@@ -287,6 +287,31 @@ static int get_existing(private_mem_pool_t *this, identification_t *id,
 		return 0;
 	}
 
+	if (peer)
+	{
+		/* check for a valid online lease to reassign during make-before-break
+		 * reauthentication */
+		enumerator = array_create_enumerator(entry->online);
+		while (enumerator->enumerate(enumerator, &lease))
+		{
+			if (lease->hash == hash_addr(peer) &&
+				(requested->is_anyaddr(requested) ||
+				 lease->offset == host2offset(this, requested)))
+			{
+				offset = lease->offset;
+				/* add an additional "online" entry */
+				array_insert(entry->online, ARRAY_TAIL, lease);
+				break;
+			}
+		}
+		enumerator->destroy(enumerator);
+		if (offset)
+		{
+			DBG1(DBG_CFG, "reassigning online lease to '%Y'", id);
+			return offset;
+		}
+	}
+
 	/* check for a valid offline lease, refresh */
 	enumerator = array_create_enumerator(entry->offline);
 	if (enumerator->enumerate(enumerator, &current))
@@ -300,30 +325,6 @@ static int get_existing(private_mem_pool_t *this, identification_t *id,
 	if (offset)
 	{
 		DBG1(DBG_CFG, "reassigning offline lease to '%Y'", id);
-		return offset;
-	}
-	if (!peer)
-	{
-		return 0;
-	}
-	/* check for a valid online lease to reassign */
-	enumerator = array_create_enumerator(entry->online);
-	while (enumerator->enumerate(enumerator, &lease))
-	{
-		if (lease->hash == hash_addr(peer) &&
-			(requested->is_anyaddr(requested) ||
-			 lease->offset == host2offset(this, requested)))
-		{
-			offset = lease->offset;
-			/* add an additional "online" entry */
-			array_insert(entry->online, ARRAY_TAIL, lease);
-			break;
-		}
-	}
-	enumerator->destroy(enumerator);
-	if (offset)
-	{
-		DBG1(DBG_CFG, "reassigning online lease to '%Y'", id);
 	}
 	return offset;
 }