child-sa: Use flags to track installation of outbound SA and policies separately
authorTobias Brunner <tobias@strongswan.org>
Tue, 11 Jul 2017 11:12:02 +0000 (13:12 +0200)
committerTobias Brunner <tobias@strongswan.org>
Mon, 7 Aug 2017 08:44:05 +0000 (10:44 +0200)
src/libcharon/sa/child_sa.c
src/libcharon/sa/child_sa.h
src/libcharon/tests/utils/sa_asserts.h

index 3d9f613..77d71ab 100644 (file)
@@ -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,
index b9a913d..e41157e 100644 (file)
@@ -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),
 };
 
 /**
index d23f724..216c150 100644 (file)
        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)); \
 })