child-sa: replace get_traffic_selectors() with create_ts_enumerator()
authorMartin Willi <martin@revosec.ch>
Wed, 17 Jul 2013 08:01:22 +0000 (10:01 +0200)
committerMartin Willi <martin@revosec.ch>
Wed, 17 Jul 2013 15:20:18 +0000 (17:20 +0200)
Not directly returning a linked list allows us to change the internals of
the CHILD_SA transparently.

12 files changed:
src/libcharon/plugins/farp/farp_listener.c
src/libcharon/plugins/ha/ha_child.c
src/libcharon/plugins/smp/smp.c
src/libcharon/plugins/stroke/stroke_list.c
src/libcharon/plugins/uci/uci_control.c
src/libcharon/sa/child_sa.c
src/libcharon/sa/child_sa.h
src/libcharon/sa/ikev1/task_manager_v1.c
src/libcharon/sa/ikev1/tasks/quick_delete.c
src/libcharon/sa/ikev1/tasks/quick_mode.c
src/libcharon/sa/ikev2/tasks/child_create.c
src/libcharon/sa/ikev2/tasks/child_delete.c

index 81d5d24..87c8435 100644 (file)
@@ -58,19 +58,30 @@ METHOD(listener_t, child_updown, bool,
        bool up)
 {
        enumerator_t *enumerator;
+       traffic_selector_t *ts;
        entry_t *entry;
 
        if (up)
        {
                INIT(entry,
-                       .local = child_sa->get_traffic_selectors(child_sa, TRUE),
-                       .remote = child_sa->get_traffic_selectors(child_sa, FALSE),
+                       .local = linked_list_create(),
+                       .remote = linked_list_create(),
                        .reqid = child_sa->get_reqid(child_sa),
                );
-               entry->local = entry->local->clone_offset(entry->local,
-                                                                               offsetof(traffic_selector_t, clone));
-               entry->remote = entry->remote->clone_offset(entry->remote,
-                                                                               offsetof(traffic_selector_t, clone));
+
+               enumerator = child_sa->create_ts_enumerator(child_sa, TRUE);
+               while (enumerator->enumerate(enumerator, &ts))
+               {
+                       entry->local->insert_last(entry->local, ts->clone(ts));
+               }
+               enumerator->destroy(enumerator);
+
+               enumerator = child_sa->create_ts_enumerator(child_sa, FALSE);
+               while (enumerator->enumerate(enumerator, &ts))
+               {
+                       entry->remote->insert_last(entry->remote, ts->clone(ts));
+               }
+               enumerator->destroy(enumerator);
 
                this->lock->write_lock(this->lock);
                this->entries->insert_last(this->entries, entry);
@@ -160,4 +171,3 @@ farp_listener_t *farp_listener_create()
 
        return &this->public;
 }
-
index 707add9..c166d72 100644 (file)
@@ -103,18 +103,22 @@ METHOD(listener_t, child_keys, bool,
                chunk_clear(&secret);
        }
 
-       local_ts = child_sa->get_traffic_selectors(child_sa, TRUE);
-       enumerator = local_ts->create_enumerator(local_ts);
+       local_ts = linked_list_create();
+       remote_ts = linked_list_create();
+
+       enumerator = child_sa->create_ts_enumerator(child_sa, TRUE);
        while (enumerator->enumerate(enumerator, &ts))
        {
                m->add_attribute(m, HA_LOCAL_TS, ts);
+               local_ts->insert_last(local_ts, ts);
        }
        enumerator->destroy(enumerator);
-       remote_ts = child_sa->get_traffic_selectors(child_sa, FALSE);
-       enumerator = remote_ts->create_enumerator(remote_ts);
+
+       enumerator = child_sa->create_ts_enumerator(child_sa, FALSE);
        while (enumerator->enumerate(enumerator, &ts))
        {
                m->add_attribute(m, HA_REMOTE_TS, ts);
+               remote_ts->insert_last(remote_ts, ts);
        }
        enumerator->destroy(enumerator);
 
@@ -128,6 +132,9 @@ METHOD(listener_t, child_keys, bool,
                seg_i, this->segments->is_active(this->segments, seg_i) ? "*" : "",
                seg_o, this->segments->is_active(this->segments, seg_o) ? "*" : "");
 
+       local_ts->destroy(local_ts);
+       remote_ts->destroy(remote_ts);
+
        this->socket->push(this->socket, m);
        m->destroy(m);
 
@@ -195,4 +202,3 @@ ha_child_t *ha_child_create(ha_socket_t *socket, ha_tunnel_t *tunnel,
 
        return &this->public;
 }
-
index d13b822..1236088 100644 (file)
@@ -165,8 +165,10 @@ static void write_childend(xmlTextWriterPtr writer, child_sa_t *child, bool loca
 
        xmlTextWriterWriteFormatElement(writer, "spi", "%x",
                                                                        htonl(child->get_spi(child, local)));
-       list = child->get_traffic_selectors(child, local);
+       list = linked_list_create_from_enumerator(
+                                                                       child->create_ts_enumerator(child, local));
        write_networks(writer, "networks", list);
+       list->destroy(list);
 }
 
 /**
index e76afec..e81f3fc 100644 (file)
@@ -207,8 +207,10 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all)
        time_t use_in, use_out, rekey, now;
        u_int64_t bytes_in, bytes_out, packets_in, packets_out;
        proposal_t *proposal;
-       child_cfg_t *config = child_sa->get_config(child_sa);
+       linked_list_t *my_ts, *other_ts;
+       child_cfg_t *config;
 
+       config = child_sa->get_config(child_sa);
        now = time_monotonic(NULL);
 
        fprintf(out, "%12s{%d}:  %N, %N%s",
@@ -319,10 +321,15 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all)
                fprintf(out, ", expires in %V", &now, &rekey);
        }
 
+       my_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, TRUE));
+       other_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, FALSE));
        fprintf(out, "\n%12s{%d}:   %#R=== %#R\n",
                        child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
-                       child_sa->get_traffic_selectors(child_sa, TRUE),
-                       child_sa->get_traffic_selectors(child_sa, FALSE));
+                       my_ts, other_ts);
+       my_ts->destroy(my_ts);
+       other_ts->destroy(other_ts);
 }
 
 /**
index 53221b7..cebc389 100644 (file)
@@ -72,6 +72,7 @@ static void write_fifo(private_uci_control_t *this, char *format, ...)
 static void status(private_uci_control_t *this, char *name)
 {
        enumerator_t *configs, *sas, *children;
+       linked_list_t *list;
        ike_sa_t *ike_sa;
        child_sa_t *child_sa;
        peer_cfg_t *peer_cfg;
@@ -108,8 +109,10 @@ static void status(private_uci_control_t *this, char *name)
                        children = ike_sa->create_child_sa_enumerator(ike_sa);
                        while (children->enumerate(children, (void**)&child_sa))
                        {
-                               fprintf(out, "%#R",
-                                               child_sa->get_traffic_selectors(child_sa, FALSE));
+                               list = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, FALSE));
+                               fprintf(out, "%#R", list);
+                               list->destroy(list);
                        }
                        children->destroy(children);
                        fprintf(out, "\n");
@@ -296,4 +299,3 @@ uci_control_t *uci_control_create()
        }
        return &this->public;
 }
-
index 1069b2d..028a2f5 100644 (file)
@@ -331,10 +331,14 @@ METHOD(child_sa_t, set_proposal, void,
        this->proposal = proposal->clone(proposal);
 }
 
-METHOD(child_sa_t, get_traffic_selectors, linked_list_t*,
-          private_child_sa_t *this, bool local)
+METHOD(child_sa_t, create_ts_enumerator, enumerator_t*,
+       private_child_sa_t *this, bool local)
 {
-       return local ? this->my_ts : this->other_ts;
+       if (local)
+       {
+               return this->my_ts->create_enumerator(this->my_ts);
+       }
+       return this->other_ts->create_enumerator(this->other_ts);
 }
 
 typedef struct policy_enumerator_t policy_enumerator_t;
@@ -1117,7 +1121,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
                        .install = _install,
                        .update = _update,
                        .add_policies = _add_policies,
-                       .get_traffic_selectors = _get_traffic_selectors,
+                       .create_ts_enumerator = _create_ts_enumerator,
                        .create_policy_enumerator = _create_policy_enumerator,
                        .destroy = _destroy,
                },
index aa44dbf..44c5293 100644 (file)
@@ -284,17 +284,20 @@ struct child_sa_t {
        mark_t (*get_mark)(child_sa_t *this, bool inbound);
 
        /**
-        * Get the traffic selectors list added for one side.
+        * Create an enumerator over traffic selectors of one side.
         *
-        * @param local         TRUE for own traffic selectors, FALSE for remote
-        * @return                      list of traffic selectors
+        * @param local         TRUE for own traffic selectors, FALSE for remote.
+        * @return                      enumerator over traffic_selector_t*
         */
-       linked_list_t* (*get_traffic_selectors) (child_sa_t *this, bool local);
+       enumerator_t* (*create_ts_enumerator)(child_sa_t *this, bool local);
 
        /**
         * Create an enumerator over installed policies.
         *
-        * @return                      enumerator over pairs of traffic selectors.
+        * The enumerated traffic selectors is a full mesh of compatible local
+        * and remote traffic selectors.
+        *
+        * @return                      enumerator over a pair of traffic_selector_t*
         */
        enumerator_t* (*create_policy_enumerator)(child_sa_t *this);
 
index 709033c..bfa6fc8 100644 (file)
@@ -1753,21 +1753,22 @@ METHOD(task_manager_t, queue_child, void,
 /**
  * Check if two CHILD_SAs have the same traffic selector
  */
-static bool have_equal_ts(child_sa_t *a, child_sa_t *b, bool local)
+static bool have_equal_ts(child_sa_t *child1, child_sa_t *child2, bool local)
 {
-       linked_list_t *list;
-       traffic_selector_t *ts_a, *ts_b;
+       enumerator_t *e1, *e2;
+       traffic_selector_t *ts1, *ts2;
+       bool equal = FALSE;
 
-       list = a->get_traffic_selectors(a, local);
-       if (list->get_first(list, (void**)&ts_a) == SUCCESS)
+       e1 = child1->create_ts_enumerator(child1, local);
+       e2 = child2->create_ts_enumerator(child2, local);
+       if (e1->enumerate(e1, &ts1) && e2->enumerate(e2, &ts2))
        {
-               list = b->get_traffic_selectors(b, local);
-               if (list->get_first(list, (void**)&ts_b) == SUCCESS)
-               {
-                       return ts_a->equals(ts_a, ts_b);
-               }
+               equal = ts1->equals(ts1, ts2);
        }
-       return FALSE;
+       e1->destroy(e1);
+       e1->destroy(e1);
+
+       return equal;
 }
 
 /**
@@ -1806,14 +1807,13 @@ static bool is_redundant(private_task_manager_t *this, child_sa_t *child_sa)
 static traffic_selector_t* get_first_ts(child_sa_t *child_sa, bool local)
 {
        traffic_selector_t *ts = NULL;
-       linked_list_t *list;
+       enumerator_t *enumerator;
 
-       list = child_sa->get_traffic_selectors(child_sa, local);
-       if (list->get_first(list, (void**)&ts) == SUCCESS)
-       {
-               return ts;
-       }
-       return NULL;
+       enumerator = child_sa->create_ts_enumerator(child_sa, local);
+       enumerator->enumerate(enumerator, &ts);
+       enumerator->destroy(enumerator);
+
+       return ts;
 }
 
 METHOD(task_manager_t, queue_child_rekey, void,
index e9f06cb..5b3e35e 100644 (file)
@@ -69,6 +69,7 @@ static bool delete_child(private_quick_delete_t *this,
 {
        u_int64_t bytes_in, bytes_out;
        child_sa_t *child_sa;
+       linked_list_t *my_ts, *other_ts;
        bool rekeyed;
 
        child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, spi, TRUE);
@@ -85,15 +86,17 @@ static bool delete_child(private_quick_delete_t *this,
        rekeyed = child_sa->get_state(child_sa) == CHILD_REKEYING;
        child_sa->set_state(child_sa, CHILD_DELETING);
 
+       my_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, TRUE));
+       other_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, FALSE));
        if (this->expired)
        {
                DBG0(DBG_IKE, "closing expired CHILD_SA %s{%d} "
                         "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
                         child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
                         ntohl(child_sa->get_spi(child_sa, TRUE)),
-                        ntohl(child_sa->get_spi(child_sa, FALSE)),
-                        child_sa->get_traffic_selectors(child_sa, TRUE),
-                        child_sa->get_traffic_selectors(child_sa, FALSE));
+                        ntohl(child_sa->get_spi(child_sa, FALSE)), my_ts, other_ts);
        }
        else
        {
@@ -105,9 +108,10 @@ static bool delete_child(private_quick_delete_t *this,
                         child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
                         ntohl(child_sa->get_spi(child_sa, TRUE)), bytes_in,
                         ntohl(child_sa->get_spi(child_sa, FALSE)), bytes_out,
-                        child_sa->get_traffic_selectors(child_sa, TRUE),
-                        child_sa->get_traffic_selectors(child_sa, FALSE));
+                        my_ts, other_ts);
        }
+       my_ts->destroy(my_ts);
+       other_ts->destroy(other_ts);
 
        if (!rekeyed)
        {
index 52ea34b..92df0f0 100644 (file)
@@ -259,7 +259,7 @@ static bool install(private_quick_mode_t *this)
 {
        status_t status, status_i, status_o;
        chunk_t encr_i, encr_r, integ_i, integ_r;
-       linked_list_t *tsi, *tsr;
+       linked_list_t *tsi, *tsr, *my_ts, *other_ts;
        child_sa_t *old = NULL;
 
        this->child_sa->set_proposal(this->child_sa, this->proposal);
@@ -362,14 +362,20 @@ static bool install(private_quick_mode_t *this)
        this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
        this->ike_sa->add_child_sa(this->ike_sa, this->child_sa);
 
+       my_ts = linked_list_create_from_enumerator(
+                               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));
+
        DBG0(DBG_IKE, "CHILD_SA %s{%d} established "
                 "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
                 this->child_sa->get_name(this->child_sa),
                 this->child_sa->get_reqid(this->child_sa),
                 ntohl(this->child_sa->get_spi(this->child_sa, TRUE)),
-                ntohl(this->child_sa->get_spi(this->child_sa, FALSE)),
-                this->child_sa->get_traffic_selectors(this->child_sa, TRUE),
-                this->child_sa->get_traffic_selectors(this->child_sa, FALSE));
+                ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), my_ts, other_ts);
+
+       my_ts->destroy(my_ts);
+       other_ts->destroy(other_ts);
 
        if (this->rekey)
        {
index dd3813e..8ae36af 100644 (file)
@@ -673,6 +673,22 @@ static status_t select_and_install(private_child_create_t *this,
        {       /* a rekeyed SA uses the same reqid, no need for a new job */
                schedule_inactivity_timeout(this);
        }
+
+       my_ts = linked_list_create_from_enumerator(
+                               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));
+
+       DBG0(DBG_IKE, "CHILD_SA %s{%d} established "
+                "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
+                this->child_sa->get_name(this->child_sa),
+                this->child_sa->get_reqid(this->child_sa),
+                ntohl(this->child_sa->get_spi(this->child_sa, TRUE)),
+                ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), my_ts, other_ts);
+
+       my_ts->destroy(my_ts);
+       other_ts->destroy(other_ts);
+
        return SUCCESS;
 }
 
