[strongSwan-dev] [PATCH] install inbound SA after outbound

Heiko Hund hhund at astaro.com
Thu May 6 15:06:03 CEST 2010


Hi list.

When pluto is the responder it installs the inbound SA when it
receives the first Quick Mode (I1) message from the initiator. The
outbound SA is installed when it receives the second one (I2).

We had a problem with Windows Vista/7 L2TP over IPsec clients
and OpenL2TPd on the server side. OpenL2TPd connect(2)s the UDP
socket when it receives the SCCRQ from Windows between I1 and I2.
Because there's no outbound SA/policy installed at that time the
L2TP packet is sent untransformed. Depending on your setup it gets
dropped by the local packetfilter or travels to the client in
plain text, which in turn drops it then because it's expecting it
to be encapsulated. Because OpenL2TPd used connect(2) there's no
other route/xfrm lookup done and retransmitted packets continue
being sent untransformed even though a policy exists.

Installing the inbound SA after the outbound SA prevents this
problem in general as it is not limited to L2TP as far as I can
tell. Thus, this patch removes the install_inbound_ipsec_sa()
function and changes to order in which IPsec SAs are installed to
outbound before inbound.

Please consider for inclusion.

Heiko

Signed-off-by: Heiko Hund <hhund at astaro.com>

---
 src/pluto/ipsec_doi.c |   15 +--------
 src/pluto/kernel.c    |   80 ++++---------------------------------------------
 src/pluto/kernel.h    |    3 +-
 3 files changed, 9 insertions(+), 89 deletions(-)

diff --git a/src/pluto/ipsec_doi.c b/src/pluto/ipsec_doi.c
index 1eaf355..be1ac6c 100644
--- a/src/pluto/ipsec_doi.c
+++ b/src/pluto/ipsec_doi.c
@@ -5175,18 +5175,7 @@ static stf_status quick_inI1_outR1_tail(struct verify_oppo_bundle *b,
 		/* Derive new keying material */
 		compute_keymats(st);
 
-		/* Tell the kernel to establish the new inbound SA
-		 * (unless the commit bit is set -- which we don't support).
-		 * We do this before any state updating so that
-		 * failure won't look like success.
-		 */
-		if (!install_inbound_ipsec_sa(st))
-		{
-			return STF_INTERNAL_ERROR;  /* ??? we may be partly committed */
-		}
-
 		/* encrypt message, except for fixed part of header */
-
 		if (!encrypt_message(&md->rbody, st))
 		{
 			return STF_INTERNAL_ERROR;  /* ??? we may be partly committed */
@@ -5353,7 +5342,7 @@ stf_status quick_inR1_outI2(struct msg_digest *md)
 	 * We do this before any state updating so that
 	 * failure won't look like success.
 	 */
-	if (!install_ipsec_sa(st, TRUE))
+	if (!install_ipsec_sas(st))
 	{
 		return STF_INTERNAL_ERROR;
 	}
@@ -5407,7 +5396,7 @@ stf_status quick_inI2(struct msg_digest *md)
 	 * We do this before any state updating so that
 	 * failure won't look like success.
 	 */
-	if (!install_ipsec_sa(st, FALSE))
+	if (!install_ipsec_sas(st))
 	{
 		return STF_INTERNAL_ERROR;
 	}
diff --git a/src/pluto/kernel.c b/src/pluto/kernel.c
index 9b65cfe..364a0a6 100644
--- a/src/pluto/kernel.c
+++ b/src/pluto/kernel.c
@@ -2430,71 +2430,6 @@ void init_kernel(void)
 #endif
 }
 
-/* Note: install_inbound_ipsec_sa is only used by the Responder.
- * The Responder will subsequently use install_ipsec_sa for the outbound.
- * The Initiator uses install_ipsec_sa to install both at once.
- */
-bool install_inbound_ipsec_sa(struct state *st)
-{
-	connection_t *const c = st->st_connection;
-
-	/* If our peer has a fixed-address client, check if we already
-	 * have a route for that client that conflicts.  We will take this
-	 * as proof that that route and the connections using it are
-	 * obsolete and should be eliminated.  Interestingly, this is
-	 * the only case in which we can tell that a connection is obsolete.
-	 */
-	passert(c->kind == CK_PERMANENT || c->kind == CK_INSTANCE);
-	if (c->spd.that.has_client)
-	{
-		for (;;)
-		{
-			struct spd_route *esr;
-			connection_t *o = route_owner(c, &esr, NULL, NULL);
-
-			if (o == NULL)
-			{
-				break;  /* nobody has a route */
-			}
-
-			/* note: we ignore the client addresses at this end */
-			if (sameaddr(&o->spd.that.host_addr, &c->spd.that.host_addr) &&
-				o->interface == c->interface)
-			{
-				break;  /* existing route is compatible */
-			}
-
-			if (o->kind == CK_TEMPLATE && streq(o->name, c->name))
-			{
-				break;  /* ??? is this good enough?? */
-			}
-
-			loglog(RC_LOG_SERIOUS, "route to peer's client conflicts with \"%s\" %s; releasing old connection to free the route"
-				, o->name, ip_str(&o->spd.that.host_addr));
-			release_connection(o, FALSE);
-		}
-	}
-
-	DBG(DBG_CONTROL, DBG_log("install_inbound_ipsec_sa() checking if we can route"));
-	/* check that we will be able to route and eroute */
-	switch (could_route(c))
-	{
-		case route_easy:
-		case route_nearconflict:
-			break;
-		default:
-			return FALSE;
-	}
-
-#ifdef KLIPS
-	/* (attempt to) actually set up the SAs */
-	return setup_half_ipsec_sa(st, TRUE);
-#else /* !KLIPS */
-	DBG(DBG_CONTROL, DBG_log("install_inbound_ipsec_sa()"));
-	return TRUE;
-#endif /* !KLIPS */
-}
-
 /* Install a route and then a prospective shunt eroute or an SA group eroute.
  * Assumption: could_route gave a go-ahead.
  * Any SA Group must have already been created.
@@ -2790,15 +2725,14 @@ bool route_and_eroute(connection_t *c USED_BY_KLIPS,
 #endif /* !KLIPS */
 }
 
