child-sa: Use flags to track installation of outbound SA and policies separately

3 files changed, 46 insertions, 29 deletions

diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c
index 3d9f6133b..77d71ab15 100644
--- a/src/libcharon/sa/child_sa.c
+++ b/src/libcharon/sa/child_sa.c
@@ -40,10 +40,10 @@ ENUM(child_sa_state_names, CHILD_CREATED, CHILD_DESTROYING,
 	"DESTROYING",
 );
 
-ENUM(child_sa_outbound_state_names, CHILD_OUTBOUND_NONE, CHILD_OUTBOUND_INSTALLED,
-	"NONE",
+ENUM_FLAGS(child_sa_outbound_state_names, CHILD_OUTBOUND_REGISTERED, CHILD_OUTBOUND_POLICIES,
 	"REGISTERED",
-	"INSTALLED",
+	"SA",
+	"POLICIES",
 );
 
 typedef struct private_child_sa_t private_child_sa_t;
@@ -547,7 +547,7 @@ static status_t update_usebytes(private_child_sa_t *this, bool inbound)
 	}
 	else
 	{
-		if (this->other_spi && this->outbound_state == CHILD_OUTBOUND_INSTALLED)
+		if (this->other_spi && (this->outbound_state & CHILD_OUTBOUND_SA))
 		{
 			kernel_ipsec_sa_id_t id = {
 				.src = this->my_addr,
@@ -788,7 +788,7 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr,
 		{
 			tfc = this->config->get_tfc(this->config);
 		}
-		this->outbound_state = CHILD_OUTBOUND_INSTALLED;
+		this->outbound_state |= CHILD_OUTBOUND_SA;
 	}
 
 	DBG2(DBG_CHD, "adding %s %N SA", inbound ? "inbound" : "outbound",
@@ -1188,6 +1188,7 @@ METHOD(child_sa_t, install_policies, status_t,
 	linked_list_t *my_ts_list, *other_ts_list;
 	traffic_selector_t *my_ts, *other_ts;
 	status_t status = SUCCESS;
+	bool install_outbound = FALSE;
 
 	if (!this->reqid_allocated && !this->static_reqid)
 	{
@@ -1207,12 +1208,17 @@ METHOD(child_sa_t, install_policies, status_t,
 		this->reqid_allocated = TRUE;
 	}
 
+	if (!(this->outbound_state & CHILD_OUTBOUND_REGISTERED))
+	{
+		install_outbound = TRUE;
+		this->outbound_state |= CHILD_OUTBOUND_POLICIES;
+	}
+
 	if (!this->config->has_option(this->config, OPT_NO_POLICIES))
 	{
 		policy_priority_t priority;
 		ipsec_sa_cfg_t my_sa, other_sa;
 		uint32_t manual_prio;
-		bool install_outbound;
 
 		prepare_sa_cfg(this, &my_sa, &other_sa);
 		manual_prio = this->config->get_manual_prio(this->config);
@@ -1222,7 +1228,6 @@ METHOD(child_sa_t, install_policies, status_t,
 		this->trap = this->state == CHILD_CREATED;
 		priority = this->trap ? POLICY_PRIORITY_ROUTED
 							  : POLICY_PRIORITY_DEFAULT;
-		install_outbound = this->outbound_state != CHILD_OUTBOUND_REGISTERED;
 
 		/* enumerate pairs of traffic selectors */
 		enumerator = create_policy_enumerator(this);
@@ -1250,7 +1255,6 @@ METHOD(child_sa_t, install_policies, status_t,
 									this->other_addr, my_ts, other_ts,
 									&my_sa, &other_sa, POLICY_IPSEC,
 									priority, manual_prio);
-
 			}
 			if (status != SUCCESS)
 			{
@@ -1281,7 +1285,7 @@ METHOD(child_sa_t, register_outbound, void,
 	this->encr_r = chunk_clone(encr);
 	this->integ_r = chunk_clone(integ);
 	this->tfcv3 = tfcv3;
-	this->outbound_state = CHILD_OUTBOUND_REGISTERED;
+	this->outbound_state |= CHILD_OUTBOUND_REGISTERED;
 }
 
 METHOD(child_sa_t, install_outbound, status_t,
@@ -1289,17 +1293,19 @@ METHOD(child_sa_t, install_outbound, status_t,
 {
 	enumerator_t *enumerator;
 	traffic_selector_t *my_ts, *other_ts;
-	status_t status;
+	status_t status = SUCCESS;
 
 	status = install_internal(this, this->encr_r, this->integ_r,
-							  this->other_spi, this->other_cpi, FALSE, FALSE,
-							  this->tfcv3);
+							  this->other_spi, this->other_cpi, FALSE,
+							  FALSE, this->tfcv3);
 	chunk_clear(&this->encr_r);
 	chunk_clear(&this->integ_r);
+	this->outbound_state &= ~CHILD_OUTBOUND_REGISTERED;
 	if (status != SUCCESS)
 	{
 		return status;
 	}
+	this->outbound_state |= CHILD_OUTBOUND_POLICIES;
 	if (!this->config->has_option(this->config, OPT_NO_POLICIES))
 	{
 		ipsec_sa_cfg_t my_sa, other_sa;
@@ -1340,20 +1346,19 @@ METHOD(child_sa_t, remove_outbound, void,
 	enumerator_t *enumerator;
 	traffic_selector_t *my_ts, *other_ts;
 
-	switch (this->outbound_state)
+	if (!(this->outbound_state & CHILD_OUTBOUND_SA))
 	{
-		case CHILD_OUTBOUND_INSTALLED:
-			break;
-		case CHILD_OUTBOUND_REGISTERED:
+		if (this->outbound_state & CHILD_OUTBOUND_REGISTERED)
+		{
 			chunk_clear(&this->encr_r);
 			chunk_clear(&this->integ_r);
 			this->outbound_state = CHILD_OUTBOUND_NONE;
-			/* fall-through */
-		case CHILD_OUTBOUND_NONE:
-			return;
+		}
+		return;
 	}
 
-	if (!this->config->has_option(this->config, OPT_NO_POLICIES))
+	if (!this->config->has_option(this->config, OPT_NO_POLICIES) &&
+		(this->outbound_state & CHILD_OUTBOUND_POLICIES))
 	{
 		ipsec_sa_cfg_t my_sa, other_sa;
 		uint32_t manual_prio;
@@ -1598,8 +1603,8 @@ METHOD(child_sa_t, destroy, void,
 
 		prepare_sa_cfg(this, &my_sa, &other_sa);
 		manual_prio = this->config->get_manual_prio(this->config);
-		del_outbound = this->trap ||
-					   this->outbound_state == CHILD_OUTBOUND_INSTALLED;
+		del_outbound = (this->outbound_state & CHILD_OUTBOUND_POLICIES) ||
+						this->trap;
 
 		/* delete all policies in the kernel */
 		enumerator = create_policy_enumerator(this);
@@ -1640,7 +1645,7 @@ METHOD(child_sa_t, destroy, void,
 		};
 		charon->kernel->del_sa(charon->kernel, &id, &sa);
 	}
-	if (this->other_spi && this->outbound_state == CHILD_OUTBOUND_INSTALLED)
+	if (this->other_spi && (this->outbound_state & CHILD_OUTBOUND_SA))
 	{
 		kernel_ipsec_sa_id_t id = {
 			.src = this->my_addr,
diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h
index b9a913da1..e41157e79 100644
--- a/src/libcharon/sa/child_sa.h
+++ b/src/libcharon/sa/child_sa.h
@@ -102,17 +102,28 @@ enum child_sa_outbound_state_t {
 	/**
 	 * Outbound SA is not installed
 	 */
-	CHILD_OUTBOUND_NONE,
+	CHILD_OUTBOUND_NONE = 0,
 
 	/**
-	 * Data for the outbound SA has been registered, but not installed yet
+	 * Data for the outbound SA has been registered during a rekeying (not set
+	 * once the SA and policies are both installed)
 	 */
-	CHILD_OUTBOUND_REGISTERED,
+	CHILD_OUTBOUND_REGISTERED = (1<<0),
 
 	/**
-	 * The outbound SA is currently installed
+	 * The outbound SA has been installed
 	 */
-	CHILD_OUTBOUND_INSTALLED,
+	CHILD_OUTBOUND_SA = (1<<1),
+
+	/**
+	 * The outbound policies have been installed
+	 */
+	CHILD_OUTBOUND_POLICIES = (1<<2),
+
+	/**
+	 * The outbound SA and policies are both installed
+	 */
+	CHILD_OUTBOUND_INSTALLED = (CHILD_OUTBOUND_SA|CHILD_OUTBOUND_POLICIES),
 };
 
 /**
diff --git a/src/libcharon/tests/utils/sa_asserts.h b/src/libcharon/tests/utils/sa_asserts.h
index d23f724f1..216c15065 100644
--- a/src/libcharon/tests/utils/sa_asserts.h
+++ b/src/libcharon/tests/utils/sa_asserts.h
@@ -121,7 +121,8 @@
 	test_assert_msg(_state == _child->get_state(_child), "%N != %N", \
 					child_sa_state_names, _state, \
 					child_sa_state_names, _child->get_state(_child)); \
-	test_assert_msg(_outbound == _child->get_outbound_state(_child), "%N != %N", \
+	typeof(outbound) _cur_out = _child->get_outbound_state(_child); \
+	test_assert_msg(_outbound == _cur_out || _outbound & _cur_out, "%N != %N", \
 					child_sa_outbound_state_names, _outbound, \
 					child_sa_outbound_state_names, _child->get_outbound_state(_child)); \
 })