From 57ea3f73bbe723243f4eb4a0cd4094e9c2d14290 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 10:47:48 +0100
Subject: [PATCH 1/8] shunt-manager: Remove first match if no namespace given
 during uninstall

Also makes namespace mandatory.
---
 src/libcharon/sa/shunt_manager.c | 13 ++++++++++---
 src/libcharon/sa/shunt_manager.h | 10 ++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/libcharon/sa/shunt_manager.c b/src/libcharon/sa/shunt_manager.c
index 3a254cea5..a83da0480 100644
--- a/src/libcharon/sa/shunt_manager.c
+++ b/src/libcharon/sa/shunt_manager.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Tobias Brunner
+ * Copyright (C) 2015-2017 Tobias Brunner
  * Copyright (C) 2011-2016 Andreas Steffen
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -198,6 +198,13 @@ METHOD(shunt_manager_t, install, bool,
 	entry_t *entry;
 	bool found = FALSE, success;
 
+	if (!ns)
+	{
+		DBG1(DBG_CFG, "missing namespace for shunt policy '%s'",
+			 cfg->get_name(cfg));
+		return FALSE;
+	}
+
 	/* check if not already installed */
 	this->lock->write_lock(this->lock);
 	if (this->installing == INSTALL_DISABLED)
@@ -224,7 +231,7 @@ METHOD(shunt_manager_t, install, bool,
 		return TRUE;
 	}
 	INIT(entry,
-		.ns = strdupnull(ns),
+		.ns = strdup(ns),
 		.cfg = cfg->get_ref(cfg),
 	);
 	this->shunts->insert_last(this->shunts, entry);
@@ -369,7 +376,7 @@ METHOD(shunt_manager_t, uninstall, bool,
 	enumerator = this->shunts->create_enumerator(this->shunts);
 	while (enumerator->enumerate(enumerator, &entry))
 	{
-		if (streq(ns, entry->ns) &&
+		if ((!ns || streq(ns, entry->ns)) &&
 			streq(name, entry->cfg->get_name(entry->cfg)))
 		{
 			this->shunts->remove_at(this->shunts, enumerator);
diff --git a/src/libcharon/sa/shunt_manager.h b/src/libcharon/sa/shunt_manager.h
index f2b721032..3d9848c93 100644
--- a/src/libcharon/sa/shunt_manager.h
+++ b/src/libcharon/sa/shunt_manager.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Tobias Brunner
+ * Copyright (C) 2015-2017 Tobias Brunner
  * Copyright (C) 2011 Andreas Steffen
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -36,8 +36,7 @@ struct shunt_manager_t {
 	/**
 	 * Install a policy as a shunt.
 	 *
-	 * @param ns		optional namespace (e.g. name of a connection or
-	 *					plugin), cloned
+	 * @param ns		namespace (e.g. name of a connection or plugin), cloned
 	 * @param child		child configuration to install as a shunt
 	 * @return			TRUE if installed successfully
 	 */
@@ -46,7 +45,10 @@ struct shunt_manager_t {
 	/**
 	 * Uninstall a shunt policy.
 	 *
-	 * @param ns		namespace (same as given during installation)
+	 * If no namespace is given the first matching child configuration is
+	 * removed.
+	 *
+	 * @param ns		namespace (same as given during installation) or NULL
 	 * @param name	 	name of child configuration to uninstall as a shunt
 	 * @return			TRUE if uninstalled successfully
 	 */

From 9d69d8a3997c4e40d0b2a62072899a8811dd8b5a Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 10:53:04 +0100
Subject: [PATCH 2/8] stroke: Remove external enumeration to unroute shunt
 policies

---
 src/libcharon/plugins/stroke/stroke_control.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/src/libcharon/plugins/stroke/stroke_control.c b/src/libcharon/plugins/stroke/stroke_control.c
index ee8306772..beda86401 100644
--- a/src/libcharon/plugins/stroke/stroke_control.c
+++ b/src/libcharon/plugins/stroke/stroke_control.c
@@ -730,30 +730,15 @@ METHOD(stroke_control_t, route, void,
 METHOD(stroke_control_t, unroute, void,
 	private_stroke_control_t *this, stroke_msg_t *msg, FILE *out)
 {
-	child_cfg_t *child_cfg;
 	child_sa_t *child_sa;
 	enumerator_t *enumerator;
-	char *ns, *found = NULL;
 	uint32_t id = 0;
 
-	enumerator = charon->shunts->create_enumerator(charon->shunts);
-	while (enumerator->enumerate(enumerator, &ns, &child_cfg))
+	if (charon->shunts->uninstall(charon->shunts, NULL, msg->unroute.name))
 	{
-		if (ns && streq(msg->unroute.name, child_cfg->get_name(child_cfg)))
-		{
-			found = strdup(ns);
-			break;
-		}
-	}
-	enumerator->destroy(enumerator);
-	if (found && charon->shunts->uninstall(charon->shunts, found,
-										   msg->unroute.name))
-	{
-		free(found);
 		fprintf(out, "shunt policy '%s' uninstalled\n", msg->unroute.name);
 		return;
 	}
-	free(found);
 
 	enumerator = charon->traps->create_enumerator(charon->traps);
 	while (enumerator->enumerate(enumerator, NULL, &child_sa))

From 6f569263a0129d4bf5238d38a8779a1ffd46cd3d Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 10:55:05 +0100
Subject: [PATCH 3/8] vici: Remove external enumeration to uninstall shunt
 policies

---
 src/libcharon/plugins/vici/vici_control.c | 28 ++---------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/src/libcharon/plugins/vici/vici_control.c b/src/libcharon/plugins/vici/vici_control.c
index afee649f7..ef33a6190 100644
--- a/src/libcharon/plugins/vici/vici_control.c
+++ b/src/libcharon/plugins/vici/vici_control.c
@@ -680,11 +680,10 @@ 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_cfg_t *child_cfg;
 	child_sa_t *child_sa;
 	enumerator_t *enumerator;
 	uint32_t reqid = 0;
-	char *child, *ike, *ns;
+	char *child, *ike;
 
 	child = request->get_str(request, NULL, "child");
 	ike = request->get_str(request, NULL, "ike");
@@ -695,30 +694,7 @@ CALLBACK(uninstall, vici_message_t*,
 
 	DBG1(DBG_CFG, "vici uninstall '%s'", child);
 
-	if (!ike)
-	{
-		enumerator = charon->shunts->create_enumerator(charon->shunts);
-		while (enumerator->enumerate(enumerator, &ns, &child_cfg))
-		{
-			if (ns && streq(child, child_cfg->get_name(child_cfg)))
-			{
-				ike = strdup(ns);
-				break;
-			}
-		}
-		enumerator->destroy(enumerator);
-		if (ike)
-		{
-			if (charon->shunts->uninstall(charon->shunts, ike, child))
-			{
-				free(ike);
-				return send_reply(this, NULL);
-			}
-			free(ike);
-			return send_reply(this, "uninstalling policy '%s' failed", child);
-		}
-	}
-	else if (charon->shunts->uninstall(charon->shunts, ike, child))
+	if (charon->shunts->uninstall(charon->shunts, ike, child))
 	{
 		return send_reply(this, NULL);
 	}

From ca213e1907a4c19120c83f0d50c7f7e22c90a269 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 11:10:16 +0100
Subject: [PATCH 4/8] 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 1a6108ed9..60004f260 100644
--- 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 beda86401..a5f2410b2 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))
+	else if (charon->traps->uninstall(charon->traps, NULL, msg->unroute.name))
 	{
-		if (streq(msg->unroute.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);
-		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 e0e2955e2..43e98c9f2 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 ef33a6190..d0e19d519 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))
+	else if (charon->traps->uninstall(charon->traps, ike, child))
 	{
-		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)
-	{
-		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 6436a2549..012f0fe46 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 083ea3dbf..9e71d7696 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.

From f42dd430d8283c4a6002087055f74468d88b95f6 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 11:26:23 +0100
Subject: [PATCH 5/8] trap-manager: Compare peer config name during
 installation

---
 src/libcharon/sa/trap_manager.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 012f0fe46..88e6bb233 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -255,7 +255,9 @@ METHOD(trap_manager_t, install, uint32_t,
 	enumerator = this->traps->create_enumerator(this->traps);
 	while (enumerator->enumerate(enumerator, &entry))
 	{
-		if (streq(entry->name, child->get_name(child)))
+		if (streq(entry->name, child->get_name(child)) &&
+			streq(entry->peer_cfg->get_name(entry->peer_cfg),
+				  peer->get_name(peer)))
 		{
 			found = entry;
 			if (entry->child_sa)

From 24fa1bb02ad5de0fe01c5aa515ee3c2dd3d42206 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 11:32:04 +0100
Subject: [PATCH 6/8] trap-manager: Remove reqid parameter from install() and
 change return type

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          |  7 +--
 src/libcharon/plugins/stroke/stroke_control.c | 45 +------------------
 src/libcharon/plugins/vici/vici_config.c      | 38 +---------------
 src/libcharon/plugins/vici/vici_control.c     | 38 +---------------
 .../processing/jobs/start_action_job.c        |  2 +-
 src/libcharon/sa/ike_sa.c                     |  3 +-
 src/libcharon/sa/ikev1/tasks/quick_delete.c   |  2 +-
 src/libcharon/sa/ikev2/tasks/child_delete.c   |  4 +-
 src/libcharon/sa/trap_manager.c               | 23 +++++-----
 src/libcharon/sa/trap_manager.h               |  6 +--
 10 files changed, 22 insertions(+), 146 deletions(-)

diff --git a/src/libcharon/plugins/ha/ha_tunnel.c b/src/libcharon/plugins/ha/ha_tunnel.c
index 60004f260..cbee1db11 100644
--- a/src/libcharon/plugins/ha/ha_tunnel.c
+++ b/src/libcharon/plugins/ha/ha_tunnel.c
@@ -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);
 }
 
diff --git a/src/libcharon/plugins/stroke/stroke_control.c b/src/libcharon/plugins/stroke/stroke_control.c
index a5f2410b2..d3c279131 100644
--- a/src/libcharon/plugins/stroke/stroke_control.c
+++ b/src/libcharon/plugins/stroke/stroke_control.c
@@ -588,47 +588,6 @@ METHOD(stroke_control_t, purge_ike, void,
 	list->destroy(list);
 }
 
-/**
- * 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
  */
@@ -636,7 +595,6 @@ 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);
 		}
diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c
index 43e98c9f2..c95d8c8be 100644
--- a/src/libcharon/plugins/vici/vici_config.c
+++ b/src/libcharon/plugins/vici/vici_config.c
@@ -1953,41 +1953,6 @@ CALLBACK(peer_sn, bool,
 	return FALSE;
 }
 
-/**
- * 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
  */
@@ -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;
diff --git a/src/libcharon/plugins/vici/vici_control.c b/src/libcharon/plugins/vici/vici_control.c
index d0e19d519..f9c7d8f19 100644
--- a/src/libcharon/plugins/vici/vici_control.c
+++ b/src/libcharon/plugins/vici/vici_control.c
@@ -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);
diff --git a/src/libcharon/processing/jobs/start_action_job.c b/src/libcharon/processing/jobs/start_action_job.c
index 654ec6abe..3a0ed879f 100644
--- a/src/libcharon/processing/jobs/start_action_job.c
+++ b/src/libcharon/processing/jobs/start_action_job.c
@@ -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:
diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index e1f4ec95a..7fe6d112b 100644
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -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;
diff --git a/src/libcharon/sa/ikev1/tasks/quick_delete.c b/src/libcharon/sa/ikev1/tasks/quick_delete.c
index 66ef50811..6b3bb9c47 100644
--- a/src/libcharon/sa/ikev1/tasks/quick_delete.c
+++ b/src/libcharon/sa/ikev1/tasks/quick_delete.c
@@ -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;
diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c
index 164f8fc03..8fec6494e 100644
--- a/src/libcharon/sa/ikev2/tasks/child_delete.c
+++ b/src/libcharon/sa/ikev2/tasks/child_delete.c
@@ -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;
diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 88e6bb233..f55821997 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -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,
diff --git a/src/libcharon/sa/trap_manager.h b/src/libcharon/sa/trap_manager.h
index 9e71d7696..63bf17fbf 100644
--- a/src/libcharon/sa/trap_manager.h
+++ b/src/libcharon/sa/trap_manager.h
@@ -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.

From c9998e5fea26d1dbe2c2a632aee4ab23b0309d6b Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 11:49:45 +0100
Subject: [PATCH 7/8] child-sa: No need to find reqid of existing trap policy

When initiating a trap policy we explicitly pass the reqid along.  I guess
the lookup was useful to get the same reqid if a trapped CHILD_SA is manually
initiated.  However, we now get the same reqid anyway if there is no
narrowing.  And if the traffic selectors do get narrowed the reqid will be
different but that shouldn't be a problem as that doesn't cause an issue with
any temporary SAs in the kernel (this is why we pass the reqid to the
triggered CHILD_SA, otherwise, no new acquire would get triggered for
traffic that doesn't match the wider trap policy).
---
 src/libcharon/sa/child_sa.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c
index a01ee9e4d..3523d4820 100644
--- a/src/libcharon/sa/child_sa.c
+++ b/src/libcharon/sa/child_sa.c
@@ -1754,7 +1754,7 @@ static host_t* get_proxy_addr(child_cfg_t *config, host_t *ike, bool local)
  * Described in header.
  */
 child_sa_t * child_sa_create(host_t *me, host_t* other,
-							 child_cfg_t *config, uint32_t rekey, bool encap,
+							 child_cfg_t *config, uint32_t reqid, bool encap,
 							 u_int mark_in, u_int mark_out)
 {
 	private_child_sa_t *this;
@@ -1865,21 +1865,15 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
 
 	if (!this->reqid)
 	{
-		/* reuse old reqid if we are rekeying an existing CHILD_SA. While the
-		 * reqid cache would find the same reqid for our selectors, this does
-		 * not work in a special case: If an SA is triggered by a trap policy,
-		 * but the negotiated SA gets narrowed, we still must reuse the same
-		 * reqid to successfully "trigger" the SA on the kernel level. Rekeying
-		 * such an SA requires an explicit reqid, as the cache currently knows
-		 * the original selectors only for that reqid. */
-		if (rekey)
-		{
-			this->reqid = rekey;
-		}
-		else
-		{
-			this->reqid = charon->traps->find_reqid(charon->traps, config);
-		}
+		/* reuse old reqid if we are rekeying an existing CHILD_SA and when
+		 * initiating a trap policy. While the reqid cache would find the same
+		 * reqid for our selectors, this does not work in a special case: If an
+		 * SA is triggered by a trap policy, but the negotiated TS get
+		 * narrowed, we still must reuse the same reqid to successfully
+		 * replace the temporary SA on the kernel level. Rekeying such an SA
+		 * requires an explicit reqid, as the cache currently knows the original
+		 * selectors only for that reqid. */
+		this->reqid = reqid;
 	}
 	else
 	{

From a541cf05c1b7db40e263603eef111ab2129d3d9c Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 3 Nov 2017 11:51:36 +0100
Subject: [PATCH 8/8] trap-manager: Remove unused find_reqid() method

It might actually have returned an incorrect result if there were child
configs for different peer configs sharing the same name.
---
 src/libcharon/sa/trap_manager.c | 26 --------------------------
 src/libcharon/sa/trap_manager.h |  8 --------
 2 files changed, 34 deletions(-)

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index f55821997..979f9290a 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -411,31 +411,6 @@ METHOD(trap_manager_t, create_enumerator, enumerator_t*,
 									(void*)this->lock->unlock);
 }
 
-METHOD(trap_manager_t, find_reqid, uint32_t,
-	private_trap_manager_t *this, child_cfg_t *child)
-{
-	enumerator_t *enumerator;
-	entry_t *entry;
-	uint32_t reqid = 0;
-
-	this->lock->read_lock(this->lock);
-	enumerator = this->traps->create_enumerator(this->traps);
-	while (enumerator->enumerate(enumerator, &entry))
-	{
-		if (streq(entry->name, child->get_name(child)))
-		{
-			if (entry->child_sa)
-			{
-				reqid = entry->child_sa->get_reqid(entry->child_sa);
-			}
-			break;
-		}
-	}
-	enumerator->destroy(enumerator);
-	this->lock->unlock(this->lock);
-	return reqid;
-}
-
 METHOD(trap_manager_t, acquire, void,
 	private_trap_manager_t *this, uint32_t reqid,
 	traffic_selector_t *src, traffic_selector_t *dst)
@@ -691,7 +666,6 @@ trap_manager_t *trap_manager_create(void)
 			.install = _install,
 			.uninstall = _uninstall,
 			.create_enumerator = _create_enumerator,
-			.find_reqid = _find_reqid,
 			.acquire = _acquire,
 			.flush = _flush,
 			.destroy = _destroy,
diff --git a/src/libcharon/sa/trap_manager.h b/src/libcharon/sa/trap_manager.h
index 63bf17fbf..1b67ff82f 100644
--- a/src/libcharon/sa/trap_manager.h
+++ b/src/libcharon/sa/trap_manager.h
@@ -61,14 +61,6 @@ struct trap_manager_t {
 	 */
 	enumerator_t* (*create_enumerator)(trap_manager_t *this);
 
-	/**
-	 * Find the reqid of a child config installed as a trap.
-	 *
-	 * @param child		CHILD_SA config to get the reqid for
-	 * @return			reqid of trap, 0 if not found
-	 */
-	uint32_t (*find_reqid)(trap_manager_t *this, child_cfg_t *child);
-
 	/**
 	 * Acquire an SA triggered by an installed trap.
 	 *