fixed some exchange collisions (except IKE/CHILD rekeying)

7 files changed, 188 insertions, 126 deletions

diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 9c50e7c23..6f15ffc00 100644
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -448,39 +448,57 @@ static void set_state(private_ike_sa_t *this, ike_sa_state_t state)
 		 ike_sa_state_names, this->state,
 		 ike_sa_state_names, state);
 	
-	if (state == IKE_ESTABLISHED)
+	switch (state)
 	{
-		job_t *job;
-		u_int32_t now = time(NULL);
-		u_int32_t soft, hard;
-		bool reauth;
-	
-		this->time.established = now;
-		/* start DPD checks */
-		send_dpd(this);
-		
-		/* schedule rekeying/reauthentication */
-		soft = this->connection->get_soft_lifetime(this->connection);
-		hard = this->connection->get_hard_lifetime(this->connection);
-		reauth = this->connection->get_reauth(this->connection);
-		DBG1(DBG_IKE, "scheduling %s in %ds, maximum lifetime %ds",
-			 reauth ? "reauthentication": "rekeying", soft, hard);
-			 
-		if (soft)
+		case IKE_ESTABLISHED:
 		{
-			this->time.rekey = now + soft;
-			job = (job_t*)rekey_ike_sa_job_create(this->ike_sa_id, reauth);
-			charon->event_queue->add_relative(charon->event_queue, job,
-											  soft * 1000);
+			if (this->state == IKE_CONNECTING)
+			{
+				job_t *job;
+				u_int32_t now = time(NULL);
+				u_int32_t soft, hard;
+				bool reauth;
+			
+				this->time.established = now;
+				/* start DPD checks */
+				send_dpd(this);
+				
+				/* schedule rekeying/reauthentication */
+				soft = this->connection->get_soft_lifetime(this->connection);
+				hard = this->connection->get_hard_lifetime(this->connection);
+				reauth = this->connection->get_reauth(this->connection);
+				DBG1(DBG_IKE, "scheduling %s in %ds, maximum lifetime %ds",
+					 reauth ? "reauthentication": "rekeying", soft, hard);
+					 
+				if (soft)
+				{
+					this->time.rekey = now + soft;
+					job = (job_t*)rekey_ike_sa_job_create(this->ike_sa_id, reauth);
+					charon->event_queue->add_relative(charon->event_queue, job,
+													  soft * 1000);
+				}
+				
+				if (hard)
+				{
+					this->time.delete = now + hard;
+					job = (job_t*)delete_ike_sa_job_create(this->ike_sa_id, TRUE);
+					charon->event_queue->add_relative(charon->event_queue, job,
+													  hard * 1000);
+				}
+			}
+			break;
 		}
-		
-		if (hard)
+		case IKE_DELETING:
 		{
-			this->time.delete = now + hard;
-			job = (job_t*)delete_ike_sa_job_create(this->ike_sa_id, TRUE);
+			/* delete may fail if a packet gets lost, so set a timeout */
+			job_t *job = (job_t*)delete_ike_sa_job_create(this->ike_sa_id, TRUE);
 			charon->event_queue->add_relative(charon->event_queue, job,
-											  hard * 1000);
+						  charon->configuration->get_half_open_ike_sa_timeout(
+					  									charon->configuration));
+			break;
 		}
+		default:
+			break;
 	}
 	
 	this->state = state;
@@ -681,6 +699,7 @@ static status_t process_message(private_ike_sa_t *this, message_t *message)
 		/* if this IKE_SA is virgin, we check for a connection */
 		if (this->connection == NULL)
 		{
+			job_t *job;
 			this->connection = charon->connections->get_connection_by_hosts(
 												charon->connections, me, other);
 			if (this->connection == NULL)
@@ -691,6 +710,11 @@ static status_t process_message(private_ike_sa_t *this, message_t *message)
 				send_notify_response(this, message, NO_PROPOSAL_CHOSEN);
 				return DESTROY_ME;
 			}
+			/* add a timeout if peer does not establish it completely */
+			job = (job_t*)delete_ike_sa_job_create(this->ike_sa_id, FALSE);
+			charon->event_queue->add_relative(charon->event_queue, job,
+						  charon->configuration->get_half_open_ike_sa_timeout(
+					  									charon->configuration));
 		}
 	
 		/* check if message is trustworthy, and update connection information */
diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index 0bcc9e140..c40ec6e51 100644
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -131,6 +131,51 @@ struct private_task_manager_t {
 };
 
 /**
+ * flush all tasks in the task manager
+ */
+static void flush(private_task_manager_t *this)
+{
+	task_t *task;
+	
+	this->queued_tasks->destroy_offset(this->queued_tasks, 
+									   offsetof(task_t, destroy));
+	this->passive_tasks->destroy_offset(this->passive_tasks,
+										offsetof(task_t, destroy));
+	
+	/* emmit outstanding signals for tasks */
+	while (this->active_tasks->remove_last(this->active_tasks,
+										   (void**)&task) == SUCCESS)
+	{
+		switch (task->get_type(task))
+		{
+			case IKE_AUTH:
+				SIG(IKE_UP_FAILED, "establishing IKE_SA failed");
+				break;
+			case IKE_DELETE:
+				SIG(IKE_DOWN_FAILED, "IKE_SA deleted");
+				break;
+			case IKE_REKEY:
+				SIG(IKE_REKEY_FAILED, "rekeying IKE_SA failed");
+				break;
+			case CHILD_CREATE:
+				SIG(CHILD_UP_FAILED, "establishing CHILD_SA failed");
+				break;
+			case CHILD_DELETE:
+				SIG(CHILD_DOWN_FAILED, "deleting CHILD_SA failed");
+				break;
+			case CHILD_REKEY:
+				SIG(IKE_REKEY_FAILED, "rekeying CHILD_SA failed");
+				break;
+			default:
+				break;
+		}
+		task->destroy(task);
+	}
+	this->queued_tasks = linked_list_create();
+	this->passive_tasks = linked_list_create();
+}
+
+/**
  * move a task of a specific type from the queue to the active list
  */
 static bool activate_task(private_task_manager_t *this, task_type_t type)
@@ -322,6 +367,7 @@ static status_t build_request(private_task_manager_t *this)
 	            /* critical failure, destroy IKE_SA */
 	            iterator->destroy(iterator);
 				message->destroy(message);
+				flush(this);
 	            return DESTROY_ME;
 	    }
 	}
@@ -335,6 +381,7 @@ static status_t build_request(private_task_manager_t *this)
 	{
 	    /* message generation failed. There is nothing more to do than to
 		 * close the SA */
+		flush(this);
 	    return DESTROY_ME;
 	}						
 	
