child-sa: Change API used to set/install policies

This way we only have to pass the traffic selectors once.

6 files changed, 119 insertions, 79 deletions

diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c
index ee66b8442..7d22257c6 100644
--- a/src/libcharon/plugins/ha/ha_dispatcher.c
+++ b/src/libcharon/plugins/ha/ha_dispatcher.c
@@ -818,14 +818,14 @@ static void process_child_add(private_ha_dispatcher_t *this,
 	}
 	enumerator->destroy(enumerator);
 
+	child_sa->set_policies(child_sa, local_ts, remote_ts);
+
 	if (initiator)
 	{
 		if (child_sa->install(child_sa, encr_r, integ_r, inbound_spi,
-							  inbound_cpi, initiator, TRUE, TRUE,
-							  local_ts, remote_ts) != SUCCESS ||
+							  inbound_cpi, initiator, TRUE, TRUE) != SUCCESS ||
 			child_sa->install(child_sa, encr_i, integ_i, outbound_spi,
-							  outbound_cpi, initiator, FALSE, TRUE,
-							  local_ts, remote_ts) != SUCCESS)
+							  outbound_cpi, initiator, FALSE, TRUE) != SUCCESS)
 		{
 			failed = TRUE;
 		}
@@ -833,11 +833,9 @@ static void process_child_add(private_ha_dispatcher_t *this,
 	else
 	{
 		if (child_sa->install(child_sa, encr_i, integ_i, inbound_spi,
-							  inbound_cpi, initiator, TRUE, TRUE,
-							  local_ts, remote_ts) != SUCCESS ||
+							  inbound_cpi, initiator, TRUE, TRUE) != SUCCESS ||
 			child_sa->install(child_sa, encr_r, integ_r, outbound_spi,
-							  outbound_cpi, initiator, FALSE, TRUE,
-							  local_ts, remote_ts) != SUCCESS)
+							  outbound_cpi, initiator, FALSE, TRUE) != SUCCESS)
 		{
 			failed = TRUE;
 		}
@@ -868,7 +866,7 @@ static void process_child_add(private_ha_dispatcher_t *this,
 		child_sa->get_unique_id(child_sa), local_ts, remote_ts,
 		seg_i, this->segments->is_active(this->segments, seg_i) ? "*" : "",
 		seg_o, this->segments->is_active(this->segments, seg_o) ? "*" : "");
-	child_sa->add_policies(child_sa, local_ts, remote_ts);
+	child_sa->install_policies(child_sa);
 	local_ts->destroy_offset(local_ts, offsetof(traffic_selector_t, destroy));
 	remote_ts->destroy_offset(remote_ts, offsetof(traffic_selector_t, destroy));
 
diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c
index a512eb8b8..dc1539fa0 100644
--- a/src/libcharon/sa/child_sa.c
+++ b/src/libcharon/sa/child_sa.c
@@ -693,12 +693,11 @@ METHOD(child_sa_t, alloc_cpi, uint16_t,
 
 METHOD(child_sa_t, install, status_t,
 	private_child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi,
-	uint16_t cpi, bool initiator, bool inbound, bool tfcv3,
-	linked_list_t *my_ts, linked_list_t *other_ts)
+	uint16_t cpi, bool initiator, bool inbound, bool tfcv3)
 {
 	uint16_t enc_alg = ENCR_UNDEFINED, int_alg = AUTH_UNDEFINED, size;
 	uint16_t esn = NO_EXT_SEQ_NUMBERS;
-	linked_list_t *src_ts = NULL, *dst_ts = NULL;
+	linked_list_t *my_ts, *other_ts, *src_ts, *dst_ts;
 	time_t now;
 	kernel_ipsec_sa_id_t id;
 	kernel_ipsec_add_sa_t sa;
@@ -708,6 +707,12 @@ METHOD(child_sa_t, install, status_t,
 	status_t status;
 	bool update = FALSE;
 
+	/* BEET requires the bound address from the traffic selectors */
+	my_ts = linked_list_create_from_enumerator(
+									array_create_enumerator(this->my_ts));
+	other_ts = linked_list_create_from_enumerator(
+									array_create_enumerator(this->other_ts));
+
 	/* now we have to decide which spi to use. Use self allocated, if "in",
 	 * or the one in the proposal, if not "in" (others). Additionally,
 	 * source and dest host switch depending on the role */
@@ -721,6 +726,8 @@ METHOD(child_sa_t, install, status_t,
 		}
 		this->my_spi = spi;
 		this->my_cpi = cpi;
+		dst_ts = my_ts;
+		src_ts = other_ts;
 	}
 	else
 	{
@@ -728,6 +735,8 @@ METHOD(child_sa_t, install, status_t,
 		dst = this->other_addr;
 		this->other_spi = spi;
 		this->other_cpi = cpi;
+		src_ts = my_ts;
+		dst_ts = other_ts;
 
 		if (tfcv3)
 		{
@@ -754,6 +763,8 @@ METHOD(child_sa_t, install, status_t,
 								this->mark_in, this->mark_out, &this->reqid);
 		if (status != SUCCESS)
 		{
+			my_ts->destroy(my_ts);
+			other_ts->destroy(other_ts);
 			return status;
 		}
 		this->reqid_allocated = TRUE;
@@ -783,18 +794,6 @@ METHOD(child_sa_t, install, status_t,
 		lifetime->time.rekey = 0;
 	}
 
-	/* BEET requires the bound address from the traffic selectors */
-	if (inbound)
-	{
-		dst_ts = my_ts;
-		src_ts = other_ts;
-	}
-	else
-	{
-		src_ts = my_ts;
-		dst_ts = other_ts;
-	}
-
 	id = (kernel_ipsec_sa_id_t){
 		.src = src,
 		.dst = dst,
@@ -827,6 +826,8 @@ METHOD(child_sa_t, install, status_t,
 
 	status = charon->kernel->add_sa(charon->kernel, &id, &sa);
 
+	my_ts->destroy(my_ts);
+	other_ts->destroy(other_ts);
 	free(lifetime);
 
 	return status;
@@ -1081,28 +1082,19 @@ static void del_policies_internal(private_child_sa_t *this,
 						 other_sa, type, priority, manual_prio);
 }
 
-METHOD(child_sa_t, add_policies, status_t,
+METHOD(child_sa_t, set_policies, void,
 	   private_child_sa_t *this, linked_list_t *my_ts_list,
 	   linked_list_t *other_ts_list)
 {
 	enumerator_t *enumerator;
 	traffic_selector_t *my_ts, *other_ts;
-	status_t status = SUCCESS;
 
-	if (!this->reqid_allocated && !this->static_reqid)
+	if (array_count(this->my_ts))
 	{
-		/* trap policy, get or confirm reqid */
-		status = charon->kernel->alloc_reqid(
-							charon->kernel, my_ts_list, other_ts_list,
-							this->mark_in, this->mark_out, &this->reqid);
-		if (status != SUCCESS)
-		{
-			return status;
-		}
-		this->reqid_allocated = TRUE;
+		array_destroy_offset(this->my_ts,
+							 offsetof(traffic_selector_t, destroy));
+		this->my_ts = array_create(0, 0);
 	}
-
-	/* apply traffic selectors */
 	enumerator = my_ts_list->create_enumerator(my_ts_list);
 	while (enumerator->enumerate(enumerator, &my_ts))
 	{
@@ -1111,6 +1103,12 @@ METHOD(child_sa_t, add_policies, status_t,
 	enumerator->destroy(enumerator);
 	array_sort(this->my_ts, (void*)traffic_selector_cmp, NULL);
 
+	if (array_count(this->other_ts))
+	{
+		array_destroy_offset(this->other_ts,
+							 offsetof(traffic_selector_t, destroy));
+		this->other_ts = array_create(0, 0);
+	}
 	enumerator = other_ts_list->create_enumerator(other_ts_list);
 	while (enumerator->enumerate(enumerator, &other_ts))
 	{
@@ -1118,6 +1116,33 @@ METHOD(child_sa_t, add_policies, status_t,
 	}
 	enumerator->destroy(enumerator);
 	array_sort(this->other_ts, (void*)traffic_selector_cmp, NULL);
+}
+
+METHOD(child_sa_t, install_policies, status_t,
+	   private_child_sa_t *this)
+{
+	enumerator_t *enumerator;
+	linked_list_t *my_ts_list, *other_ts_list;
+	traffic_selector_t *my_ts, *other_ts;
+	status_t status = SUCCESS;
+
+	if (!this->reqid_allocated && !this->static_reqid)
+	{
+		my_ts_list = linked_list_create_from_enumerator(
+									array_create_enumerator(this->my_ts));
+		other_ts_list = linked_list_create_from_enumerator(
+									array_create_enumerator(this->other_ts));
+		status = charon->kernel->alloc_reqid(
+							charon->kernel, my_ts_list, other_ts_list,
+							this->mark_in, this->mark_out, &this->reqid);
+		my_ts_list->destroy(my_ts_list);
+		other_ts_list->destroy(other_ts_list);
+		if (status != SUCCESS)
+		{
+			return status;
+		}
+		this->reqid_allocated = TRUE;
+	}
 
 	if (!this->config->has_option(this->config, OPT_NO_POLICIES))
 	{
@@ -1505,7 +1530,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
 			.alloc_cpi = _alloc_cpi,
 			.install = _install,
 			.update = _update,
-			.add_policies = _add_policies,
+			.set_policies = _set_policies,
+			.install_policies = _install_policies,
 			.create_ts_enumerator = _create_ts_enumerator,
 			.create_policy_enumerator = _create_policy_enumerator,
 			.destroy = _destroy,
diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h
index bc7df996a..bc5f919ae 100644
--- a/src/libcharon/sa/child_sa.h
+++ b/src/libcharon/sa/child_sa.h
@@ -1,8 +1,8 @@
 /*
- * Copyright (C) 2006-2008 Tobias Brunner
+ * Copyright (C) 2006-2017 Tobias Brunner
  * Copyright (C) 2006-2008 Martin Willi
  * Copyright (C) 2006 Daniel Roethlisberger
- * Hochschule fuer Technik Rapperswil
+ * HSR Hochschule fuer Technik Rapperswil
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -347,6 +347,8 @@ struct child_sa_t {
 	/**
 	 * Install an IPsec SA for one direction.
 	 *
+	 * set_policies() should be called before calling this.
+	 *
 	 * @param encr		encryption key, if any
 	 * @param integ		integrity key
 	 * @param spi		SPI to use, allocated for inbound
@@ -354,26 +356,36 @@ struct child_sa_t {
 	 * @param initiator	TRUE if initiator of exchange resulting in this SA
 	 * @param inbound	TRUE to install an inbound SA, FALSE for outbound
 	 * @param tfcv3		TRUE if peer supports ESPv3 TFC
-	 * @param my_ts		negotiated local traffic selector list
-	 * @param other_ts	negotiated remote traffic selector list
 	 * @return			SUCCESS or FAILED
 	 */
 	status_t (*install)(child_sa_t *this, chunk_t encr, chunk_t integ,
 						uint32_t spi, uint16_t cpi,
-						bool initiator, bool inbound, bool tfcv3,
-						linked_list_t *my_ts, linked_list_t *other_ts);
+						bool initiator, bool inbound, bool tfcv3);
+
 	/**
-	 * Install the policies using some traffic selectors.
+	 * Configure the policies using some traffic selectors.
 	 *
 	 * Supplied lists of traffic_selector_t's specify the policies
 	 * to use for this child sa.
 	 *
-	 * @param my_ts		traffic selectors for local site
-	 * @param other_ts	traffic selectors for remote site
+	 * Install the policies by calling install_policies().
+	 *
+	 * This should be called before calling install() so the traffic selectors
+	 * may be passed to the kernel interface when installing the SAs.
+	 *
+	 * @param my_ts		traffic selectors for local site (cloned)
+	 * @param other_ts	traffic selectors for remote site (cloned)
+	 */
+	void (*set_policies)(child_sa_t *this, linked_list_t *my_ts_list,
+						 linked_list_t *other_ts_list);
+
+	/**
+	 * Install the configured policies.
+	 *
 	 * @return			SUCCESS or FAILED
 	 */
-	status_t (*add_policies)(child_sa_t *this, linked_list_t *my_ts_list,
-							 linked_list_t *other_ts_list);
+	status_t (*install_policies)(child_sa_t *this);
+
 	/**
 	 * Update hosts and ecapulation mode in the kernel SAs and policies.
 	 *
diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c
index d65db2875..8be82ebe2 100644
--- a/src/libcharon/sa/ikev1/tasks/quick_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c
@@ -325,6 +325,17 @@ static bool install(private_quick_mode_t *this)
 		return FALSE;
 	}
 
+	if (this->initiator)
+	{
+		this->child_sa->set_policies(this->child_sa, tsi, tsr);
+	}
+	else
+	{
+		this->child_sa->set_policies(this->child_sa, tsr, tsi);
+	}
+	tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy));
+	tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy));
+
 	if (this->keymat->derive_child_keys(this->keymat, this->proposal, this->dh,
 						this->spi_i, this->spi_r, this->nonce_i, this->nonce_r,
 						&encr_i, &integ_i, &encr_r, &integ_r))
@@ -333,19 +344,19 @@ static bool install(private_quick_mode_t *this)
 		{
 			status_i = this->child_sa->install(this->child_sa,
 									encr_r, integ_r, this->spi_i, this->cpi_i,
-									this->initiator, TRUE, FALSE, tsi, tsr);
+									this->initiator, TRUE, FALSE);
 			status_o = this->child_sa->install(this->child_sa,
 									encr_i, integ_i, this->spi_r, this->cpi_r,
-									this->initiator, FALSE, FALSE, tsi, tsr);
+									this->initiator, FALSE, FALSE);
 		}
 		else
 		{
 			status_i = this->child_sa->install(this->child_sa,
 									encr_i, integ_i, this->spi_r, this->cpi_r,
-									this->initiator, TRUE, FALSE, tsr, tsi);
+									this->initiator, TRUE, FALSE);
 			status_o = this->child_sa->install(this->child_sa,
 									encr_r, integ_r, this->spi_i, this->cpi_i,
-									this->initiator, FALSE, FALSE, tsr, tsi);
+									this->initiator, FALSE, FALSE);
 		}
 	}
 
@@ -355,22 +366,12 @@ static bool install(private_quick_mode_t *this)
 			(status_i != SUCCESS) ? "inbound " : "",
 			(status_i != SUCCESS && status_o != SUCCESS) ? "and ": "",
 			(status_o != SUCCESS) ? "outbound " : "");
-		tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy));
-		tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy));
 		status = FAILED;
 	}
 	else
 	{
-		if (this->initiator)
-		{
-			status = this->child_sa->add_policies(this->child_sa, tsi, tsr);
-		}
-		else
-		{
-			status = this->child_sa->add_policies(this->child_sa, tsr, tsi);
-		}
-		tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy));
-		tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy));
+		status = this->child_sa->install_policies(this->child_sa);
+
 		if (status != SUCCESS)
 		{
 			DBG1(DBG_IKE, "unable to install IPsec policies (SPD) in kernel");
diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c
index 03e2c0029..d4d05c9c1 100644
--- a/src/libcharon/sa/ikev2/tasks/child_create.c
+++ b/src/libcharon/sa/ikev2/tasks/child_create.c
@@ -649,6 +649,15 @@ static status_t select_and_install(private_child_create_t *this,
 		}
 	}
 
+	this->child_sa->set_policies(this->child_sa, my_ts, other_ts);
+	if (!this->initiator)
+	{
+		my_ts->destroy_offset(my_ts,
+							  offsetof(traffic_selector_t, destroy));
+		other_ts->destroy_offset(other_ts,
+							  offsetof(traffic_selector_t, destroy));
+	}
+
 	this->child_sa->set_state(this->child_sa, CHILD_INSTALLING);
 	this->child_sa->set_ipcomp(this->child_sa, this->ipcomp);
 	this->child_sa->set_mode(this->child_sa, this->mode);
@@ -668,19 +677,19 @@ 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, my_ts, other_ts);
+							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, my_ts, other_ts);
+							FALSE, this->tfcv3);
 		}
 		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, my_ts, other_ts);
+							TRUE, 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, my_ts, other_ts);
+							FALSE, this->tfcv3);
 		}
 	}
 
@@ -696,15 +705,8 @@ static status_t select_and_install(private_child_create_t *this,
 	}
 	else
 	{
-		status = this->child_sa->add_policies(this->child_sa, my_ts, other_ts);
+		status = this->child_sa->install_policies(this->child_sa);
 
-		if (!this->initiator)
-		{
-			my_ts->destroy_offset(my_ts,
-								  offsetof(traffic_selector_t, destroy));
-			other_ts->destroy_offset(other_ts,
-								  offsetof(traffic_selector_t, destroy));
-		}
 		if (status != SUCCESS)
 		{
 			DBG1(DBG_IKE, "unable to install IPsec policies (SPD) in kernel");
diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 40a0682f2..51df7a0db 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -272,7 +272,8 @@ METHOD(trap_manager_t, install, uint32_t,
 	proposals->destroy_offset(proposals, offsetof(proposal_t, destroy));
 	child_sa->set_protocol(child_sa, proto);
 	child_sa->set_mode(child_sa, child->get_mode(child));
-	status = child_sa->add_policies(child_sa, my_ts, other_ts);
+	child_sa->set_policies(child_sa, my_ts, other_ts);
+	status = child_sa->install_policies(child_sa);
 	my_ts->destroy_offset(my_ts, offsetof(traffic_selector_t, destroy));
 	other_ts->destroy_offset(other_ts, offsetof(traffic_selector_t, destroy));
 	if (status != SUCCESS)