From ca213e1907a4c19120c83f0d50c7f7e22c90a269 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 3 Nov 2017 11:10:16 +0100 Subject: [PATCH] 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. --- src/libcharon/plugins/ha/ha_tunnel.c | 8 +++--- src/libcharon/plugins/stroke/stroke_control.c | 22 ++-------------- src/libcharon/plugins/vici/vici_config.c | 18 +------------ src/libcharon/plugins/vici/vici_control.c | 25 ++----------------- src/libcharon/sa/trap_manager.c | 11 ++++---- src/libcharon/sa/trap_manager.h | 11 +++++--- 6 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/libcharon/plugins/ha/ha_tunnel.c b/src/libcharon/plugins/ha/ha_tunnel.c index 1a6108ed98..60004f2603 100644 --- a/src/libcharon/plugins/ha/ha_tunnel.c +++ b/src/libcharon/plugins/ha/ha_tunnel.c @@ -20,6 +20,8 @@ #include #include +#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 beda864018..a5f2410b20 100644 --- 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 e0e2955e20..43e98c9f26 100644 --- 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 ef33a61905..d0e19d519b 100644 --- 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 6436a25491..012f0fe46c 100644 --- 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 083ea3dbfb..9e71d76966 100644 --- 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. -- 2.47.2