-bool install_ipsec_sa(struct state *st, bool inbound_also USED_BY_KLIPS)
+bool install_ipsec_sas(struct state *st)
 {
 #ifdef KLIPS
 	struct spd_route *sr;
 
-	DBG(DBG_CONTROL, DBG_log("install_ipsec_sa() for #%ld: %s"
-							 , st->st_serialno
-							 , inbound_also?
-							 "inbound and outbound" : "outbound only"));
+	DBG(DBG_CONTROL,
+		DBG_log("install_ipsec_sas() for #%ld: inbound and outbound"
+				, st->st_serialno));
 
 	switch (could_route(st->st_connection))
 	{
@@ -2810,8 +2744,7 @@ bool install_ipsec_sa(struct state *st, bool inbound_also USED_BY_KLIPS)
 	}
 
 	/* (attempt to) actually set up the SA group */
-	if ((inbound_also && !setup_half_ipsec_sa(st, TRUE)) ||
-		!setup_half_ipsec_sa(st, FALSE))
+	if (!setup_half_ipsec_sa(st, FALSE) || !setup_half_ipsec_sa(st, TRUE))
 	{
 		return FALSE;
 	}
@@ -2843,8 +2776,7 @@ bool install_ipsec_sa(struct state *st, bool inbound_also USED_BY_KLIPS)
 		}
 	}
 #else /* !KLIPS */
-	DBG(DBG_CONTROL, DBG_log("install_ipsec_sa() %s"
-		, inbound_also? "inbound and oubound" : "outbound only"));
+	DBG(DBG_CONTROL, DBG_log("install_ipsec_sas() inbound and oubound"));
 
 	switch (could_route(st->st_connection))
 	{
diff --git a/src/pluto/kernel.h b/src/pluto/kernel.h
index 36f87d2..5f50b03 100644
--- a/src/pluto/kernel.h
+++ b/src/pluto/kernel.h
@@ -184,8 +184,7 @@ extern ipsec_spi_t get_ipsec_spi(ipsec_spi_t avoid
 								 , bool tunnel_mode);
 extern ipsec_spi_t get_my_cpi(struct spd_route *sr, bool tunnel_mode);
 
-extern bool install_inbound_ipsec_sa(struct state *st);
-extern bool install_ipsec_sa(struct state *st, bool inbound_also);
+extern bool install_ipsec_sas(struct state *st);
 extern void delete_ipsec_sa(struct state *st, bool inbound_only);
 extern bool route_and_eroute(struct connection *c
 							 , struct spd_route *sr
-- 
tg: (9a76690..) t/inbound_sa_install_after_outbound (depends on: t/vpn_id_type_in_xauth_peer_info)

-- 
Heiko Hund | Software Engineer | Phone +49-721-25516-237 | Fax -200
Astaro GmbH & Co. KG | An der RaumFabrik 33a | 76227 Karlsruhe | Germany

Astaro GmbH & Co. KG
Commercial Register: Mannheim HRA 702710
Headquarter Location: Karlsruhe
 
Represented by the General Partner Astaro Verwaltungs GmbH
An der RaumFabrik 33a | 76227 Karlsruhe | Germany 
Commercial Register: Mannheim HRB 708248
Executive Board: Gert Hansen, Markus Hennig, Jan Hichert, Guenter Junk, Dr. Frank Nellissen





More information about the Dev mailing list