some task queueing improvements:

	- do not pass CHILD_SAs to task constructor, might not
	  be valid anymore during execution (late lookup)
	- use sub-tasks to delete CHILD/IKE_SA after rekeying,
	  as we want to execute the delete before additional
	  queued tasks

8 files changed, 186 insertions, 63 deletions

diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 3e13249c9..979fe1980 100644
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -1623,37 +1623,27 @@ static iterator_t* create_child_sa_iterator(private_ike_sa_t *this)
 /**
  * Implementation of ike_sa_t.rekey_child_sa.
  */
-static status_t rekey_child_sa(private_ike_sa_t *this, protocol_id_t protocol, u_int32_t spi)
+static status_t rekey_child_sa(private_ike_sa_t *this, protocol_id_t protocol,
+							   u_int32_t spi)
 {
-	child_sa_t *child_sa;
 	child_rekey_t *child_rekey;
 	
-	child_sa = get_child_sa(this, protocol, spi, TRUE);
-	if (child_sa)
-	{
-		child_rekey = child_rekey_create(&this->public, child_sa);
-		this->task_manager->queue_task(this->task_manager, &child_rekey->task);
-		return this->task_manager->initiate(this->task_manager);
-	}
-	return FAILED;
+	child_rekey = child_rekey_create(&this->public, protocol, spi);
+	this->task_manager->queue_task(this->task_manager, &child_rekey->task);
+	return this->task_manager->initiate(this->task_manager);
 }
 
 /**
  * Implementation of ike_sa_t.delete_child_sa.
  */
-static status_t delete_child_sa(private_ike_sa_t *this, protocol_id_t protocol, u_int32_t spi)
+static status_t delete_child_sa(private_ike_sa_t *this, protocol_id_t protocol,
+								u_int32_t spi)
 {
-	child_sa_t *child_sa;
 	child_delete_t *child_delete;
 	
-	child_sa = get_child_sa(this, protocol, spi, TRUE);
-	if (child_sa)
-	{
-		child_delete = child_delete_create(&this->public, child_sa);
-		this->task_manager->queue_task(this->task_manager, &child_delete->task);
-		return this->task_manager->initiate(this->task_manager);
-	}
-	return FAILED;
+	child_delete = child_delete_create(&this->public, protocol, spi);
+	this->task_manager->queue_task(this->task_manager, &child_delete->task);
+	return this->task_manager->initiate(this->task_manager);
 }
 
 /**
diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index 135dd7fa8..211e64fd2 100644
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -417,7 +417,7 @@ static status_t build_request(private_task_manager_t *this)
 	message->set_exchange_type(message, exchange);
 	this->initiating.type = exchange;
 	this->initiating.retransmitted = 0;
-
+	
 	iterator = this->active_tasks->create_iterator(this->active_tasks, TRUE);
 	while (iterator->iterate(iterator, (void*)&task))
 	{
@@ -442,6 +442,9 @@ static status_t build_request(private_task_manager_t *this)
 	}
 	iterator->destroy(iterator);
 	
+	/* update exchange type if a task changed it */
+	this->initiating.type = message->get_exchange_type(message);
+	
 	DESTROY_IF(this->initiating.packet);
 	status = this->ike_sa->generate_message(this->ike_sa, message,
 											&this->initiating.packet);
@@ -495,7 +498,9 @@ static status_t process_response(private_task_manager_t *this,
 			case FAILED:
 			default:
 				/* critical failure, destroy IKE_SA */
+				iterator->remove(iterator);
 				iterator->destroy(iterator);
+				task->destroy(task);
 				return DESTROY_ME;
 		}
 		if (this->reset)
