Merge branch 'vici-reload-tasks'

Consider queued child-creating tasks when reloading configs that have
`start` as start action.  Besides some possible corner cases it fixes
handling IKE_SAs that are current getting established and have no
established CHILD_SAs yet.

Closes strongswan/strongswan#2418

src/libcharon/plugins/vici/vici_config.c

@@ -49,6 +49,8 @@
 #include <collections/array.h>
 #include <collections/hashtable.h>
 #include <collections/linked_list.h>
+#include <sa/ikev2/tasks/child_create.h>
+#include <sa/ikev1/tasks/quick_mode.h>
 
 #include <pubkey_cert.h>
 
@@ -2294,6 +2296,102 @@ typedef struct {
 	char *name;
 } child_name_id_t;
 
+/**
+ * Get the child config of the given task depending on its type.
+ */
+static child_cfg_t *get_task_config(task_t *task, task_type_t type)
+{
+	child_create_t *child_create = (child_create_t*)task;
+
+	if (type == TASK_QUICK_MODE)
+	{
+		quick_mode_t *quick_mode = (quick_mode_t*)task;
+		return quick_mode->get_config(quick_mode);
+	}
+	return child_create->get_config(child_create);
+}
+
+/**
+ * Abort the given child-creating task depending on its type.
+ */
+static void abort_child_task(task_t *task, task_type_t type)
+{
+	child_create_t *child_create = (child_create_t*)task;
+
+	if (type == TASK_QUICK_MODE)
+	{
+		quick_mode_t *quick_mode = (quick_mode_t*)task;
+		return quick_mode->abort(quick_mode);
+	}
+	return child_create->abort(child_create);
+}
+
+/**
+ * Check if there are child-creating tasks queued that should be aborted, as
+ * well as if there are any others that should prevent the deletion of the
+ * IKE_SA.
+ */
+static void check_queued_tasks(ike_sa_t *ike_sa, hashtable_t *to_terminate,
+							   bool *others)
+{
+	enumerator_t *enumerator;
+	child_cfg_t *cfg;
+	task_t *task;
+	task_type_t type = TASK_CHILD_CREATE;
+
+	if (ike_sa->get_version(ike_sa) == IKEV1)
+	{
+		type = TASK_QUICK_MODE;
+	}
+
+	/* if we find an active task, we can't remove it as there is no way to
+	 * abort an exchange (i.e. the peer will have created the SA even if we
+	 * don't process the response). so we instruct the task to immediately
+	 * delete the CHILD_SA once it's created */
+	enumerator = ike_sa->create_task_enumerator(ike_sa, TASK_QUEUE_ACTIVE);
+	while (enumerator->enumerate(enumerator, &task))
+	{
+		if (task->get_type(task) == type)
+		{
+			cfg = get_task_config(task, type);
+			if (to_terminate->get(to_terminate, cfg->get_name(cfg)))
+			{
+				DBG1(DBG_CFG, "vici aborting %N task for CHILD_SA '%s'",
+					 task_type_names, type, cfg->get_name(cfg));
+				abort_child_task(task, type);
+			}
+			else
+			{
+				*others = TRUE;
+			}
+		}
+	}
+	enumerator->destroy(enumerator);
+
+	/* for the queued tasks, we just remove any that use a config that is to
+	 * be terminated, but also note if there are others */
+	enumerator = ike_sa->create_task_enumerator(ike_sa, TASK_QUEUE_QUEUED);
+	while (enumerator->enumerate(enumerator, &task))
+	{
+		if (task->get_type(task) == type)
+		{
+			cfg = get_task_config(task, type);
+			if (to_terminate->get(to_terminate, cfg->get_name(cfg)))
+			{
+				DBG1(DBG_CFG, "vici removing %N task for CHILD_SA '%s'",
+					 task_type_names, type, cfg->get_name(cfg));
+				ike_sa->remove_task(ike_sa, enumerator);
+				task->destroy(task);
+			}
+			else
+			{
+				*others = TRUE;
+			}
+		}
+	}
+	enumerator->destroy(enumerator);
+}
+
 /**
  * Terminate given CHILD_SAs and optionally terminate any IKE_SA without other
  * children.
@@ -2332,9 +2430,11 @@ static void terminate_for_action(private_vici_config_t *this, char *peer_name,
 		}
 		children->destroy(children);
 
-		if (delete_ike && (!others || !ike_sa->get_child_count(ike_sa)))
+		check_queued_tasks(ike_sa, to_terminate, &others);
+
+		if (delete_ike && !others)
 		{
-			/* found no children or only matching, delete IKE_SA */
+			/* found no children/tasks or only matching, delete IKE_SA */
 			id = ike_sa->get_unique_id(ike_sa);
 			array_insert_create_value(&ikeids, sizeof(id),
 									  ARRAY_TAIL, &id);