@@ -1245,15 +1261,6 @@ METHOD(task_t, build_r, status_t,
 
        build_payloads(this, message);
 
-       DBG0(DBG_IKE, "CHILD_SA %s{%d} established "
-                "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
-                this->child_sa->get_name(this->child_sa),
-                this->child_sa->get_reqid(this->child_sa),
-                ntohl(this->child_sa->get_spi(this->child_sa, TRUE)),
-                ntohl(this->child_sa->get_spi(this->child_sa, FALSE)),
-                this->child_sa->get_traffic_selectors(this->child_sa, TRUE),
-                this->child_sa->get_traffic_selectors(this->child_sa, FALSE));
-
        if (!this->rekey)
        {       /* invoke the child_up() hook if we are not rekeying */
                charon->bus->child_updown(charon->bus, this->child_sa, TRUE);
@@ -1431,15 +1438,6 @@ METHOD(task_t, process_i, status_t,
 
        if (select_and_install(this, no_dh, ike_auth) == SUCCESS)
        {
-               DBG0(DBG_IKE, "CHILD_SA %s{%d} established "
-                        "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
-                        this->child_sa->get_name(this->child_sa),
-                        this->child_sa->get_reqid(this->child_sa),
-                        ntohl(this->child_sa->get_spi(this->child_sa, TRUE)),
-                        ntohl(this->child_sa->get_spi(this->child_sa, FALSE)),
-                        this->child_sa->get_traffic_selectors(this->child_sa, TRUE),
-                        this->child_sa->get_traffic_selectors(this->child_sa, FALSE));
-
                if (!this->rekey)
                {       /* invoke the child_up() hook if we are not rekeying */
                        charon->bus->child_updown(charon->bus, this->child_sa, TRUE);
index 9f12d73..eaaca20 100644 (file)
@@ -249,6 +249,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
  */
 static void log_children(private_child_delete_t *this)
 {
+       linked_list_t *my_ts, *other_ts;
        enumerator_t *enumerator;
        child_sa_t *child_sa;
        u_int64_t bytes_in, bytes_out;
@@ -256,15 +257,17 @@ static void log_children(private_child_delete_t *this)
        enumerator = this->child_sas->create_enumerator(this->child_sas);
        while (enumerator->enumerate(enumerator, (void**)&child_sa))
        {
+               my_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, TRUE));
+               other_ts = linked_list_create_from_enumerator(
+                                                       child_sa->create_ts_enumerator(child_sa, FALSE));
                if (this->expired)
                {
                        DBG0(DBG_IKE, "closing expired CHILD_SA %s{%d} "
                                 "with SPIs %.8x_i %.8x_o and TS %#R=== %#R",
                                 child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
                                 ntohl(child_sa->get_spi(child_sa, TRUE)),
-                                ntohl(child_sa->get_spi(child_sa, FALSE)),
-                                child_sa->get_traffic_selectors(child_sa, TRUE),
-                                child_sa->get_traffic_selectors(child_sa, FALSE));
+                                ntohl(child_sa->get_spi(child_sa, FALSE)), my_ts, other_ts);
                }
                else
                {
@@ -276,9 +279,10 @@ static void log_children(private_child_delete_t *this)
                                 child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
                                 ntohl(child_sa->get_spi(child_sa, TRUE)), bytes_in,
                                 ntohl(child_sa->get_spi(child_sa, FALSE)), bytes_out,
-                                child_sa->get_traffic_selectors(child_sa, TRUE),
-                                child_sa->get_traffic_selectors(child_sa, FALSE));
+                                my_ts, other_ts);
                }
+               my_ts->destroy(my_ts);
+               other_ts->destroy(other_ts);
        }
        enumerator->destroy(enumerator);
 }