child-sa: Change API used to set/install policies
authorTobias Brunner <tobias@strongswan.org>
Wed, 1 Mar 2017 13:40:15 +0000 (14:40 +0100)
committerTobias Brunner <tobias@strongswan.org>
Tue, 23 May 2017 16:41:31 +0000 (18:41 +0200)
This way we only have to pass the traffic selectors once.

src/libcharon/plugins/ha/ha_dispatcher.c
src/libcharon/sa/child_sa.c
src/libcharon/sa/child_sa.h
src/libcharon/sa/ikev1/tasks/quick_mode.c
src/libcharon/sa/ikev2/tasks/child_create.c
src/libcharon/sa/trap_manager.c

index ee66b84..7d22257 100644 (file)
@@ -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));
 
index a512eb8..dc1539f 100644 (file)
@@ -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,
index bc7df99..bc5f919 100644 (file)
@@ -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.
         *
index d65db28..8be82eb 100644 (file)
@@ -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");
index 03e2c00..d4d05c9 100644 (file)
@@ -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");
index 40a0682..51df7a0 100644 (file)
@@ -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)