@@ -608,6 +655,7 @@ static status_t process_message(private_task_manager_t *this, message_t *msg)
 		{
 			if (process_request(this, msg) != SUCCESS)
 			{
+				flush(this);
 				return DESTROY_ME;
 			}
 			this->responding.mid++;
@@ -632,6 +680,7 @@ static status_t process_message(private_task_manager_t *this, message_t *msg)
 		{
 			if (process_response(this, msg) != SUCCESS)
 			{
+				flush(this);
 				return DESTROY_ME;
 			}
 		}
@@ -716,43 +765,12 @@ static void reset(private_task_manager_t *this)
  */
 static void destroy(private_task_manager_t *this)
 {
-	task_t *task;
-	
-	this->queued_tasks->destroy_offset(this->queued_tasks, 
-									   offsetof(task_t, destroy));
-	this->passive_tasks->destroy_offset(this->passive_tasks,
-										offsetof(task_t, destroy));
+	flush(this);
 	
-	/* emmit outstanding signals for tasks */
-	while (this->active_tasks->remove_last(this->active_tasks,
-										   (void**)&task) == SUCCESS)
-	{
-		switch (task->get_type(task))
-		{
-			case IKE_AUTH:
-				SIG(IKE_UP_FAILED, "establishing IKE_SA failed");
-				break;
-			case IKE_DELETE:
-				SIG(IKE_DOWN_FAILED, "IKE_SA deleted");
-				break;
-			case IKE_REKEY:
-				SIG(IKE_REKEY_FAILED, "rekeying IKE_SA failed");
-				break;
-			case CHILD_CREATE:
-				SIG(CHILD_UP_FAILED, "establishing CHILD_SA failed");
-				break;
-			case CHILD_DELETE:
-				SIG(CHILD_DOWN_FAILED, "deleting CHILD_SA failed");
-				break;
-			case CHILD_REKEY:
-				SIG(IKE_REKEY_FAILED, "rekeying CHILD_SA failed");
-				break;
-			default:
-				break;
-		}
-		task->destroy(task);
-	}
 	this->active_tasks->destroy(this->active_tasks);
+	this->queued_tasks->destroy(this->queued_tasks);
+	this->passive_tasks->destroy(this->passive_tasks);
+	
 	DESTROY_IF(this->responding.packet);
 	DESTROY_IF(this->initiating.packet);
 	free(this);
diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index e97f7814c..9526146a8 100644
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -553,6 +553,13 @@ static status_t build_r(private_child_create_t *this, message_t *message)
 			break;
 	}
 	
+	if (this->ike_sa->get_state(this->ike_sa) == IKE_REKEYING)
+	{
+		SIG(CHILD_UP_FAILED, "unable to create CHILD_SA while rekeying IKE_SA");
+		message->add_notify(message, TRUE, NO_ADDITIONAL_SAS, chunk_empty);
+		return SUCCESS;
+	}
+	
 	if (this->policy == NULL)
 	{
 		SIG(CHILD_UP_FAILED, "no acceptable policy found");
diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c
index 92c79ed29..5258aa02a 100644
--- a/src/charon/sa/tasks/child_delete.c
+++ b/src/charon/sa/tasks/child_delete.c
@@ -126,12 +126,21 @@ static void process_payloads(private_child_delete_t *this, message_t *message)
 				{
 					DBG1(DBG_IKE, "received DELETE for %N CHILD_SA with SPI 0x%x, "
 						 "but no such SA", protocol_id_names, protocol, ntohl(*spi));
-					break;
+					continue;
 				}
 				
-				if (child_sa->get_state(child_sa) == CHILD_REKEYING)
+				switch (child_sa->get_state(child_sa))
 				{
-					/* TODO: handle rekeying */
+					case CHILD_REKEYING:
+						/* we reply as usual, rekeying will fail */
+						break;
+					case CHILD_DELETING:
+						/* we don't send back a delete */
+						this->ike_sa->destroy_child_sa(this->ike_sa,
+													   protocol, *spi);
+						continue;
+					default:
+						break;
 				}
 				
 				this->child_sas->insert_last(this->child_sas, child_sa);
@@ -155,7 +164,6 @@ static void destroy_children(private_child_delete_t *this)
 	iterator = this->child_sas->create_iterator(this->child_sas, TRUE);
 	while (iterator->iterate(iterator, (void**)&child_sa))
 	{
-		/* TODO: can we do this more cleanly? */
 		spi = child_sa->get_spi(child_sa, TRUE);
 		protocol = child_sa->get_protocol(child_sa);
 		this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi);
@@ -200,7 +208,11 @@ static status_t process_r(private_child_delete_t *this, message_t *message)
  */
 static status_t build_r(private_child_delete_t *this, message_t *message)
 {
-	build_payloads(this, message);
+	/* if we are rekeying, we send an empty informational */
+	if (this->ike_sa->get_state(this->ike_sa) != IKE_REKEYING)
+	{
+		build_payloads(this, message);	
+	}
 	destroy_children(this);
 	return SUCCESS;
 }
diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c
index 4d2b6a0ba..660a9c15f 100644
--- a/src/charon/sa/tasks/child_rekey.c
+++ b/src/charon/sa/tasks/child_rekey.c
@@ -141,53 +141,6 @@ static void find_child(private_child_rekey_t *this, message_t *message)
 	iterator->destroy(iterator);
 }
 
-#if 0
-/**
- * handle a detected simultaneous rekeying situation as responder
- */
-static void simultaneous_r(private_child_rekey_t *this, message_t *message)
-{
-	private_child_rekey_t *other = NULL;
-	task_t *task;
-	iterator_t *iterator;
-	
-	this->ike_sa->create_task_iterator(this->ike_sa);
-	while (iterator->iterate(iterator, (void**)&task))
-	{
-		if (task->get_type(task) == CHILD_REKEY)
-		{
-			other = (private_child_rekey_t*)task;
-			break;
-		}
-	}
-	iterator->destroy(iterator);
-	
-	if (other)
-	{
-		other->simultaneous = this->child_create->get_child(this->child_create);
-	
-		if (!get_nonce(other, message))
-		{
-			/* this wins the race, other lost */
-			other->winner = FALSE;
-		}
-	}
-}
-
-/**
- * was there a simultaneous rekeying, did we win the nonce compare?
- */
-static bool simultaneous_i(private_child_rekey_t *this, message_t *message)
-{
-	if (this->winner || get_nonce(this, message))
-	{
-		/* we have the lower nonce and win */
-		return TRUE;
-	}
-	return FALSE;
-}
-#endif
-
 /**
  * Implementation of task_t.build for initiator
  */
@@ -247,11 +200,20 @@ static status_t build_r(private_child_rekey_t *this, message_t *message)
 	reqid = this->child_sa->get_reqid(this->child_sa);
 	this->child_create->use_reqid(this->child_create, reqid);
 	this->child_create->task.build(&this->child_create->task, message);
+	
+	if (message->get_payload(message, SECURITY_ASSOCIATION) == NULL)
+	{
+		/* rekeying failed, reuse old child */
+		this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
+		/* TODO: reschedule rekeying */
+		return SUCCESS;
+	}
+	
 	get_nonce(this, message);
 
 	if (this->child_sa->get_state(this->child_sa) == CHILD_REKEYING)
 	{
-		/* simultaneous_detected(this); */
+		/* TODO: handle simultaneous rekeying */
 	}
 	
 	this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
@@ -268,16 +230,17 @@ static status_t process_i(private_child_rekey_t *this, message_t *message)
 	u_int32_t spi;
 	
 	this->child_create->task.process(&this->child_create->task, message);
-	
-	/*if (!simultaneous_won(this, message))
+	if (message->get_payload(message, SECURITY_ASSOCIATION) == NULL)
 	{
-		* delete the redundant CHILD_SA, instead of the rekeyed *
-		this->child_sa = this->create_child->get_child(this->create_child);
-	}*/
+		/* establishing new child failed, fallback */
+		this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
+		/* TODO: rescedule rekeying */
+		return SUCCESS;
+	}
+	
+	/* TODO: delete the redundant CHILD_SA, instead of the rekeyed */
 	spi = this->child_sa->get_spi(this->child_sa, TRUE);
 	protocol = this->child_sa->get_protocol(this->child_sa);
