mem-pool: Pass the remote IKE address, to re-acquire() an address during reauth

With make-before-break IKEv2 re-authentication, virtual IP addresses must be
assigned overlapping to the same peer. With the remote IKE address, the backend
can detect re-authentication attempts by comparing the remote host address and
port. This allows proper reassignment of the virtual IP if it is re-requested.

This change removes the mem-pool.reassign_online option, as it is obsolete now.
IPs get automatically reassigned if a peer re-requests the same address, and
additionally connects from the same address and port.

7 files changed, 87 insertions, 55 deletions

diff --git a/conf/options/charon.opt b/conf/options/charon.opt
index fc38a1451..f0969e6c6 100644
--- a/conf/options/charon.opt
+++ b/conf/options/charon.opt
@@ -206,10 +206,6 @@ charon.make_before_break = no
 	gaps during reauthentication, but requires support for overlapping SAs by
 	the peer. strongSwan can handle such overlapping SAs since version 5.3.0.
 
-charon.mem-pool.reassign_online = no
-	Reassign an online IP address lease from an in-memory address pool if a
-	client with the same identity requests it explicitly.
-
 charon.multiple_authentication = yes
 	Enable multiple authentication exchanges (RFC 4739).
 
diff --git a/src/libcharon/attributes/mem_pool.c b/src/libcharon/attributes/mem_pool.c
index f35ffaa9c..759d94126 100644
--- a/src/libcharon/attributes/mem_pool.c
+++ b/src/libcharon/attributes/mem_pool.c
@@ -70,20 +70,25 @@ struct private_mem_pool_t {
 	 * lock to safely access the pool
 	 */
 	mutex_t *mutex;
-
-	/**
-	 * Do we reassign online leases to the same identity, if requested?
-	 */
-	bool reassign_online;
 };
 
 /**
+ * A unique lease address offset, with a hash of the peer host address
+ */
+typedef struct {
+	/** lease, as offset */
+	u_int offset;
+	/** hash of remote address, to allow duplicates */
+	u_int hash;
+} unique_lease_t;
+
+/**
  * Lease entry.
  */
 typedef struct {
 	/* identitiy reference */
 	identification_t *id;
-	/* array of online leases, as u_int offset */
+	/* array of online leases, as unique_lease_t */
 	array_t *online;
 	/* array of offline leases, as u_int offset */
 	array_t *offline;
@@ -98,7 +103,7 @@ static entry_t* entry_create(identification_t *id)
 
 	INIT(entry,
 		.id = id->clone(id),
-		.online = array_create(sizeof(u_int), 0),
+		.online = array_create(sizeof(unique_lease_t), 0),
 		.offline = array_create(sizeof(u_int), 0),
 	);
 	return entry;
@@ -240,12 +245,25 @@ METHOD(mem_pool_t, get_offline, u_int,
 }
 
 /**
+ * Create a unique hash for a remote address
+ */
+static u_int hash_addr(host_t *addr)
+{
+	if (addr)
+	{
+		return chunk_hash_inc(addr->get_address(addr), addr->get_port(addr));
+	}
+	return 0;
+}
+
+/**
  * Get an existing lease for id
  */
 static int get_existing(private_mem_pool_t *this, identification_t *id,
-						host_t *requested)
+						host_t *requested, host_t *peer)
 {
 	enumerator_t *enumerator;
+	unique_lease_t *lease, reassign;
 	u_int *current;
 	entry_t *entry;
 	int offset = 0;
@@ -260,8 +278,9 @@ static int get_existing(private_mem_pool_t *this, identification_t *id,
 	enumerator = array_create_enumerator(entry->offline);
 	if (enumerator->enumerate(enumerator, &current))
 	{
-		offset = *current;
-		array_insert(entry->online, ARRAY_TAIL, current);
+		reassign.offset = offset = *current;
+		reassign.hash = hash_addr(peer);
+		array_insert(entry->online, ARRAY_TAIL, &reassign);
 		array_remove_at(entry->offline, enumerator);
 	}
 	enumerator->destroy(enumerator);
@@ -270,19 +289,20 @@ static int get_existing(private_mem_pool_t *this, identification_t *id,
 		DBG1(DBG_CFG, "reassigning offline lease to '%Y'", id);
 		return offset;
 	}
-	if (!this->reassign_online)
+	if (!peer)
 	{
 		return 0;
 	}
 	/* check for a valid online lease to reassign */
 	enumerator = array_create_enumerator(entry->online);
-	while (enumerator->enumerate(enumerator, &current))
+	while (enumerator->enumerate(enumerator, &lease))
 	{
-		if (*current == host2offset(this, requested))
+		if (lease->offset == host2offset(this, requested) &&
+			lease->hash == hash_addr(peer))
 		{
-			offset = *current;
+			offset = lease->offset;
 			/* add an additional "online" entry */
-			array_insert(entry->online, ARRAY_TAIL, current);
+			array_insert(entry->online, ARRAY_TAIL, lease);
 			break;
 		}
 	}
@@ -297,10 +317,10 @@ static int get_existing(private_mem_pool_t *this, identification_t *id,
 /**
  * Get a new lease for id
  */
-static int get_new(private_mem_pool_t *this, identification_t *id)
+static int get_new(private_mem_pool_t *this, identification_t *id, host_t *peer)
 {
 	entry_t *entry;
-	u_int offset = 0;
+	unique_lease_t lease = {};
 
 	if (this->unused < this->size)
 	{
@@ -311,28 +331,31 @@ static int get_new(private_mem_pool_t *this, identification_t *id)
 			this->leases->put(this->leases, entry->id, entry);
 		}
 		/* assigning offset, starting by 1 */
-		offset = ++this->unused + (this->base_is_network_id ? 1 : 0);
-		array_insert(entry->online, ARRAY_TAIL, &offset);
+		lease.offset = ++this->unused + (this->base_is_network_id ? 1 : 0);
+		lease.hash = hash_addr(peer);
+		array_insert(entry->online, ARRAY_TAIL, &lease);
 		DBG1(DBG_CFG, "assigning new lease to '%Y'", id);
 	}
-	return offset;
+	return lease.offset;
 }
 
 /**
  * Get a reassigned lease for id in case the pool is full
  */
-static int get_reassigned(private_mem_pool_t *this, identification_t *id)
+static int get_reassigned(private_mem_pool_t *this, identification_t *id,
+						  host_t *peer)
 {
 	enumerator_t *enumerator;
 	entry_t *entry;
-	u_int current, offset = 0;
+	u_int current;
+	unique_lease_t lease = {};
 
 	enumerator = this->leases->create_enumerator(this->leases);
 	while (enumerator->enumerate(enumerator, NULL, &entry))
 	{
 		if (array_remove(entry->offline, ARRAY_HEAD, &current))
 		{
-			offset = current;
+			lease.offset = current;
 			DBG1(DBG_CFG, "reassigning existing offline lease by '%Y'"
 				 " to '%Y'", entry->id, id);
 			break;
@@ -340,7 +363,7 @@ static int get_reassigned(private_mem_pool_t *this, identification_t *id)
 	}
 	enumerator->destroy(enumerator);
 
-	if (offset)
+	if (lease.offset)
 	{
 		entry = this->leases->get(this->leases, id);
 		if (!entry)
@@ -348,14 +371,15 @@ static int get_reassigned(private_mem_pool_t *this, identification_t *id)
 			entry = entry_create(id);
 			this->leases->put(this->leases, entry->id, entry);
 		}
-		array_insert(entry->online, ARRAY_TAIL, &offset);
+		lease.hash = hash_addr(peer);
+		array_insert(entry->online, ARRAY_TAIL, &lease);
 	}
-	return offset;
+	return lease.offset;
 }
 
 METHOD(mem_pool_t, acquire_address, host_t*,
 	private_mem_pool_t *this, identification_t *id, host_t *requested,
-	mem_pool_op_t operation)
+	mem_pool_op_t operation, host_t *peer)
 {
 	int offset = 0;
 
@@ -376,13 +400,13 @@ METHOD(mem_pool_t, acquire_address, host_t*,
 	switch (operation)
 	{
 		case MEM_POOL_EXISTING:
-			offset = get_existing(this, id, requested);
+			offset = get_existing(this, id, requested, peer);
 			break;
 		case MEM_POOL_NEW:
-			offset = get_new(this, id);
+			offset = get_new(this, id, peer);
 			break;
 		case MEM_POOL_REASSIGN:
-			offset = get_reassigned(this, id);
+			offset = get_reassigned(this, id, peer);
 			if (!offset)
 			{
 				DBG1(DBG_CFG, "pool '%s' is full, unable to assign address",
@@ -407,7 +431,8 @@ METHOD(mem_pool_t, release_address, bool,
 	enumerator_t *enumerator;
 	bool found = FALSE, more = FALSE;
 	entry_t *entry;
-	u_int offset, *current;
+	u_int offset;
+	unique_lease_t *current;
 
 	if (this->size != 0)
 	{
@@ -420,7 +445,7 @@ METHOD(mem_pool_t, release_address, bool,
 			enumerator = array_create_enumerator(entry->online);
 			while (enumerator->enumerate(enumerator, &current))
 			{
-				if (*current == offset)
+				if (current->offset == offset)
 				{
 					if (!found)
 					{	/* remove the first entry only */
@@ -472,6 +497,7 @@ METHOD(enumerator_t, lease_enumerate, bool,
 	lease_enumerator_t *this, identification_t **id, host_t **addr, bool *online)
 {
 	u_int *offset;
+	unique_lease_t *lease;
 
 	DESTROY_IF(this->addr);
 	this->addr = NULL;
@@ -480,10 +506,10 @@ METHOD(enumerator_t, lease_enumerate, bool,
 	{
 		if (this->entry)
 		{
-			if (this->online->enumerate(this->online, &offset))
+			if (this->online->enumerate(this->online, &lease))
 			{
 				*id = this->entry->id;
-				*addr = this->addr = offset2host(this->pool, *offset);
+				*addr = this->addr = offset2host(this->pool, lease->offset);
 				*online = TRUE;
 				return TRUE;
 			}
@@ -581,8 +607,6 @@ static private_mem_pool_t *create_generic(char *name)
 		.leases = hashtable_create((hashtable_hash_t)id_hash,
 								   (hashtable_equals_t)id_equals, 16),
 		.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
-		.reassign_online = lib->settings->get_bool(lib->settings,
-								"%s.mem-pool.reassign_online", FALSE, lib->ns),
 	);
 
 	return this;
diff --git a/src/libcharon/attributes/mem_pool.h b/src/libcharon/attributes/mem_pool.h
index 7347bb547..3ee1dd37d 100644
--- a/src/libcharon/attributes/mem_pool.h
+++ b/src/libcharon/attributes/mem_pool.h
@@ -87,13 +87,21 @@ struct mem_pool_t {
 	 * acquire a new lease (MEM_POOL_NEW), and if the pool is full once again
 	 * to assign an existing offline lease (MEM_POOL_REASSIGN).
 	 *
+	 * If the same identity requests a virtual IP that is already assigned to
+	 * it, the peer address and port is used to check if it is the same client
+	 * instance that is connecting. If this is true, the request is considered
+	 * a request for a reauthentication attempt, and the same virtual IP gets
+	 * assigned to the peer.
+	 *
 	 * @param id		the id to acquire an address for
 	 * @param requested	acquire this address, if possible
 	 * @param operation	acquire operation to perform, see above
+	 * @param peer		optional remote IKE address and port
 	 * @return			the acquired address
 	 */
 	host_t* (*acquire_address)(mem_pool_t *this, identification_t *id,
-							   host_t *requested, mem_pool_op_t operation);
+							   host_t *requested, mem_pool_op_t operation,
+							   host_t *peer);
 
 	/**
 	 * Release a previously acquired address.
diff --git a/src/libcharon/plugins/load_tester/load_tester_config.c b/src/libcharon/plugins/load_tester/load_tester_config.c
index bc7c0ffbc..65575c6ac 100644
--- a/src/libcharon/plugins/load_tester/load_tester_config.c
+++ b/src/libcharon/plugins/load_tester/load_tester_config.c
@@ -618,7 +618,7 @@ static host_t *allocate_addr(private_load_tester_config_t *this, uint num)
 	enumerator = this->pools->create_enumerator(this->pools);
 	while (enumerator->enumerate(enumerator, &pool))
 	{
-		found = pool->acquire_address(pool, id, requested, MEM_POOL_NEW);
+		found = pool->acquire_address(pool, id, requested, MEM_POOL_NEW, NULL);
 		if (found)
 		{
 			iface = (char*)pool->get_name(pool);
diff --git a/src/libcharon/plugins/stroke/stroke_attribute.c b/src/libcharon/plugins/stroke/stroke_attribute.c
index 131253c69..cd1b4d093 100644
--- a/src/libcharon/plugins/stroke/stroke_attribute.c
+++ b/src/libcharon/plugins/stroke/stroke_attribute.c
@@ -94,7 +94,7 @@ static mem_pool_t *find_pool(private_stroke_attribute_t *this, char *name)
  */
 static host_t *find_addr(private_stroke_attribute_t *this, linked_list_t *pools,
 						 identification_t *id, host_t *requested,
-						 mem_pool_op_t operation)
+						 mem_pool_op_t operation, host_t *peer)
 {
 	host_t *addr = NULL;
 	enumerator_t *enumerator;
@@ -107,7 +107,7 @@ static host_t *find_addr(private_stroke_attribute_t *this, linked_list_t *pools,
 		pool = find_pool(this, name);
 		if (pool)
 		{
-			addr = pool->acquire_address(pool, id, requested, operation);
+			addr = pool->acquire_address(pool, id, requested, operation, peer);
 			if (addr)
 			{
 				break;
@@ -124,19 +124,20 @@ METHOD(attribute_provider_t, acquire_address, host_t*,
 	host_t *requested)
 {
 	identification_t *id;
-	host_t *addr;
+	host_t *addr, *peer;
 
 	id = ike_sa->get_other_eap_id(ike_sa);
+	peer = ike_sa->get_other_host(ike_sa);
 
 	this->lock->read_lock(this->lock);
 
-	addr = find_addr(this, pools, id, requested, MEM_POOL_EXISTING);
+	addr = find_addr(this, pools, id, requested, MEM_POOL_EXISTING, peer);
 	if (!addr)
 	{
-		addr = find_addr(this, pools, id, requested, MEM_POOL_NEW);
+		addr = find_addr(this, pools, id, requested, MEM_POOL_NEW, peer);
 		if (!addr)
 		{
-			addr = find_addr(this, pools, id, requested, MEM_POOL_REASSIGN);
+			addr = find_addr(this, pools, id, requested, MEM_POOL_REASSIGN, peer);
 		}
 	}
 
diff --git a/src/libcharon/plugins/vici/vici_attribute.c b/src/libcharon/plugins/vici/vici_attribute.c
index 320fe5568..f04bae774 100644
--- a/src/libcharon/plugins/vici/vici_attribute.c
+++ b/src/libcharon/plugins/vici/vici_attribute.c
@@ -96,7 +96,8 @@ static void pool_destroy(pool_t *pool)
  * Find an existing or not yet existing lease
  */
 static host_t *find_addr(private_vici_attribute_t *this, linked_list_t *pools,
-					identification_t *id, host_t *requested, mem_pool_op_t op)
+						 identification_t *id, host_t *requested,
+						 mem_pool_op_t op, host_t *peer)
 {
 	enumerator_t *enumerator;
 	host_t *addr = NULL;
@@ -109,7 +110,8 @@ static host_t *find_addr(private_vici_attribute_t *this, linked_list_t *pools,
 		pool = this->pools->get(this->pools, name);
 		if (pool)
 		{
-			addr = pool->vips->acquire_address(pool->vips, id, requested, op);
+			addr = pool->vips->acquire_address(pool->vips, id, requested,
+											   op, peer);
 			if (addr)
 			{
 				break;
@@ -126,19 +128,20 @@ METHOD(attribute_provider_t, acquire_address, host_t*,
 	host_t *requested)
 {
 	identification_t *id;
-	host_t *addr;
+	host_t *addr, *peer;
 
 	id = ike_sa->get_other_eap_id(ike_sa);
+	peer = ike_sa->get_other_host(ike_sa);
 
 	this->lock->read_lock(this->lock);
 
-	addr = find_addr(this, pools, id, requested, MEM_POOL_EXISTING);
+	addr = find_addr(this, pools, id, requested, MEM_POOL_EXISTING, peer);
 	if (!addr)
 	{
-		addr = find_addr(this, pools, id, requested, MEM_POOL_NEW);
+		addr = find_addr(this, pools, id, requested, MEM_POOL_NEW, peer);
 		if (!addr)
 		{
-			addr = find_addr(this, pools, id, requested, MEM_POOL_REASSIGN);
+			addr = find_addr(this, pools, id, requested, MEM_POOL_REASSIGN, peer);
 		}
 	}
 
diff --git a/src/libcharon/tests/suites/test_mem_pool.c b/src/libcharon/tests/suites/test_mem_pool.c
index c47a92f78..4204d4bab 100644
--- a/src/libcharon/tests/suites/test_mem_pool.c
+++ b/src/libcharon/tests/suites/test_mem_pool.c
@@ -43,7 +43,7 @@ static void assert_acquire(mem_pool_t *pool, char *requested, char *expected,
 	id = identification_create_from_string("tester");
 	req = host_create_from_string(requested, 0);
 
-	acquired = pool->acquire_address(pool, id, req, operation);
+	acquired = pool->acquire_address(pool, id, req, operation, NULL);
 	assert_host(expected, acquired);
 	DESTROY_IF(acquired);