Imported Upstream version 4.5.2

4 files changed, 101 insertions, 110 deletions

diff --git a/src/libcharon/sa/tasks/child_delete.c b/src/libcharon/sa/tasks/child_delete.c
index 45e97e4cd..e6834a93c 100644
--- a/src/libcharon/sa/tasks/child_delete.c
+++ b/src/libcharon/sa/tasks/child_delete.c
@@ -163,6 +163,7 @@ static void process_payloads(private_child_delete_t *this, message_t *message)
 														   protocol, spi);
 							continue;
 						}
+						/* fall through */
 					case CHILD_INSTALLED:
 						if (!this->initiator)
 						{	/* reestablish installed children if required */
diff --git a/src/libcharon/sa/tasks/child_rekey.c b/src/libcharon/sa/tasks/child_rekey.c
index e74ca4eef..b39a5fc67 100644
--- a/src/libcharon/sa/tasks/child_rekey.c
+++ b/src/libcharon/sa/tasks/child_rekey.c
@@ -382,7 +382,7 @@ static void collide(private_child_rekey_t *this, task_t *other)
 	if (other->get_type(other) == CHILD_REKEY)
 	{
 		private_child_rekey_t *rekey = (private_child_rekey_t*)other;
-		if (rekey == NULL || rekey->child_sa != this->child_sa)
+		if (rekey->child_sa != this->child_sa)
 		{
 			/* not the same child => no collision */
 			other->destroy(other);
@@ -399,7 +399,7 @@ static void collide(private_child_rekey_t *this, task_t *other)
 			other->destroy(other);
 			return;
 		}
-		if (del == NULL || del->get_child(del) != this->child_sa)
+		if (del->get_child(del) != this->child_sa)
 		{
 			/* not the same child => no collision */
 			other->destroy(other);
@@ -412,6 +412,8 @@ static void collide(private_child_rekey_t *this, task_t *other)
 		other->destroy(other);
 		return;
 	}
+	DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, CHILD_REKEY,
+		 task_type_names, other->get_type(other));
 	DESTROY_IF(this->collision);
 	this->collision = other;
 }
diff --git a/src/libcharon/sa/tasks/ike_config.c b/src/libcharon/sa/tasks/ike_config.c
index c92b5bca5..a61663c48 100644
--- a/src/libcharon/sa/tasks/ike_config.c
+++ b/src/libcharon/sa/tasks/ike_config.c
@@ -317,7 +317,7 @@ static status_t build_r(private_ike_config_t *this, message_t *message)
 		id = this->ike_sa->get_other_eap_id(this->ike_sa);
 
 		config = this->ike_sa->get_peer_cfg(this->ike_sa);