-	
-	/* TODO: don't delete when rekeying failed */
 	if (this->ike_sa->delete_child_sa(this->ike_sa, protocol, spi) != SUCCESS)
 	{
 		return FAILED;
diff --git a/src/charon/sa/tasks/ike_delete.c b/src/charon/sa/tasks/ike_delete.c
index b9efc4ea9..9c4fdac0e 100644
--- a/src/charon/sa/tasks/ike_delete.c
+++ b/src/charon/sa/tasks/ike_delete.c
@@ -92,11 +92,14 @@ static status_t process_r(private_ike_delete_t *this, message_t *message)
 			break;
 		case IKE_ESTABLISHED:
 			DBG1(DBG_IKE, "deleting IKE_SA on request");
-			/* warn only if we are established */
+			break;
+		case IKE_REKEYING:
+			DBG1(DBG_IKE, "initiated rekeying, but received delete for IKE_SA");
+			break;
 		default:
-			this->ike_sa->set_state(this->ike_sa, IKE_DELETING);
 			break;
 	}
+	this->ike_sa->set_state(this->ike_sa, IKE_DELETING);
 	return NEED_MORE;
 }
 
@@ -107,7 +110,7 @@ static status_t build_r(private_ike_delete_t *this, message_t *message)
 {
 	if (this->simultaneous)
 	{
-		/* wait for peers response for our delete request */
+		/* wait for peers response for our delete request, but set a timeout */
 		return SUCCESS;
 	}
 	/* completed, delete IKE_SA by returning FAILED */
diff --git a/src/charon/sa/tasks/ike_rekey.c b/src/charon/sa/tasks/ike_rekey.c
index bbbd310bc..01698b11e 100644
--- a/src/charon/sa/tasks/ike_rekey.c
+++ b/src/charon/sa/tasks/ike_rekey.c
@@ -98,6 +98,32 @@ static status_t process_r(private_ike_rekey_t *this, message_t *message)
 	connection_t *connection;
 	policy_t *policy;
 	ike_sa_id_t *id;
+	iterator_t *iterator;
+	child_sa_t *child_sa;
+	
+	if (this->ike_sa->get_state(this->ike_sa) == IKE_DELETING)
+	{
+		DBG1(DBG_IKE, "peer initiated rekeying, but we are deleting");
+		return NEED_MORE;
+	}
+
+	iterator = this->ike_sa->create_child_sa_iterator(this->ike_sa);
+	while (iterator->iterate(iterator, (void**)&child_sa))
+	{
+		switch (child_sa->get_state(child_sa))
+		{
+			case CHILD_CREATED:
+			case CHILD_REKEYING:
+			case CHILD_DELETING:
+				/* we do not allow rekeying while we have children in-progress */
+				DBG1(DBG_IKE, "peer initiated rekeying, but a child is half-open");
+				iterator->destroy(iterator);
+				return NEED_MORE;
+			default:
+				break;
+		}
+	}
+	iterator->destroy(iterator);
 	
 	id = ike_sa_id_create(0, 0, FALSE);
 	this->new_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, id);
@@ -119,6 +145,13 @@ static status_t process_r(private_ike_rekey_t *this, message_t *message)
  */
 static status_t build_r(private_ike_rekey_t *this, message_t *message)
 {
+	if (this->new_sa == NULL)
+	{
+		/* IKE_SA/a CHILD_SA is in an inacceptable state, deny rekeying */
+		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
+		return SUCCESS;
+	}
+
 	if (this->ike_init->task.build(&this->ike_init->task, message) == FAILED)
 	{
 		return SUCCESS;
@@ -142,6 +175,9 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 
 	if (this->ike_init->task.process(&this->ike_init->task, message) == FAILED)
 	{
+		/* rekeying failed, fallback to old SA */
+		this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
+		/* TODO: reschedule rekeying */
 		return SUCCESS;
 	}
 	
@@ -152,7 +188,6 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 	
 	job = (job_t*)delete_ike_sa_job_create(this->ike_sa->get_id(this->ike_sa),
 										   TRUE);
-	
 	charon->job_queue->add(charon->job_queue, job);
 	return SUCCESS;
 }