src/libcharon/sa/ikev1/tasks/quick_mode.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2019 Tobias Brunner
+ * Copyright (C) 2012-2024 Tobias Brunner
  * Copyright (C) 2011 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -164,6 +164,11 @@ struct private_quick_mode_t {
 	 */
 	bool delete;
 
+	/**
+	 * Whether the task was aborted
+	 */
+	bool aborted;
+
 	/**
 	 * Negotiated mode, tunnel or transport
 	 */
@@ -955,6 +960,13 @@ METHOD(task_t, build_i, status_t,
 		}
 		case QM_NEGOTIATED:
 		{
+			if (this->aborted)
+			{
+				this->ike_sa->queue_task(this->ike_sa,
+				(task_t*)quick_delete_create(this->ike_sa,
+								this->proposal->get_protocol(this->proposal),
+								this->spi_i, TRUE, FALSE));
+			}
 			return SUCCESS;
 		}
 		default:
@@ -1435,6 +1447,12 @@ METHOD(quick_mode_t, get_mid, uint32_t,
 	return this->mid;
 }
 
+METHOD(quick_mode_t, get_config, child_cfg_t*,
+	private_quick_mode_t *this)
+{
+	return this->initiator ? this->config : NULL;
+}
+
 METHOD(quick_mode_t, use_reqid, void,
 	private_quick_mode_t *this, uint32_t reqid)
 {
@@ -1470,6 +1488,12 @@ METHOD(quick_mode_t, rekey, void,
 	this->rekey = spi;
 }
 
+METHOD(quick_mode_t, abort_, void,
+	private_quick_mode_t *this)
+{
+	this->aborted = TRUE;
+}
+
 METHOD(task_t, migrate, void,
 	private_quick_mode_t *this, ike_sa_t *ike_sa)
 {
@@ -1534,10 +1558,12 @@ quick_mode_t *quick_mode_create(ike_sa_t *ike_sa, child_cfg_t *config,
 				.destroy = _destroy,
 			},
 			.get_mid = _get_mid,
+			.get_config = _get_config,
 			.use_reqid = _use_reqid,
 			.use_marks = _use_marks,
 			.use_if_ids = _use_if_ids,
 			.rekey = _rekey,
+			.abort = _abort_,
 		},
 		.ike_sa = ike_sa,
 		.initiator = config != NULL,

src/libcharon/sa/ikev1/tasks/quick_mode.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2019 Tobias Brunner
+ * Copyright (C) 2015-2024 Tobias Brunner
  * Copyright (C) 2011 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -47,6 +47,13 @@ struct quick_mode_t {
 	 */
 	uint32_t (*get_mid)(quick_mode_t *this);
 
+	/**
+	 * Get the child config of this task as initiator.
+	 *
+	 * @return				config for the CHILD_SA, NULL as responder
+	 */
+	child_cfg_t *(*get_config)(quick_mode_t *this);
+
 	/**
 	 * Use a specific reqid to install this CHILD_SA.
 	 *
@@ -80,6 +87,12 @@ struct quick_mode_t {
 	 * @param spi			spi of SA to rekey
 	 */
 	void (*rekey)(quick_mode_t *this, uint32_t spi);
+
+	/**
+	 * Mark this active task as being aborted, i.e. cause a deletion of the
+	 * created CHILD_SA immediately after its successful creation.
+	 */
+	void (*abort)(quick_mode_t *this);
 };
 
 /**

src/libcharon/sa/ikev2/tasks/child_create.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2020 Tobias Brunner
+ * Copyright (C) 2008-2024 Tobias Brunner
  * Copyright (C) 2005-2008 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  *
@@ -231,6 +231,11 @@ struct private_child_create_t {
 	 * whether we are retrying with another KE method
 	 */
 	bool retry;
+
+	/**
+	 * Whether the task was aborted
+	 */
+	bool aborted;
 };
 
 /**
@@ -1689,6 +1694,11 @@ static void handle_child_sa_failure(private_child_create_t *this,
 {
 	bool is_first;
 
+	if (this->aborted)
+	{
+		return;
+	}
+
 	is_first = message->get_exchange_type(message) == IKE_AUTH;
 	if (is_first &&
 		lib->settings->get_bool(lib->settings,
@@ -2232,7 +2242,7 @@ METHOD(task_t, process_i_multi_ke, status_t,
 	{
 		DBG1(DBG_IKE, "received %N notify", notify_type_names,
 			 TEMPORARY_FAILURE);
-		if (!this->rekey)
+		if (!this->rekey && !this->aborted)
 		{	/* the rekey task will retry itself if necessary */
 			schedule_delayed_retry(this);
 		}
