trap-manager: Uninstall trap policies by name and not reqid

If a trap policy is concurrently uninstalled and reinstalled under a
different name the reqid will be the same so the wrong trap might be
removed.

diff --git a/src/libcharon/plugins/ha/ha_tunnel.c b/src/libcharon/plugins/ha/ha_tunnel.c
index 1a6108ed98eca95284b2eee979bc1c7e7f2abaf2..60004f2603479b8eef999013e05567d2d461e5fe 100644 (file)
--- a/src/libcharon/plugins/ha/ha_tunnel.c
+++ b/src/libcharon/plugins/ha/ha_tunnel.c
@@ -20,6 +20,8 @@
 #include <utils/identification.h>
 #include <processing/jobs/callback_job.h>
 
+#define HA_CFG_NAME "ha"
+
 typedef struct private_ha_tunnel_t private_ha_tunnel_t;
 typedef struct ha_backend_t ha_backend_t;
 typedef struct ha_creds_t ha_creds_t;
@@ -225,7 +227,7 @@ static void setup_tunnel(private_ha_tunnel_t *this,
                                                         remote, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0);
        ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE));
        ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE));
-       peer_cfg = peer_cfg_create("ha", ike_cfg, &peer);
+       peer_cfg = peer_cfg_create(HA_CFG_NAME, ike_cfg, &peer);
 
        auth_cfg = auth_cfg_create();
        auth_cfg->add(auth_cfg, AUTH_RULE_AUTH_CLASS, AUTH_CLASS_PSK);
@@ -239,7 +241,7 @@ static void setup_tunnel(private_ha_tunnel_t *this,
                                  identification_create_from_string(remote));
        peer_cfg->add_auth_cfg(peer_cfg, auth_cfg, FALSE);
 
-       child_cfg = child_cfg_create("ha", &child);
+       child_cfg = child_cfg_create(HA_CFG_NAME, &child);
        ts = traffic_selector_create_dynamic(IPPROTO_UDP, HA_PORT, HA_PORT);
        child_cfg->add_traffic_selector(child_cfg, TRUE, ts);
        ts = traffic_selector_create_dynamic(IPPROTO_ICMP, 0, 65535);
@@ -280,7 +282,7 @@ METHOD(ha_tunnel_t, destroy, void,
        this->creds.remote->destroy(this->creds.remote);
        if (this->trap)
        {
-               charon->traps->uninstall(charon->traps, this->trap);
+               charon->traps->uninstall(charon->traps, HA_CFG_NAME, HA_CFG_NAME);
        }
        free(this);
 }