-		if (config && this->virtual_ip)
+		if (this->virtual_ip)
 		{
 			DBG1(DBG_IKE, "peer requested virtual IP %H", this->virtual_ip);
 			if (config->get_pool(config))
diff --git a/src/libcharon/sa/tasks/ike_rekey.c b/src/libcharon/sa/tasks/ike_rekey.c
index 44c55036e..c055dabc1 100644
--- a/src/libcharon/sa/tasks/ike_rekey.c
+++ b/src/libcharon/sa/tasks/ike_rekey.c
@@ -68,9 +68,45 @@ struct private_ike_rekey_t {
 };
 
 /**
- * Implementation of task_t.build for initiator, after rekeying
+ * Establish the new replacement IKE_SA
  */
-static status_t build_i_delete(private_ike_rekey_t *this, message_t *message)
+static void establish_new(private_ike_rekey_t *this)
+{
+	if (this->new_sa)
+	{
+		this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
+		DBG0(DBG_IKE, "IKE_SA %s[%d] rekeyed between %H[%Y]...%H[%Y]",
+			 this->new_sa->get_name(this->new_sa),
+			 this->new_sa->get_unique_id(this->new_sa),
+			 this->ike_sa->get_my_host(this->ike_sa),
+			 this->ike_sa->get_my_id(this->ike_sa),
+			 this->ike_sa->get_other_host(this->ike_sa),
+			 this->ike_sa->get_other_id(this->ike_sa));
+
+		this->new_sa->inherit(this->new_sa, this->ike_sa);
+		charon->bus->ike_rekey(charon->bus, this->ike_sa, this->new_sa);
+		charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
+		this->new_sa = NULL;
+		/* set threads active IKE_SA after checkin */
+		charon->bus->set_sa(charon->bus, this->ike_sa);
+	}
+}
+
+METHOD(task_t, process_r_delete, status_t,
+	private_ike_rekey_t *this, message_t *message)
+{
+	establish_new(this);
+	return this->ike_delete->task.process(&this->ike_delete->task, message);
+}
+
+METHOD(task_t, build_r_delete, status_t,
+	private_ike_rekey_t *this, message_t *message)
+{
+	return this->ike_delete->task.build(&this->ike_delete->task, message);
+}
+
+METHOD(task_t, build_i_delete, status_t,
+	private_ike_rekey_t *this, message_t *message)
 {
 	/* update exchange type to INFORMATIONAL for the delete */
 	message->set_exchange_type(message, INFORMATIONAL);
@@ -78,18 +114,14 @@ static status_t build_i_delete(private_ike_rekey_t *this, message_t *message)
 	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)
+METHOD(task_t, process_i_delete, status_t,
+	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)
+METHOD(task_t, build_i, status_t,
+	private_ike_rekey_t *this, message_t *message)
 {
 	peer_cfg_t *peer_cfg;
 	host_t *other_host;
@@ -112,10 +144,8 @@ static status_t build_i(private_ike_rekey_t *this, message_t *message)
 	return NEED_MORE;
 }
 
-/**
- * Implementation of task_t.process for responder
- */
-static status_t process_r(private_ike_rekey_t *this, message_t *message)
+METHOD(task_t, process_r, status_t,
+	private_ike_rekey_t *this, message_t *message)
 {
 	peer_cfg_t *peer_cfg;
 	iterator_t *iterator;
@@ -156,10 +186,8 @@ static status_t process_r(private_ike_rekey_t *this, message_t *message)
 	return NEED_MORE;
 }
 
-/**
- * Implementation of task_t.build for responder
- */
-static status_t build_r(private_ike_rekey_t *this, message_t *message)
+METHOD(task_t, build_r, status_t,
+	private_ike_rekey_t *this, message_t *message)
 {
 	if (this->new_sa == NULL)
 	{
@@ -174,22 +202,17 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message)
 	}
 
 	this->ike_sa->set_state(this->ike_sa, IKE_REKEYING);
-	this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
-	DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
-		 this->new_sa->get_name(this->new_sa),
-		 this->new_sa->get_unique_id(this->new_sa),
-		 this->ike_sa->get_my_host(this->ike_sa),
-		 this->ike_sa->get_my_id(this->ike_sa),
-		 this->ike_sa->get_other_host(this->ike_sa),
-		 this->ike_sa->get_other_id(this->ike_sa));
-
-	return SUCCESS;
+
+	/* rekeying successful, delete the IKE_SA using a subtask */
+	this->ike_delete = ike_delete_create(this->ike_sa, FALSE);
+	this->public.task.build = _build_r_delete;
+	this->public.task.process = _process_r_delete;
+
+	return NEED_MORE;
 }
 
-/**
- * Implementation of task_t.process for initiator
- */
-static status_t process_i(private_ike_rekey_t *this, message_t *message)
+METHOD(task_t, process_i, status_t,
+	private_ike_rekey_t *this, message_t *message)
 {
 	if (message->get_notify(message, NO_ADDITIONAL_SAS))
 	{
@@ -228,15 +251,6 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 			break;
 	}
 
-	this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
-	DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
-		 this->new_sa->get_name(this->new_sa),
-		 this->new_sa->get_unique_id(this->new_sa),
-		 this->ike_sa->get_my_host(this->ike_sa),
-		 this->ike_sa->get_my_id(this->ike_sa),
-		 this->ike_sa->get_other_host(this->ike_sa),
-		 this->ike_sa->get_other_id(this->ike_sa));
-
 	/* check for collisions */
 	if (this->collision &&
 		this->collision->get_type(this->collision) == IKE_REKEY)
@@ -275,21 +289,20 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 				host = this->ike_sa->get_other_host(this->ike_sa);
 				this->new_sa->set_other_host(this->new_sa, host->clone(host));
 				this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
+				this->new_sa->set_state(this->new_sa, IKE_REKEYING);
 				if (this->new_sa->delete(this->new_sa) == DESTROY_ME)
 				{
-					charon->ike_sa_manager->checkin_and_destroy(
-								charon->ike_sa_manager, this->new_sa);
+					this->new_sa->destroy(this->new_sa);
 				}
 				else
 				{
 					charon->ike_sa_manager->checkin(
 								charon->ike_sa_manager, this->new_sa);
+					/* set threads active IKE_SA after checkin */
+					charon->bus->set_sa(charon->bus, this->ike_sa);
 				}
