child-rekey: Don't install outbound SA in case of lost collisions

This splits the SA installation also on the initiator, so we can avoid
installing the outbound SA if we lost a rekey collision, which might
have caused traffic loss depending on the timing of the DELETEs that are
sent in both directions.

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

@@ -478,6 +478,7 @@ static status_t select_and_install(private_child_create_t *this,
 								   bool no_dh, bool ike_auth)
 {
 	status_t status, status_i, status_o;
+	child_sa_outbound_state_t out_state;
 	chunk_t nonce_i, nonce_r;
 	chunk_t encr_i = chunk_empty, encr_r = chunk_empty;
 	chunk_t integ_i = chunk_empty, integ_r = chunk_empty;
@@ -678,29 +679,42 @@ static status_t select_and_install(private_child_create_t *this,
 			status_i = this->child_sa->install(this->child_sa, encr_r, integ_r,
 							this->my_spi, this->my_cpi, this->initiator,
 							TRUE, this->tfcv3);
-			status_o = this->child_sa->install(this->child_sa, encr_i, integ_i,
-							this->other_spi, this->other_cpi, this->initiator,
-							FALSE, this->tfcv3);
 		}
-		else if (!this->rekey)
+		else
 		{
 			status_i = this->child_sa->install(this->child_sa, encr_i, integ_i,
 							this->my_spi, this->my_cpi, this->initiator,
 							TRUE, this->tfcv3);
-			status_o = this->child_sa->install(this->child_sa, encr_r, integ_r,
+		}
+		if (this->rekey)
+		{	/* during rekeyings we install the outbound SA and/or policies
+			 * separately: as responder when we receive the delete for the old
+			 * SA, as initiator pretty much immediately in the ike-rekey task,
+			 * unless there was a rekey collision that we lost */
+			if (this->initiator)
+			{
+				status_o = this->child_sa->register_outbound(this->child_sa,
+							encr_i, integ_i, this->other_spi, this->other_cpi,
+							this->tfcv3);
+			}
+			else
+			{
+				status_o = this->child_sa->register_outbound(this->child_sa,
+							encr_r, integ_r, this->other_spi, this->other_cpi,
+							this->tfcv3);
+			}
+		}
+		else if (this->initiator)
+		{
+			status_o = this->child_sa->install(this->child_sa, encr_i, integ_i,
 							this->other_spi, this->other_cpi, this->initiator,
 							FALSE, this->tfcv3);
 		}
 		else
-		{	/* as responder during a rekeying we only install the inbound
-			 * SA now, the outbound SA and policies are installed when we
-			 * receive the delete for the old SA */
-			status_i = this->child_sa->install(this->child_sa, encr_i, integ_i,
-							this->my_spi, this->my_cpi, this->initiator,
-							TRUE, this->tfcv3);
-			status_o = this->child_sa->register_outbound(this->child_sa, encr_r,
-							integ_r, this->other_spi, this->other_cpi,
-							this->tfcv3);
+		{
+			status_o = this->child_sa->install(this->child_sa, encr_r, integ_r,
+							this->other_spi, this->other_cpi, this->initiator,
+							FALSE, this->tfcv3);
 		}
 	}
 
@@ -749,10 +763,11 @@ static status_t select_and_install(private_child_create_t *this,
 				this->child_sa->create_ts_enumerator(this->child_sa, TRUE));
 	other_ts = linked_list_create_from_enumerator(
 				this->child_sa->create_ts_enumerator(this->child_sa, FALSE));
+	out_state = this->child_sa->get_outbound_state(this->child_sa);
 
 	DBG0(DBG_IKE, "%sCHILD_SA %s{%d} established "
 		 "with SPIs %.8x_i %.8x_o and TS %#R === %#R",
-		 this->rekey && !this->initiator ? "inbound " : "",
+		 (out_state == CHILD_OUTBOUND_INSTALLED) ? "" : "inbound ",
 		 this->child_sa->get_name(this->child_sa),
 		 this->child_sa->get_unique_id(this->child_sa),
 		 ntohl(this->child_sa->get_spi(this->child_sa, TRUE)),

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

@@ -196,7 +196,6 @@ static void install_outbound(private_child_delete_t *this,
 		/* FIXME: delete the new child_sa? */
 		return;
 	}
-	child_sa->set_state(child_sa, CHILD_INSTALLED);
 
 	my_ts = linked_list_create_from_enumerator(
 							child_sa->create_ts_enumerator(child_sa, TRUE));

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

@@ -283,7 +283,8 @@ METHOD(task_t, build_r, status_t,
 /**
  * Handle a rekey collision
  */
-static child_sa_t *handle_collision(private_child_rekey_t *this)
+static child_sa_t *handle_collision(private_child_rekey_t *this,
+									child_sa_t **to_install)
 {
 	child_sa_t *to_delete;
 
@@ -303,6 +304,7 @@ static child_sa_t *handle_collision(private_child_rekey_t *this)
 			child_sa_t *child_sa;
 
 			DBG1(DBG_IKE, "CHILD_SA rekey collision won, deleting old child");
+			*to_install = this->child_create->get_child(this->child_create);
 			to_delete = this->child_sa;
 			/* don't touch child other created, it has already been deleted */
 			if (!this->other_child_destroyed)
@@ -353,7 +355,7 @@ METHOD(task_t, process_i, status_t,
 {
 	protocol_id_t protocol;
 	uint32_t spi;
-	child_sa_t *to_delete;
+	child_sa_t *to_delete, *to_install = NULL;
 
 	if (message->get_notify(message, NO_ADDITIONAL_SAS))
 	{
@@ -415,19 +417,48 @@ METHOD(task_t, process_i, status_t,
 	/* check for rekey collisions */
 	if (this->collision)
 	{
-		to_delete = handle_collision(this);
+		to_delete = handle_collision(this, &to_install);
 	}
 	else
 	{
+		to_install = this->child_create->get_child(this->child_create);
 		to_delete = this->child_sa;
 	}
+	if (to_install)
+	{
+		if (to_install->install_outbound(to_install) != SUCCESS)
+		{
+			DBG1(DBG_IKE, "unable to install outbound IPsec SA (SAD) in kernel");
+			charon->bus->alert(charon->bus, ALERT_INSTALL_CHILD_SA_FAILED,
+							   to_install);
+			/* FIXME: delete the child_sa? fail the task? */
+		}
+		else
+		{
+			linked_list_t *my_ts, *other_ts;
 
+			my_ts = linked_list_create_from_enumerator(
+						to_install->create_ts_enumerator(to_install, TRUE));
+			other_ts = linked_list_create_from_enumerator(
+						to_install->create_ts_enumerator(to_install, FALSE));
+
+			DBG0(DBG_IKE, "outbound CHILD_SA %s{%d} established "
+				 "with SPIs %.8x_i %.8x_o and TS %#R === %#R",
+				 to_install->get_name(to_install),
+				 to_install->get_unique_id(to_install),
+				 ntohl(to_install->get_spi(to_install, TRUE)),
+				 ntohl(to_install->get_spi(to_install, FALSE)),
+				 my_ts, other_ts);
+
+			my_ts->destroy(my_ts);
+			other_ts->destroy(other_ts);
+		}
+	}
 	if (to_delete != this->child_create->get_child(this->child_create))
 	{	/* invoke rekey hook if rekeying successful */
 		charon->bus->child_rekey(charon->bus, this->child_sa,
 							this->child_create->get_child(this->child_create));
 	}
-
 	if (to_delete == NULL)
 	{
 		return SUCCESS;