diff --git a/src/libcharon/plugins/stroke/stroke_control.c b/src/libcharon/plugins/stroke/stroke_control.c
index beda864018123c7e1cce340bdb73e1a1356a33bf..a5f2410b20af0016d3a91d842f65829e0b9295bd 100644 (file)
--- a/src/libcharon/plugins/stroke/stroke_control.c
+++ b/src/libcharon/plugins/stroke/stroke_control.c
@@ -730,31 +730,13 @@ METHOD(stroke_control_t, route, void,
 METHOD(stroke_control_t, unroute, void,
        private_stroke_control_t *this, stroke_msg_t *msg, FILE *out)
 {
-       child_sa_t *child_sa;
-       enumerator_t *enumerator;
-       uint32_t id = 0;
-
        if (charon->shunts->uninstall(charon->shunts, NULL, msg->unroute.name))
        {
                fprintf(out, "shunt policy '%s' uninstalled\n", msg->unroute.name);
-               return;
        }
-
-       enumerator = charon->traps->create_enumerator(charon->traps);
-       while (enumerator->enumerate(enumerator, NULL, &child_sa))
-       {
-               if (streq(msg->unroute.name, child_sa->get_name(child_sa)))
-               {
-                       id = child_sa->get_reqid(child_sa);
-                       break;
-               }
-       }
-       enumerator->destroy(enumerator);
-
-       if (id)
+       else if (charon->traps->uninstall(charon->traps, NULL, msg->unroute.name))
        {
-               charon->traps->uninstall(charon->traps, id);
-               fprintf(out, "configuration '%s' unrouted\n", msg->unroute.name);
+               fprintf(out, "trap policy '%s' unrouted\n", msg->unroute.name);
        }
        else
        {

diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c
index e0e2955e20bdf70fd8a052beac1b46f5a945f40e..43e98c9f26f60405d30f679d28c8860c350c41e5 100644 (file)
--- a/src/libcharon/plugins/vici/vici_config.c
+++ b/src/libcharon/plugins/vici/vici_config.c
@@ -2030,7 +2030,6 @@ static void clear_start_action(private_vici_config_t *this, char *peer_name,
 {
        enumerator_t *enumerator, *children;
        child_sa_t *child_sa;
-       peer_cfg_t *peer_cfg;
        ike_sa_t *ike_sa;
        uint32_t id = 0, others;
        array_t *ids = NULL, *ikeids = NULL;
@@ -2121,22 +2120,7 @@ static void clear_start_action(private_vici_config_t *this, char *peer_name,
                                        charon->shunts->uninstall(charon->shunts, peer_name, name);
                                        break;
                                default:
-                                       enumerator = charon->traps->create_enumerator(charon->traps);
-                                       while (enumerator->enumerate(enumerator, &peer_cfg,
-                                                                                                &child_sa))
-                                       {
-                                               if (streq(peer_name, peer_cfg->get_name(peer_cfg)) &&
-                                                       streq(name, child_sa->get_name(child_sa)))
-                                               {
-                                                       id = child_sa->get_reqid(child_sa);
-                                                       break;
-                                               }
-                                       }
-                                       enumerator->destroy(enumerator);
-                                       if (id)
-                                       {
-                                               charon->traps->uninstall(charon->traps, id);
-                                       }
+                                       charon->traps->uninstall(charon->traps, peer_name, name);
                                        break;
                        }
                        break;

diff --git a/src/libcharon/plugins/vici/vici_control.c b/src/libcharon/plugins/vici/vici_control.c
index ef33a61905d2f096cebb1baf2293208dffd422c1..d0e19d519b71cc0fa794e8ad16c09e0d279c9478 100644 (file)
--- a/src/libcharon/plugins/vici/vici_control.c
+++ b/src/libcharon/plugins/vici/vici_control.c
@@ -679,10 +679,6 @@ CALLBACK(install, vici_message_t*,
 CALLBACK(uninstall, vici_message_t*,
        private_vici_control_t *this, char *name, u_int id, vici_message_t *request)
 {
-       peer_cfg_t *peer_cfg;
-       child_sa_t *child_sa;
-       enumerator_t *enumerator;
-       uint32_t reqid = 0;
        char *child, *ike;
 
        child = request->get_str(request, NULL, "child");
@@ -698,26 +694,9 @@ CALLBACK(uninstall, vici_message_t*,
        {
                return send_reply(this, NULL);
        }
-
-       enumerator = charon->traps->create_enumerator(charon->traps);
-       while (enumerator->enumerate(enumerator, &peer_cfg, &child_sa))
-       {
-               if ((!ike || streq(ike, peer_cfg->get_name(peer_cfg))) &&
-                       streq(child, child_sa->get_name(child_sa)))
-               {
-                       reqid = child_sa->get_reqid(child_sa);
-                       break;
-               }
-       }
-       enumerator->destroy(enumerator);
-
-       if (reqid)
+       else if (charon->traps->uninstall(charon->traps, ike, child))
        {
-               if (charon->traps->uninstall(charon->traps, reqid))
-               {
-                       return send_reply(this, NULL);
-               }
-               return send_reply(this, "uninstalling policy '%s' failed", child);
+               return send_reply(this, NULL);
        }
        return send_reply(this, "policy '%s' not found", child);
 }

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 6436a2549191c18a6a38885d6c569d5c1e89d9ec..012f0fe46c24e080a4da63abd9101606ba358ea1 100644 (file)
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -1,7 +1,7 @@
 /*
- * Copyright (C) 2011-2015 Tobias Brunner
+ * Copyright (C) 2011-2017 Tobias Brunner
  * Copyright (C) 2009 Martin Willi
- * 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,7 +347,7 @@ METHOD(trap_manager_t, install, uint32_t,
 }
 
 METHOD(trap_manager_t, uninstall, bool,
-       private_trap_manager_t *this, uint32_t reqid)
+       private_trap_manager_t *this, char *peer, char *child)
 {
        enumerator_t *enumerator;
        entry_t *entry, *found = NULL;
@@ -356,8 +356,8 @@ METHOD(trap_manager_t, uninstall, bool,
        enumerator = this->traps->create_enumerator(this->traps);
        while (enumerator->enumerate(enumerator, &entry))
        {
-               if (entry->child_sa &&
-                       entry->child_sa->get_reqid(entry->child_sa) == reqid)
+               if (streq(entry->name, child) &&
+                  (!peer || streq(peer, entry->peer_cfg->get_name(entry->peer_cfg))))
                {
                        this->traps->remove_at(this->traps, enumerator);
                        found = entry;
@@ -369,7 +369,6 @@ METHOD(trap_manager_t, uninstall, bool,
 
        if (!found)
        {
-               DBG1(DBG_CFG, "trap %d not found to uninstall", reqid);
                return FALSE;
        }
        destroy_entry(found);

diff --git a/src/libcharon/sa/trap_manager.h b/src/libcharon/sa/trap_manager.h
index 083ea3dbfb211d24d6d3128918ad62e5fccd937e..9e71d769665e3cebc4f939ccd9f6ad737b21444c 100644 (file)
--- a/src/libcharon/sa/trap_manager.h
+++ b/src/libcharon/sa/trap_manager.h
@@ -1,6 +1,7 @@
 /*
+ * Copyright (C) 2013-2017 Tobias Brunner
  * Copyright (C) 2009 Martin Willi
- * 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
@@ -46,10 +47,14 @@ struct trap_manager_t {
        /**
         * Uninstall a trap policy.
         *
-        * @param id            reqid of CHILD_SA to uninstall, returned by install()
+        * If no peer configuration name is given the first matching child
+        * configuration is uninstalled.
+        *
+        * @param peer          peer configuration name or NULL
+        * @param child         child configuration name
         * @return                      TRUE if uninstalled successfully
         */
-       bool (*uninstall)(trap_manager_t *this, uint32_t reqid);
+       bool (*uninstall)(trap_manager_t *this, char *peer, char *child);
 
        /**
         * Create an enumerator over all installed traps.