-				/* set threads active IKE_SA after checkin */
-				charon->bus->set_sa(charon->bus, this->ike_sa);
-				/* inherit to other->new_sa in destroy() */
-				this->new_sa = other->new_sa;
-				other->new_sa = NULL;
+				this->new_sa = NULL;
+				establish_new(other);
 				return SUCCESS;
 			}
 		}
@@ -297,32 +310,33 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
 		charon->bus->set_sa(charon->bus, this->ike_sa);
 	}
 
+	establish_new(this);
+
 	/* 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;
+	this->public.task.build = _build_i_delete;
+	this->public.task.process = _process_i_delete;
 
 	return NEED_MORE;
 }
 
-/**
- * Implementation of task_t.get_type
- */
-static task_type_t get_type(private_ike_rekey_t *this)
+METHOD(task_t, get_type, task_type_t,
+	private_ike_rekey_t *this)
 {
 	return IKE_REKEY;
 }
 
-static void collide(private_ike_rekey_t* this, task_t *other)
+METHOD(ike_rekey_t, collide, void,
+	private_ike_rekey_t* this, task_t *other)
 {
+	DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, IKE_REKEY,
+		 task_type_names, other->get_type(other));
 	DESTROY_IF(this->collision);
 	this->collision = other;
 }
 
-/**
- * Implementation of task_t.migrate
- */
-static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
+METHOD(task_t, migrate, void,
+	private_ike_rekey_t *this, ike_sa_t *ike_sa)
 {
 	if (this->ike_init)
 	{
@@ -332,13 +346,7 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
 	{
 		this->ike_delete->task.destroy(&this->ike_delete->task);
 	}
-	if (this->new_sa)
-	{
-		charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
-													this->new_sa);
-		/* set threads active IKE_SA after checkin */
-		charon->bus->set_sa(charon->bus, this->ike_sa);
-	}
+	DESTROY_IF(this->new_sa);
 	DESTROY_IF(this->collision);
 
 	this->collision = NULL;
@@ -348,28 +356,9 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
 	this->ike_delete = NULL;
 }
 
-/**
- * Implementation of task_t.destroy
- */
-static void destroy(private_ike_rekey_t *this)
+METHOD(task_t, destroy, void,
+	private_ike_rekey_t *this)
 {
-	if (this->new_sa)
-	{
-		if (this->new_sa->get_state(this->new_sa) == IKE_ESTABLISHED &&
-			this->new_sa->inherit(this->new_sa, this->ike_sa) != DESTROY_ME)
-		{
-			/* invoke hook if rekeying was successful */
-			charon->bus->ike_rekey(charon->bus, this->ike_sa, this->new_sa);
-			charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
-		}
-		else
-		{
-			charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
-														this->new_sa);
-		}
-		/* set threads active IKE_SA after checkin */
-		charon->bus->set_sa(charon->bus, this->ike_sa);
-	}
 	if (this->ike_init)
 	{
 		this->ike_init->task.destroy(&this->ike_init->task);
@@ -378,6 +367,7 @@ static void destroy(private_ike_rekey_t *this)
 	{
 		this->ike_delete->task.destroy(&this->ike_delete->task);
 	}
+	DESTROY_IF(this->new_sa);
 	DESTROY_IF(this->collision);
 	free(this);
 }
@@ -387,29 +377,27 @@ static void destroy(private_ike_rekey_t *this)
  */
 ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator)
 {
-	private_ike_rekey_t *this = malloc_thing(private_ike_rekey_t);
-
-	this->public.collide = (void(*)(ike_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;
+	private_ike_rekey_t *this;
+
+	INIT(this,
+		.public = {
+			.task = {
+				.get_type = _get_type,
+				.build = _build_r,
+				.process = _process_r,
+				.migrate = _migrate,
+				.destroy = _destroy,
+			},
+			.collide = _collide,
+		},
+		.ike_sa = ike_sa,
+		.initiator = initiator,
+	);
 	if (initiator)
 	{
-		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->public.task.build = _build_i;
+		this->public.task.process = _process_i;
 	}
-	else
-	{
-		this->public.task.build = (status_t(*)(task_t*,message_t*))build_r;
-		this->public.task.process = (status_t(*)(task_t*,message_t*))process_r;
-	}
-
-	this->ike_sa = ike_sa;
-	this->new_sa = NULL;
-	this->ike_init = NULL;
-	this->ike_delete = NULL;
-	this->initiator = initiator;
-	this->collision = NULL;
 
 	return &this->public;
 }