@@ -2241,7 +2251,7 @@ METHOD(task_t, process_i_multi_ke, status_t,
 
 	process_payloads_multi_ke(this, message);
 
-	if (this->ke_failed)
+	if (this->ke_failed || this->aborted)
 	{
 		handle_child_sa_failure(this, message);
 		return delete_failed_sa(this);
@@ -2313,7 +2323,7 @@ METHOD(task_t, process_i, status_t,
 				{
 					DBG1(DBG_IKE, "received %N notify", notify_type_names, type);
 					enumerator->destroy(enumerator);
-					if (!this->rekey)
+					if (!this->rekey && !this->aborted)
 					{	/* the rekey task will retry itself if necessary */
 						schedule_delayed_retry(this);
 					}
@@ -2324,6 +2334,14 @@ METHOD(task_t, process_i, status_t,
 					chunk_t data;
 					uint16_t alg = KE_NONE;
 
+					if (this->aborted)
+					{	/* nothing to do if the task was aborted */
+						DBG1(DBG_IKE, "received %N notify in aborted %N task",
+							 notify_type_names, type, task_type_names,
+							 TASK_CHILD_CREATE);
+						enumerator->destroy(enumerator);
+						return SUCCESS;
+					}
 					data = notify->get_notification_data(notify);
 					if (data.len == sizeof(alg))
 					{
@@ -2380,6 +2398,17 @@ METHOD(task_t, process_i, status_t,
 	this->other_spi = this->proposal->get_spi(this->proposal);
 	this->proposal->set_spi(this->proposal, this->my_spi);
 
+	if (this->aborted)
+	{
+		DBG1(DBG_IKE, "deleting CHILD_SA %s{%d} with SPIs %.8x_i %.8x_o of "
+			 "aborted %N task",
+			 this->child_sa->get_name(this->child_sa),
+			 this->child_sa->get_unique_id(this->child_sa),
+			 ntohl(this->my_spi), ntohl(this->other_spi),
+			 task_type_names, TASK_CHILD_CREATE);
+		return delete_failed_sa(this);
+	}
+
 	if (this->ipcomp == IPCOMP_NONE && this->ipcomp_received != IPCOMP_NONE)
 	{
 		DBG1(DBG_IKE, "received an IPCOMP_SUPPORTED notify without requesting"
@@ -2504,6 +2533,12 @@ METHOD(child_create_t, set_config, void,
 	this->config = cfg;
 }
 
+METHOD(child_create_t, get_config, child_cfg_t*,
+	private_child_create_t *this)
+{
+	return this->initiator ? this->config : NULL;
+}
+
 METHOD(child_create_t, get_lower_nonce, chunk_t,
 	private_child_create_t *this)
 {
@@ -2518,6 +2553,12 @@ METHOD(child_create_t, get_lower_nonce, chunk_t,
 	}
 }
 
+METHOD(child_create_t, abort_, void,
+	private_child_create_t *this)
+{
+	this->aborted = TRUE;
+}
+
 METHOD(task_t, get_type, task_type_t,
 	private_child_create_t *this)
 {
@@ -2637,12 +2678,14 @@ child_create_t *child_create_create(ike_sa_t *ike_sa,
 			.get_child = _get_child,
 			.get_other_spi = _get_other_spi,
 			.set_config = _set_config,
+			.get_config = _get_config,
 			.get_lower_nonce = _get_lower_nonce,
 			.use_reqid = _use_reqid,
 			.use_marks = _use_marks,
 			.use_if_ids = _use_if_ids,
 			.use_label = _use_label,
 			.use_ke_method = _use_ke_method,
+			.abort = _abort_,
 			.task = {
 				.get_type = _get_type,
 				.migrate = _migrate,

src/libcharon/sa/ikev2/tasks/child_create.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2019 Tobias Brunner
+ * Copyright (C) 2018-2024 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -116,6 +116,20 @@ struct child_create_t {
 	 * @param cfg		configuration to enforce, reference gets owned
 	 */
 	void (*set_config)(child_create_t *this, child_cfg_t *cfg);
+
+	/**
+	 * Get the child config of this task as initiator.
+	 *
+	 * @return				config for the CHILD_SA, NULL as responder
+	 */
+	child_cfg_t *(*get_config)(child_create_t *this);
+
+	/**
+	 * Mark this active task as being aborted, i.e. cause a deletion of the
+	 * created CHILD_SA immediately after its creation (any failures to create
+	 * the SA are ignored).
+	 */
+	void (*abort)(child_create_t *this);
 };
 
 /**