trap-manager: Remove reqid parameter from install() and change return type
authorTobias Brunner <tobias@strongswan.org>
Fri, 3 Nov 2017 10:32:04 +0000 (11:32 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 22 Feb 2018 10:31:05 +0000 (11:31 +0100)
Reqids for the same traffic selectors are now stable so we don't have to
pass reqids of previously installed CHILD_SAs.  Likewise, we don't need
to know the reqid of the newly installed trap policy as we now uninstall
by name.

src/libcharon/plugins/ha/ha_tunnel.c
src/libcharon/plugins/stroke/stroke_control.c
src/libcharon/plugins/vici/vici_config.c
src/libcharon/plugins/vici/vici_control.c
src/libcharon/processing/jobs/start_action_job.c
src/libcharon/sa/ike_sa.c
src/libcharon/sa/ikev1/tasks/quick_delete.c
src/libcharon/sa/ikev2/tasks/child_delete.c
src/libcharon/sa/trap_manager.c
src/libcharon/sa/trap_manager.h

index 60004f2..cbee1db 100644 (file)
@@ -262,7 +262,7 @@ static void setup_tunnel(private_ha_tunnel_t *this,
        charon->backends->add_backend(charon->backends, &this->backend.public);
 
        /* install an acquiring trap */
-       this->trap = charon->traps->install(charon->traps, peer_cfg, child_cfg, 0);
+       charon->traps->install(charon->traps, peer_cfg, child_cfg);
 }
 
 METHOD(ha_tunnel_t, destroy, void,
@@ -280,10 +280,7 @@ METHOD(ha_tunnel_t, destroy, void,
        }
        this->creds.local->destroy(this->creds.local);
        this->creds.remote->destroy(this->creds.remote);
-       if (this->trap)
-       {
-               charon->traps->uninstall(charon->traps, HA_CFG_NAME, HA_CFG_NAME);
-       }
+       charon->traps->uninstall(charon->traps, HA_CFG_NAME, HA_CFG_NAME);
        free(this);
 }
 
index a5f2410..d3c2791 100644 (file)
@@ -589,54 +589,12 @@ METHOD(stroke_control_t, purge_ike, void,
 }
 
 /**
- * Find an existing CHILD_SA/reqid
- */
-static uint32_t find_reqid(child_cfg_t *child_cfg)
-{
-       enumerator_t *enumerator, *children;
-       child_sa_t *child_sa;
-       ike_sa_t *ike_sa;
-       char *name;
-       uint32_t reqid;
-
-       reqid = charon->traps->find_reqid(charon->traps, child_cfg);
-       if (reqid)
-       {       /* already trapped */
-               return reqid;
-       }
-
-       name = child_cfg->get_name(child_cfg);
-       enumerator = charon->controller->create_ike_sa_enumerator(
-                                                                                                       charon->controller, TRUE);
-       while (enumerator->enumerate(enumerator, &ike_sa))
-       {
-               children = ike_sa->create_child_sa_enumerator(ike_sa);
-               while (children->enumerate(children, (void**)&child_sa))
-               {
-                       if (streq(name, child_sa->get_name(child_sa)))
-                       {
-                               reqid = child_sa->get_reqid(child_sa);
-                               break;
-                       }
-               }
-               children->destroy(children);
-               if (reqid)
-               {
-                       break;
-               }
-       }
-       enumerator->destroy(enumerator);
-       return reqid;
-}
-
-/**
  * call charon to install a shunt or trap
  */
 static void charon_route(peer_cfg_t *peer_cfg, child_cfg_t *child_cfg,
                                                 char *name, FILE *out)
 {
        ipsec_mode_t mode;
-       uint32_t reqid;
 
        mode = child_cfg->get_mode(child_cfg);
        if (mode == MODE_PASS || mode == MODE_DROP)
@@ -655,8 +613,7 @@ static void charon_route(peer_cfg_t *peer_cfg, child_cfg_t *child_cfg,
        }
        else
        {
-               reqid = find_reqid(child_cfg);
-               if (charon->traps->install(charon->traps, peer_cfg, child_cfg, reqid))
+               if (charon->traps->install(charon->traps, peer_cfg, child_cfg))
                {
                        fprintf(out, "'%s' routed\n", name);
                }
index 43e98c9..c95d8c8 100644 (file)
@@ -1954,41 +1954,6 @@ CALLBACK(peer_sn, bool,
 }
 
 /**
- * Find reqid of an existing CHILD_SA
- */
-static uint32_t find_reqid(child_cfg_t *cfg)
-{
-       enumerator_t *enumerator, *children;
-       child_sa_t *child_sa;
-       ike_sa_t *ike_sa;
-       uint32_t reqid;
-
-       reqid = charon->traps->find_reqid(charon->traps, cfg);
-       if (reqid)
-       {       /* already trapped */
-               return reqid;
-       }
-
-       enumerator = charon->controller->create_ike_sa_enumerator(
-                                                                                                       charon->controller, TRUE);
-       while (!reqid && enumerator->enumerate(enumerator, &ike_sa))
-       {
-               children = ike_sa->create_child_sa_enumerator(ike_sa);
-               while (children->enumerate(children, &child_sa))
-               {
-                       if (streq(cfg->get_name(cfg), child_sa->get_name(child_sa)))
-                       {
-                               reqid = child_sa->get_reqid(child_sa);
-                               break;
-                       }
-               }
-               children->destroy(children);
-       }
-       enumerator->destroy(enumerator);
-       return reqid;
-}
-
-/**
  * Perform start actions associated with a child config
  */
 static void run_start_action(private_vici_config_t *this, peer_cfg_t *peer_cfg,
@@ -2012,8 +1977,7 @@ static void run_start_action(private_vici_config_t *this, peer_cfg_t *peer_cfg,
                                                                        peer_cfg->get_name(peer_cfg), child_cfg);
                                        break;
                                default:
-                                       charon->traps->install(charon->traps, peer_cfg, child_cfg,
-                                                                                  find_reqid(child_cfg));
+                                       charon->traps->install(charon->traps, peer_cfg, child_cfg);
                                        break;
                        }
                        break;
index d0e19d5..f9c7d8f 100644 (file)
@@ -601,41 +601,6 @@ CALLBACK(redirect, vici_message_t*,
        return builder->finalize(builder);
 }
 
-/**
- * Find reqid of an existing CHILD_SA
- */
-static uint32_t find_reqid(child_cfg_t *cfg)
-{
-       enumerator_t *enumerator, *children;
-       child_sa_t *child_sa;
-       ike_sa_t *ike_sa;
-       uint32_t reqid;
-
-       reqid = charon->traps->find_reqid(charon->traps, cfg);
-       if (reqid)
-       {       /* already trapped */
-               return reqid;
-       }
-
-       enumerator = charon->controller->create_ike_sa_enumerator(
-                                                                                                       charon->controller, TRUE);
-       while (!reqid && enumerator->enumerate(enumerator, &ike_sa))
-       {
-               children = ike_sa->create_child_sa_enumerator(ike_sa);
-               while (children->enumerate(children, &child_sa))
-               {
-                       if (streq(cfg->get_name(cfg), child_sa->get_name(child_sa)))
-                       {
-                               reqid = child_sa->get_reqid(child_sa);
-                               break;
-                       }
-               }
-               children->destroy(children);
-       }
-       enumerator->destroy(enumerator);
-       return reqid;
-}
-
 CALLBACK(install, vici_message_t*,
        private_vici_control_t *this, char *name, u_int id, vici_message_t *request)
 {
@@ -666,8 +631,7 @@ CALLBACK(install, vici_message_t*,
                                                                        peer_cfg->get_name(peer_cfg), child_cfg);
                        break;
                default:
-                       ok = charon->traps->install(charon->traps, peer_cfg, child_cfg,
-                                                                               find_reqid(child_cfg));
+                       ok = charon->traps->install(charon->traps, peer_cfg, child_cfg);
                        break;
        }
        peer_cfg->destroy(peer_cfg);
index 654ec6a..3a0ed87 100644 (file)
@@ -75,7 +75,7 @@ METHOD(job_t, execute, job_requeue_t,
                                        else
                                        {
                                                charon->traps->install(charon->traps, peer_cfg,
-                                                                                          child_cfg, 0);
+                                                                                          child_cfg);
                                        }
                                        break;
                                case ACTION_NONE:
index e1f4ec9..7fe6d11 100644 (file)
@@ -2035,8 +2035,7 @@ METHOD(ike_sa_t, reestablish, status_t,
                                        break;
                                case ACTION_ROUTE:
                                        charon->traps->install(charon->traps, this->peer_cfg,
-                                                                                  child_sa->get_config(child_sa),
-                                                                                  child_sa->get_reqid(child_sa));
+                                                                                  child_sa->get_config(child_sa));
                                        break;
                                default:
                                        break;
index 66ef508..6b3bb9c 100644 (file)
@@ -154,7 +154,7 @@ static bool delete_child(private_quick_delete_t *this, protocol_id_t protocol,
                                case ACTION_ROUTE:
                                        charon->traps->install(charon->traps,
                                                                        this->ike_sa->get_peer_cfg(this->ike_sa),
-                                                                       child_cfg, child_sa->get_reqid(child_sa));
+                                                                       child_cfg);
                                        break;
                                default:
                                        break;
index 164f8fc..8fec649 100644 (file)
@@ -374,8 +374,8 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
                                        break;
                                case ACTION_ROUTE:
                                        charon->traps->install(charon->traps,
-                                                       this->ike_sa->get_peer_cfg(this->ike_sa), child_cfg,
-                                                       reqid);
+                                                                       this->ike_sa->get_peer_cfg(this->ike_sa),
+                                                                       child_cfg);
                                        break;
                                default:
                                        break;
index 88e6bb2..f558219 100644 (file)
@@ -183,9 +183,8 @@ static bool dynamic_remote_ts(child_cfg_t *child)
        return found;
 }
 
-METHOD(trap_manager_t, install, uint32_t,
-       private_trap_manager_t *this, peer_cfg_t *peer, child_cfg_t *child,
-       uint32_t reqid)
+METHOD(trap_manager_t, install, bool,
+       private_trap_manager_t *this, peer_cfg_t *peer, child_cfg_t *child)
 {
        entry_t *entry, *found = NULL;
        ike_cfg_t *ike_cfg;
@@ -197,7 +196,7 @@ METHOD(trap_manager_t, install, uint32_t,
        linked_list_t *proposals;
        proposal_t *proposal;
        protocol_id_t proto = PROTO_ESP;
-       bool wildcard = FALSE;
+       bool result = FALSE, wildcard = FALSE;
 
        /* try to resolve addresses */
        ike_cfg = peer->get_ike_cfg(peer);
@@ -213,7 +212,7 @@ METHOD(trap_manager_t, install, uint32_t,
        {
                other->destroy(other);
                DBG1(DBG_CFG, "installing trap failed, remote address unknown");
-               return 0;
+               return FALSE;
        }
        else
        {       /* depending on the traffic selectors we don't really need a remote
@@ -223,7 +222,7 @@ METHOD(trap_manager_t, install, uint32_t,
                         * which is probably not what users expect*/
                        DBG1(DBG_CFG, "installing trap failed, remote address unknown with "
                                 "dynamic traffic selector");
-                       return 0;
+                       return FALSE;
                }
                me = ike_cfg->resolve_me(ike_cfg, other ? other->get_family(other)
                                                                                                : AF_UNSPEC);
@@ -250,7 +249,7 @@ METHOD(trap_manager_t, install, uint32_t,
                this->lock->unlock(this->lock);
                other->destroy(other);
                me->destroy(me);
-               return 0;
+               return FALSE;
        }
        enumerator = this->traps->create_enumerator(this->traps);
        while (enumerator->enumerate(enumerator, &entry))
@@ -277,11 +276,10 @@ METHOD(trap_manager_t, install, uint32_t,
                        this->lock->unlock(this->lock);
                        other->destroy(other);
                        me->destroy(me);
-                       return 0;
+                       return FALSE;
                }
                /* config might have changed so update everything */
                DBG1(DBG_CFG, "updating already routed CHILD_SA '%s'", found->name);
-               reqid = found->child_sa->get_reqid(found->child_sa);
        }
 
        INIT(entry,
@@ -295,7 +293,7 @@ METHOD(trap_manager_t, install, uint32_t,
        this->lock->unlock(this->lock);
 
        /* create and route CHILD_SA */
-       child_sa = child_sa_create(me, other, child, reqid, FALSE, 0, 0);
+       child_sa = child_sa_create(me, other, child, 0, FALSE, 0, 0);
 
        list = linked_list_create_with_items(me, NULL);
        my_ts = child->get_traffic_selectors(child, TRUE, NULL, list);
@@ -327,14 +325,13 @@ METHOD(trap_manager_t, install, uint32_t,
                this->lock->unlock(this->lock);
                entry->child_sa = child_sa;
                destroy_entry(entry);
-               reqid = 0;
        }
        else
        {
-               reqid = child_sa->get_reqid(child_sa);
                this->lock->write_lock(this->lock);
                entry->child_sa = child_sa;
                this->lock->unlock(this->lock);
+               result = TRUE;
        }
        if (found)
        {
@@ -345,7 +342,7 @@ METHOD(trap_manager_t, install, uint32_t,
        this->installing--;
        this->condvar->signal(this->condvar);
        this->lock->unlock(this->lock);
-       return reqid;
+       return result;
 }
 
 METHOD(trap_manager_t, uninstall, bool,
index 9e71d76..63bf17f 100644 (file)
@@ -38,11 +38,9 @@ struct trap_manager_t {
         *
         * @param peer          peer configuration to initiate on trap
         * @param child         child configuration to install as a trap
-        * @param reqid         optional reqid to use
-        * @return                      reqid of installed CHILD_SA, 0 if failed
+        * @return                      TRUE if successfully installed
         */
-       uint32_t (*install)(trap_manager_t *this, peer_cfg_t *peer,
-                                                child_cfg_t *child, uint32_t reqid);
+       bool (*install)(trap_manager_t *this, peer_cfg_t *peer, child_cfg_t *child);
 
        /**
         * Uninstall a trap policy.