@@ -716,7 +721,8 @@ static status_t process_request(private_task_manager_t *this,
 			{
 				if (notify_found)
 				{
-					task = (task_t*)child_rekey_create(this->ike_sa, NULL);
+					task = (task_t*)child_rekey_create(this->ike_sa,
+													   PROTO_NONE, 0);
 				}
 				else
 				{
@@ -773,7 +779,8 @@ static status_t process_request(private_task_manager_t *this,
 						}
 						else
 						{
-							task = (task_t*)child_delete_create(this->ike_sa, NULL);
+							task = (task_t*)child_delete_create(this->ike_sa,
+																PROTO_NONE, 0);
 						}
 						break;
 					}
@@ -822,7 +829,9 @@ static status_t process_request(private_task_manager_t *this,
 			case FAILED:
 			default:
 				/* critical failure, destroy IKE_SA */
+				iterator->remove(iterator);
 				iterator->destroy(iterator);
+				task->destroy(task);
 				return DESTROY_ME;
 		}
 	}
diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index f1453f0c5..164bc076e 100644
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -1119,6 +1119,8 @@ static void migrate(private_child_create_t *this, ike_sa_t *ike_sa)
 	}
 	
 	this->ike_sa = ike_sa;
+	this->keymat = ike_sa->get_keymat(ike_sa);
+	this->proposal = NULL;
 	this->proposals = NULL;
 	this->tsi = NULL;
 	this->tsr = NULL;
diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c
index 45c7775bb..36d260f99 100644
--- a/src/charon/sa/tasks/child_delete.c
+++ b/src/charon/sa/tasks/child_delete.c
@@ -44,9 +44,19 @@ struct private_child_delete_t {
 	bool initiator;
 	
 	/**
- 	 * wheter to enforce delete action policy
- 	 */
- 	bool check_delete_action;
+	 * Protocol of CHILD_SA to delete
+	 */
+	protocol_id_t protocol;
+	
+	/**
+	 * Inbound SPI of CHILD_SA to delete
+	 */
+	u_int32_t spi;
+	
+	/**
+	 * wheter to enforce delete action policy
+	 */
+	bool check_delete_action;
 	
 	/**
 	 * CHILD_SAs which get deleted
@@ -238,6 +248,16 @@ static void log_children(private_child_delete_t *this)
  */
 static status_t build_i(private_child_delete_t *this, message_t *message)
 {
+	child_sa_t *child_sa;
+	
+	child_sa = this->ike_sa->get_child_sa(this->ike_sa, this->protocol,
+										  this->spi, TRUE);
+	if (!child_sa)
+	{	/* child does not exist anymore */
+		return SUCCESS;
+	}
+	this->child_sas->insert_last(this->child_sas, child_sa);
+	
 	log_children(this);
 	build_payloads(this, message);
 	return NEED_MORE;
@@ -323,7 +343,8 @@ static void destroy(private_child_delete_t *this)
 /*
  * Described in header.
  */
-child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
+child_delete_t *child_delete_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+									u_int32_t spi)
 {
 	private_child_delete_t *this = malloc_thing(private_child_delete_t);
 
@@ -335,13 +356,14 @@ child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
 	this->ike_sa = ike_sa;
 	this->check_delete_action = FALSE;
 	this->child_sas = linked_list_create();
+	this->protocol = protocol;
+	this->spi = spi;
 	
-	if (child_sa != NULL)
+	if (protocol != PROTO_NONE)
 	{
 		this->public.task.build = (status_t(*)(task_t*,message_t*))build_i;
 		this->public.task.process = (status_t(*)(task_t*,message_t*))process_i;
 		this->initiator = TRUE;
-		this->child_sas->insert_last(this->child_sas, child_sa);
 	}
 	else
 	{
diff --git a/src/charon/sa/tasks/child_delete.h b/src/charon/sa/tasks/child_delete.h
index 1aa60993e..9503ff0e6 100644
--- a/src/charon/sa/tasks/child_delete.h
+++ b/src/charon/sa/tasks/child_delete.h
@@ -52,9 +52,11 @@ struct child_delete_t {
  * Create a new child_delete task.
  *
  * @param ike_sa		IKE_SA this task works for
- * @param child_sa		CHILD_SA to delete, or NULL as responder
+ * @param protocol		protocol of CHILD_SA to delete, PROTO_NONE as responder
+ * @param spi			inbound SPI of CHILD_SA to delete
  * @return				child_delete task to handle by the task_manager
  */
-child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa);
+child_delete_t *child_delete_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+									u_int32_t spi);
 
 #endif /* CHILD_DELETE_H_ @} */
diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c
index 3a22d5040..127280efc 100644
--- a/src/charon/sa/tasks/child_rekey.c
+++ b/src/charon/sa/tasks/child_rekey.c
@@ -49,11 +49,26 @@ struct private_child_rekey_t {
 	bool initiator;
 	
 	/**
+	 * Protocol of CHILD_SA to rekey
+	 */
+	protocol_id_t protocol;
+	
+	/**
+	 * Inbound SPI of CHILD_SA to rekey
+	 */
+	u_int32_t spi;
+	
+	/**
 	 * the CHILD_CREATE task which is reused to simplify rekeying
 	 */
 	child_create_t *child_create;
 	
 	/**
+	 * the CHILD_DELETE task to delete rekeyed CHILD_SA
+	 */
+	child_delete_t *child_delete;
+	
+	/**
 	 * CHILD_SA which gets rekeyed
 	 */
 	child_sa_t *child_sa;
@@ -65,6 +80,25 @@ struct private_child_rekey_t {
 };
 
 /**
+ * Implementation of task_t.build for initiator, after rekeying
+ */
+static status_t build_i_delete(private_child_rekey_t *this, message_t *message)
+{
+	/* update exchange type to INFORMATIONAL for the delete */
+	message->set_exchange_type(message, INFORMATIONAL);
+	
+	return this->child_delete->task.build(&this->child_delete->task, message);
+}
+
+/**
+ * Implementation of task_t.process for initiator, after rekeying
+ */
+static status_t process_i_delete(private_child_rekey_t *this, message_t *message)
+{
+	return this->child_delete->task.process(&this->child_delete->task, message);
+}
+
+/**
  * find a child using the REKEY_SA notify
  */
 static void find_child(private_child_rekey_t *this, message_t *message)
@@ -104,25 +138,33 @@ static void find_child(private_child_rekey_t *this, message_t *message)
  * Implementation of task_t.build for initiator
  */
 static status_t build_i(private_child_rekey_t *this, message_t *message)
-{	
+{
 	notify_payload_t *notify;
-	protocol_id_t protocol;
-	u_int32_t spi, reqid;
+	u_int32_t reqid;
+	child_cfg_t *config;
+	
+	this->child_sa = this->ike_sa->get_child_sa(this->ike_sa, this->protocol,
+												this->spi, TRUE);
+	if (!this->child_sa)
+	{	/* CHILD_SA is gone, unable to rekey */
+		return SUCCESS;
+	}
+	config = this->child_sa->get_config(this->child_sa);
 	
 	/* we just need the rekey notify ... */
-	protocol = this->child_sa->get_protocol(this->child_sa);
-	spi = this->child_sa->get_spi(this->child_sa, TRUE);
-	notify = notify_payload_create_from_protocol_and_type(protocol, REKEY_SA);
-	notify->set_spi(notify, spi);
+	notify = notify_payload_create_from_protocol_and_type(this->protocol,
+														  REKEY_SA);
+	notify->set_spi(notify, this->spi);
 	message->add_payload(message, (payload_t*)notify);
-
+	
 	/* ... our CHILD_CREATE task does the hard work for us. */
 	reqid = this->child_sa->get_reqid(this->child_sa);
+	this->child_create = child_create_create(this->ike_sa, config);
 	this->child_create->use_reqid(this->child_create, reqid);
 	this->child_create->task.build(&this->child_create->task, message);
 	
 	this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
-
+	
 	return NEED_MORE;
 }
 
@@ -133,7 +175,7 @@ static status_t process_r(private_child_rekey_t *this, message_t *message)
 {
 	/* let the CHILD_CREATE task process the message */
 	this->child_create->task.process(&this->child_create->task, message);
-
+	
 	find_child(this, message);
 	
 	return NEED_MORE;
@@ -265,11 +307,13 @@ static status_t process_i(private_child_rekey_t *this, message_t *message)
 	
 	spi = to_delete->get_spi(to_delete, TRUE);
 	protocol = to_delete->get_protocol(to_delete);
-	if (this->ike_sa->delete_child_sa(this->ike_sa, protocol, spi) != SUCCESS)
-	{
-		return FAILED;
-	}
-	return SUCCESS;
+	
+	/* rekeying done, delete the obsolete CHILD_SA using a subtask */
+	this->child_delete = child_delete_create(this->ike_sa, protocol, spi);
+	this->public.task.build = (status_t(*)(task_t*,message_t*))build_i_delete;
+	this->public.task.process = (status_t(*)(task_t*,message_t*))process_i_delete;
+	
+	return NEED_MORE;
 }
 
 /**
@@ -319,9 +363,16 @@ static void collide(private_child_rekey_t *this, task_t *other)
  */
 static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
 {	
-	this->child_create->task.migrate(&this->child_create->task, ike_sa);
+	if (this->child_create)
+	{
+		this->child_create->task.migrate(&this->child_create->task, ike_sa);
+	}
+	if (this->child_delete)
+	{
+		this->child_delete->task.migrate(&this->child_delete->task, ike_sa);
+	}
 	DESTROY_IF(this->collision);
-
+	
 	this->ike_sa = ike_sa;
 	this->collision = NULL;
 }
@@ -331,7 +382,14 @@ static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
  */
 static void destroy(private_child_rekey_t *this)
 {
-	this->child_create->task.destroy(&this->child_create->task);
+	if (this->child_create)
+	{
+		this->child_create->task.destroy(&this->child_create->task);
+	}
+	if (this->child_delete)
+	{
+		this->child_delete->task.destroy(&this->child_delete->task);
+	}
 	DESTROY_IF(this->collision);
 	free(this);
 }
@@ -339,22 +397,21 @@ static void destroy(private_child_rekey_t *this)
 /*
  * Described in header.
  */
-child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
+child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+								  u_int32_t spi)
 {
-	child_cfg_t *config;
 	private_child_rekey_t *this = malloc_thing(private_child_rekey_t);
-
+	
 	this->public.collide = (void (*)(child_rekey_t*,task_t*))collide;
 	this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
 	this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;
 	this->public.task.destroy = (void(*)(task_t*))destroy;
-	if (child_sa != NULL)
+	if (protocol != PROTO_NONE)
 	{
 		this->public.task.build = (status_t(*)(task_t*,message_t*))build_i;
 		this->public.task.process = (status_t(*)(task_t*,message_t*))process_i;
 		this->initiator = TRUE;
-		config = child_sa->get_config(child_sa);
-		this->child_create = child_create_create(ike_sa, config);
+		this->child_create = NULL;
 	}
 	else
 	{
@@ -365,8 +422,11 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
 	}
 	
 	this->ike_sa = ike_sa;
-	this->child_sa = child_sa;
+	this->child_sa = NULL;
+	this->protocol = protocol;
+	this->spi = spi;
 	this->collision = NULL;
+	this->child_delete = NULL;
 	
 	return &this->public;
 }
diff --git a/src/charon/sa/tasks/child_rekey.h b/src/charon/sa/tasks/child_rekey.h
index 382cf4a31..b0c4cec2e 100644
--- a/src/charon/sa/tasks/child_rekey.h
+++ b/src/charon/sa/tasks/child_rekey.h
@@ -56,9 +56,11 @@ struct child_rekey_t {
  * Create a new CHILD_REKEY task.
  *
  * @param ike_sa		IKE_SA this task works for
- * @param child_sa 		child_sa to rekey, NULL if responder
+ * @param protocol		protocol of CHILD_SA to rekey, PROTO_NONE as responder
+ * @param spi			inbound SPI of CHILD_SA to rekey
  * @return			  	child_rekey task to handle by the task_manager
  */
-child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa);
+child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+								  u_int32_t spi);
 
 #endif /* CHILD_REKEY_H_ @} */
diff --git a/src/charon/sa/tasks/ike_rekey.c b/src/charon/sa/tasks/ike_rekey.c
index d094a0481..6dbad5ff6 100644
--- a/src/charon/sa/tasks/ike_rekey.c
+++ b/src/charon/sa/tasks/ike_rekey.c
@@ -21,6 +21,7 @@
 #include <daemon.h>
 #include <encoding/payloads/notify_payload.h>
 #include <sa/tasks/ike_init.h>
+#include <sa/tasks/ike_delete.h>
 #include <processing/jobs/delete_ike_sa_job.h>
 #include <processing/jobs/rekey_ike_sa_job.h>
 
@@ -58,12 +59,36 @@ struct private_ike_rekey_t {
 	ike_init_t *ike_init;
 	
 	/**
+	 * IKE_DELETE task to delete the old IKE_SA after rekeying was successful
+	 */
+	ike_delete_t *ike_delete;
+	
+	/**
 	 * colliding task detected by the task manager
 	 */
 	task_t *collision;
 };
 
 /**
+ * Implementation of task_t.build for initiator, after rekeying
+ */
+static status_t build_i_delete(private_ike_rekey_t *this, message_t *message)
+{
+	/* update exchange type to INFORMATIONAL for the delete */
+	message->set_exchange_type(message, INFORMATIONAL);
+	
+	return this->ike_delete->task.build(&this->ike_delete->task, message);
+}
+
+/**
+ * Implementation of task_t.process for initiator, after rekeying
+ */
+static status_t process_i_delete(private_ike_rekey_t *this, message_t *message)
+{
+	return this->ike_delete->task.process(&this->ike_delete->task, message);
+}
+
+/**
  * Implementation of task_t.build for initiator
  */
 static status_t build_i(private_ike_rekey_t *this, message_t *message)
@@ -168,7 +193,6 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message)
  */
 static status_t process_i(private_ike_rekey_t *this, message_t *message)
 {
-	job_t *job;
 	ike_sa_id_t *to_delete;
 	iterator_t *iterator;
 	payload_t *payload;
@@ -271,10 +295,12 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 		charon->bus->set_sa(charon->bus, this->ike_sa);
 	}
 	
-	job = (job_t*)delete_ike_sa_job_create(to_delete, TRUE);
-	charon->processor->queue_job(charon->processor, job);	
+	/* rekeying successful, delete the IKE_SA using a subtask */
+	this->ike_delete = ike_delete_create(this->ike_sa, TRUE);
+	this->public.task.build = (status_t(*)(task_t*,message_t*))build_i_delete;
+	this->public.task.process = (status_t(*)(task_t*,message_t*))process_i_delete;
 	
-	return SUCCESS;
+	return NEED_MORE;
 }
 
 /**
@@ -300,6 +326,10 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
 	{
 		this->ike_init->task.destroy(&this->ike_init->task);
 	}
+	if (this->ike_delete)
+	{
+		this->ike_delete->task.destroy(&this->ike_delete->task);
+	}
 	if (this->new_sa)
 	{
 		charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
@@ -308,11 +338,12 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
 		charon->bus->set_sa(charon->bus, this->ike_sa);
 	}
 	DESTROY_IF(this->collision);
-
+	
 	this->collision = NULL;
 	this->ike_sa = ike_sa;
 	this->new_sa = NULL;
 	this->ike_init = NULL;
+	this->ike_delete = NULL;
 }
 
 /**
@@ -339,6 +370,10 @@ static void destroy(private_ike_rekey_t *this)
 	{
 		this->ike_init->task.destroy(&this->ike_init->task);
 	}
+	if (this->ike_delete)
+	{
+		this->ike_delete->task.destroy(&this->ike_delete->task);
+	}
 	DESTROY_IF(this->collision);
 	free(this);
 }
@@ -368,6 +403,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator)
 	this->ike_sa = ike_sa;
 	this->new_sa = NULL;
 	this->ike_init = NULL;
+	this->ike_delete = NULL;
 	this->initiator = initiator;
 	this->collision = NULL;